x/git-upstream
Code Issues Proposed changes

Find commits when parent is before import merge

Browse Source 
Switch to --simplify-merges to limit finding of commits to those that
contribute changes to the current tree which will ignore those discarded
by the previous import merge.

Use of --ancestry-path would miss commits that had a parent that was not
of the direct line between the commits being examined. This would mean
any commit created from before the merge to import the latest upstream
plus carried changes into the target branch would not be picked up on
the next import.

Remove the KNOWN-ISSUES markdown file as this was the only major known
issue that was contained within.

Change-Id: Id64e3b362b58263ec689fb5b9d0627faebcc1725
Closes-Bug: 1380520
This commit is contained in:
Darragh Bailey
2014-10-10 19:10:26 +01:00
parent 8129865433
commit 04cc82285d
6 changed files with 8 additions and 85 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
DESCRIPTION
View File

@@ -17,7 +17,7 @@ patches on top of upstream repositories. It provides commands to ease the use
of git for who needs to integrate big upstream projects in their environment.
The operations are performed using Git commands.
See also README.md, USAGE.md and KNOWN-ISSUES.md files.
See also README.md, and USAGE.md files.
- What git-upstream is not

67
KNOWN-ISSUES.md
View File

@@ -1,67 +0,0 @@
# git-upstream import could miss some local changes
## Problem description
If previous local changes were based off of a point before the previous import
merge was complete, and were not rebased to be applied after this point before
being merged, the import command will miss including these when discarding
previous obsolete history.
Given a previous import like the following where previously changes B & C were
replayed, and the import was merged as E. Where subsequently a change D which
was proposed before the import, is merged to mainline, a subsequent run of the
tool to import K will fail to include D along with B', C' & G' when discarding
the assumed unnecessary merges E & F. While these should be discarded, the tool
needs to check to make sure if there are dangling changes that should be
included.
D----
/ \
B---C---E---F---L mainline
/ /
/ B'--C' import/G
/ /
A---G---H---I---J---K upstream
The tool will find the following commit path to consider:
E---F---L
/
B'--C'
/
G
## Implementing a fix (WiP)
With respect to the previous diagrams, the correct behaviour is to find the
following set:
D----
\
E---F---L
/
B'--C'
/
G
This requires walking the list of initial found (2nd diagram) and examining
each merge commit up to the previous import to see if there is a change that
shares history with the other parent. i.e. look at F and see if D contains a
merge-base in common with E, an thus include. Or locate E and find any
differences between E & L that should be included.
## Workaround
Waiting for a proper fix to be implemented, a workaround for this issue would
be to perform the import only into a branch (usually master) which has no
"pending" commits in other branches to be merged that span across different
imports.
For gerrit this means not having pending reviews spanning across different
git-upstream import command runs.
This could be achieved in two different ways:
1) merging all pending reviews before an import;
2) rebasing all pending reviews after an import.

4
README.md
View File

@@ -268,10 +268,6 @@ different system, **notes must be present on the latter**. You can push notes
directly to git repository on the target system or push them in a different
repository and then pull notes from your target system.
# Known issues
Please see `KNOWN-ISSUES.md`.
# Authors
git-upstream was written by Darragh Bailey <dbailey@hp.com>.

8
git_upstream/lib/searchers.py
View File

@@ -80,8 +80,8 @@ class Searcher(GitMixin):
# commits to the caller
self.log.info(
"""\
Walking the ancestry path between found commit and target
git rev-list --parents --ancestry-path %s..%s
Walking the changes between found commit and target
git rev-list --parents --simplify-merges %s..%s
""", self.commit.hexsha, self.branch)
# depending on how many commits are returned, may need to examine use
@@ -90,7 +90,7 @@ class Searcher(GitMixin):
commit_list = Commit.iter_items(self.repo,
"{0}..{1}".format(self.commit.hexsha,
self.branch),
topo_order=True, ancestry_path=True)
topo_order=True, simplify_merges=True)
# chain the filters as generators so that we don't need to allocate new
# lists for each step in the filter chain.
@@ -280,7 +280,7 @@ class UpstreamMergeBaseSearcher(LogDedentMixin, Searcher):
def list(self, include_all=False):
"""
If a particular commit has been merged in mulitple times, walking the
ancestry path and returning all results will return all commits from
history and returning all results will return all commits from
each merge point, not just the last set which are usually what is
desired.

8
git_upstream/tests/test_searchers.py
View File

@@ -178,9 +178,7 @@ class TestUpstreamMergeBaseSearcher(BaseTestCase):
}
expected_changes = ["H", "D1", "E1", "I", "J", "K", "O", "P", "Q"]
self.expectFailure(
"Should fail to find change 'O'",
self._verify_expected, tree, branches, expected_changes)
self._verify_expected(tree, branches, expected_changes)
def test_search_multi_changes_upload_prior_to_import(self):
"""Construct a repo layout where using a complex layout involving
@@ -232,6 +230,4 @@ class TestUpstreamMergeBaseSearcher(BaseTestCase):
}
expected_changes = ["H", "D1", "E1", "I", "J", "K", "L", "M", "O", "P"]
self.expectFailure(
"Should fail to find changes 'J' and 'L'",
self._verify_expected, tree, branches, expected_changes)
self._verify_expected(tree, branches, expected_changes)

4
git_upstream/tests/test_strategies.py
View File

@@ -182,6 +182,4 @@ class TestStrategies(BaseTestCase):
}
expected_changes = ["D1", "E1", "J", "K", "O", "Q"]
self.expectFailure(
"Should fail to find change 'O'",
self._verify_expected, tree, branches, expected_changes)
self._verify_expected(tree, branches, expected_changes)