diff --git a/forwardport/models/project.py b/forwardport/models/project.py index 89a71d37..9fd8632d 100644 --- a/forwardport/models/project.py +++ b/forwardport/models/project.py @@ -496,13 +496,28 @@ In the former case, you may want to edit this PR message as well. try: root._cherry_pick(working_copy) except CherrypickError as e: - # apply the diff of the PR to the target - # in git diff, "A...B" selects the bits of B not in A - # (which is the other way around from gitrevisions(7) - # because git) - diff = working_copy.stdout().diff('%s...%s' % root._reference_branches()).stdout - # apply probably fails (because conflict, we don't care - working_copy.with_config(input=diff).apply('-3', '-') + # 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 + root_base, root_branch = root._reference_branches() + working_copy.checkout('-bsquashed', root_branch) + root_commits = list( + working_copy.lazy().stdout() + .rev_list('--reverse', '%s..%s' % (root_base, root_branch)) + .stdout + ) + # squash the PR to a single commit + working_copy.reset('--soft', 'HEAD~%d' % len(root_commits)) + working_copy.commit(a=True, message="temp") + squashed = working_copy.stdout().rev_parse('HEAD').stdout.strip().decode() + + # switch back to the PR branch + working_copy.checkout(fp_branch_name) + # cherry-pick the squashed commit + working_copy.with_config(check=False).cherry_pick(squashed) working_copy.commit( a=True, allow_empty=True, diff --git a/forwardport/tests/conftest.py b/forwardport/tests/conftest.py index dbd5ba26..46f16c4f 100644 --- a/forwardport/tests/conftest.py +++ b/forwardport/tests/conftest.py @@ -367,6 +367,8 @@ class Repo: hashes = [] for commit in commits: + if commit.reset: + tree = None r = self._session.post('https://api.github.com/repos/{}/git/trees'.format(self.name), json={ 'tree': [ {'path': k, 'mode': '100644', 'type': 'blob', 'content': v} diff --git a/forwardport/tests/test_simple.py b/forwardport/tests/test_simple.py index d997c369..331e5bf0 100644 --- a/forwardport/tests/test_simple.py +++ b/forwardport/tests/test_simple.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- import collections -import re +import pprint import sys import time from operator import itemgetter @@ -328,12 +328,12 @@ def test_update_pr(env, config, make_repo): assert prod.read_tree(prod.commit(pr2.head)) == { 'f': 'c', 'g': 'a', - 'h': '''<<<<<<< ours + 'h': re_matches(r'''<<<<<<< HEAD a ======= 0 ->>>>>>> theirs -''', +>>>>>>> [0-9a-f]{7,}(...)? temp +'''), } def test_conflict(env, config, make_repo): @@ -378,12 +378,12 @@ def test_conflict(env, config, make_repo): assert pr1.state == 'opened' assert prod.read_tree(prod.commit(pr1.head)) == { 'f': 'c', - 'g': '''<<<<<<< ours + 'g': re_matches(r'''<<<<<<< HEAD a ======= xxx ->>>>>>> theirs -''', +>>>>>>> [0-9a-f]{7,}(...)? temp +'''), } # check that CI passing does not create more PRs @@ -442,6 +442,77 @@ xxx 'h': 'a', } +def test_conflict_deleted(env, config, make_repo): + prod, other = make_basic(env, config, make_repo) + # remove f from b + with prod: + prod.make_commits( + 'b', Commit('33', tree={'g': 'c'}, reset=True), + ref='heads/b' + ) + + # generate a conflict: update f in a + with prod: + [p_0] = prod.make_commits( + 'a', Commit('p_0', tree={'f': 'xxx'}), + ref='heads/conflicting' + ) + pr = prod.make_pr(target='a', head='conflicting') + prod.post_status(p_0, 'success', 'legal/cla') + prod.post_status(p_0, 'success', 'ci/runbot') + pr.post_comment('hansen r+', config['role_reviewer']['token']) + + env.run_crons() + with prod: + prod.post_status('staging.a', 'success', 'legal/cla') + prod.post_status('staging.a', 'success', 'ci/runbot') + + env.run_crons() + # wait a bit for PR webhook... ? + time.sleep(5) + env.run_crons() + + # should have created a new PR + pr0, pr1 = env['runbot_merge.pull_requests'].search([], order='number') + # but it should not have a parent + assert not pr1.parent_id + assert pr1.source_id == pr0 + assert prod.read_tree(prod.commit('b')) == { + 'g': 'c', + } + assert pr1.state == 'opened' + # NOTE: no actual conflict markers because pr1 essentially adds f de-novo + assert prod.read_tree(prod.commit(pr1.head)) == { + 'f': 'xxx', + 'g': 'c', + } + + # check that CI passing does not create more PRs + with prod: + validate_all([prod], [pr1.head]) + env.run_crons() + time.sleep(5) + env.run_crons() + assert pr0 | pr1 == env['runbot_merge.pull_requests'].search([], order='number'),\ + "CI passing should not have resumed the FP process on a conflicting / draft PR" + + # fix the PR, should behave as if this were a normal PR + get_pr = prod.get_pr(pr1.number) + pr_repo, pr_ref = get_pr.branch + with pr_repo: + pr_repo.make_commits( + # if just given a branch name, goes and gets it from pr_repo whose + # "b" was cloned before that branch got rolled back + prod.commit('b').id, + Commit('f should indeed be removed', tree={'g': 'c'}, reset=True), + ref='heads/%s' % pr_ref + ) + env.run_crons() + assert prod.read_tree(prod.commit(pr1.head)) == { + 'g': 'c', + } + assert pr1.state == 'opened', "state should be open still" + def test_empty(env, config, make_repo, users): """ Cherrypick of an already cherrypicked (or separately implemented) commit -> create draft PR. diff --git a/forwardport/tests/utils.py b/forwardport/tests/utils.py index 857f42e8..e68be339 100644 --- a/forwardport/tests/utils.py +++ b/forwardport/tests/utils.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- # target branch '-' source branch '-' base32 unique '-forwardport' import itertools +import re MESSAGE_TEMPLATE = """{message} @@ -10,15 +11,26 @@ closes {repo}#{number} REF_PATTERN = r'{target}-{source}-\w{{8}}-forwardport' class Commit: - def __init__(self, message, *, author=None, committer=None, tree): + def __init__(self, message, *, author=None, committer=None, tree, reset=False): self.id = None self.message = message self.author = author self.committer = committer self.tree = tree + self.reset = reset def validate_all(repos, refs, contexts=('ci/runbot', 'legal/cla')): """ Post a "success" status for each context on each ref of each repo """ for repo, branch, context in itertools.product(repos, refs, contexts): repo.post_status(branch, 'success', context) + +class re_matches: + def __init__(self, pattern, flags=0): + self._r = re.compile(pattern, flags) + + def __eq__(self, text): + return self._r.match(text) + + def __repr__(self): + return '~' + self._r.pattern + '~'