diff --git a/forwardport/models/__init__.py b/forwardport/models/__init__.py index f3fb02c7..96d53ab5 100644 --- a/forwardport/models/__init__.py +++ b/forwardport/models/__init__.py @@ -1,3 +1,4 @@ # -*- coding: utf-8 -*- from . import project +from . import project_freeze from . import forwardport diff --git a/forwardport/models/project_freeze.py b/forwardport/models/project_freeze.py new file mode 100644 index 00000000..8168e14a --- /dev/null +++ b/forwardport/models/project_freeze.py @@ -0,0 +1,24 @@ +from odoo import models + + +class FreezeWizard(models.Model): + """ Override freeze wizard to disable the forward port cron when one is + created (so there's a freeze ongoing) and re-enable it once all freezes are + done. + + If there ever is a case where we have lots of projects, + """ + _inherit = 'runbot_merge.project.freeze' + + def create(self, vals_list): + r = super().create(vals_list) + self.env.ref('forwardport.port_forward').active = False + return r + + def unlink(self): + r = super().unlink() + if not self.search_count([]): + self.env.ref('forwardport.port_forward').active = True + return r + + diff --git a/runbot_merge/__manifest__.py b/runbot_merge/__manifest__.py index 6092c4bd..e44f1830 100644 --- a/runbot_merge/__manifest__.py +++ b/runbot_merge/__manifest__.py @@ -9,6 +9,7 @@ 'data/merge_cron.xml', 'views/res_partner.xml', 'views/runbot_merge_project.xml', + 'models/project_freeze/views.xml', 'views/mergebot.xml', 'views/templates.xml', ], diff --git a/runbot_merge/models/__init__.py b/runbot_merge/models/__init__.py index 8e548f8f..b457f468 100644 --- a/runbot_merge/models/__init__.py +++ b/runbot_merge/models/__init__.py @@ -1,3 +1,4 @@ from . import res_partner from . import project from . import pull_requests +from . import project_freeze diff --git a/runbot_merge/models/project.py b/runbot_merge/models/project.py index cf25498f..dc83b8f4 100644 --- a/runbot_merge/models/project.py +++ b/runbot_merge/models/project.py @@ -42,6 +42,9 @@ class Project(models.Model): "will lead to webhook rejection. Should only use ASCII." ) + freeze_id = fields.Many2one('runbot_merge.project.freeze', compute='_compute_freeze') + freeze_reminder = fields.Text() + def _check_stagings(self, commit=False): for branch in self.search([]).mapped('branch_ids').filtered('active'): staging = branch.active_staging_id @@ -81,3 +84,37 @@ class Project(models.Model): LIMIT 1 """, (self.id, name)) return bool(self.env.cr.rowcount) + + def _next_freeze(self): + prev = self.branch_ids[1:2].name + if not prev: + return None + + m = re.search(r'(\d+)(?:\.(\d+))?$', prev) + if m: + return "%s.%d" % (m[1], (int(m[2] or 0) + 1)) + else: + return f'post-{prev}' + + def _compute_freeze(self): + freezes = { + f.project_id.id: f.id + for f in self.env['runbot_merge.project.freeze'].search([('project_id', 'in', self.ids)]) + } + print(freezes) + for project in self: + project.freeze_id = freezes.get(project.id) or False + + def action_prepare_freeze(self): + """ Initialises the freeze wizard and returns the corresponding action. + """ + self.check_access_rights('write') + self.check_access_rule('write') + Freeze = self.env['runbot_merge.project.freeze'].sudo() + + 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] + }) + return w.action_freeze() diff --git a/runbot_merge/models/project_freeze/__init__.py b/runbot_merge/models/project_freeze/__init__.py new file mode 100644 index 00000000..f795261d --- /dev/null +++ b/runbot_merge/models/project_freeze/__init__.py @@ -0,0 +1,198 @@ +import enum +import itertools +import logging +import time + +from odoo import models, fields, api +from odoo.exceptions import UserError + +_logger = logging.getLogger(__name__) +class FreezeWizard(models.Model): + _name = 'runbot_merge.project.freeze' + _description = "Wizard for freezing a project('s master)" + + project_id = fields.Many2one('runbot_merge.project', required=True) + branch_name = fields.Char(required=True, help="Name of the new branches to create") + release_pr_ids = fields.One2many( + 'runbot_merge.project.freeze.prs', 'wizard_id', + string="Release pull requests", + help="Pull requests used as tips for the freeze branches, one per repository" + ) + required_pr_ids = fields.Many2many( + 'runbot_merge.pull_requests', string="Required Pull Requests", + domain="[('state', 'not in', ('closed', 'merged'))]", + help="Pull requests which must have been merged before the freeze is allowed", + ) + errors = fields.Text(compute='_compute_errors') + + _sql_constraints = [ + ('unique_per_project', 'unique (project_id)', + "There should be only one ongoing freeze per project"), + ] + + @api.depends('release_pr_ids.pr_id.label', 'required_pr_ids.state') + def _compute_errors(self): + errors = [] + without = self.release_pr_ids.filtered(lambda p: not p.pr_id) + if without: + errors.append("* Every repository must have a release PR, missing release PRs for %s." % ', '.join( + p.repository_id.name for p in without + )) + + 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)) + + unready = sum(p.state not in ('closed', 'merged') for p in self.required_pr_ids) + if unready: + errors.append(f"* {unready} required PRs not ready.") + + self.errors = '\n'.join(errors) or False + + def action_cancel(self): + self.project_id.check_access_rights('write') + self.project_id.check_access_rule('write') + self.sudo().unlink() + + return {'type': 'ir.actions.act_window_close'} + + 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, + } + + # 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" + master, rest = project_id.branch_ids[0], project_id.branch_ids[1:] + seq = itertools.count(start=1) # start reseq at 1 + commands = [ + (1, master.id, {'sequence': next(seq)}), + (0, 0, { + 'name': self.branch_name, + 'sequence': next(seq), + }) + ] + for s, b in zip(seq, rest): + commands.append((1, b.id, {'sequence': s})) + project_id.branch_ids = commands + + # update release PRs to get merged on the newly created branch + new_branch = project_id.branch_ids - master - rest + self.release_pr_ids.mapped('pr_id').write({'target': new_branch.id, 'priority': 0}) + + # create new branch on every repository + errors = [] + repository = None + for repository in project_id.repo_ids: + gh = repository.github() + # annoyance: can't directly alias a ref to an other ref, need to + # resolve the "old" branch explicitely + prev = gh('GET', f'git/refs/heads/{master.name}') + if not prev.ok: + errors.append(f"Unable to resolve branch {master.name} of repository {repository.name} to a commit.") + break + new_branch = gh('POST', 'git/refs', json={ + 'ref': 'refs/heads/' + self.branch_name, + 'sha': prev.json()['object']['sha'], + }, check=False) + if not new_branch.ok: + err = new_branch.json()['message'] + errors.append(f"Unable to create branch {master.name} of repository {repository.name}: {err}.") + break + time.sleep(1) + + # 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: + break + + deletion = r.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) + raise UserError('\n'.join(errors)) + + # delete wizard + self.sudo().unlink() + # managed to create all the things, show reminder text (or close) + if project_id.freeze_reminder: + return { + 'type': 'ir.actions.act_window', + 'target': 'new', + 'name': f'Freeze reminder {project_id.name}', + 'view_mode': 'form', + 'res_model': project_id._name, + 'res_id': project_id.id, + 'view_id': self.env.ref('runbot_merge.project_freeze_reminder').id + } + + return {'type': 'ir.actions.act_window_close'} + +class ReleasePullRequest(models.Model): + _name = 'runbot_merge.project.freeze.prs' + _description = "links to pull requests used to \"cap\" freezes" + + wizard_id = fields.Many2one('runbot_merge.project.freeze', required=True, index=True, ondelete='cascade') + repository_id = fields.Many2one('runbot_merge.repository', required=True) + pr_id = fields.Many2one( + 'runbot_merge.pull_requests', + domain='[("repository", "=", repository_id), ("state", "not in", ("closed", "merged"))]', + string="Release Pull Request", + ) + + def write(self, vals): + # only the pr should be writeable after initial creation + assert 'wizard_id' not in vals + assert 'repository_id' not in vals + # and if the PR gets set, it should match the requested repository + if 'pr_id' in vals: + assert self.env['runbot_merge.pull_requests'].browse(vals['pr_id'])\ + .repository == self.repository_id + + return super().write(vals) + +@enum.unique +class Colors(enum.IntEnum): + No = 0 + Red = 1 + Orange = 2 + Yellow = 3 + LightBlue = 4 + DarkPurple = 5 + Salmon = 6 + MediumBlue = 7 + DarkBlue = 8 + Fuchsia = 9 + Green = 10 + Purple = 11 + +STATE_COLORMAP = { + 'opened': Colors.No, + 'closed': Colors.Orange, + 'validated': Colors.No, + 'approved': Colors.No, + 'ready': Colors.LightBlue, + 'merged': Colors.Green, + 'error': Colors.Red, +} +class PullRequestColor(models.Model): + _inherit = 'runbot_merge.pull_requests' + + state_color = fields.Integer(compute='_compute_state_color') + + @api.depends('state') + def _compute_state_color(self): + for p in self: + p.state_color = STATE_COLORMAP[p.state] diff --git a/runbot_merge/models/project_freeze/views.xml b/runbot_merge/models/project_freeze/views.xml new file mode 100644 index 00000000..255be83d --- /dev/null +++ b/runbot_merge/models/project_freeze/views.xml @@ -0,0 +1,52 @@ + + + + + Freeze Wizard Configuration Screen + runbot_merge.project.freeze + +
+ + + + + + + + + + + + + + + + + + + + +
+ + + diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index c83bcff6..c0eae29d 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -1851,14 +1851,10 @@ class Stagings(models.Model): self, prs ) prs.write({'state': 'merged'}) + pseudobranch = None - if self.target == project.branch_ids[:1] and project.branch_ids[1:2]: - prev = project.branch_ids[1:2].name - m = re.search(r'(\d+)(?:\.(\d+))?$', prev) - if m: - pseudobranch = "%s.%d" % (m[1], (int(m[2] or 0) + 1)) - else: - pseudobranch = 'post-' + prev + if self.target == project.branch_ids[:1]: + pseudobranch = project._next_freeze() for pr in prs: self.env['runbot_merge.pull_requests.feedback'].create({ diff --git a/runbot_merge/security/ir.model.access.csv b/runbot_merge/security/ir.model.access.csv index 42dc408c..5815aed8 100644 --- a/runbot_merge/security/ir.model.access.csv +++ b/runbot_merge/security/ir.model.access.csv @@ -1,5 +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_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/static/project_freeze/index.js b/runbot_merge/static/project_freeze/index.js new file mode 100644 index 00000000..a0568962 --- /dev/null +++ b/runbot_merge/static/project_freeze/index.js @@ -0,0 +1,62 @@ +odoo.define('runbot_merge.index', function (require) { +"use strict"; +const FormController = require('web.FormController'); +const FormView = require('web.FormView'); +const viewRegistry = require('web.view_registry'); + +/** + * Attept at a "smart" controller for the freeze wizard: keeps triggering + * onchange() on the form in order to try and update the error information, as + * some of the "errors" are not under direct operator control. Hopefully this + * allows the operator to just keep the wizard open and wait until the error + * messages disappear so they can proceed. + */ +const FreezeController = FormController.extend({ + async _checkState() { + const record = this.model.get(this.handle) + const requiredPrIds = record.data.required_pr_ids; + + // we're inside the form's mutex, so can use `_applyChange` directly + const changed = await this.model._applyChange(this.handle, { + required_pr_ids: { + operation: 'REPLACE_WITH', + ids: requiredPrIds.res_ids, + } + }); + // not sure why we need to wait for the round *after* the error update + // notification, but even debouncing the rest of the method is not + // sufficient (so it's not just a problem of being behind the mutex, + // there's something wonky going on) + if (!this._updateNext) { + this._updateNext = changed.includes('errors'); + return; + } + + this._updateNext = false; + for(const p of requiredPrIds.data) { + this.renderer.updateState(p.id, {fieldNames: ['state_color']}); + } + this.renderer.updateState(record, {fieldNames: ['errors', 'required_pr_ids']}); + }, + /** + * @override + */ + async start(...args) { + const checker = async () => { + if (this.isDestroyed()) { return; } + await this.model.mutex.exec(() => this._checkState()); + setTimeout(checker, 1000); + }; + const started = await this._super(...args); + const _ = checker(); + return started; + }, +}); + +viewRegistry.add('freeze_wizard', FormView.extend({ + config: Object.assign({}, FormView.prototype.config, { + Controller: FreezeController, + }) +})); +}); + diff --git a/runbot_merge/tests/test_multirepo.py b/runbot_merge/tests/test_multirepo.py index 8b1e8f02..f2aaf471 100644 --- a/runbot_merge/tests/test_multirepo.py +++ b/runbot_merge/tests/test_multirepo.py @@ -1074,3 +1074,154 @@ def test_multi_project(env, make_repo, setreviewers, users, config, assert pr2.comments == [ (users['user'], f'[Pull request status dashboard]({pr2_id.url}).'), ] + +def test_freeze_complete(env, project, repo_a, repo_b, repo_c, users, config): + """ Tests the freeze wizard feature (aside from the UI): + + * have a project with 3 repos, and two branches (1.0 and master) each + * have 2 PRs required for the freeze + * prep 3 freeze PRs + * trigger the freeze wizard + * trigger it again (check that the same object is returned, there should + only be one freeze per project at a time) + * configure the freeze + * check that it doesn't go through + * merge required PRs + * check that freeze goes through + * check that reminder is shown + * check that new branches are created w/ correct parent & commit info + """ + project.freeze_reminder = "Don't forget to like and subscribe" + + # 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')) + + # 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') + 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') + pr_required_c = repo_c.make_pr(target='master', head='cpr') + + # have 3 release PRs, only the first one updates the tree (version file) + with repo_a: + repo_a.make_commits( + masters[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( + masters[1], + Commit('Release 1.1 (B)', tree=None), + 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( + masters[2], + Commit('Release 1.1 (C)', tree=None), + ref='heads/release-1.1' + ) + pr_rel_c = repo_c.make_pr(target='master', head='release-1.1') + env.run_crons() # process the PRs + + release_prs = { + repo_a.name: to_pr(env, pr_rel_a), + 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" + + 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" + assert len(w_id.release_pr_ids) == len(project.repo_ids), \ + "should ask for a many release PRs as we have repositories" + + # configure required PRs + w_id.required_pr_ids = (to_pr(env, pr_required_a) | to_pr(env, pr_required_c)).ids + # configure releases + for r in w_id.release_pr_ids: + r.pr_id = release_prs[r.repository_id.name].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." + + 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') + 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') + env.run_crons() + + for repo in [repo_a, repo_b, repo_c]: + with repo: + repo.post_status('staging.master', 'success', 'ci/runbot') + repo.post_status('staging.master', 'success', 'legal/cla') + env.run_crons() + + assert to_pr(env, pr_required_a).state == 'merged' + assert to_pr(env, pr_required_c).state == 'merged' + + assert not w_id.errors + + r = w_id.action_freeze() + # check that the wizard was deleted + assert not w_id.exists() + # check that the wizard pops out a reminder dialog (kinda) + assert r['res_model'] == 'runbot_merge.project' + assert r['res_id'] == project.id + + env.run_crons() # stage the release prs + for repo in [repo_a, repo_b, repo_c]: + with repo: + repo.post_status('staging.1.1', 'success', 'ci/runbot') + repo.post_status('staging.1.1', 'success', 'legal/cla') + env.run_crons() # get the release prs merged + for pr_id in release_prs.values(): + assert pr_id.target.name == '1.1' + assert pr_id.state == 'merged' + + c_a = repo_a.commit('1.1') + assert c_a.message.startswith('Release 1.1 (A)') + assert repo_a.read_tree(c_a) == { + 'f': '1', # from master + 'g': 'x', # from required pr + 'version': '1.1', # from release commit + } + c_a_parent = repo_a.commit(c_a.parents[0]) + assert c_a_parent.message.startswith('super important file') + assert c_a_parent.parents[0] == masters[0] + + c_b = repo_b.commit('1.1') + assert c_b.message.startswith('Release 1.1 (B)') + assert repo_b.read_tree(c_b) == {'f': '1', 'version': ''} + assert c_b.parents[0] == masters[1] + + c_c = repo_c.commit('1.1') + 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] diff --git a/runbot_merge/views/runbot_merge_project.xml b/runbot_merge/views/runbot_merge_project.xml index ee688756..2f670ed3 100644 --- a/runbot_merge/views/runbot_merge_project.xml +++ b/runbot_merge/views/runbot_merge_project.xml @@ -4,8 +4,14 @@ runbot_merge.project
+
-
@@ -27,6 +33,15 @@ + + + + + + + @@ -48,4 +63,16 @@ + + + Project Form + runbot_merge.project + +
+ + + +
+ +