[IMP] forwardport: provide clearer picture of conflicts

On conflicts in multi-commit PRs developers sometimes get confused as
to what happened why.

If a conflict occurs and the source pull request had multiple
commits, list all the source commits and show which one broke.

Related to #505
This commit is contained in:
Xavier Morel 2021-08-11 14:08:18 +02:00 committed by xmo-odoo
parent 82174ae66e
commit 678d2216b8
2 changed files with 22 additions and 13 deletions

View File

@ -127,7 +127,7 @@ class UpdateQueue(models.Model, Queue):
conflicts, working_copy = previous._create_fp_branch( conflicts, working_copy = previous._create_fp_branch(
child.target, child.refname, s) child.target, child.refname, s)
if conflicts: if conflicts:
_, out, err = conflicts _, out, err, _ = conflicts
Feedback.create({ Feedback.create({
'repository': previous.repository.id, 'repository': previous.repository.id,
'pull_request': previous.number, 'pull_request': previous.number,

View File

@ -711,22 +711,27 @@ The next pull request (%s) is in conflict. You can merge the chain up to here by
for pr, new_pr in zip(self, new_batch): for pr, new_pr in zip(self, new_batch):
source = pr.source_id or pr source = pr.source_id or pr
(h, out, err) = conflicts.get(pr) or (None, None, None) (h, out, err, hh) = conflicts.get(pr) or (None, None, None, None)
if h: if h:
sout = serr = '' sout = serr = ''
if out.strip(): if out.strip():
sout = "\nstdout:\n```\n%s\n```\n" % out sout = f"\nstdout:\n```\n{out}\n```\n"
if err.strip(): if err.strip():
serr = "\nstderr:\n```\n%s\n```\n" % err serr = f"\nstderr:\n```\n{err}\n```\n"
message = source._pingline() + """ lines = ''
Cherrypicking %s of source %s failed if len(hh) > 1:
%s%s lines = '\n' + ''.join(
'* %s%s\n' % (sha, ' <- on this commit' if sha == h else '')
for sha in hh
)
message = f"""{source._pingline()} cherrypicking of pull request {source.display_name} failed.
{lines}{sout}{serr}
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.
""" % (h, source.display_name, sout, serr) """
elif has_conflicts: elif has_conflicts:
message = """%s message = """%s
While this was properly forward-ported, at least one co-dependent PR (%s) did not succeed. You will need to fix it before this can be merged. While this was properly forward-ported, at least one co-dependent PR (%s) did not succeed. You will need to fix it before this can be merged.
@ -798,8 +803,11 @@ This PR targets %s and is part of the forward-port chain. Further PRs will be cr
: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 fp_branch_name: the name of the branch to create the FP under
:param ExitStack cleanup: so the working directories can be cleaned up :param ExitStack cleanup: so the working directories can be cleaned up
:return: (conflictp, working_copy) :return: A pair of an optional conflict information and a repository. If
:rtype: (bool, Repo) 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[str]), Repo)
""" """
source = self._get_local_directory() source = self._get_local_directory()
# update all the branches & PRs # update all the branches & PRs
@ -888,7 +896,7 @@ This PR targets %s and is part of the forward-port chain. Further PRs will be cr
.with_config(check=False)\ .with_config(check=False)\
.cherry_pick(squashed, no_commit=True) .cherry_pick(squashed, no_commit=True)
status = conf.stdout().status(short=True, untracked_files='no').stdout.decode() status = conf.stdout().status(short=True, untracked_files='no').stdout.decode()
h, out, err = e.args h, out, err, hh = e.args
if err.strip(): if err.strip():
err = err.rstrip() + '\n----------\nstatus:\n' + status err = err.rstrip() + '\n----------\nstatus:\n' + status
else: else:
@ -910,7 +918,7 @@ stdout:
stderr: stderr:
%s %s
""" % (h, out, err)) """ % (h, out, err))
return (h, out, err), working_copy return (h, out, err, hh), working_copy
def _cherry_pick(self, working_copy): def _cherry_pick(self, working_copy):
""" Cherrypicks ``self`` into the working copy """ Cherrypicks ``self`` into the working copy
@ -968,7 +976,8 @@ stderr:
raise CherrypickError( raise CherrypickError(
commit_sha, commit_sha,
r.stdout.decode(), r.stdout.decode(),
_clean_rename(r.stderr.decode()) _clean_rename(r.stderr.decode()),
[commit['sha'] for commit in commits]
) )
msg = self._make_fp_message(commit) msg = self._make_fp_message(commit)