Add support and CLI option to search multiple refs for last import
When auto-detecting the most recent point in the history of the current branch to be updated that came from upstream, need to support searching more than the given upstream reference where the user is switching the upstream branch being tracked. Use a default of 'upstream/*' and add a CLI option to set a different pattern to be searched in cases where the repository is not using the same naming. The CLI option '--search-refs' may be specified multiple times to provide multiple ref patterns to be searched using simple globbing. The default if none provided by the user is 'upstream/*'. Change-Id: If890d1bee015d6b495a5caa60dd8f7783cf7c4e0 Closes-Bug: #1380652
This commit is contained in:
@@ -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)
|
||||
|
@@ -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='<pattern>',
|
||||
default=['upstream/*'], dest='search_refs',
|
||||
help='Refs to search for previous import commit. May be '
|
||||
'specified multiple times.')
|
||||
@subcommand.arg('--into', dest='branch', metavar='<branch>', 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")
|
||||
|
@@ -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):
|
||||
"""
|
||||
|
@@ -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()
|
||||
|
||||
|
@@ -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))
|
||||
|
@@ -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')
|
||||
|
@@ -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()])
|
||||
|
Reference in New Issue
Block a user