x/git-upstream
Code Issues Proposed changes

Improve detection of the previous import merge

Browse Source 
Ensure detection of the previous import merge commit checks whether the
parents being considered for exclusion are actually reachable from the
previous upstream commit that was imported last time.

This establishes which parts of the tree should be excluded in the cases
where the previous import merge was itself merged in due to another
change landing on the tree between the time the import was done and it
was approved.

Include two new unit tests that describe these scenarios.

Change-Id: I638784167190c40f76d87be98885de3f1f3a15c6
Closes-Bug: #1515583
This commit is contained in:
Darragh Bailey
2015-12-18 21:00:24 +00:00
parent 5c523d6655
commit c13695d82b
3 changed files with 207 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

102
git_upstream/lib/searchers.py
View File

@@ -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 '<commitish>' 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

57
git_upstream/tests/searchers/scenarios/changes_merge_result_identical_on_both_sides.yaml Normal file
View File

@@ -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]

58
git_upstream/tests/searchers/scenarios/changes_merge_result_identical_to_previous.yaml Normal file
View File

@@ -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]