From a15086a8a91570176e111a1c16e81b3c625f551a Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Fri, 27 Oct 2023 11:15:05 +0200 Subject: [PATCH] [FIX] runbot_merge: "not something we can merge" freeze error During the 17.0 freezeathon, the freeze wizard blew up with MergeError: merge-tree: {oid} - not something we can merge Turns out when freezes were moved to local (4d2c0f86e1f3e058f81aabad832a666739d980e3) I forgot to fetch the heads of the release and bump PRs into the local repo, so rebasing them atop their branch would fail because the local repository would just not find the object being rebased. I had missed that case in testing as well, but in fairness even if I had tried testing it I'd likely have missed it: implementation limitations (shortcuts) of dummy central mean it currently ignores what objects the client requests and bundles everything it can find associated with the repository (meaning it sends the entire network). This is not usually an issue because the test repos are pretty small, but it means the client can have objects they should not because they never requested them and might not even be supposed to be aware of their existence. Anyway solve by doing the obvious: fetch the heads of the release and bump PRs at the same time we update the branch being forked off. Also update the freeze tests to trigger the issue (by creating the release / bump PRs in different repos) and running the tests against github actual to make sure we can actually see them fail (correctly, the merge error we expect) not via errors in the test), and we do fix them. Fixes #821 --- .../models/project_freeze/__init__.py | 5 ++ runbot_merge/tests/test_multirepo.py | 57 ++++++++++--------- 2 files changed, 36 insertions(+), 26 deletions(-) diff --git a/runbot_merge/models/project_freeze/__init__.py b/runbot_merge/models/project_freeze/__init__.py index 7684249d..77cffc96 100644 --- a/runbot_merge/models/project_freeze/__init__.py +++ b/runbot_merge/models/project_freeze/__init__.py @@ -218,6 +218,11 @@ class FreezeWizard(models.Model): } for repo, copy in repos.items(): copy.fetch(git.source_url(repo, 'github'), '+refs/heads/*:refs/heads/*') + for pr in self.release_pr_ids.pr_id | self.bump_pr_ids.pr_id: + repos[pr.repository].fetch( + git.source_url(pr.repository, 'github'), + pr.head, + ) # prep new branch (via tmp refs) on every repo rel_heads: Dict[Repository, str] = {} diff --git a/runbot_merge/tests/test_multirepo.py b/runbot_merge/tests/test_multirepo.py index 59b3f932..3ba105ca 100644 --- a/runbot_merge/tests/test_multirepo.py +++ b/runbot_merge/tests/test_multirepo.py @@ -1171,20 +1171,19 @@ def test_freeze_complete(env, project, repo_a, repo_b, repo_c, users, config): ]}) r = w_id.action_freeze() assert r == w, "the freeze is not ready so the wizard should redirect to itself" - owner = repo_c.owner assert w_id.errors == f"""\ -* All release PRs must have the same label, found '{owner}:release-1.1, {owner}:whocares'. +* All release PRs must have the same label, found '{pr_rel_c.user}:release-1.1, {pr_other.user}:whocares'. * 2 required PRs not ready.""" w_id.release_pr_ids[-1].pr_id = release_prs[repo_c.name].id with repo_a: pr_required_a.post_comment('hansen r+', config['role_reviewer']['token']) - repo_a.post_status('apr', 'success', 'ci/runbot') - repo_a.post_status('apr', 'success', 'legal/cla') + repo_a.post_status(pr_required_a.head, 'success', 'ci/runbot') + repo_a.post_status(pr_required_a.head, 'success', 'legal/cla') with repo_c: pr_required_c.post_comment('hansen r+', config['role_reviewer']['token']) - repo_c.post_status('cpr', 'success', 'ci/runbot') - repo_c.post_status('cpr', 'success', 'legal/cla') + repo_c.post_status(pr_required_c.head, 'success', 'ci/runbot') + repo_c.post_status(pr_required_c.head, 'success', 'legal/cla') env.run_crons() for repo in [repo_a, repo_b, repo_c]: @@ -1275,45 +1274,51 @@ def setup_mess(repo_a, repo_b, repo_c): ref='heads/1.0' ) master_heads.extend(r.make_commits(root, Commit('other', tree={'f': '1'}), ref='heads/master')) + + a_fork = repo_a.fork() + b_fork = repo_b.fork() + c_fork = repo_c.fork() + assert a_fork.owner == b_fork.owner == c_fork.owner + owner = a_fork.owner # have 2 PRs required for the freeze - with repo_a: - repo_a.make_commits(master_heads[0], Commit('super important file', tree={'g': 'x'}), ref='heads/apr') - pr_required_a = repo_a.make_pr(target='master', head='apr') - with repo_c: - repo_c.make_commits(master_heads[2], Commit('update thing', tree={'f': '2'}), ref='heads/cpr') - pr_required_c = repo_c.make_pr(target='master', head='cpr') + with repo_a, a_fork: + a_fork.make_commits(master_heads[0], Commit('super important file', tree={'g': 'x'}), ref='heads/apr') + pr_required_a = repo_a.make_pr(target='master', head=f'{owner}:apr', title="xxx") + with repo_c, c_fork: + c_fork.make_commits(master_heads[2], Commit('update thing', tree={'f': '2'}), ref='heads/cpr') + pr_required_c = repo_c.make_pr(target='master', head=f'{owner}:cpr', title="yyy") # have 3 release PRs, only the first one updates the tree (version file) - with repo_a: - repo_a.make_commits( + with repo_a, a_fork: + a_fork.make_commits( master_heads[0], Commit('Release 1.1 (A)', tree={'version': '1.1'}), ref='heads/release-1.1' ) - pr_rel_a = repo_a.make_pr(target='master', head='release-1.1') - with repo_b: - repo_b.make_commits( + pr_rel_a = repo_a.make_pr(target='master', head=f'{owner}:release-1.1', title="zzz") + with repo_b, b_fork: + b_fork.make_commits( master_heads[1], Commit('Release 1.1 (B)', tree={'version': '1.1'}), ref='heads/release-1.1' ) - pr_rel_b = repo_b.make_pr(target='master', head='release-1.1') - with repo_c: - repo_c.make_commits(master_heads[2], Commit("Some change", tree={'a': '1'}), ref='heads/whocares') - pr_other = repo_c.make_pr(target='master', head='whocares') - repo_c.make_commits( + pr_rel_b = repo_b.make_pr(target='master', head=f'{owner}:release-1.1', title="000") + with repo_c, c_fork: + c_fork.make_commits(master_heads[2], Commit("Some change", tree={'a': '1'}), ref='heads/whocares') + pr_other = repo_c.make_pr(target='master', head=f'{owner}:whocares', title="111") + c_fork.make_commits( master_heads[2], Commit('Release 1.1 (C)', tree={'version': '1.1'}), ref='heads/release-1.1' ) - pr_rel_c = repo_c.make_pr(target='master', head='release-1.1') + pr_rel_c = repo_c.make_pr(target='master', head=f'{owner}:release-1.1', title="222") # have one bump PR on repo A - with repo_a: - repo_a.make_commits( + with repo_a, a_fork: + a_fork.make_commits( master_heads[0], Commit("Bump A", tree={'version': '1.2-alpha'}), ref='heads/bump-1.1', ) - pr_bump_a = repo_a.make_pr(target='master', head='bump-1.1') + pr_bump_a = repo_a.make_pr(target='master', head=f'{owner}:bump-1.1', title="333") return master_heads, (pr_required_a, None, pr_required_c), (pr_rel_a, pr_rel_b, pr_rel_c), pr_bump_a, pr_other def test_freeze_subset(env, project, repo_a, repo_b, repo_c, users, config):