[FIX] forwardport: change method for conflicting working copy

The original method was to `git diff | git apply` in order to get a
complete overview of conflicts generated by the forward port (if
any).

However this turns out to have a huge issue in the presence of renamed
or removed files: in that case `git apply` will simply not do
anything, and fail with a completely clean working copy. Which is very
much undesirable.

-> alternative method, squash the PR to a single commit then
cherry-pick that single commit, this should provide us with proper
conflicts & their markers.

Also add tests for conflicts due to deleted files...

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,

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}

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.

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 + '~'