diff --git a/forwardport/models/forwardport.py b/forwardport/models/forwardport.py index 54521621..a0604da7 100644 --- a/forwardport/models/forwardport.py +++ b/forwardport/models/forwardport.py @@ -161,94 +161,94 @@ class ForwardPortTasks(models.Model, Queue): # only then create the PR objects # TODO: maybe do that after making forward-port WC-less, so all the # branches can be pushed atomically at once - with contextlib.ExitStack() as s: - for descendant in self.batch_id.descendants(): - target = pr._find_next_target() - if target is None: - _logger.info("Will not forward-port %s: no next target", pr.display_name) - return + for descendant in self.batch_id.descendants(): + target = pr._find_next_target() + if target is None: + _logger.info("Will not forward-port %s: no next target", pr.display_name) + return - if PullRequests.search_count([ - ('source_id', '=', source.id), - ('target', '=', target.id), - ('state', 'not in', ('closed', 'merged')), - ]): - _logger.warning("Will not forward-port %s: already ported", pr.display_name) - return + if PullRequests.search_count([ + ('source_id', '=', source.id), + ('target', '=', target.id), + ('state', 'not in', ('closed', 'merged')), + ]): + _logger.warning("Will not forward-port %s: already ported", pr.display_name) + return - if target != descendant.target: - self.env['runbot_merge.pull_requests.feedback'].create({ - 'repository': repository.id, - 'pull_request': source.id, - 'token_field': 'fp_github_token', - 'message': """\ + if target != descendant.target: + self.env['runbot_merge.pull_requests.feedback'].create({ + 'repository': repository.id, + 'pull_request': source.id, + 'token_field': 'fp_github_token', + 'message': """\ {pr.ping}unable to port this PR forwards due to inconsistency: goes from \ {pr.target.name} to {next_target.name} but {batch} ({batch_prs}) targets \ {batch.target.name}. """.format(pr=pr, next_target=target, batch=descendant, batch_prs=', '.join(descendant.mapped('prs.display_name'))) - }) - return - - ref = descendant.prs[:1].refname - # NOTE: ports the new source everywhere instead of porting each - # PR to the next step as it does not *stop* on conflict - conflict, working_copy = source._create_fp_branch(target, ref, s) - working_copy.push('target', ref) - - remote_target = repository.fp_remote_target - owner, _ = remote_target.split('/', 1) - message = source.message + f"\n\nForward-Port-Of: {pr.display_name}" - - title, body = re.match(r'(?P[^\n]+)\n*(?P<body>.*)', message, flags=re.DOTALL).groups() - r = gh.post(f'https://api.github.com/repos/{pr.repository.name}/pulls', json={ - 'base': target.name, - 'head': f'{owner}:{ref}', - 'title': '[FW]' + (' ' if title[0] != '[' else '') + title, - 'body': body }) - if not r.ok: - _logger.warning("Failed to create forward-port PR for %s, deleting branches", pr.display_name) - # delete all the branches this should automatically close the - # PRs if we've created any. Using the API here is probably - # simpler than going through the working copies - d = gh.delete(f'https://api.github.com/repos/{remote_target}/git/refs/heads/{ref}') - if d.ok: - _logger.info("Deleting %s:%s=success", remote_target, ref) - else: - _logger.warning("Deleting %s:%s=%s", remote_target, ref, d.text) - raise RuntimeError(f"Forwardport failure: {pr.display_name} ({r.text})") + return - new_pr = PullRequests._from_gh(r.json()) - _logger.info("Created forward-port PR %s", new_pr) - new_pr.write({ - 'batch_id': descendant.id, # should already be set correctly but... - 'merge_method': pr.merge_method, - 'source_id': source.id, - # only link to previous PR of sequence if cherrypick passed - # FIXME: apply parenting of siblings? Apply parenting *to* siblings? - 'parent_id': pr.id if not conflict else False, - 'detach_reason': "{1}\n{2}".format(*conflict).strip() if conflict else None, - }) + ref = descendant.prs[:1].refname + # 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) + repo.push(git.fw_url(pr.repository), f'{head}:refs/heads/{ref}') - if conflict: - self.env.ref('runbot_merge.forwardport.failure.conflict')._send( - repository=pr.repository, - pull_request=pr.number, - token_field='fp_github_token', - format_args={'source': source, 'pr': pr, 'new': new_pr, 'footer': FOOTER}, - ) - new_pr._fp_conflict_feedback(pr, {pr: conflict}) + remote_target = repository.fp_remote_target + owner, _ = remote_target.split('/', 1) + message = source.message + f"\n\nForward-Port-Of: {pr.display_name}" - labels = ['forwardport'] - if conflict: - labels.append('conflict') - self.env['runbot_merge.pull_requests.tagging'].create({ - 'repository': new_pr.repository.id, - 'pull_request': new_pr.number, - 'tags_add': labels, - }) + title, body = re.match(r'(?P<title>[^\n]+)\n*(?P<body>.*)', message, flags=re.DOTALL).groups() + r = gh.post(f'https://api.github.com/repos/{pr.repository.name}/pulls', json={ + 'base': target.name, + 'head': f'{owner}:{ref}', + 'title': '[FW]' + (' ' if title[0] != '[' else '') + title, + 'body': body + }) + if not r.ok: + _logger.warning("Failed to create forward-port PR for %s, deleting branches", pr.display_name) + # delete all the branches this should automatically close the + # PRs if we've created any. Using the API here is probably + # simpler than going through the working copies + d = gh.delete(f'https://api.github.com/repos/{remote_target}/git/refs/heads/{ref}') + if d.ok: + _logger.info("Deleting %s:%s=success", remote_target, ref) + else: + _logger.warning("Deleting %s:%s=%s", remote_target, ref, d.text) + raise RuntimeError(f"Forwardport failure: {pr.display_name} ({r.text})") - pr = new_pr + new_pr = PullRequests._from_gh(r.json()) + _logger.info("Created forward-port PR %s", new_pr) + new_pr.write({ + 'batch_id': descendant.id, # should already be set correctly but... + 'merge_method': pr.merge_method, + 'source_id': source.id, + # only link to previous PR of sequence if cherrypick passed + # FIXME: apply parenting of siblings? Apply parenting *to* siblings? + 'parent_id': pr.id if not conflict else False, + 'detach_reason': "{1}\n{2}".format(*conflict).strip() if conflict else None, + }) + + if conflict: + self.env.ref('runbot_merge.forwardport.failure.conflict')._send( + repository=pr.repository, + pull_request=pr.number, + token_field='fp_github_token', + format_args={'source': source, 'pr': pr, 'new': new_pr, 'footer': FOOTER}, + ) + new_pr._fp_conflict_feedback(pr, {pr: conflict}) + + labels = ['forwardport'] + if conflict: + labels.append('conflict') + self.env['runbot_merge.pull_requests.tagging'].create({ + 'repository': new_pr.repository.id, + 'pull_request': new_pr.number, + 'tags_add': labels, + }) + + pr = new_pr class UpdateQueue(models.Model, Queue): _name = 'forwardport.updates' @@ -286,8 +286,9 @@ class UpdateQueue(models.Model, Queue): ) return - conflicts, working_copy = previous._create_fp_branch( - child.target, child.refname, s) + repo = git.get_local(previous.repository) + conflicts, new_head = previous._create_fp_branch(repo, child.target) + if conflicts: _, out, err, _ = conflicts self.env.ref('runbot_merge.forwardport.updates.conflict.parent')._send( @@ -308,9 +309,8 @@ class UpdateQueue(models.Model, Queue): }, ) - new_head = working_copy.stdout().rev_parse(child.refname).stdout.decode().strip() - commits_count = int(working_copy.stdout().rev_list( - f'{child.target.name}..{child.refname}', + commits_count = int(repo.stdout().rev_list( + f'{child.target.name}..{new_head}', count=True ).stdout.decode().strip()) old_head = child.head @@ -320,16 +320,11 @@ class UpdateQueue(models.Model, Queue): # 'state': 'opened', 'squash': commits_count == 1, }) - # push the new head to the local cache: in some cases github - # doesn't propagate revisions fast enough so on the next loop we - # can't find the revision we just pushed - dummy_branch = str(uuid.uuid4()) - ref = git.get_local(previous.repository, 'fp_github') - working_copy.push(ref._directory, f'{new_head}:refs/heads/{dummy_branch}') - ref.branch('--delete', '--force', dummy_branch) # then update the child's branch to the new head - working_copy.push(f'--force-with-lease={child.refname}:{old_head}', - 'target', child.refname) + repo.push( + f'--force-with-lease={child.refname}:{old_head}', + git.fw_url(child.repository), + f"{new_head}:refs/heads/{child.refname}") # committing here means github could technically trigger its # webhook before sending a response, but committing before diff --git a/forwardport/models/project.py b/forwardport/models/project.py index 5c664c25..31e0ec9c 100644 --- a/forwardport/models/project.py +++ b/forwardport/models/project.py @@ -19,9 +19,7 @@ import json import logging import operator import subprocess -import tempfile import typing -from pathlib import Path import dateutil.relativedelta import requests @@ -30,7 +28,6 @@ from odoo import models, fields, api from odoo.exceptions import UserError from odoo.osv import expression from odoo.tools.misc import topological_sort, groupby -from odoo.tools.appdirs import user_cache_dir from odoo.addons.base.models.res_partner import Partner from odoo.addons.runbot_merge import git, utils from odoo.addons.runbot_merge.models.pull_requests import Branch @@ -289,17 +286,11 @@ class PullRequests(models.Model): return sorted(commits, key=lambda c: idx[c['sha']]) - def _create_fp_branch(self, target_branch, fp_branch_name, cleanup): + def _create_fp_branch(self, source, target_branch): """ Creates a forward-port for the current PR to ``target_branch`` under ``fp_branch_name``. :param target_branch: the branch to port forward to - :param fp_branch_name: the name of the branch to create the FP under - :param ExitStack cleanup: so the working directories can be cleaned up - :return: A pair of an optional conflict information and a repository. If - present the conflict information is composed of the hash of the - conflicting commit, the stderr and stdout of the failed - cherrypick and a list of all PR commit hashes :rtype: (None | (str, str, str, list[commit]), Repo) """ logger = _logger.getChild(str(self.id)) @@ -308,104 +299,52 @@ class PullRequests(models.Model): "Forward-porting %s (%s) to %s", self.display_name, root.display_name, target_branch.name ) - source = git.get_local(self.repository, 'fp_github') - r = source.with_config(stdout=subprocess.PIPE, stderr=subprocess.STDOUT).fetch() - logger.info("Updated cache repo %s:\n%s", source._directory, r.stdout.decode()) + tree = source.with_config(stdout=subprocess.PIPE, stderr=subprocess.STDOUT).fetch() + logger.info("Updated cache repo %s:\n%s", source._directory, tree.stdout.decode()) - logger.info("Create working copy...") - cache_dir = user_cache_dir('forwardport') - # PullRequest.display_name is `owner/repo#number`, so `owner` becomes a - # directory, `TemporaryDirectory` only creates the leaf, so we need to - # make sure `owner` exists in `cache_dir`. - Path(cache_dir, root.repository.name).parent.mkdir(parents=True, exist_ok=True) - working_copy = source.clone( - cleanup.enter_context( - tempfile.TemporaryDirectory( - prefix=f'{root.display_name}-to-{target_branch.name}', - dir=cache_dir)), - branch=target_branch.name - ) - - r = working_copy.with_config(stdout=subprocess.PIPE, stderr=subprocess.STDOUT) \ - .fetch(git.source_url(self.repository, 'fp_github'), root.head) + tree = source.with_config(stdout=subprocess.PIPE, stderr=subprocess.STDOUT) \ + .fetch(git.source_url(self.repository), root.head) logger.info( - "Fetched head of %s into %s:\n%s", + "Fetched head of %s (%s):\n%s", root.display_name, - working_copy._directory, - r.stdout.decode() + root.head, + tree.stdout.decode() ) - if working_copy.check(False).cat_file(e=root.head).returncode: + if source.check(False).cat_file(e=root.head).returncode: raise ForwardPortError( f"During forward port of {self.display_name}, unable to find " f"expected head of {root.display_name} ({root.head})" ) - project_id = self.repository.project_id - # add target remote - working_copy.remote( - 'add', 'target', - 'https://{p.fp_github_token}@github.com/{r.fp_remote_target}'.format( - r=self.repository, - p=project_id - ) - ) - logger.info("Create FP branch %s in %s", fp_branch_name, working_copy._directory) - working_copy.checkout(b=fp_branch_name) - try: - root._cherry_pick(working_copy) - return None, working_copy + return None, root._cherry_pick(source, target_branch.name) except CherrypickError as e: h, out, err, commits = e.args - # using git diff | git apply -3 to get the entire conflict set - # turns out to not work correctly: in case files have been moved - # / removed (which turns out to be a common source of conflicts - # when forward-porting) it'll just do nothing to the working copy - # so the "conflict commit" will be empty - # switch to a squashed-pr branch - working_copy.check(True).checkout('-bsquashed', root.head) # commits returns oldest first, so youngest (head) last head_commit = commits[-1]['commit'] to_tuple = operator.itemgetter('name', 'email') - def to_dict(term, vals): - return {'GIT_%s_NAME' % term: vals[0], 'GIT_%s_EMAIL' % term: vals[1], 'GIT_%s_DATE' % term: vals[2]} authors, committers = set(), set() - for c in (c['commit'] for c in commits): - authors.add(to_tuple(c['author'])) - committers.add(to_tuple(c['committer'])) - fp_authorship = (project_id.fp_github_name, '', '') + for commit in (c['commit'] for c in commits): + authors.add(to_tuple(commit['author'])) + committers.add(to_tuple(commit['committer'])) + fp_authorship = (self.repository.project_id.fp_github_name, '', '') author = fp_authorship if len(authors) != 1 \ else authors.pop() + (head_commit['author']['date'],) committer = fp_authorship if len(committers) != 1 \ else committers.pop() + (head_commit['committer']['date'],) - conf = working_copy.with_config(env={ - **to_dict('AUTHOR', author), - **to_dict('COMMITTER', committer), - 'GIT_COMMITTER_DATE': '', - }) - # squash to a single commit - conf.reset('--soft', commits[0]['parents'][0]['sha']) - conf.commit(a=True, message="temp") - squashed = conf.stdout().rev_parse('HEAD').stdout.strip().decode() - - # switch back to the PR branch - conf.checkout(fp_branch_name) - # cherry-pick the squashed commit to generate the conflict - conf.with_params( + conf = source.with_params( 'merge.renamelimit=0', 'merge.renames=copies', 'merge.conflictstyle=zdiff3' - ) \ - .with_config(check=False) \ - .cherry_pick(squashed, no_commit=True, strategy="ort") - status = conf.stdout().status(short=True, untracked_files='no').stdout.decode() - if err.strip(): - err = err.rstrip() + '\n----------\nstatus:\n' + status - else: - err = 'status:\n' + status + ).with_config(stdout=subprocess.PIPE, stderr=subprocess.PIPE) + tree = conf.with_config(check=False).merge_tree( + '--merge-base', commits[0]['parents'][0]['sha'], + target_branch.name, + root.head, + ) # if there was a single commit, reuse its message when committing # the conflict if len(commits) == 1: @@ -421,77 +360,107 @@ stderr: {err} """ - conf.with_config(input=str(msg).encode()) \ - .commit(all=True, allow_empty=True, file='-') + target_head = source.stdout().rev_parse(target_branch.name).stdout.decode().strip() + commit = conf.commit_tree( + tree=tree.stdout.decode().splitlines(keepends=False)[0], + parents=[target_head], + message=str(msg), + author=author, + committer=committer[:2], + ) + assert commit.returncode == 0,\ + f"commit failed\n\n{commit.stdout.decode()}\n\n{commit.stderr.decode}" + hh = commit.stdout.strip() - return (h, out, err, [c['sha'] for c in commits]), working_copy + return (h, out, err, [c['sha'] for c in commits]), hh - def _cherry_pick(self, working_copy): - """ Cherrypicks ``self`` into the working copy + def _cherry_pick(self, repo: git.Repo, branch: Branch) -> str: + """ Cherrypicks ``self`` into ``branch`` - :return: ``True`` if the cherrypick was successful, ``False`` otherwise + :return: the HEAD of the forward-port is successful + :raises CherrypickError: in case of conflict """ # <xxx>.cherrypick.<number> logger = _logger.getChild(str(self.id)).getChild('cherrypick') - # original head so we can reset - original_head = working_copy.stdout().rev_parse('HEAD').stdout.decode().strip() + # target's head + head = repo.stdout().rev_parse(branch).stdout.decode().strip() commits = self.commits() - logger.info("%s: copy %s commits to %s\n%s", self, len(commits), original_head, '\n'.join( - '- %s (%s)' % (c['sha'], c['commit']['message'].splitlines()[0]) - for c in commits - )) + logger.info( + "%s: copy %s commits to %s (%s)%s", + self, len(commits), branch, head, ''.join( + '\n- %s: %s' % (c['sha'], c['commit']['message'].splitlines()[0]) + for c in commits + ) + ) - conf_base = working_copy.with_params( + conf = repo.with_params( 'merge.renamelimit=0', 'merge.renames=copies', ).with_config( stdout=subprocess.PIPE, stderr=subprocess.PIPE, - check=False + check=False, ) for commit in commits: commit_sha = commit['sha'] - # config (global -c) or commit options don't really give access to - # setting dates - cm = commit['commit'] # get the "git" commit object rather than the "github" commit resource - env = { - 'GIT_AUTHOR_NAME': cm['author']['name'], - 'GIT_AUTHOR_EMAIL': cm['author']['email'], - 'GIT_AUTHOR_DATE': cm['author']['date'], - 'GIT_COMMITTER_NAME': cm['committer']['name'], - 'GIT_COMMITTER_EMAIL': cm['committer']['email'], - } - configured = working_copy.with_config(env=env) + # merge-tree is a bit stupid and gets confused when the options + # follow the parameters + r = conf.merge_tree('--merge-base', commit['parents'][0]['sha'], head, commit_sha) + new_tree = r.stdout.decode().splitlines(keepends=False)[0] + if r.returncode: + # For merge-tree the stdout on conflict is of the form + # + # oid of toplevel tree + # conflicted file info+ + # + # informational messages+ + # + # to match cherrypick we only want the informational messages, + # so strip everything else + r.stdout = r.stdout.split(b'\n\n')[-1] + else: + # By default cherry-pick fails if a non-empty commit becomes + # empty (--empty=stop), also it fails when cherrypicking already + # empty commits which I didn't think we prevented but clearly we + # do...? + parent_tree = conf.rev_parse(f'{head}^{{tree}}').stdout.decode().strip() + if parent_tree == new_tree: + r.returncode = 1 + r.stdout = f"You are currently cherry-picking commit {commit_sha}.".encode() + r.stderr = b"The previous cherry-pick is now empty, possibly due to conflict resolution." - conf = conf_base.with_config(env={**env, 'GIT_TRACE': 'true'}) - # first try with default / low renamelimit - r = conf.cherry_pick(commit_sha, strategy="ort") logger.debug("Cherry-picked %s: %s\n%s\n%s", commit_sha, r.returncode, r.stdout.decode(), _clean_rename(r.stderr.decode())) - - if r.returncode: # pick failed, reset and bail + if r.returncode: # pick failed, bail # try to log inflateInit: out of memory errors as warning, they - # seem to return the status code 128 + # seem to return the status code 128 (nb: may not work anymore + # with merge-tree, idk) logger.log( logging.WARNING if r.returncode == 128 else logging.INFO, "forward-port of %s (%s) failed at %s", self, self.display_name, commit_sha) - configured.reset('--hard', original_head) + raise CherrypickError( commit_sha, r.stdout.decode(), _clean_rename(r.stderr.decode()), commits ) + # get the "git" commit object rather than the "github" commit resource + cc = conf.commit_tree( + tree=new_tree, + parents=[head], + message=str(self._make_fp_message(commit)), + author=map_author(commit['commit']['author']), + committer=map_committer(commit['commit']['committer']), + ) + if cc.returncode: + raise CherrypickError(commit_sha, cc.stdout.decode(), cc.stderr.decode(), commits) - msg = self._make_fp_message(commit) + head = cc.stdout.strip() + logger.info('%s -> %s', commit_sha, head) - # replace existing commit message with massaged one - configured \ - .with_config(input=str(msg).encode())\ - .commit(amend=True, file='-') - result = configured.stdout().rev_parse('HEAD').stdout.decode() - logger.info('%s: success -> %s', commit_sha, result) + return head def _make_fp_message(self, commit): cmap = json.loads(self.commits_map) @@ -541,6 +510,11 @@ stderr: format_args={'pr': pr, 'source': source}, ) + +map_author = operator.itemgetter('name', 'email', 'date') +map_committer = operator.itemgetter('name', 'email') + + class Stagings(models.Model): _inherit = 'runbot_merge.stagings' diff --git a/forwardport/tests/test_batches.py b/forwardport/tests/test_batches.py index b0b7155a..2ecab8b6 100644 --- a/forwardport/tests/test_batches.py +++ b/forwardport/tests/test_batches.py @@ -381,7 +381,7 @@ def test_add_to_forward_port_conflict(env, config, make_repo, users): assert pr2_c.comments == [ seen(env, pr2_c, users), # should have conflicts - (users['user'], matches("""@{user} cherrypicking of pull request {previous.display_name} failed. + (users['user'], """@{user} cherrypicking of pull request {previous.display_name} failed. stdout: ``` @@ -390,11 +390,6 @@ CONFLICT (add/add): Merge conflict in b ``` -stderr: -``` -$$ -``` - Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?). In the former case, you may want to edit this PR message as well. @@ -402,5 +397,5 @@ In the former case, you may want to edit this PR message as well. :warning: after resolving this conflict, you will need to merge it via @{project.github_prefix}. More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port -""".format(project=project, previous=pr2_b_id, **users))) +""".format(project=project, previous=pr2_b_id, **users)) ] diff --git a/forwardport/tests/test_conflicts.py b/forwardport/tests/test_conflicts.py index 030e534b..7011b1b3 100644 --- a/forwardport/tests/test_conflicts.py +++ b/forwardport/tests/test_conflicts.py @@ -62,17 +62,17 @@ def test_conflict(env, config, make_repo, users): assert prod.read_tree(c) == { 'f': 'c', 'g': 'a', - 'h': matches('''<<<\x3c<<< HEAD + 'h': matches('''<<<\x3c<<< $$ a -||||||| parent of $$ (temp) +||||||| $$ ======= xxx ->>>\x3e>>> $$ (temp) +>>>\x3e>>> $$ '''), } assert prc.comments == [ seen(env, prc, users), - (users['user'], matches( + (users['user'], f'''@{users['user']} @{users['reviewer']} cherrypicking of pull request {pra_id.display_name} failed. stdout: @@ -82,11 +82,6 @@ CONFLICT (add/add): Merge conflict in h ``` -stderr: -``` -$$ -``` - Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?). In the former case, you may want to edit this PR message as well. @@ -94,7 +89,7 @@ In the former case, you may want to edit this PR message as well. :warning: after resolving this conflict, you will need to merge it via @{project.github_prefix}. More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port -''')) +''') ] prb = prod.get_pr(prb_id.number) diff --git a/forwardport/tests/test_updates.py b/forwardport/tests/test_updates.py index d68c6c92..b13cd16a 100644 --- a/forwardport/tests/test_updates.py +++ b/forwardport/tests/test_updates.py @@ -445,12 +445,12 @@ def test_subsequent_conflict(env, make_repo, config, users): assert repo.read_tree(repo.commit(pr3_id.head)) == { 'f': 'c', 'g': 'a', - 'h': matches('''<<<\x3c<<< HEAD + 'h': matches('''<<<\x3c<<< $$ a -||||||| parent of $$ (temp) +||||||| $$ ======= conflict! ->>>\x3e>>> $$ (temp) +>>>\x3e>>> $$ '''), 'x': '0', } @@ -470,7 +470,7 @@ conflict! # 1. link to status page # 2. forward-port chain thing assert repo.get_pr(pr3_id.number).comments[2:] == [ - (users['user'], matches(f'''\ + (users['user'], f'''\ @{users['user']} @{users['reviewer']} WARNING: the update of {pr2_id.display_name} to {pr2_id.head} has caused a \ conflict in this pull request, data may have been lost. @@ -478,20 +478,5 @@ stdout: ``` Auto-merging h CONFLICT (add/add): Merge conflict in h -``` - -stderr: -``` -$$ trace: built-in: git cherry-pick {pr2_id.head} --strategy ort -error: could not apply {pr2_id.head[:7]}... newfiles -hint: After resolving the conflicts, mark them with -hint: "git add/rm <pathspec>", then run -hint: "git cherry-pick --continue". -hint: You can instead skip this commit with "git cherry-pick --skip". -hint: To abort and get back to the state before "git cherry-pick", -hint: run "git cherry-pick --abort". -hint: Disable this message with "git config advice.mergeConflict false" ----------- -status: -```''')) +```'''), ] diff --git a/runbot_merge/git.py b/runbot_merge/git.py index f44c1cd2..caaa6e5d 100644 --- a/runbot_merge/git.py +++ b/runbot_merge/git.py @@ -13,16 +13,21 @@ from .github import MergeError, PrCommit _logger = logging.getLogger(__name__) - -def source_url(repository, prefix: str) -> str: +def source_url(repository) -> str: return 'https://{}@github.com/{}'.format( - repository.project_id[f'{prefix}_token'], + repository.project_id.github_token, repository.name, ) +def fw_url(repository) -> str: + return 'https://{}@github.com/{}'.format( + repository.project_id.fp_github_token, + repository.fp_remote_target, + ) + Authorship = Union[Tuple[str, str], Tuple[str, str, str]] -def get_local(repository, prefix: Optional[str]) -> 'Optional[Repo]': +def get_local(repository, *, clone: bool = True) -> 'Optional[Repo]': repos_dir = pathlib.Path(user_cache_dir('mergebot')) repos_dir.mkdir(parents=True, exist_ok=True) # NB: `repository.name` is `$org/$name` so this will be a subdirectory, probably @@ -30,9 +35,9 @@ def get_local(repository, prefix: Optional[str]) -> 'Optional[Repo]': if repo_dir.is_dir(): return git(repo_dir) - elif prefix: + elif clone: _logger.info("Cloning out %s to %s", repository.name, repo_dir) - subprocess.run(['git', 'clone', '--bare', source_url(repository, prefix), str(repo_dir)], check=True) + subprocess.run(['git', 'clone', '--bare', source_url(repository), str(repo_dir)], check=True) # bare repos don't have fetch specs by default, and fetching *into* # them is a pain in the ass, configure fetch specs so `git fetch` # works properly diff --git a/runbot_merge/models/batch.py b/runbot_merge/models/batch.py index c85eb4f7..de70d67b 100644 --- a/runbot_merge/models/batch.py +++ b/runbot_merge/models/batch.py @@ -13,7 +13,7 @@ from psycopg2 import sql from odoo import models, fields, api from .utils import enum - +from .. import git _logger = logging.getLogger(__name__) FOOTER = '\nMore info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port\n' @@ -330,12 +330,11 @@ class Batch(models.Model): base64.urlsafe_b64encode(os.urandom(3)).decode() ) conflicts = {} - with contextlib.ExitStack() as s: - for pr in prs: - conflicts[pr], working_copy = pr._create_fp_branch( - target, new_branch, s) + for pr in prs: + repo = git.get_local(pr.repository) + conflicts[pr], head = pr._create_fp_branch(repo, target) - working_copy.push('target', new_branch) + repo.push(git.fw_url(pr.repository), f"{head}:refs/heads/{new_branch}") gh = requests.Session() gh.headers['Authorization'] = 'token %s' % proj.fp_github_token diff --git a/runbot_merge/models/crons/git_maintenance.py b/runbot_merge/models/crons/git_maintenance.py index 2b0e2182..29dd028a 100644 --- a/runbot_merge/models/crons/git_maintenance.py +++ b/runbot_merge/models/crons/git_maintenance.py @@ -24,7 +24,7 @@ class GC(models.TransientModel): # run on all repos with a forwardport target (~ forwardport enabled) for repo in self.env['runbot_merge.repository'].search([]): - repo_git = get_local(repo, prefix=None) + repo_git = get_local(repo, clone=False) if not repo_git: continue diff --git a/runbot_merge/models/project_freeze/__init__.py b/runbot_merge/models/project_freeze/__init__.py index 3c645969..0a39ceae 100644 --- a/runbot_merge/models/project_freeze/__init__.py +++ b/runbot_merge/models/project_freeze/__init__.py @@ -213,15 +213,15 @@ class FreezeWizard(models.Model): gh_sessions = {r: r.github() for r in self.project_id.repo_ids} repos: Dict[Repository, git.Repo] = { - r: git.get_local(r, 'github').check(False) + r: git.get_local(r).check(False) for r in self.project_id.repo_ids } for repo, copy in repos.items(): - copy.fetch(git.source_url(repo, 'github'), '+refs/heads/*:refs/heads/*') + copy.fetch(git.source_url(repo), '+refs/heads/*:refs/heads/*') all_prs = self.release_pr_ids.pr_id | self.bump_pr_ids.pr_id for pr in all_prs: repos[pr.repository].fetch( - git.source_url(pr.repository, 'github'), + git.source_url(pr.repository), pr.head, ) @@ -282,7 +282,7 @@ class FreezeWizard(models.Model): repo_id = rel.repository_id if repos[repo_id].push( - git.source_url(repo_id, 'github'), + git.source_url(repo_id), f'{rel_heads[repo_id]}:refs/heads/{self.branch_name}', ).returncode: failure = ('create', repo_id.name, self.branch_name) @@ -294,7 +294,7 @@ class FreezeWizard(models.Model): for bump in self.bump_pr_ids: repo_id = bump.repository_id if repos[repo_id].push( - git.source_url(repo_id, 'github'), + git.source_url(repo_id), f'{bump_heads[repo_id]}:refs/heads/{master_name}' ).returncode: failure = ('fast-forward', repo_id.name, master_name) @@ -310,7 +310,7 @@ class FreezeWizard(models.Model): for prev_id in to_revert: if repos[prev_id].push( '-f', - git.source_url(prev_id, 'github'), + git.source_url(prev_id), f'{prevs[prev_id]}:refs/heads/{master_name}', ).returncode: failures.append(prev_id.name) @@ -323,7 +323,7 @@ class FreezeWizard(models.Model): for prev_id in to_delete: if repos[prev_id].push( - git.source_url(prev_id, 'github'), + git.source_url(prev_id), f':refs/heads/{self.branch_name}' ).returncode: failures.append(prev_id.name) diff --git a/runbot_merge/models/stagings_create.py b/runbot_merge/models/stagings_create.py index ddc9c3d4..2005416b 100644 --- a/runbot_merge/models/stagings_create.py +++ b/runbot_merge/models/stagings_create.py @@ -177,7 +177,7 @@ For-Commit-Id: {it.head} ) it.repo.stdout(False).check(True).push( '-f', - git.source_url(repo, 'github'), + git.source_url(repo), f'{it.head}:refs/heads/staging.{branch.name}', ) @@ -231,9 +231,9 @@ def staging_setup( gh = repo.github() head = gh.head(target.name) - source = git.get_local(repo, 'github') + source = git.get_local(repo) source.fetch( - git.source_url(repo, 'github'), + git.source_url(repo), # a full refspec is necessary to ensure we actually fetch the ref # (not just the commit it points to) and update it. # `git fetch $remote $branch` seems to work locally, but it might