From 6565a0f9a1e534a195bcfeab171903c065a7205e Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Tue, 23 Oct 2018 14:03:45 +0200 Subject: [PATCH] [FIX] runbot_merge: don't batch patch-X PRs across repos Normally, two PRs from the same owner with the same branch name (source) are batched together, and that's represented by batching them by label e.g. two PRs labelled `odoo-dev:12.0-snailmail-address-format-anp` means they probably should be merged together somehow. *However* this is an issue when editing via the web interface: if the editor doesn't have write access to the repository, github automatically createa a branch called `patch-` under their ownership, where `` is a sequence (of sorts?) *within the user's fork*. This means it's possible (and indeed easy) to create :patch-1 on different (non-forks) but related repositories without them actually being co-dependent, at which point they get blocked on one another, which can lead to them being blocked (period) due to runbot not currently handling co-dependencies between PRs. --- runbot_merge/models/pull_requests.py | 14 ++++++++++++-- runbot_merge/tests/test_multirepo.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index 988b21f8..e55b6561 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -276,7 +276,12 @@ class Branch(models.Model): -- deleting branches & reusing labels) AND pr.state != 'merged' AND pr.state != 'closed' - GROUP BY pr.label + GROUP BY + CASE + WHEN pr.label SIMILAR TO '%%:patch-[[:digit:]]+' + THEN pr.id::text + ELSE pr.label + END HAVING (bool_or(pr.priority = 0) AND NOT bool_or(pr.state = 'error')) OR bool_and(pr.state = 'ready') ORDER BY min(pr.priority), min(pr.id) @@ -729,7 +734,12 @@ class PullRequests(models.Model): -- deleting branches & reusing labels) pr.state != 'merged' AND pr.state != 'closed' - GROUP BY pr.label + GROUP BY + CASE + WHEN pr.label SIMILAR TO '%%:patch-[[:digit:]]+' + THEN pr.id::text + ELSE pr.label + END HAVING -- one of the batch's PRs should be ready & not marked bool_or(pr.state = 'ready' AND NOT pr.link_warned) diff --git a/runbot_merge/tests/test_multirepo.py b/runbot_merge/tests/test_multirepo.py index 91ed8e36..fdae3d8a 100644 --- a/runbot_merge/tests/test_multirepo.py +++ b/runbot_merge/tests/test_multirepo.py @@ -102,6 +102,34 @@ def test_stage_match(env, project, repo_a, repo_b): assert pr_a.staging_id == pr_b.staging_id, \ "branch-matched PRs should be part of the same staging" +def test_unmatch_patch(env, project, repo_a, repo_b): + """ When editing files via the UI for a project you don't have write + access to, a branch called patch-XXX is automatically created in your + profile to hold the change. + + This means it's possible to create a:patch-1 and b:patch-1 without + intending them to be related in any way, and more likely than the opposite + since there is no user control over the branch names (save by actually + creating/renaming branches afterwards before creating the PR). + + -> PRs with a branch name of patch-* should not be label-matched + """ + project.batch_limit = 1 + make_branch(repo_a, 'master', 'initial', {'a': 'a_0'}) + pr_a = make_pr(repo_a, 'A', [{'a': 'a_1'}], label='patch-1') + + make_branch(repo_b, 'master', 'initial', {'a': 'b_0'}) + pr_b = make_pr(repo_b, 'B', [{'a': 'b_1'}], label='patch-1') + + env['runbot_merge.project']._check_progress() + + pr_a = to_pr(env, pr_a) + pr_b = to_pr(env, pr_b) + assert pr_a.state == 'ready' + assert pr_a.staging_id + assert pr_b.state == 'ready' + assert not pr_b.staging_id, 'patch-* PRs should not be branch-matched' + def test_sub_match(env, project, repo_a, repo_b, repo_c): """ Branch-matching should work on a subset of repositories """