[ADD] runbot_merge: ~~tree~~ freeze wizard

Provides a less manual interface for creating the freeze:

* takes the name of the branch to create
* takes any number of PRs which must be part of the freeze
* takes PRs representing the HEADs of the new branches

Then essentially takes care of the test.

Implementation of the actual wizard is not trivial but fairly
straightforward and linear, biggest issue is not being able to
`project_id.branch_ids[1]` to get the new branch, not sure why but it
seems to ignore the ordering, clearing the cache doens't fix it.

When creating the branches, add a sleep after each one for secondary
rate limiting purposes. Same when deleting branches.

Also the forwardbot has been updated to disable the forwardport cron
while a freeze is ongoing, this simplifies the freezing process.

Note: after recommendation of @aab-odoo, tried using `_applyChanges`
in `_checkState` but it simply did not work: the two relational fields
got completely frozen and were impossible to update, which is less
than ideal. Oh well, hopefully it works well enough like this for now.
This commit is contained in:
Xavier Morel 2021-11-12 16:04:34 +01:00
parent dbc001f841
commit 4da0f5df69
12 changed files with 560 additions and 8 deletions

View File

@ -1,3 +1,4 @@
# -*- coding: utf-8 -*-
from . import project
from . import project_freeze
from . import forwardport

View File

@ -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

View File

@ -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',
],

View File

@ -1,3 +1,4 @@
from . import res_partner
from . import project
from . import pull_requests
from . import project_freeze

View File

@ -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()

View File

@ -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]

View File

@ -0,0 +1,52 @@
<odoo>
<template id="runbot_merge_freeze_assets" inherit_id="web.assets_backend" active="True">
<xpath expr="." position="inside">
<script type="text/javascript" src="/runbot_merge/static/project_freeze/index.js"></script>
</xpath>
</template>
<record id="runbot_merge_project_freeze_form" model="ir.ui.view">
<field name="name">Freeze Wizard Configuration Screen</field>
<field name="model">runbot_merge.project.freeze</field>
<field name="arch" type="xml">
<form js_class="freeze_wizard">
<sheet>
<div class="alert alert-warning" role="alert"
attrs="{'invisible': [('errors', '=', False)]}">
<field name="errors" readonly="True"/>
</div>
<group>
<group colspan="2">
<field name="branch_name"/>
<field name="required_pr_ids" widget="many2many_tags"
options="{'color_field': 'state_color', 'no_create': True}"/>
</group>
</group>
<group>
<group colspan="2">
<field name="release_pr_ids" nolabel="1">
<tree editable="bottom" create="false" unlink="false">
<field name="repository_id" readonly="1"/>
<field name="pr_id" options="{'no_create': True}"/>
</tree>
</field>
</group>
</group>
<footer>
<!--
the operator should always be able to try freezing, in
case the smart form blows up or whatever, but change
the style of the button if the form has "no errors"
-->
<button string="Freeze" type="object" name="action_freeze"
class="btn-success" attrs="{'invisible': [('errors', '!=', False)]}"/>
<button string="Freeze" type="object" name="action_freeze"
class="btn-primary" attrs="{'invisible': [('errors', '=', False)]}"/>
<button string="Save &amp; Close" special="save"/>
<button string="Cancel" type="object" name="action_cancel" class="btn-warning"/>
</footer>
</sheet>
</form>
</field>
</record>
</odoo>

View File

@ -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({

View File

@ -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

1 id name model_id:id group_id:id perm_read perm_write perm_create perm_unlink
2 access_runbot_merge_project_admin Admin access to project model_runbot_merge_project runbot_merge.group_admin 1 1 1 1
3 access_runbot_merge_project_freeze Admin access to freeze wizard model_runbot_merge_project_freeze runbot_merge.group_admin 1 1 0 0
4 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
5 access_runbot_merge_repository_admin Admin access to repo model_runbot_merge_repository runbot_merge.group_admin 1 1 1 1
6 access_runbot_merge_repository_status_admin Admin access to repo statuses model_runbot_merge_repository_status runbot_merge.group_admin 1 1 1 1
7 access_runbot_merge_branch_admin Admin access to branches model_runbot_merge_branch runbot_merge.group_admin 1 1 1 1

View File

@ -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,
})
}));
});

View File

@ -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]

View File

@ -4,8 +4,14 @@
<field name="model">runbot_merge.project</field>
<field name="arch" type="xml">
<form>
<field name="freeze_id" invisible="1"/>
<header>
<button type="object" name="action_freeze" string="Freeze" class="oe_highlight"/>
<button type="object" name="action_prepare_freeze"
string="Freeze"
attrs="{'invisible': [('freeze_id', '!=', False)]}"/>
<button type="object" name="action_prepare_freeze"
string="View Freeze" class="oe_highlight"
attrs="{'invisible': [('freeze_id', '=', False)]}"/>
</header>
<sheet>
<div class="oe_title">
@ -27,6 +33,15 @@
</group>
</group>
<group class="oe_edit_only">
<group colspan="4">
<label for="freeze_reminder">
Reminder to show after freeze
</label>
<field colspan="4" name="freeze_reminder" nolabel="1"/>
</group>
</group>
<separator string="Repositories"/>
<field name="repo_ids">
<tree>
@ -48,4 +63,16 @@
</form>
</field>
</record>
<record id="project_freeze_reminder" model="ir.ui.view">
<field name="name">Project Form</field>
<field name="model">runbot_merge.project</field>
<field name="arch" type="xml">
<form>
<sheet>
<field name="freeze_reminder" nolabel="1" readonly="1"/>
</sheet>
</form>
</field>
</record>
</odoo>