diff --git a/git_upstream/commands/__init__.py b/git_upstream/commands/__init__.py index 2908c14..a48cecf 100644 --- a/git_upstream/commands/__init__.py +++ b/git_upstream/commands/__init__.py @@ -16,10 +16,28 @@ # limitations under the License. # +import argparse import os import sys +class AppendReplaceAction(argparse._AppendAction): + """Allows setting of a default value which is overriden by the first use + of the option, while subsequent calls will then append. + """ + def __init__(self, *args, **kwargs): + super(AppendReplaceAction, self).__init__(*args, **kwargs) + self._reset_default = False + self.default = list(self.default) + + def __call__(self, parser, namespace, values, option_string=None): + if not self._reset_default: + setattr(namespace, self.dest, []) + self._reset_default = True + super(AppendReplaceAction, self).__call__(parser, namespace, values, + option_string) + + def get_subcommands(subparsers): subcommands = _find_actions(subparsers, os.path.dirname(__file__)) @@ -45,6 +63,7 @@ def _find_actions(subparsers, module_path): command, help=help, description=desc) + subparser.register('action', 'append_replace', AppendReplaceAction) for (args, kwargs) in args: subparser.add_argument(*args, **kwargs) diff --git a/git_upstream/commands/import.py b/git_upstream/commands/import.py index 2bd276a..672a9f3 100644 --- a/git_upstream/commands/import.py +++ b/git_upstream/commands/import.py @@ -498,24 +498,29 @@ class LocateChangesWalk(LocateChangesStrategy): _strategy = "drop" - def __init__(self, branch="HEAD", search_ref=None, *args, **kwargs): + def __init__(self, branch="HEAD", upstream="upstream/master", + search_refs=None, *args, **kwargs): + + if not search_refs: + search_refs = [] + search_refs.insert(0, upstream) + self.searcher = UpstreamMergeBaseSearcher(branch=branch, - pattern=search_ref) - self.search_ref = search_ref + patterns=search_refs) + self.upstream = upstream super(LocateChangesWalk, self).__init__(*args, **kwargs) def filtered_iter(self): # may wish to make class used to remove duplicate objects configurable # through git-upstream specific 'git config' settings - if self.search_ref: - self.filters.append( - DiscardDuplicateGerritChangeId(self.search_ref, - limit=self.searcher.commit)) + self.filters.append( + DiscardDuplicateGerritChangeId(self.upstream, + limit=self.searcher.commit)) self.filters.append(NoMergeCommitFilter()) self.filters.append(ReverseCommitFilter()) self.filters.append(DroppedCommitFilter()) self.filters.append( - SupersededCommitFilter(self.search_ref, + SupersededCommitFilter(self.upstream, limit=self.searcher.commit)) return super(LocateChangesWalk, self).filtered_iter() @@ -542,6 +547,10 @@ class LocateChangesWalk(LocateChangesStrategy): default=LocateChangesWalk.get_strategy_name(), help='Use the given strategy to re-apply locally carried ' 'changes to the import branch. (default: %(default)s)') +@subcommand.arg('--search-refs', action='append_replace', metavar='', + default=['upstream/*'], dest='search_refs', + help='Refs to search for previous import commit. May be ' + 'specified multiple times.') @subcommand.arg('--into', dest='branch', metavar='', default='HEAD', help='Branch to take changes from, and replace with imported ' 'branch.') @@ -579,7 +588,8 @@ def do_import(args): logger.notice("Searching for previous import") strategy = ImportStrategiesFactory.create_strategy( - args.strategy, branch=args.branch, search_ref=args.upstream_branch) + args.strategy, branch=args.branch, upstream=args.upstream_branch, + search_refs=args.search_refs) if len(strategy) == 0: raise ImportUpstreamError("Cannot find previous import") diff --git a/git_upstream/lib/searchers.py b/git_upstream/lib/searchers.py index 25c3b52..3999c6d 100644 --- a/git_upstream/lib/searchers.py +++ b/git_upstream/lib/searchers.py @@ -124,34 +124,40 @@ class UpstreamMergeBaseSearcher(LogDedentMixin, Searcher): base available. """ - def __init__(self, pattern="upstream/*", search_tags=False, remotes=None, + def __init__(self, branch, patterns=None, search_tags=False, remotes=None, *args, **kwargs): + super(UpstreamMergeBaseSearcher, self).__init__(branch, *args, + **kwargs) + + if not patterns: + patterns = ["upstream/*"] + self._patterns = patterns + self._references = ["refs/heads/{0}".format(ref) + for ref in self.patterns] + if not remotes: remotes = [] - self._pattern = pattern - self._references = ["refs/heads/{0}".format(self.pattern)] - - super(UpstreamMergeBaseSearcher, self).__init__(*args, **kwargs) if remotes: self._references.extend( - ["refs/remotes/{0}/{1}".format(s, self.pattern) - for s in remotes]) + ["refs/remotes/{0}/{1}".format(s, ref) + for s in remotes for ref in self.patterns]) else: - self._references.append( - "refs/remotes/*/{0}".format(self.pattern)) + self._references.extend( + ["refs/remotes/*/{0}".format(ref) for ref in self.patterns]) if search_tags: - self._references.append("refs/tags/{0}".format(self.pattern)) + self._references.append( + ["refs/tags/{0}".format(ref) for ref in self.patterns]) @property - def pattern(self): + def patterns(self): """ Pattern to limit which references are searched when looking for a merge base commit. """ - return self._pattern + return self._patterns def find(self): """ diff --git a/git_upstream/tests/base.py b/git_upstream/tests/base.py index 4f5ad3c..737efbf 100644 --- a/git_upstream/tests/base.py +++ b/git_upstream/tests/base.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +import logging import os import tempfile @@ -130,6 +131,8 @@ class GitRepo(fixtures.Fixture): class BaseTestCase(testtools.TestCase): """Base Test Case for all tests.""" + logging.basicConfig() + def setUp(self): super(BaseTestCase, self).setUp() diff --git a/git_upstream/tests/test_import_cmd.py b/git_upstream/tests/test_import_cmd.py index 1f76ea6..863361b 100644 --- a/git_upstream/tests/test_import_cmd.py +++ b/git_upstream/tests/test_import_cmd.py @@ -21,6 +21,7 @@ import mock from testtools.matchers import Equals from git_upstream import main +from git_upstream.lib.pygitcompat import Commit from git_upstream.tests.base import BaseTestCase from string import lower @@ -158,3 +159,204 @@ class TestImportCommand(BaseTestCase): mock_logger.warning.assert_called_with( SubstringMatcher( containing="Previous import merged additional")) + + def test_import_switch_branches_search(self): + """Test that the import sub-command can correctly switch branches when + importing from upstream when given a usable search-ref. + + Repository layout being checked (assumed already replayed) + + E---F local/master + / + C---D upstream/stable + / + A---B---G---H upstream/master + + New branch to be tracked will be upstream/master, so the resulting + commits found should just be E & F. + + Test that result is as follows + + E---F---I local/master + / / + C---D / upstream/stable + / / + / E1--F1 + / / + A---B---G---H upstream/master + + """ + + tree = [ + ('A', []), + ('B', ['A']), + ('C', ['B']), + ('D', ['C']), + ('E', ['D']), + ('F', ['E']), + ('G', ['B']), + ('H', ['G']) + ] + + branches = { + 'head': ('master', 'F'), + 'upstream': ('upstream/master', 'H'), + 'stable': ('upstream/stable', 'D') + } + + self._build_git_tree(tree, branches.values()) + self.git.tag(inspect.currentframe().f_code.co_name, 'upstream/master') + args = self.parser.parse_args(['-q', 'import']) + self.assertThat(args.func(args), Equals(True), + "import command failed to complete succesfully") + changes = list(Commit.iter_items( + self.repo, 'upstream/master..master^2')) + self.assertThat(len(changes), Equals(2), + "should only have seen two changes, got: %s" % + ", ".join(["%s:%s" % (commit.hexsha, + commit.message.splitlines()[0]) + for commit in changes])) + for commit, node in zip(changes, ['F', 'E']): + subject = commit.message.splitlines()[0] + node_subject = self._graph[node].message.splitlines()[0] + self.assertThat(subject, Equals(node_subject), + "subject '%s' of commit '%s' does not match " + "subject '%s' of node '%s'" % ( + subject, commit.hexsha, node_subject, node)) + + def test_import_switch_branches_fails_without_search_ref(self): + """Test that the import sub-command finds additional changes when + not given a search-ref to look under. + + Repository layout being checked (assumed already replayed) + + E---F local/master + / + C---D upstream/stable + / + A---B---G---H upstream/master + + New branch to be tracked will be upstream/master, so the resulting + commits found will be C, D, E & F because of not telling the searcher + to look under all of the namespace. + + Test that result is as follows + + E---F-------------I local/master + / / + C---D / upstream/stable + / / + / C1---D1---E1--F1 + / / + A---B---G---H upstream/master + + """ + + tree = [ + ('A', []), + ('B', ['A']), + ('C', ['B']), + ('D', ['C']), + ('E', ['D']), + ('F', ['E']), + ('G', ['B']), + ('H', ['G']) + ] + + # use 'custom/*' to ensure defaults are overriden correctly + branches = { + 'head': ('master', 'F'), + 'upstream': ('custom/master', 'H'), + 'stable': ('custom/stable', 'D') + } + + self._build_git_tree(tree, branches.values()) + + self.git.tag(inspect.currentframe().f_code.co_name, 'custom/master') + args = self.parser.parse_args(['-q', 'import', + '--into=master', 'custom/master']) + self.assertThat(args.func(args), Equals(True), + "import command failed to complete succesfully") + changes = list(Commit.iter_items( + self.repo, 'custom/master..master^2')) + local_rebased = ['F', 'E', 'D', 'C'] + self.assertThat(len(changes), Equals(len(local_rebased)), + "should only have seen two changes, got: %s" % + ", ".join(["%s:%s" % (commit.hexsha, + commit.message.splitlines()[0]) + for commit in changes])) + for commit, node in zip(changes, local_rebased): + subject = commit.message.splitlines()[0] + node_subject = self._graph[node].message.splitlines()[0] + self.assertThat(subject, Equals(node_subject), + "subject '%s' of commit '%s' does not match " + "subject '%s' of node '%s'" % ( + subject, commit.hexsha, node_subject, node)) + + def test_import_switch_branches_search_ref_custom_namespace(self): + """Test that the import sub-command can correctly switch branches when + importing from upstream when given a usable search-ref. + + Repository layout being checked (assumed already replayed) + + E---F local/master + / + C---D upstream/stable + / + A---B---G---H upstream/master + + New branch to be tracked will be upstream/master, so the resulting + commits found should just be E & F. + + Test that result is as follows + + E---F---I local/master + / / + C---D / upstream/stable + / / + / E1--F1 + / / + A---B---G---H upstream/master + + """ + + tree = [ + ('A', []), + ('B', ['A']), + ('C', ['B']), + ('D', ['C']), + ('E', ['D']), + ('F', ['E']), + ('G', ['B']), + ('H', ['G']) + ] + + # use 'custom/*' to ensure defaults are overriden correctly + branches = { + 'head': ('master', 'F'), + 'upstream': ('custom/master', 'H'), + 'stable': ('custom/stable', 'D') + } + + self._build_git_tree(tree, branches.values()) + self.git.tag(inspect.currentframe().f_code.co_name, 'custom/master') + args = self.parser.parse_args(['-q', 'import', + '--search-refs=custom/*', + '--search-refs=custom-d/*', + '--into=master', 'custom/master']) + self.assertThat(args.func(args), Equals(True), + "import command failed to complete succesfully") + changes = list(Commit.iter_items( + self.repo, 'custom/master..master^2')) + self.assertThat(len(changes), Equals(2), + "should only have seen two changes, got: %s" % + ", ".join(["%s:%s" % (commit.hexsha, + commit.message.splitlines()[0]) + for commit in changes])) + for commit, node in zip(changes, ['F', 'E']): + subject = commit.message.splitlines()[0] + node_subject = self._graph[node].message.splitlines()[0] + self.assertThat(subject, Equals(node_subject), + "subject '%s' of commit '%s' does not match " + "subject '%s' of node '%s'" % ( + subject, commit.hexsha, node_subject, node)) diff --git a/git_upstream/tests/test_searchers.py b/git_upstream/tests/test_searchers.py index 36f3907..c8fca70 100644 --- a/git_upstream/tests/test_searchers.py +++ b/git_upstream/tests/test_searchers.py @@ -23,8 +23,10 @@ class TestUpstreamMergeBaseSearcher(BaseTestCase): def _verify_expected(self, tree, branches, expected_nodes, pattern=None): self._build_git_tree(tree, branches.values()) - searcher = UpstreamMergeBaseSearcher( - pattern=pattern or branches['upstream'][0], repo=self.repo) + if pattern: + pattern = [pattern] + searcher = UpstreamMergeBaseSearcher(branch=branches['head'][0], + patterns=pattern, repo=self.repo) self.assertEquals(self._commits_from_nodes(reversed(expected_nodes)), searcher.list()) @@ -312,4 +314,5 @@ class TestUpstreamMergeBaseSearcher(BaseTestCase): } expected_changes = ['E', 'I', 'B1', 'J'] - self._verify_expected(tree, branches, expected_changes) + self._verify_expected(tree, branches, expected_changes, + pattern='upstream/master') diff --git a/git_upstream/tests/test_strategies.py b/git_upstream/tests/test_strategies.py index 3d49d42..e47971d 100644 --- a/git_upstream/tests/test_strategies.py +++ b/git_upstream/tests/test_strategies.py @@ -28,7 +28,7 @@ class TestStrategies(BaseTestCase): self._build_git_tree(tree, branches.values()) strategy = LocateChangesWalk(branch=branches['head'][0], - search_ref=branches['upstream'][0]) + search_refs=[branches['upstream'][0]]) self.assertEquals(self._commits_from_nodes(expected_nodes), [c for c in strategy.filtered_iter()])