diff --git a/forwardport/models/forwardport.py b/forwardport/models/forwardport.py index 9440c389..e8eca387 100644 --- a/forwardport/models/forwardport.py +++ b/forwardport/models/forwardport.py @@ -34,7 +34,7 @@ class Queue: def _search_domain(self): return [] -class BatchQueue(models.Model, Queue): +class ForwardPortTasks(models.Model, Queue): _name = 'forwardport.batches' _description = 'batches which got merged and are candidates for forward-porting' diff --git a/forwardport/models/project.py b/forwardport/models/project.py index a1d939a5..49f3e202 100644 --- a/forwardport/models/project.py +++ b/forwardport/models/project.py @@ -176,7 +176,7 @@ class Branch(models.Model): _inherit = 'runbot_merge.branch' fp_sequence = fields.Integer(default=50) - fp_target = fields.Boolean(default=False) + fp_target = fields.Boolean(default=True) fp_enabled = fields.Boolean(compute='_compute_fp_enabled') @api.depends('active', 'fp_target') diff --git a/forwardport/models/project_freeze.py b/forwardport/models/project_freeze.py index 8168e14a..ddd9f222 100644 --- a/forwardport/models/project_freeze.py +++ b/forwardport/models/project_freeze.py @@ -21,4 +21,12 @@ class FreezeWizard(models.Model): self.env.ref('forwardport.port_forward').active = True return r - + def action_freeze(self): + # have to store wizard content as it's removed during freeze + project = self.project_id + branches_before = project.branch_ids + prs = self.mapped('release_pr_ids.pr_id') + r = super().action_freeze() + new_branch = project.branch_ids - branches_before + prs.write({'limit_id': new_branch.id}) + return r diff --git a/forwardport/tests/test_weird.py b/forwardport/tests/test_weird.py index 57915f4b..31c9e777 100644 --- a/forwardport/tests/test_weird.py +++ b/forwardport/tests/test_weird.py @@ -24,9 +24,9 @@ def make_basic(env, config, make_repo, *, fp_token, fp_remote): 'github_prefix': 'hansen', 'fp_github_token': fp_token and config['github']['token'], 'branch_ids': [ - (0, 0, {'name': 'a', 'fp_sequence': 2, 'fp_target': True}), - (0, 0, {'name': 'b', 'fp_sequence': 1, 'fp_target': True}), - (0, 0, {'name': 'c', 'fp_sequence': 0, 'fp_target': True}), + (0, 0, {'name': 'a', 'sequence': 2, 'fp_target': True}), + (0, 0, {'name': 'b', 'sequence': 1, 'fp_target': True}), + (0, 0, {'name': 'c', 'sequence': 0, 'fp_target': True}), ], }) @@ -683,3 +683,47 @@ def test_approve_draft(env, config, make_repo, users): pr.post_comment('hansen r+', config['role_reviewer']['token']) env.run_crons() assert pr_id.state == 'approved' + +def test_freeze(env, config, make_repo, users): + """Freeze: + + - should not forward-port the freeze PRs themselves + """ + project, prod, _ = make_basic(env, config, make_repo, fp_token=True, fp_remote=True) + # branches here are "a" (older), "b", and "c" (master) + with prod: + [root, _] = prod.make_commits( + None, + Commit('base', tree={'version': '', 'f': '0'}), + Commit('release 1.0', tree={'version': '1.0'}), + ref='heads/b' + ) + prod.make_commits(root, Commit('other', tree={'f': '1'}), ref='heads/c') + with prod: + prod.make_commits( + 'c', + Commit('Release 1.1', tree={'version': '1.1'}), + ref='heads/release-1.1' + ) + release = prod.make_pr(target='c', head='release-1.1') + env.run_crons() + + w = project.action_prepare_freeze() + assert w['res_model'] == 'runbot_merge.project.freeze' + w_id = env[w['res_model']].browse([w['res_id']]) + assert w_id.release_pr_ids.repository_id.name == prod.name + release_id = to_pr(env, release) + w_id.release_pr_ids.pr_id = release_id.id + + assert not w_id.errors + w_id.action_freeze() + env.run_crons() # stage freeze PRs + with prod: + prod.post_status('staging.post-b', 'success', 'ci/runbot') + prod.post_status('staging.post-b', 'success', 'legal/cla') + env.run_crons() + + assert release_id.state == 'merged' + assert not env['runbot_merge.pull_requests'].search([ + ('state', '!=', 'merged') + ]), "the release PRs should not be forward-ported" diff --git a/runbot_merge/__manifest__.py b/runbot_merge/__manifest__.py index e44f1830..8375a3d8 100644 --- a/runbot_merge/__manifest__.py +++ b/runbot_merge/__manifest__.py @@ -9,9 +9,9 @@ 'data/merge_cron.xml', 'views/res_partner.xml', 'views/runbot_merge_project.xml', - 'models/project_freeze/views.xml', 'views/mergebot.xml', 'views/templates.xml', + 'models/project_freeze/views.xml', ], 'post_load': 'enable_sentry', 'pre_init_hook': '_check_citext', diff --git a/runbot_merge/models/project.py b/runbot_merge/models/project.py index e69468d4..f7296dc8 100644 --- a/runbot_merge/models/project.py +++ b/runbot_merge/models/project.py @@ -114,6 +114,10 @@ class Project(models.Model): w = Freeze.search([('project_id', '=', self.id)]) or Freeze.create({ 'project_id': self.id, 'branch_name': self._next_freeze(), - 'release_pr_ids': [(0, 0, {'repository_id': repo.id}) for repo in self.repo_ids] + 'release_pr_ids': [ + (0, 0, {'repository_id': repo.id}) + for repo in self.repo_ids + if repo.freeze + ] }) - return w.action_freeze() + return w.action_open() diff --git a/runbot_merge/models/project_freeze/__init__.py b/runbot_merge/models/project_freeze/__init__.py index f795261d..07c26d35 100644 --- a/runbot_merge/models/project_freeze/__init__.py +++ b/runbot_merge/models/project_freeze/__init__.py @@ -41,7 +41,7 @@ class FreezeWizard(models.Model): labels = set(self.mapped('release_pr_ids.pr_id.label')) if len(labels) != 1: - errors.append("* All release PRs must have the same label, found %r." % ', '.join(labels)) + errors.append("* All release PRs must have the same label, found %r." % ', '.join(sorted(labels))) unready = sum(p.state not in ('closed', 'merged') for p in self.required_pr_ids) if unready: @@ -56,21 +56,24 @@ class FreezeWizard(models.Model): return {'type': 'ir.actions.act_window_close'} + def action_open(self): + return { + 'type': 'ir.actions.act_window', + 'target': 'new', + 'name': f'Freeze project {self.project_id.name}', + 'view_mode': 'form', + 'res_model': self._name, + 'res_id': self.id, + } + def action_freeze(self): """ Attempts to perform the freeze. """ - project_id = self.project_id # if there are still errors, reopen the wizard if self.errors: - return { - 'type': 'ir.actions.act_window', - 'target': 'new', - 'name': f'Freeze project {project_id.name}', - 'view_mode': 'form', - 'res_model': self._name, - 'res_id': self.id, - } + return self.action_open() + project_id = self.project_id # need to create the new branch, but at the same time resequence # everything so the new branch is the second one, just after the branch # it "forks" @@ -94,7 +97,8 @@ class FreezeWizard(models.Model): # create new branch on every repository errors = [] repository = None - for repository in project_id.repo_ids: + for rel in self.release_pr_ids: + repository = rel.repository_id gh = repository.github() # annoyance: can't directly alias a ref to an other ref, need to # resolve the "old" branch explicitely @@ -114,11 +118,11 @@ class FreezeWizard(models.Model): # if an error occurred during creation, try to clean up then raise error if errors: - for r in project_id.repo_ids: - if r == repository: + for r in self.release_pr_ids: + if r.repository_id == repository: break - deletion = r.github().delete(f'git/refs/heads/{self.branch_name}') + deletion = r.repository_id.github().delete(f'git/refs/heads/{self.branch_name}') if not deletion.ok: errors.append(f"Consequently unable to delete branch {self.branch_name} of repository {r.name}.") time.sleep(1) @@ -163,6 +167,11 @@ class ReleasePullRequest(models.Model): return super().write(vals) +class RepositoryFreeze(models.Model): + _inherit = 'runbot_merge.repository' + freeze = fields.Boolean(required=True, default=True, + help="Freeze this repository by default") + @enum.unique class Colors(enum.IntEnum): No = 0 diff --git a/runbot_merge/models/project_freeze/views.xml b/runbot_merge/models/project_freeze/views.xml index 255be83d..2e6ba031 100644 --- a/runbot_merge/models/project_freeze/views.xml +++ b/runbot_merge/models/project_freeze/views.xml @@ -25,9 +25,10 @@ - + - + @@ -49,4 +50,15 @@ + + + Add freeze field to repo form + runbot_merge.repository + + + + + + + diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index c0eae29d..c003d85d 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -531,6 +531,9 @@ class PullRequests(models.Model): url = fields.Char(compute='_compute_url') github_url = fields.Char(compute='_compute_url') + repo_name = fields.Char(related='repository.name') + message_title = fields.Char(compute='_compute_message_title') + @api.depends('repository.name', 'number') def _compute_url(self): base = werkzeug.urls.url_parse(self.env['ir.config_parameter'].sudo().get_param('web.base.url', 'http://localhost:8069')) @@ -540,15 +543,20 @@ class PullRequests(models.Model): pr.url = str(base.join(path)) pr.github_url = str(gh_base.join(path)) - @api.depends('repository.name', 'number') + @api.depends('message') + def _compute_message_title(self): + for pr in self: + pr.message_title = next(iter(pr.message.splitlines()), '') + + @api.depends('repository.name', 'number', 'message') def _compute_display_name(self): return super(PullRequests, self)._compute_display_name() def name_get(self): - return [ - (p.id, '%s#%d' % (p.repository.name, p.number)) - for p in self - ] + name_template = '%(repo_name)s#%(number)d' + if self.env.context.get('pr_include_title'): + name_template += ' (%(message_title)s)' + return [(p.id, name_template % p) for p in self] @api.model def name_search(self, name='', args=None, operator='ilike', limit=100): diff --git a/runbot_merge/security/ir.model.access.csv b/runbot_merge/security/ir.model.access.csv index 5815aed8..4bf9e0d2 100644 --- a/runbot_merge/security/ir.model.access.csv +++ b/runbot_merge/security/ir.model.access.csv @@ -1,7 +1,7 @@ id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink access_runbot_merge_project_admin,Admin access to project,model_runbot_merge_project,runbot_merge.group_admin,1,1,1,1 access_runbot_merge_project_freeze,Admin access to freeze wizard,model_runbot_merge_project_freeze,runbot_merge.group_admin,1,1,0,0 -access_runbot_merge_project_freeze_prs,Admin access to freeze wizard prs,model_runbot_merge_project_freeze_prs,runbot_merge.group_admin,1,1,0,0 +access_runbot_merge_project_freeze_prs,Admin access to freeze wizard prs,model_runbot_merge_project_freeze_prs,runbot_merge.group_admin,1,1,0,1 access_runbot_merge_repository_admin,Admin access to repo,model_runbot_merge_repository,runbot_merge.group_admin,1,1,1,1 access_runbot_merge_repository_status_admin,Admin access to repo statuses,model_runbot_merge_repository_status,runbot_merge.group_admin,1,1,1,1 access_runbot_merge_branch_admin,Admin access to branches,model_runbot_merge_branch,runbot_merge.group_admin,1,1,1,1 diff --git a/runbot_merge/tests/test_multirepo.py b/runbot_merge/tests/test_multirepo.py index f2aaf471..a51d99b7 100644 --- a/runbot_merge/tests/test_multirepo.py +++ b/runbot_merge/tests/test_multirepo.py @@ -1112,10 +1112,10 @@ def test_freeze_complete(env, project, repo_a, repo_b, repo_c, users, config): # have 2 PRs required for the freeze with repo_a: - repo_a.make_commits('master', Commit('super important file', tree={'g': 'x'}), ref='heads/apr') + repo_a.make_commits(masters[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', Commit('update thing', tree={'f': '2'}), ref='heads/cpr') + repo_c.make_commits(masters[2], Commit('update thing', tree={'f': '2'}), ref='heads/cpr') pr_required_c = repo_c.make_pr(target='master', head='cpr') # have 3 release PRs, only the first one updates the tree (version file) @@ -1134,6 +1134,8 @@ def test_freeze_complete(env, project, repo_a, repo_b, repo_c, users, config): ) pr_rel_b = repo_b.make_pr(target='master', head='release-1.1') with repo_c: + repo_c.make_commits(masters[2], Commit("Some change", tree={'a': '1'}), ref='heads/whocares') + pr_other = repo_c.make_pr(target='master', head='whocares') repo_c.make_commits( masters[2], Commit('Release 1.1 (C)', tree=None), @@ -1147,11 +1149,11 @@ def test_freeze_complete(env, project, repo_a, repo_b, repo_c, users, config): repo_b.name: to_pr(env, pr_rel_b), repo_c.name: to_pr(env, pr_rel_c), } - # trigger the ~~tree~~ freeze wizard w = project.action_prepare_freeze() w2 = project.action_prepare_freeze() assert w == w2, "each project should only have one freeze wizard active at a time" + assert w['res_model'] == 'runbot_merge.project.freeze' w_id = env[w['res_model']].browse([w['res_id']]) assert w_id.branch_name == '1.1', "check that the forking incremented the minor by 1" @@ -1163,9 +1165,14 @@ def test_freeze_complete(env, project, repo_a, repo_b, repo_c, users, config): # configure releases for r in w_id.release_pr_ids: r.pr_id = release_prs[r.repository_id.name].id + w_id.release_pr_ids[-1].pr_id = to_pr(env, pr_other).id r = w_id.action_freeze() assert r == w, "the freeze is not ready so the wizard should redirect to itself" - assert w_id.errors == "* 2 required PRs not ready." + owner = repo_c.owner + assert w_id.errors == f"""\ +* All release PRs must have the same label, found '{owner}:release-1.1, {owner}: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']) @@ -1188,6 +1195,14 @@ def test_freeze_complete(env, project, repo_a, repo_b, repo_c, users, config): assert not w_id.errors + # assume the wizard is closed, re-open it + w = project.action_prepare_freeze() + assert w['res_model'] == 'runbot_merge.project.freeze' + assert w['res_id'] == w_id.id, "check that we're still getting the old wizard" + w_id = env[w['res_model']].browse([w['res_id']]) + assert w_id.exists() + + # actually perform the freeze r = w_id.action_freeze() # check that the wizard was deleted assert not w_id.exists() @@ -1225,3 +1240,75 @@ def test_freeze_complete(env, project, repo_a, repo_b, repo_c, users, config): assert c_c.message.startswith('Release 1.1 (C)') assert repo_c.read_tree(c_c) == {'f': '2', 'version': ''} assert repo_c.commit(c_c.parents[0]).parents[0] == masters[2] + + +def test_freeze_subset(env, project, repo_a, repo_b, repo_c, users, config): + """It should be possible to only freeze a subset of a project when e.g. one + of the repository is managed differently than the rest and has + non-synchronous releases. + + - it should be possible to mark repositories as non-freezed (just opted out + of the entire thing), in which case no freeze PRs should be asked of them + - it should be possible to remove repositories from the freeze wizard + - repositories which are not in the freeze wizard should just not be frozen + + To do things correctly that should probably match with the branch filters + and stuff, but that's a configuration concern. + """ + # have a project with 3 repos, and two branches (1.0 and master) + project.branch_ids = [ + (1, project.branch_ids.id, {'sequence': 1}), + (0, 0, {'name': '1.0', 'sequence': 2}), + ] + + masters = [] + for r in [repo_a, repo_b, repo_c]: + with r: + [root, _] = r.make_commits( + None, + Commit('base', tree={'version': '', 'f': '0'}), + Commit('release 1.0', tree={'version': '1.0'} if r is repo_a else None), + ref='heads/1.0' + ) + masters.extend(r.make_commits(root, Commit('other', tree={'f': '1'}), ref='heads/master')) + + with repo_a: + repo_a.make_commits( + masters[0], + Commit('Release 1.1', tree={'version': '1.1'}), + ref='heads/release-1.1' + ) + pr_rel_a = repo_a.make_pr(target='master', head='release-1.1') + + # the third repository we opt out of freezing + project.repo_ids.filtered(lambda r: r.name == repo_c.name).freeze = False + env.run_crons() # process the PRs + + # open the freeze wizard + w = project.action_prepare_freeze() + w_id = env[w['res_model']].browse([w['res_id']]) + # check that there are only rels for repos A and B + assert w_id.mapped('release_pr_ids.repository_id.name') == [repo_a.name, repo_b.name] + # remove B from the set + b_id = w_id.release_pr_ids.filtered(lambda r: r.repository_id.name == repo_b.name) + w_id.write({'release_pr_ids': [(3, b_id.id, 0)]}) + assert len(w_id.release_pr_ids) == 1 + # set lone release PR + w_id.release_pr_ids.pr_id = to_pr(env, pr_rel_a).id + assert not w_id.errors + + w_id.action_freeze() + assert not w_id.exists() + + assert repo_a.commit('1.1'), "should have created branch in repo A" + try: + repo_b.commit('1.1') + pytest.fail("should *not* have created branch in repo B") + except AssertionError: + ... + try: + repo_c.commit('1.1') + pytest.fail("should *not* have created branch in repo C") + except AssertionError: + ... + # can't stage because we (wilfully) don't have branches 1.1 in repos B and C