Detect cherry-picked changes without Change-Id

Support detecting simple cherry-picking of changes to support use with
repositories that are not using Gerrit/git-review.

Refactor the searcher list method to emulate git-rebase behaviour in
looking at a single side of the changes between two revisions and using
'--cherry-pick' to drop commits with the same patch id.

Change-Id: I75b0086203ea614c4c58873292c8dbf9eca00f64

git_upstream/lib/searchers.py

@@ -126,11 +126,13 @@ class Searcher(GitMixin):
         return mergecommit, ["^%s" % ip
                              for ip in mergecommit.parents if ip != parent]
 
-    def list(self):
+    def list(self, upstream=None):
         """
         Returns a list of Commit objects, between the '<commitish>' revision
         given in the constructor, and the commit object returned by the find()
-        method.
+        method. If given an upstream branch, uses --cherry-pick/--left-only to
+        exclude commits that are identical to those already on the upstream
+        branch.
         """
         if not self.commit:
             self.find()
@@ -182,34 +184,52 @@ class Searcher(GitMixin):
         # the tip of the head to avoid inversion where older commits
         # started before the previous import merge and approved afterwards
         # are not sorted by 'rev-list' predictably.
-        if previous_import:
-            search_list = [
-                (previous_import, self.branch),
-                (self.commit, previous_import),
-            ]
-        else:
-            search_list = [(self.commit, self.branch)]
-
         commit_list = []
-        extra_args.append('--')
+        if upstream is None:
-        for start, end in search_list:
+            if previous_import:
-            revision_spec = "{0}..{1}".format(start, end)
+                search_list = [
+                    (previous_import, self.branch, None),
+                    (self.commit, previous_import, None),
+                ]
+            else:
+                search_list = [(self.commit, self.branch, None)]
+            rev_spec = "{0}..{1}"
+            git_args = {}
+        else:
+            if previous_import:
+                search_list = [
+                    (self.branch, upstream, "^%s" % previous_import),
+                    (previous_import, upstream, "^%s~1" % previous_import)
+                ]
+            else:
+                search_list = [(self.branch, upstream, None)]
+            rev_spec = "{0}...{1}"
+            git_args = {'cherry_pick': True, 'left_only': True}
+            extra_args.append("^%s" % self.commit)
 
+        for start, end, exclude in search_list:
+            extra = list(extra_args)
+            if exclude:
+                extra.append(exclude)
+            extra.append("--")
+            revision_spec = rev_spec.format(start, end)
             self.log.info(
                 """
                 Walking the changes between found commit and target, excluding
                 those behind the previous import or merged as an additional
                 branch during the previous import
-                    git rev-list --topo-order %s %s
+                    git rev-list --topo-order %s %s %s
-                """, revision_spec, " ".join(extra_args))
+                """, self.git.transform_kwargs(git_args), revision_spec,
+                " ".join(extra))
 
             commit_list.append(
                 Commit._iter_from_process_or_stream(
                     self.repo,
                     self.git.rev_list(revision_spec,
-                                      *extra_args,
+                                      *extra,
                                       as_process=True,
-                                      topo_order=True)))
+                                      topo_order=True,
+                                      **git_args)))
 
         # chain the filters as generators so that we don't need to allocate new
         # lists for each step in the filter chain.
@@ -444,7 +464,7 @@ class CommitMessageSearcher(LogDedentMixin, Searcher):
 
         return self.commit.hexsha
 
-    def list(self, include=True):
+    def list(self, upstream=None, include=True):
         """
         Override parent implementation to permit inclusion of the found commit
         to be returned in the list of changes. This will help in cases where
@@ -452,7 +472,7 @@ class CommitMessageSearcher(LogDedentMixin, Searcher):
         branches that would be returned by the generic upstream searcher.
         """
 
-        commits = super(CommitMessageSearcher, self).list()
+        commits = super(CommitMessageSearcher, self).list(upstream)
         if include:
             commits.append(self.commit)
 

git_upstream/lib/strategies.py

@@ -99,7 +99,7 @@ class LocateChangesStrategy(GitMixin, Sequence):
 
     def _popdata(self):
         """Should return the list of commits from the searcher object"""
-        return self.searcher.list()
+        return self.searcher.list(upstream=self.upstream)
 
 
 class LocateChangesWalk(LocateChangesStrategy):

git_upstream/tests/searchers/scenarios/drop_cherry_picked_changes.yaml

@@ -0,0 +1,42 @@
+#
+# Copyright (c) 2016 Hewlett Packard Enterprise Development LP
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+---
+- desc: |
+    Test if searcher handles simple cherry picks
+
+    Checks that importing an upstream where all local changes have
+    gone upstream, will spot that the commits are identical using
+    cherry-pick mechanism of git rev-list.
+
+          C---D                 local/master
+         /
+    A---B---E---C1--D1          upstream/master
+
+  tree:
+    - [A, []]
+    - [B, [A]]
+    - [C, [B]]
+    - [D, [C]]
+    - [C1, [E]]
+    - [D1, [C1]]
+    - [E, [B]]
+
+  branches:
+    head: [master, D]
+    upstream: [upstream/master, D1]
+
+  expected_changes: []

git_upstream/tests/searchers/scenarios/drop_cherry_picked_changes_complex.yaml

@@ -0,0 +1,63 @@
+#
+# Copyright (c) 2016 Hewlett Packard Enterprise Development LP
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+---
+- desc: |
+    Test cherry-picked changes found with complex layout
+
+    Construct a complex repo layout which contains additional include branches
+    and a previous import from upstream. Test that cherry-picked changes are
+    correctly spotted having been upstreamed and removed from the set to be
+    rebased.
+
+    Repository layout being tested
+
+    B--
+     \ \
+      \ \         L----------------
+       \ \       /                 \
+        C---D---E-------I---J---K1--M---N   master
+       /   \           /
+      /     --H---D1--E1
+     /       /
+    A---F---G---D2--E2--K                  upstream/master
+
+  tree:
+    - [A, []]
+    - [B, []]
+    - [C, [A, B]]
+    - [D, [C]]
+    - [D1, [H]]
+    - [D2, [G]]
+    - [E, [D]]
+    - [E1, [D1]]
+    - [E2, [D2]]
+    - [F, [A]]
+    - [G, [F]]
+    - [H, [G, B]]
+    - [I, [E, =E1]]
+    - [J, [I]]
+    - [K, [E2]]
+    - [K1, [J]]
+    - [L, [E]]
+    - [M, [K1, L]]
+    - [N, [M]]
+
+  branches:
+    head: [master, N]
+    upstream: [upstream/master, K]
+
+  expected_changes: [H, I, J, L, M, N]

git_upstream/tests/searchers/test_searchers.py

@@ -52,4 +52,4 @@ class TestUpstreamMergeBaseSearcher(TestWithScenarios, BaseTestCase):
                                              patterns=pattern, repo=self.repo)
         self.assertEqual(
             self.gittree._commits_from_nodes(reversed(self.expected_changes)),
-            searcher.list())
+            searcher.list(self.branches['upstream'][0]))