diff --git a/git_upstream/lib/searchers.py b/git_upstream/lib/searchers.py index dc86864..5d65d4f 100644 --- a/git_upstream/lib/searchers.py +++ b/git_upstream/lib/searchers.py @@ -63,6 +63,67 @@ class Searcher(GitMixin): """ pass + def _check_merge_is_previous(self, mergecommit, parent, last_merge): + """Check if merge commit and parent describes previous import + + Method checks if the given merge and parent commits describe a + previous import merge commit and returns a tuple of the merge + commit if it is the previous import merge, and a list of additional + commits to be excluded from any history when looking for carried + changes. + """ + + # if the parent tree doesn't match the merge commit tree, we can + # skip inspecting it as it and it's parent commit must be + # included as it contributes changes to the tree. + if (self.git.rev_parse("%s^{tree}" % parent) != + self.git.rev_parse("%s^{tree}" % mergecommit)): + return False, [] + + mergebase = self.git.merge_base(parent, self.commit, + with_exceptions=False) + self.log.debug( + """\ + previous upstream: %s + merge-base: %s + parent: %s + """, self.commit, mergebase, parent.hexsha) + + # if not a valid response from merge-base, we have an additional + # branch with unrelated history that can be ignored + if not mergebase: + self.log.info( + """\ + Found merge of additional branch: + %s + """, mergecommit) + return False, ["^%s" % parent] + + # otherwise we have a descendant commit with the same tree that + # requires further inspection to determine if it is really the + # previous import merge. + + # if the parent is not a descendent of the previous upstream, will + # need to determine whether to exclude + if mergebase != self.commit.hexsha: + # if we're checking the last merge in the list, then looking at + # the previous mainline that was replaced and should ignore + if mergecommit == last_merge: + # also means we've found the previous import + return True, ["^%s" % parent] + + # otherwise this an unusual state where we are looking at the + # merge of the previous import with a change that landed on the + # previous target mainline but was not included in the changes + # that where on the previous import. This can occur due to a + # change being approved/landed after the import was performed + return False, [] + + # otherwise looking at the previous import merge commit and the parent + # from the previous import branch, so exclude all other parents. + return True, ["^%s" % ip + for ip in mergecommit.parents if ip != parent] + def list(self): """ Returns a list of Commit objects, between the '' revision @@ -88,20 +149,41 @@ class Searcher(GitMixin): topo_order=True, ancestry_path=True, merges=True)) ignore_args = [] - for c in merge_list: - for p in c.parents: - # previous merge using 'ours' strategy to ignore commits - if (self.git.rev_parse("%s^{tree}" % p.hexsha) == - self.git.rev_parse("%s^{tree}" % c.hexsha)): - ignore_args.extend(["^%s" % ip - for ip in c.parents if ip != p]) - break + previous_import = False + for mergecommit, parent in ((mc, p) + for mc in merge_list + for p in mc.parents): + # inspect each + previous_import, ignores = self._check_merge_is_previous( + mergecommit, parent, merge_list[-1]) - if merge_list: + if ignores: + self.log.debug( + """\ + Adding following to ignore list: + %s + """, "\n ".join(ignores)) + ignore_args.extend(ignores) + + if previous_import: + self.log.info( + """\ + Found the previous import merge: + %s + """, mergecommit) + break + + # handle old scenario where upstream would be merged in first using + # 'ours' to replace the existing target, followed by applying the + # replayed local changes. Possibly removable as defaulted to using + # an import branch and only merging the final result of upstream + + # local changes. + if merge_list and not previous_import: for p in merge_list[-1].parents: if p.hexsha == self.commit.hexsha: ignore_args.extend(["^%s" % ip - for ip in c.parents if ip != p]) + for ip in merge_list[-1].parents + if ip != p]) # walk the tree and find all commits that lie in the path between the # commit found by find() and head of the branch to provide a list of diff --git a/git_upstream/tests/searchers/scenarios/changes_merge_result_identical_on_both_sides.yaml b/git_upstream/tests/searchers/scenarios/changes_merge_result_identical_on_both_sides.yaml new file mode 100644 index 0000000..b8d9c93 --- /dev/null +++ b/git_upstream/tests/searchers/scenarios/changes_merge_result_identical_on_both_sides.yaml @@ -0,0 +1,57 @@ +# +# Copyright (c) 2016 Hewlett-Packard Enterprise Development Company, L.P. +# +# 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: | + Construct a repo layout where using a complex layout involving additional + branches having been included, and a previous import from upstream having + been completed, test that if the previous import resulted in the same + commits and tree contents on both sides, that the tool can still correctly + find the previous import point. + + Repository layout being tested + + B---C---D---G---H---I---J master + / / + / G1 import/next + / / + / B1--C1--D1 + / / + A---E---F---K---L upstream/master + + tree: + - [A, []] + - [B, [A]] + - [C, [B]] + - [D, [C]] + - [E, [A]] + - [F, [E]] + - [G, [D]] + - [B1, [F]] + - [C1, [B1]] + - [D1, [C1]] + - [G1, [D1]] + - [H, [=G, G1]] + - [I, [H]] + - [J, [I]] + - [K, [F]] + - [L, [K]] + + branches: + head: [master, J] + upstream: [upstream/master, L] + + expected_changes: [B1, C1, D1, G1, H, I, J] diff --git a/git_upstream/tests/searchers/scenarios/changes_merge_result_identical_to_previous.yaml b/git_upstream/tests/searchers/scenarios/changes_merge_result_identical_to_previous.yaml new file mode 100644 index 0000000..8194021 --- /dev/null +++ b/git_upstream/tests/searchers/scenarios/changes_merge_result_identical_to_previous.yaml @@ -0,0 +1,58 @@ +# +# Copyright (c) 2016 Hewlett-Packard Enterprise Development Company, L.P. +# +# 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: | + Construct a repo layout where using a complex layout involving additional + branches having been included, and a previous import from upstream having + been completed, test that if the previous import resulted in the same + contents of the tree after merge as was there before that we can still + correctly find the previous import point. + + Repository layout being tested + + B---C---D---G---I---J---K master + / \ / + / ----H + / / + / B1--C1--D1-G1 import/next + / / + A---E---F---L---M upstream/master + + tree: + - [A, []] + - [B, [A]] + - [C, [B]] + - [D, [C]] + - [E, [A]] + - [F, [E]] + - [G, [D]] + - [B1, [F]] + - [C1, [B1]] + - [D1, [C1]] + - [G1, [D1]] + - [H, [G, =G1]] + - [I, [G]] + - [J, [=I, H]] + - [K, [J]] + - [L, [F]] + - [M, [L]] + + branches: + head: [master, K] + upstream: [upstream/master, M] + + expected_changes: [I, B1, C1, D1, G1, H, J, K]