-
Notifications
You must be signed in to change notification settings - Fork 134
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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
- Loading branch information
Showing
7 changed files
with
280 additions
and
7 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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: ' |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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, | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters