[CHG] forwardport: perform forward porting without working copies

The goal is to reduce maintenance and odd disk interactions &
concurrency issues, by not creating concurrent clones, not having to
push forks back in the repository, etc... it also removes the need to
cleanup "scratch" working copies though that looks not to have been an
issue in a while.

The work is done on isolated objects without using or mutating refs,
so even concurrent work should not be a problem.

This turns out to not be any more verbose (less so if anything) than
using `cherry-pick`, as that is not really designed for scripted /
non-interactive use, or for squashing commits thereafter. Working
directly with trees and commits is quite a bit cleaner even without a
ton of helpers.

Much of the credit goes to Julia Evans for [their investigation of
3-way merges as the underpinnings of cherry-picking][3-way merge],
this would have been a lot more difficult if I'd had to rediscover the
merge-base trick independently.

A few things have been changed by this:

- The old trace/stderr from cherrypick has disappeared as it's
  generated by cherrypick, but for a non-interactive use it's kinda
  useless anyway so I probably should have looked into removing it
  earlier (I think the main use was investigation of the inflateinit
  issue).
- Error on emptied commits has to be hand-rolled as `merge-tree`
  couldn't care less, this is not hard but is a bit annoying.
- `merge-tree`'s conflict information only references raw commits,
  which makes sense, but requires updating a bunch of tests. Then
  again so does the fact that it *usually* doesn't send anything to
  stderr, so that's usually disappearing.

Conveniently `merge-tree` merges the conflict marker directly in the
files / tree so we don't have to mess about moving them back out of
the repository and into the working copy as I assume cherry-pick does,
which means we don't have to try and commit them back in ether. That
is a huge part of the gain over faffing about with the working copy.

Fixes #847

[3-way merge]: https://jvns.ca/blog/2023/11/10/how-cherry-pick-and-revert-work/
This commit is contained in:
Xavier Morel 2024-07-05 13:32:02 +02:00
parent 3062f30245
commit b1d3278de1
10 changed files with 220 additions and 272 deletions

View File

@ -161,94 +161,94 @@ class ForwardPortTasks(models.Model, Queue):
# only then create the PR objects # only then create the PR objects
# TODO: maybe do that after making forward-port WC-less, so all the # TODO: maybe do that after making forward-port WC-less, so all the
# branches can be pushed atomically at once # branches can be pushed atomically at once
with contextlib.ExitStack() as s: for descendant in self.batch_id.descendants():
for descendant in self.batch_id.descendants(): target = pr._find_next_target()
target = pr._find_next_target() if target is None:
if target is None: _logger.info("Will not forward-port %s: no next target", pr.display_name)
_logger.info("Will not forward-port %s: no next target", pr.display_name) return
return
if PullRequests.search_count([ if PullRequests.search_count([
('source_id', '=', source.id), ('source_id', '=', source.id),
('target', '=', target.id), ('target', '=', target.id),
('state', 'not in', ('closed', 'merged')), ('state', 'not in', ('closed', 'merged')),
]): ]):
_logger.warning("Will not forward-port %s: already ported", pr.display_name) _logger.warning("Will not forward-port %s: already ported", pr.display_name)
return return
if target != descendant.target: if target != descendant.target:
self.env['runbot_merge.pull_requests.feedback'].create({ self.env['runbot_merge.pull_requests.feedback'].create({
'repository': repository.id, 'repository': repository.id,
'pull_request': source.id, 'pull_request': source.id,
'token_field': 'fp_github_token', 'token_field': 'fp_github_token',
'message': """\ 'message': """\
{pr.ping}unable to port this PR forwards due to inconsistency: goes from \ {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 \ {pr.target.name} to {next_target.name} but {batch} ({batch_prs}) targets \
{batch.target.name}. {batch.target.name}.
""".format(pr=pr, next_target=target, batch=descendant, batch_prs=', '.join(descendant.mapped('prs.display_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<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: return
_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})")
new_pr = PullRequests._from_gh(r.json()) ref = descendant.prs[:1].refname
_logger.info("Created forward-port PR %s", new_pr) # NOTE: ports the new source everywhere instead of porting each
new_pr.write({ # PR to the next step as it does not *stop* on conflict
'batch_id': descendant.id, # should already be set correctly but... repo = git.get_local(source.repository)
'merge_method': pr.merge_method, conflict, head = source._create_fp_branch(repo, target)
'source_id': source.id, repo.push(git.fw_url(pr.repository), f'{head}:refs/heads/{ref}')
# 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: remote_target = repository.fp_remote_target
self.env.ref('runbot_merge.forwardport.failure.conflict')._send( owner, _ = remote_target.split('/', 1)
repository=pr.repository, message = source.message + f"\n\nForward-Port-Of: {pr.display_name}"
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'] title, body = re.match(r'(?P<title>[^\n]+)\n*(?P<body>.*)', message, flags=re.DOTALL).groups()
if conflict: r = gh.post(f'https://api.github.com/repos/{pr.repository.name}/pulls', json={
labels.append('conflict') 'base': target.name,
self.env['runbot_merge.pull_requests.tagging'].create({ 'head': f'{owner}:{ref}',
'repository': new_pr.repository.id, 'title': '[FW]' + (' ' if title[0] != '[' else '') + title,
'pull_request': new_pr.number, 'body': body
'tags_add': labels, })
}) 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): class UpdateQueue(models.Model, Queue):
_name = 'forwardport.updates' _name = 'forwardport.updates'
@ -286,8 +286,9 @@ class UpdateQueue(models.Model, Queue):
) )
return return
conflicts, working_copy = previous._create_fp_branch( repo = git.get_local(previous.repository)
child.target, child.refname, s) conflicts, new_head = previous._create_fp_branch(repo, child.target)
if conflicts: if conflicts:
_, out, err, _ = conflicts _, out, err, _ = conflicts
self.env.ref('runbot_merge.forwardport.updates.conflict.parent')._send( 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(repo.stdout().rev_list(
commits_count = int(working_copy.stdout().rev_list( f'{child.target.name}..{new_head}',
f'{child.target.name}..{child.refname}',
count=True count=True
).stdout.decode().strip()) ).stdout.decode().strip())
old_head = child.head old_head = child.head
@ -320,16 +320,11 @@ class UpdateQueue(models.Model, Queue):
# 'state': 'opened', # 'state': 'opened',
'squash': commits_count == 1, '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 # then update the child's branch to the new head
working_copy.push(f'--force-with-lease={child.refname}:{old_head}', repo.push(
'target', child.refname) 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 # committing here means github could technically trigger its
# webhook before sending a response, but committing before # webhook before sending a response, but committing before

View File

@ -19,9 +19,7 @@ import json
import logging import logging
import operator import operator
import subprocess import subprocess
import tempfile
import typing import typing
from pathlib import Path
import dateutil.relativedelta import dateutil.relativedelta
import requests import requests
@ -30,7 +28,6 @@ from odoo import models, fields, api
from odoo.exceptions import UserError from odoo.exceptions import UserError
from odoo.osv import expression from odoo.osv import expression
from odoo.tools.misc import topological_sort, groupby 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.base.models.res_partner import Partner
from odoo.addons.runbot_merge import git, utils from odoo.addons.runbot_merge import git, utils
from odoo.addons.runbot_merge.models.pull_requests import Branch 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']]) 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 """ Creates a forward-port for the current PR to ``target_branch`` under
``fp_branch_name``. ``fp_branch_name``.
:param target_branch: the branch to port forward to :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) :rtype: (None | (str, str, str, list[commit]), Repo)
""" """
logger = _logger.getChild(str(self.id)) logger = _logger.getChild(str(self.id))
@ -308,104 +299,52 @@ class PullRequests(models.Model):
"Forward-porting %s (%s) to %s", "Forward-porting %s (%s) to %s",
self.display_name, root.display_name, target_branch.name self.display_name, root.display_name, target_branch.name
) )
source = git.get_local(self.repository, 'fp_github') tree = source.with_config(stdout=subprocess.PIPE, stderr=subprocess.STDOUT).fetch()
r = 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("Updated cache repo %s:\n%s", source._directory, r.stdout.decode())
logger.info("Create working copy...") tree = source.with_config(stdout=subprocess.PIPE, stderr=subprocess.STDOUT) \
cache_dir = user_cache_dir('forwardport') .fetch(git.source_url(self.repository), root.head)
# 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)
logger.info( logger.info(
"Fetched head of %s into %s:\n%s", "Fetched head of %s (%s):\n%s",
root.display_name, root.display_name,
working_copy._directory, root.head,
r.stdout.decode() 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( raise ForwardPortError(
f"During forward port of {self.display_name}, unable to find " f"During forward port of {self.display_name}, unable to find "
f"expected head of {root.display_name} ({root.head})" 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: try:
root._cherry_pick(working_copy) return None, root._cherry_pick(source, target_branch.name)
return None, working_copy
except CherrypickError as e: except CherrypickError as e:
h, out, err, commits = e.args 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 # commits returns oldest first, so youngest (head) last
head_commit = commits[-1]['commit'] head_commit = commits[-1]['commit']
to_tuple = operator.itemgetter('name', 'email') 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() authors, committers = set(), set()
for c in (c['commit'] for c in commits): for commit in (c['commit'] for c in commits):
authors.add(to_tuple(c['author'])) authors.add(to_tuple(commit['author']))
committers.add(to_tuple(c['committer'])) committers.add(to_tuple(commit['committer']))
fp_authorship = (project_id.fp_github_name, '', '') fp_authorship = (self.repository.project_id.fp_github_name, '', '')
author = fp_authorship if len(authors) != 1 \ author = fp_authorship if len(authors) != 1 \
else authors.pop() + (head_commit['author']['date'],) else authors.pop() + (head_commit['author']['date'],)
committer = fp_authorship if len(committers) != 1 \ committer = fp_authorship if len(committers) != 1 \
else committers.pop() + (head_commit['committer']['date'],) else committers.pop() + (head_commit['committer']['date'],)
conf = working_copy.with_config(env={ conf = source.with_params(
**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(
'merge.renamelimit=0', 'merge.renamelimit=0',
'merge.renames=copies', 'merge.renames=copies',
'merge.conflictstyle=zdiff3' 'merge.conflictstyle=zdiff3'
) \ ).with_config(stdout=subprocess.PIPE, stderr=subprocess.PIPE)
.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
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 # if there was a single commit, reuse its message when committing
# the conflict # the conflict
if len(commits) == 1: if len(commits) == 1:
@ -421,77 +360,107 @@ stderr:
{err} {err}
""" """
conf.with_config(input=str(msg).encode()) \ target_head = source.stdout().rev_parse(target_branch.name).stdout.decode().strip()
.commit(all=True, allow_empty=True, file='-') 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): def _cherry_pick(self, repo: git.Repo, branch: Branch) -> str:
""" Cherrypicks ``self`` into the working copy """ 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> # <xxx>.cherrypick.<number>
logger = _logger.getChild(str(self.id)).getChild('cherrypick') logger = _logger.getChild(str(self.id)).getChild('cherrypick')
# original head so we can reset # target's head
original_head = working_copy.stdout().rev_parse('HEAD').stdout.decode().strip() head = repo.stdout().rev_parse(branch).stdout.decode().strip()
commits = self.commits() commits = self.commits()
logger.info("%s: copy %s commits to %s\n%s", self, len(commits), original_head, '\n'.join( logger.info(
'- %s (%s)' % (c['sha'], c['commit']['message'].splitlines()[0]) "%s: copy %s commits to %s (%s)%s",
for c in commits 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.renamelimit=0',
'merge.renames=copies', 'merge.renames=copies',
).with_config( ).with_config(
stdout=subprocess.PIPE, stderr=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
check=False check=False,
) )
for commit in commits: for commit in commits:
commit_sha = commit['sha'] commit_sha = commit['sha']
# config (global -c) or commit options don't really give access to # merge-tree is a bit stupid and gets confused when the options
# setting dates # follow the parameters
cm = commit['commit'] # get the "git" commit object rather than the "github" commit resource r = conf.merge_tree('--merge-base', commit['parents'][0]['sha'], head, commit_sha)
env = { new_tree = r.stdout.decode().splitlines(keepends=False)[0]
'GIT_AUTHOR_NAME': cm['author']['name'], if r.returncode:
'GIT_AUTHOR_EMAIL': cm['author']['email'], # For merge-tree the stdout on conflict is of the form
'GIT_AUTHOR_DATE': cm['author']['date'], #
'GIT_COMMITTER_NAME': cm['committer']['name'], # oid of toplevel tree
'GIT_COMMITTER_EMAIL': cm['committer']['email'], # conflicted file info+
} #
configured = working_copy.with_config(env=env) # 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())) 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, bail
if r.returncode: # pick failed, reset and bail
# try to log inflateInit: out of memory errors as warning, they # 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( logger.log(
logging.WARNING if r.returncode == 128 else logging.INFO, logging.WARNING if r.returncode == 128 else logging.INFO,
"forward-port of %s (%s) failed at %s", "forward-port of %s (%s) failed at %s",
self, self.display_name, commit_sha) self, self.display_name, commit_sha)
configured.reset('--hard', original_head)
raise CherrypickError( raise CherrypickError(
commit_sha, commit_sha,
r.stdout.decode(), r.stdout.decode(),
_clean_rename(r.stderr.decode()), _clean_rename(r.stderr.decode()),
commits 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 return head
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)
def _make_fp_message(self, commit): def _make_fp_message(self, commit):
cmap = json.loads(self.commits_map) cmap = json.loads(self.commits_map)
@ -541,6 +510,11 @@ stderr:
format_args={'pr': pr, 'source': source}, format_args={'pr': pr, 'source': source},
) )
map_author = operator.itemgetter('name', 'email', 'date')
map_committer = operator.itemgetter('name', 'email')
class Stagings(models.Model): class Stagings(models.Model):
_inherit = 'runbot_merge.stagings' _inherit = 'runbot_merge.stagings'

View File

@ -381,7 +381,7 @@ def test_add_to_forward_port_conflict(env, config, make_repo, users):
assert pr2_c.comments == [ assert pr2_c.comments == [
seen(env, pr2_c, users), seen(env, pr2_c, users),
# should have conflicts # 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: 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?). 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. 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}. :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 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))
] ]

View File

@ -62,17 +62,17 @@ def test_conflict(env, config, make_repo, users):
assert prod.read_tree(c) == { assert prod.read_tree(c) == {
'f': 'c', 'f': 'c',
'g': 'a', 'g': 'a',
'h': matches('''<<<\x3c<<< HEAD 'h': matches('''<<<\x3c<<< $$
a a
||||||| parent of $$ (temp) ||||||| $$
======= =======
xxx xxx
>>>\x3e>>> $$ (temp) >>>\x3e>>> $$
'''), '''),
} }
assert prc.comments == [ assert prc.comments == [
seen(env, prc, users), seen(env, prc, users),
(users['user'], matches( (users['user'],
f'''@{users['user']} @{users['reviewer']} cherrypicking of pull request {pra_id.display_name} failed. f'''@{users['user']} @{users['reviewer']} cherrypicking of pull request {pra_id.display_name} failed.
stdout: 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?). 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. 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}. :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 More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
''')) ''')
] ]
prb = prod.get_pr(prb_id.number) prb = prod.get_pr(prb_id.number)

View File

@ -445,12 +445,12 @@ def test_subsequent_conflict(env, make_repo, config, users):
assert repo.read_tree(repo.commit(pr3_id.head)) == { assert repo.read_tree(repo.commit(pr3_id.head)) == {
'f': 'c', 'f': 'c',
'g': 'a', 'g': 'a',
'h': matches('''<<<\x3c<<< HEAD 'h': matches('''<<<\x3c<<< $$
a a
||||||| parent of $$ (temp) ||||||| $$
======= =======
conflict! conflict!
>>>\x3e>>> $$ (temp) >>>\x3e>>> $$
'''), '''),
'x': '0', 'x': '0',
} }
@ -470,7 +470,7 @@ conflict!
# 1. link to status page # 1. link to status page
# 2. forward-port chain thing # 2. forward-port chain thing
assert repo.get_pr(pr3_id.number).comments[2:] == [ 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 \ @{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. conflict in this pull request, data may have been lost.
@ -478,20 +478,5 @@ stdout:
``` ```
Auto-merging h Auto-merging h
CONFLICT (add/add): Merge conflict in 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:
```'''))
] ]

View File

@ -13,16 +13,21 @@ from .github import MergeError, PrCommit
_logger = logging.getLogger(__name__) _logger = logging.getLogger(__name__)
def source_url(repository) -> str:
def source_url(repository, prefix: str) -> str:
return 'https://{}@github.com/{}'.format( return 'https://{}@github.com/{}'.format(
repository.project_id[f'{prefix}_token'], repository.project_id.github_token,
repository.name, 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]] 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 = pathlib.Path(user_cache_dir('mergebot'))
repos_dir.mkdir(parents=True, exist_ok=True) repos_dir.mkdir(parents=True, exist_ok=True)
# NB: `repository.name` is `$org/$name` so this will be a subdirectory, probably # 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(): if repo_dir.is_dir():
return git(repo_dir) return git(repo_dir)
elif prefix: elif clone:
_logger.info("Cloning out %s to %s", repository.name, repo_dir) _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* # 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` # them is a pain in the ass, configure fetch specs so `git fetch`
# works properly # works properly

View File

@ -13,7 +13,7 @@ from psycopg2 import sql
from odoo import models, fields, api from odoo import models, fields, api
from .utils import enum from .utils import enum
from .. import git
_logger = logging.getLogger(__name__) _logger = logging.getLogger(__name__)
FOOTER = '\nMore info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port\n' 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() base64.urlsafe_b64encode(os.urandom(3)).decode()
) )
conflicts = {} conflicts = {}
with contextlib.ExitStack() as s: for pr in prs:
for pr in prs: repo = git.get_local(pr.repository)
conflicts[pr], working_copy = pr._create_fp_branch( conflicts[pr], head = pr._create_fp_branch(repo, target)
target, new_branch, s)
working_copy.push('target', new_branch) repo.push(git.fw_url(pr.repository), f"{head}:refs/heads/{new_branch}")
gh = requests.Session() gh = requests.Session()
gh.headers['Authorization'] = 'token %s' % proj.fp_github_token gh.headers['Authorization'] = 'token %s' % proj.fp_github_token

View File

@ -24,7 +24,7 @@ class GC(models.TransientModel):
# run on all repos with a forwardport target (~ forwardport enabled) # run on all repos with a forwardport target (~ forwardport enabled)
for repo in self.env['runbot_merge.repository'].search([]): 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: if not repo_git:
continue continue

View File

@ -213,15 +213,15 @@ class FreezeWizard(models.Model):
gh_sessions = {r: r.github() for r in self.project_id.repo_ids} gh_sessions = {r: r.github() for r in self.project_id.repo_ids}
repos: Dict[Repository, git.Repo] = { 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 r in self.project_id.repo_ids
} }
for repo, copy in repos.items(): 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 all_prs = self.release_pr_ids.pr_id | self.bump_pr_ids.pr_id
for pr in all_prs: for pr in all_prs:
repos[pr.repository].fetch( repos[pr.repository].fetch(
git.source_url(pr.repository, 'github'), git.source_url(pr.repository),
pr.head, pr.head,
) )
@ -282,7 +282,7 @@ class FreezeWizard(models.Model):
repo_id = rel.repository_id repo_id = rel.repository_id
if repos[repo_id].push( 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}', f'{rel_heads[repo_id]}:refs/heads/{self.branch_name}',
).returncode: ).returncode:
failure = ('create', repo_id.name, self.branch_name) failure = ('create', repo_id.name, self.branch_name)
@ -294,7 +294,7 @@ class FreezeWizard(models.Model):
for bump in self.bump_pr_ids: for bump in self.bump_pr_ids:
repo_id = bump.repository_id repo_id = bump.repository_id
if repos[repo_id].push( 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}' f'{bump_heads[repo_id]}:refs/heads/{master_name}'
).returncode: ).returncode:
failure = ('fast-forward', repo_id.name, master_name) failure = ('fast-forward', repo_id.name, master_name)
@ -310,7 +310,7 @@ class FreezeWizard(models.Model):
for prev_id in to_revert: for prev_id in to_revert:
if repos[prev_id].push( if repos[prev_id].push(
'-f', '-f',
git.source_url(prev_id, 'github'), git.source_url(prev_id),
f'{prevs[prev_id]}:refs/heads/{master_name}', f'{prevs[prev_id]}:refs/heads/{master_name}',
).returncode: ).returncode:
failures.append(prev_id.name) failures.append(prev_id.name)
@ -323,7 +323,7 @@ class FreezeWizard(models.Model):
for prev_id in to_delete: for prev_id in to_delete:
if repos[prev_id].push( if repos[prev_id].push(
git.source_url(prev_id, 'github'), git.source_url(prev_id),
f':refs/heads/{self.branch_name}' f':refs/heads/{self.branch_name}'
).returncode: ).returncode:
failures.append(prev_id.name) failures.append(prev_id.name)

View File

@ -177,7 +177,7 @@ For-Commit-Id: {it.head}
) )
it.repo.stdout(False).check(True).push( it.repo.stdout(False).check(True).push(
'-f', '-f',
git.source_url(repo, 'github'), git.source_url(repo),
f'{it.head}:refs/heads/staging.{branch.name}', f'{it.head}:refs/heads/staging.{branch.name}',
) )
@ -231,9 +231,9 @@ def staging_setup(
gh = repo.github() gh = repo.github()
head = gh.head(target.name) head = gh.head(target.name)
source = git.get_local(repo, 'github') source = git.get_local(repo)
source.fetch( source.fetch(
git.source_url(repo, 'github'), git.source_url(repo),
# a full refspec is necessary to ensure we actually fetch the ref # a full refspec is necessary to ensure we actually fetch the ref
# (not just the commit it points to) and update it. # (not just the commit it points to) and update it.
# `git fetch $remote $branch` seems to work locally, but it might # `git fetch $remote $branch` seems to work locally, but it might