From 8ab6ccc3bee6b9cd355b3e99f2fcefc6a69b4a9c Mon Sep 17 00:00:00 2001 From: Darragh Bailey Date: Mon, 27 Jun 2016 18:43:07 +0100 Subject: [PATCH] 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 | 58 +++++++++++------ git_upstream/lib/strategies.py | 2 +- .../scenarios/drop_cherry_picked_changes.yaml | 42 +++++++++++++ .../drop_cherry_picked_changes_complex.yaml | 63 +++++++++++++++++++ .../tests/searchers/test_searchers.py | 2 +- 5 files changed, 146 insertions(+), 21 deletions(-) create mode 100644 git_upstream/tests/searchers/scenarios/drop_cherry_picked_changes.yaml create mode 100644 git_upstream/tests/searchers/scenarios/drop_cherry_picked_changes_complex.yaml diff --git a/git_upstream/lib/searchers.py b/git_upstream/lib/searchers.py index 81f9fa4..a62ec72 100644 --- a/git_upstream/lib/searchers.py +++ b/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 '' 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('--') - for start, end in search_list: - revision_spec = "{0}..{1}".format(start, end) + if upstream is None: + if previous_import: + 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 - """, revision_spec, " ".join(extra_args)) + git rev-list --topo-order %s %s %s + """, 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) diff --git a/git_upstream/lib/strategies.py b/git_upstream/lib/strategies.py index 6da4558..6142431 100644 --- a/git_upstream/lib/strategies.py +++ b/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): diff --git a/git_upstream/tests/searchers/scenarios/drop_cherry_picked_changes.yaml b/git_upstream/tests/searchers/scenarios/drop_cherry_picked_changes.yaml new file mode 100644 index 0000000..e5c1d48 --- /dev/null +++ b/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: [] diff --git a/git_upstream/tests/searchers/scenarios/drop_cherry_picked_changes_complex.yaml b/git_upstream/tests/searchers/scenarios/drop_cherry_picked_changes_complex.yaml new file mode 100644 index 0000000..5fb5c45 --- /dev/null +++ b/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] diff --git a/git_upstream/tests/searchers/test_searchers.py b/git_upstream/tests/searchers/test_searchers.py index 8e4e441..aa0758f 100644 --- a/git_upstream/tests/searchers/test_searchers.py +++ b/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]))