mirror of
https://github.com/odoo/runbot.git
synced 2025-03-15 23:45:44 +07:00
[ADD] *: PR backport wizard
This is not a full user-driven backport thingie for now, just one admins can use to facilitate thing and debug issues with the system. May eventually graduate to a frontend feature. Fixes #925
This commit is contained in:
parent
ed1f084c4f
commit
cf4d162907
@ -202,7 +202,7 @@ class ForwardPortTasks(models.Model, Queue):
|
||||
# NOTE: ports the new source everywhere instead of porting each
|
||||
# PR to the next step as it does not *stop* on conflict
|
||||
repo = git.get_local(source.repository)
|
||||
conflict, head = source._create_fp_branch(repo, target)
|
||||
conflict, head = source._create_port_branch(repo, target, forward=True)
|
||||
repo.push(git.fw_url(pr.repository), f'{head}:refs/heads/{ref}')
|
||||
|
||||
remote_target = repository.fp_remote_target
|
||||
@ -302,7 +302,7 @@ class UpdateQueue(models.Model, Queue):
|
||||
return
|
||||
|
||||
repo = git.get_local(previous.repository)
|
||||
conflicts, new_head = previous._create_fp_branch(repo, child.target)
|
||||
conflicts, new_head = previous._create_port_branch(repo, child.target, forward=True)
|
||||
|
||||
if conflicts:
|
||||
_, out, err, _ = conflicts
|
||||
|
@ -34,6 +34,8 @@ from odoo.addons.runbot_merge import git, utils
|
||||
from odoo.addons.runbot_merge.models.pull_requests import Branch
|
||||
from odoo.addons.runbot_merge.models.stagings_create import Message
|
||||
|
||||
Conflict = tuple[str, str, str, list[str]]
|
||||
|
||||
DEFAULT_DELTA = dateutil.relativedelta.relativedelta(days=3)
|
||||
|
||||
_logger = logging.getLogger('odoo.addons.forwardport')
|
||||
@ -287,17 +289,29 @@ class PullRequests(models.Model):
|
||||
return sorted(commits, key=lambda c: idx[c['sha']])
|
||||
|
||||
|
||||
def _create_fp_branch(self, source, target_branch):
|
||||
def _create_port_branch(
|
||||
self,
|
||||
source: git.Repo,
|
||||
target_branch: Branch,
|
||||
*,
|
||||
forward: bool,
|
||||
) -> tuple[typing.Optional[Conflict], str]:
|
||||
""" Creates a forward-port for the current PR to ``target_branch`` under
|
||||
``fp_branch_name``.
|
||||
|
||||
:param target_branch: the branch to port forward to
|
||||
:rtype: (None | (str, str, str, list[commit]), Repo)
|
||||
:param source: the git repository to work with / in
|
||||
:param target_branch: the branch to port ``self`` to
|
||||
:param forward: whether this is a forward (``True``) or a back
|
||||
(``False``) port
|
||||
:returns: a conflict if one happened, and the head of the port branch
|
||||
(either a succcessful port of the entire `self`, or a conflict
|
||||
commit)
|
||||
"""
|
||||
logger = _logger.getChild(str(self.id))
|
||||
root = self.root_id
|
||||
logger.info(
|
||||
"Forward-porting %s (%s) to %s",
|
||||
"%s %s (%s) to %s",
|
||||
"Forward-porting" if forward else "Back-porting",
|
||||
self.display_name, root.display_name, target_branch.name
|
||||
)
|
||||
fetch = source.with_config(stdout=subprocess.PIPE, stderr=subprocess.STDOUT).fetch()
|
||||
|
117
forwardport/tests/test_backport.py
Normal file
117
forwardport/tests/test_backport.py
Normal file
@ -0,0 +1,117 @@
|
||||
from xmlrpc.client import Fault
|
||||
|
||||
import pytest
|
||||
|
||||
from utils import make_basic, Commit, to_pr, seen
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def repo(env, config, make_repo):
|
||||
repo, _ = make_basic(env, config, make_repo, statuses="default")
|
||||
return repo
|
||||
|
||||
@pytest.fixture
|
||||
def pr_id(env, repo, config):
|
||||
with repo:
|
||||
repo.make_commits('c', Commit("c", tree={'x': '1'}), ref='heads/aref')
|
||||
pr = repo.make_pr(target='c', head='aref')
|
||||
repo.post_status('aref', 'success')
|
||||
pr.post_comment('hansen r+', config['role_reviewer']['token'])
|
||||
env.run_crons()
|
||||
with repo:
|
||||
repo.post_status('staging.c', 'success')
|
||||
env.run_crons()
|
||||
pr_id = to_pr(env, pr)
|
||||
assert pr_id.merge_date
|
||||
return pr_id
|
||||
|
||||
@pytest.fixture
|
||||
def backport_id(env, pr_id):
|
||||
action = pr_id.backport()
|
||||
backport_id = env[action['res_model']].browse([action['res_id']])
|
||||
assert backport_id._name == 'runbot_merge.pull_requests.backport'
|
||||
assert backport_id
|
||||
return backport_id
|
||||
|
||||
def test_golden_path(env, repo, config, pr_id, backport_id, users):
|
||||
branch_a, branch_b, _branch_c = env['runbot_merge.branch'].search([], order='name')
|
||||
backport_id.target = branch_a.id
|
||||
act2 = backport_id.action_apply()
|
||||
env.run_crons() # run cron to update labels
|
||||
|
||||
_, bp_id = env['runbot_merge.pull_requests'].search([], order='number')
|
||||
assert bp_id.limit_id == branch_b
|
||||
assert bp_id._name == act2['res_model']
|
||||
assert bp_id.id == act2['res_id']
|
||||
bp_head = repo.commit(bp_id.head)
|
||||
assert repo.read_tree(bp_head) == {
|
||||
'f': 'e',
|
||||
'x': '1',
|
||||
}
|
||||
assert bp_head.message == f"""c
|
||||
|
||||
X-original-commit: {pr_id.head}\
|
||||
"""
|
||||
assert bp_id.message == f"[Backport] c\n\nBackport of {pr_id.display_name}"
|
||||
assert repo.get_pr(bp_id.number).labels == {"backport"}
|
||||
|
||||
# check that the backport can actually be merged and forward-ports successfully...
|
||||
with repo:
|
||||
repo.post_status(bp_id.head, 'success')
|
||||
repo.get_pr(bp_id.number).post_comment("hansen r+", config['role_reviewer']['token'])
|
||||
env.run_crons()
|
||||
with repo:
|
||||
repo.post_status('staging.a', 'success')
|
||||
env.run_crons()
|
||||
_pr, _backport, fw_id = env['runbot_merge.pull_requests'].search([], order='number')
|
||||
fw_pr = repo.get_pr(fw_id.number)
|
||||
assert fw_pr.comments == [
|
||||
seen(env, fw_pr, users),
|
||||
(users['user'], '''\
|
||||
@{user} @{reviewer} this PR targets b and is the last of the forward-port chain.
|
||||
|
||||
To merge the full chain, use
|
||||
> @hansen r+
|
||||
|
||||
More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
|
||||
'''.format_map(users)),
|
||||
]
|
||||
|
||||
def test_conflict(env, repo, config, backport_id):
|
||||
with repo:
|
||||
repo.make_commits('a', Commit('conflict', tree={'x': '-1'}), ref='heads/a', make=False)
|
||||
|
||||
branch_a, _branch_b, _branch_c = env['runbot_merge.branch'].search([], order='name')
|
||||
backport_id.target = branch_a.id
|
||||
with pytest.raises(Fault) as exc:
|
||||
backport_id.action_apply()
|
||||
assert exc.value.faultString == """\
|
||||
backport conflict:
|
||||
|
||||
Auto-merging x
|
||||
CONFLICT (add/add): Merge conflict in x
|
||||
"""
|
||||
|
||||
def test_target_error(env, config, backport_id):
|
||||
branch_a, _branch_b, branch_c = env['runbot_merge.branch'].search([], order='name')
|
||||
with pytest.raises(Fault) as exc:
|
||||
backport_id.action_apply()
|
||||
assert exc.value.faultString == "A backport needs a backport target"
|
||||
|
||||
backport_id.target = branch_c.id
|
||||
with pytest.raises(Fault) as exc:
|
||||
backport_id.action_apply()
|
||||
assert exc.value.faultString == "The backport branch needs to be before the source's branch (got 'c' and 'c')"
|
||||
|
||||
backport_id.target = branch_a.id
|
||||
backport_id.action_apply()
|
||||
|
||||
@pytest.mark.skip(
|
||||
reason="Currently no way to make just the PR creation fail, swapping the "
|
||||
"fp_github_token for an invalid one breaks git itself"
|
||||
)
|
||||
def test_pr_fail(env, config, repo, pr_id, backport_id):
|
||||
backport_id.target = env['runbot_merge.branch'].search([], order='name', limit=1).id
|
||||
with pytest.raises(Fault) as exc:
|
||||
backport_id.action_apply()
|
||||
assert exc.value.faultString == 'Backport PR creation failure: '
|
@ -8,5 +8,6 @@ from . import patcher
|
||||
from . import project_freeze
|
||||
from . import stagings_create
|
||||
from . import staging_cancel
|
||||
from . import backport
|
||||
from . import events_sources
|
||||
from . import crons
|
||||
|
140
runbot_merge/models/backport/__init__.py
Normal file
140
runbot_merge/models/backport/__init__.py
Normal file
@ -0,0 +1,140 @@
|
||||
import logging
|
||||
import re
|
||||
import secrets
|
||||
import subprocess
|
||||
|
||||
import requests
|
||||
|
||||
from odoo import models, fields
|
||||
from odoo.exceptions import UserError
|
||||
|
||||
from ..batch import Batch
|
||||
from ..project import Project
|
||||
from ..pull_requests import Repository
|
||||
from ... import git
|
||||
|
||||
_logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class PullRequest(models.Model):
|
||||
_inherit = 'runbot_merge.pull_requests'
|
||||
|
||||
id: int
|
||||
display_name: str
|
||||
project: Project
|
||||
repository: Repository
|
||||
batch_id: Batch
|
||||
|
||||
def backport(self) -> dict:
|
||||
if len(self) != 1:
|
||||
raise UserError(f"Backporting works one PR at a time, got {len(self)}")
|
||||
|
||||
if len(self.batch_id.prs) > 1:
|
||||
raise UserError("Automatic backport of multi-pr batches is not currently supported")
|
||||
|
||||
if not self.project.fp_github_token:
|
||||
raise UserError(f"Can not backport {self.display_name}: no token on project {self.project.display_name}")
|
||||
|
||||
if not self.repository.fp_remote_target:
|
||||
raise UserError(f"Can not backport {self.display_name}: no remote on {self.project.display_name}")
|
||||
|
||||
w = self.env['runbot_merge.pull_requests.backport'].create({
|
||||
'pr_id': self.id,
|
||||
})
|
||||
return {
|
||||
'type': 'ir.actions.act_window',
|
||||
'name': f"Backport of {self.display_name}",
|
||||
'views': [('form', False)],
|
||||
'target': 'new',
|
||||
'res_model': w._name,
|
||||
'res_id': w.id,
|
||||
}
|
||||
|
||||
class PullRequestBackport(models.TransientModel):
|
||||
_name = 'runbot_merge.pull_requests.backport'
|
||||
_rec_name = 'pr_id'
|
||||
|
||||
pr_id = fields.Many2one('runbot_merge.pull_requests', required=True)
|
||||
target = fields.Many2one('runbot_merge.branch')
|
||||
|
||||
def action_apply(self) -> dict:
|
||||
if not self.target:
|
||||
raise UserError("A backport needs a backport target")
|
||||
|
||||
project = self.pr_id.project
|
||||
branches = project._forward_port_ordered().ids
|
||||
source = self.pr_id.source_id or self.pr_id
|
||||
source_idx = branches.index(source.target.id)
|
||||
if branches.index(self.target.id) >= source_idx:
|
||||
raise UserError(
|
||||
"The backport branch needs to be before the source's branch "
|
||||
f"(got {self.target.name!r} and {source.target.name!r})"
|
||||
)
|
||||
|
||||
_logger.info(
|
||||
"backporting %s (on %s) to %s",
|
||||
self.pr_id.display_name,
|
||||
self.pr_id.target.name,
|
||||
self.target.name,
|
||||
)
|
||||
|
||||
bp_branch = "%s-%s-%s-bp" % (
|
||||
self.target.name,
|
||||
self.pr_id.refname,
|
||||
secrets.token_urlsafe(3),
|
||||
)
|
||||
repo_id = self.pr_id.repository
|
||||
repo = git.get_local(repo_id)
|
||||
|
||||
old_map = self.pr_id.commits_map
|
||||
self.pr_id.commits_map = "{}"
|
||||
conflict, head = self.pr_id._create_port_branch(repo, self.target, forward=False)
|
||||
self.pr_id.commits_map = old_map
|
||||
|
||||
if conflict:
|
||||
feedback = "\n".join(filter(None, conflict[1:3]))
|
||||
raise UserError(f"backport conflict:\n\n{feedback}")
|
||||
repo.push(git.fw_url(repo_id), f"{head}:refs/heads/{bp_branch}")
|
||||
|
||||
self.env.cr.execute('LOCK runbot_merge_pull_requests IN SHARE MODE')
|
||||
|
||||
owner, _repo = repo_id.fp_remote_target.split('/', 1)
|
||||
message = source.message + f"\n\nBackport of {self.pr_id.display_name}"
|
||||
title, body = re.fullmatch(r'(?P<title>[^\n]+)\n*(?P<body>.*)', message, flags=re.DOTALL).groups()
|
||||
|
||||
r = requests.post(
|
||||
f'https://api.github.com/repos/{repo_id.name}/pulls',
|
||||
headers={'Authorization': f'token {project.fp_github_token}'},
|
||||
json={
|
||||
'base': self.target.name,
|
||||
'head': f'{owner}:{bp_branch}',
|
||||
'title': '[Backport]' + ('' if title[0] == '[' else ' ') + title,
|
||||
'body': body
|
||||
}
|
||||
)
|
||||
if not r.ok:
|
||||
raise UserError(f"Backport PR creation failure: {r.text}")
|
||||
|
||||
backport = self.env['runbot_merge.pull_requests']._from_gh(r.json())
|
||||
_logger.info("Created backport %s for %s", backport.display_name, self.pr_id.display_name)
|
||||
|
||||
backport.write({
|
||||
'merge_method': self.pr_id.merge_method,
|
||||
# the backport's own forwardport should stop right before the
|
||||
# original PR by default
|
||||
'limit_id': branches[source_idx - 1],
|
||||
})
|
||||
self.env['runbot_merge.pull_requests.tagging'].create({
|
||||
'repository': repo_id.id,
|
||||
'pull_request': backport.number,
|
||||
'tags_add': ['backport'],
|
||||
})
|
||||
# scheduling fp followup probably doesn't make sense since we don't copy the fw_policy...
|
||||
|
||||
return {
|
||||
'type': 'ir.actions.act_window',
|
||||
'name': "new backport",
|
||||
'views': [('form', False)],
|
||||
'res_model': backport._name,
|
||||
'res_id': backport.id,
|
||||
}
|
@ -326,7 +326,7 @@ class Batch(models.Model):
|
||||
conflicts = {}
|
||||
for pr in prs:
|
||||
repo = git.get_local(pr.repository)
|
||||
conflicts[pr], head = pr._create_fp_branch(repo, target)
|
||||
conflicts[pr], head = pr._create_port_branch(repo, target, forward=True)
|
||||
|
||||
repo.push(git.fw_url(pr.repository), f"{head}:refs/heads/{new_branch}")
|
||||
|
||||
|
@ -32,3 +32,4 @@ access_runbot_merge_review_rights_2,Users can see partners,model_res_partner_rev
|
||||
access_runbot_merge_review_override_2,Users can see partners,model_res_partner_override,base.group_user,1,0,0,0
|
||||
access_runbot_merge_pull_requests_feedback_template,access_runbot_merge_pull_requests_feedback_template,runbot_merge.model_runbot_merge_pull_requests_feedback_template,base.group_system,1,1,0,0
|
||||
access_runbot_merge_patch,Patcher access,runbot_merge.model_runbot_merge_patch,runbot_merge.group_patcher,1,1,1,0
|
||||
access_runbot_merge_backport_admin,Admin access to backport wizard,model_runbot_merge_pull_requests_backport,runbot_merge.group_admin,1,1,1,0
|
||||
|
|
Loading…
Reference in New Issue
Block a user