diff --git a/DESCRIPTION b/DESCRIPTION index b89e82f..c6432b0 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -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 diff --git a/KNOWN-ISSUES.md b/KNOWN-ISSUES.md deleted file mode 100644 index cbe39b2..0000000 --- a/KNOWN-ISSUES.md +++ /dev/null @@ -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. diff --git a/README.md b/README.md index ca5ab05..5800936 100644 --- a/README.md +++ b/README.md @@ -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 . diff --git a/git_upstream/lib/searchers.py b/git_upstream/lib/searchers.py index f970e35..c39eaa4 100644 --- a/git_upstream/lib/searchers.py +++ b/git_upstream/lib/searchers.py @@ -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. diff --git a/git_upstream/tests/test_searchers.py b/git_upstream/tests/test_searchers.py index 455d0a7..4ff4619 100644 --- a/git_upstream/tests/test_searchers.py +++ b/git_upstream/tests/test_searchers.py @@ -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) diff --git a/git_upstream/tests/test_strategies.py b/git_upstream/tests/test_strategies.py index efae8a0..3d49d42 100644 --- a/git_upstream/tests/test_strategies.py +++ b/git_upstream/tests/test_strategies.py @@ -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)