Merge "Find commits when parent is before import merge"
This commit is contained in:
		| @@ -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 | ||||
|  | ||||
|   | ||||
| @@ -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. | ||||
| @@ -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>. | ||||
|  | ||||
|   | ||||
| @@ -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. | ||||
|  | ||||
|   | ||||
| @@ -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) | ||||
|   | ||||
| @@ -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) | ||||
|   | ||||
		Reference in New Issue
	
	Block a user
	 Jenkins
					Jenkins