diff --git a/git_cl.py b/git_cl.py index ba5c54f5f..6c779728d 100755 --- a/git_cl.py +++ b/git_cl.py @@ -996,24 +996,22 @@ class _CQState(object): class _ParsedIssueNumberArgument(object): - def __init__(self, issue=None, patchset=None, hostname=None, codereview=None): + def __init__(self, issue=None, patchset=None, hostname=None): self.issue = issue self.patchset = patchset self.hostname = hostname - assert codereview in (None, 'gerrit', 'rietveld') - self.codereview = codereview @property def valid(self): return self.issue is not None -def ParseIssueNumberArgument(arg, codereview=None): +def ParseIssueNumberArgument(arg): """Parses the issue argument and returns _ParsedIssueNumberArgument.""" fail_result = _ParsedIssueNumberArgument() if arg.isdigit(): - return _ParsedIssueNumberArgument(issue=int(arg), codereview=codereview) + return _ParsedIssueNumberArgument(issue=int(arg)) if not arg.startswith('http'): return fail_result @@ -1023,10 +1021,6 @@ def ParseIssueNumberArgument(arg, codereview=None): except ValueError: return fail_result - if codereview is not None: - parsed = _CODEREVIEW_IMPLEMENTATIONS[codereview].ParseIssueURL(parsed_url) - return parsed or fail_result - return _GerritChangelistImpl.ParseIssueURL(parsed_url) or fail_result @@ -1064,26 +1058,16 @@ _CommentSummary = collections.namedtuple( class Changelist(object): """Changelist works with one changelist in local branch. - Supports two codereview backends: Rietveld or Gerrit, selected at object - creation. - Notes: * Not safe for concurrent multi-{thread,process} use. * Caches values from current branch. Therefore, re-use after branch change with great care. """ - def __init__(self, branchref=None, issue=None, codereview=None, **kwargs): + def __init__(self, branchref=None, issue=None, **kwargs): """Create a new ChangeList instance. - If issue is given, the codereview must be given too. - - If `codereview` is given, it must be 'rietveld' or 'gerrit'. - Otherwise, it's decided based on current configuration of the local branch, - with default being 'rietveld' for backwards compatibility. - See _load_codereview_impl for more details. - - **kwargs will be passed directly to codereview implementation. + **kwargs will be passed directly to Gerrit implementation. """ # Poke settings so we get the "configure your server" message if necessary. global settings @@ -1091,9 +1075,6 @@ class Changelist(object): # Happens when git_cl.py is used as a utility library. settings = Settings() - if issue: - assert codereview, 'codereview must be known, if issue is known' - self.branchref = branchref if self.branchref: assert branchref.startswith('refs/heads/') @@ -1112,44 +1093,7 @@ class Changelist(object): self._remote = None self._cached_remote_url = (False, None) # (is_cached, value) - self._codereview_impl = None - self._codereview = None - self._load_codereview_impl(codereview, **kwargs) - assert self._codereview_impl - assert self._codereview in _CODEREVIEW_IMPLEMENTATIONS - - def _load_codereview_impl(self, codereview=None, **kwargs): - if codereview: - assert codereview in _CODEREVIEW_IMPLEMENTATIONS, ( - 'codereview {} not in {}'.format(codereview, - _CODEREVIEW_IMPLEMENTATIONS)) - cls = _CODEREVIEW_IMPLEMENTATIONS[codereview] - self._codereview = codereview - self._codereview_impl = cls(self, **kwargs) - return - - # Automatic selection based on issue number set for a current branch. - # Rietveld takes precedence over Gerrit. - assert not self.issue - # Whether we find issue or not, we are doing the lookup. - self.lookedup_issue = True - if self.GetBranch(): - for codereview, cls in _CODEREVIEW_IMPLEMENTATIONS.iteritems(): - issue = _git_get_branch_config_value( - cls.IssueConfigKey(), value_type=int, branch=self.GetBranch()) - if issue: - self._codereview = codereview - self._codereview_impl = cls(self, **kwargs) - self.issue = int(issue) - return - - # No issue is set for this branch, so default to gerrit. - return self._load_codereview_impl( - codereview='gerrit', - **kwargs) - - def IsGerrit(self): - return self._codereview == 'gerrit' + self._codereview_impl = _GerritChangelistImpl(self, **kwargs) def GetCCList(self): """Returns the users cc'd on this CL. @@ -1476,6 +1420,7 @@ class Changelist(object): 'Removing it.') RunGit(['commit', '--amend', '-m', git_footers.remove_footer(msg, 'Change-Id')]) + self.lookedup_issue = True self.issue = None self.patchset = None @@ -1571,7 +1516,7 @@ class Changelist(object): except presubmit_support.PresubmitFailure as e: DieWithError('%s\nMaybe your depot_tools is out of date?' % e) - def CMDPatchIssue(self, issue_arg, reject, nocommit, directory): + def CMDPatchIssue(self, issue_arg, nocommit): """Fetches and applies the issue patch from codereview to local branch.""" if isinstance(issue_arg, (int, long)) or issue_arg.isdigit(): parsed_issue_arg = _ParsedIssueNumberArgument(int(issue_arg)) @@ -1583,11 +1528,10 @@ class Changelist(object): DieWithError('Failed to parse issue argument "%s". ' 'Must be an issue number or a valid URL.' % issue_arg) return self._codereview_impl.CMDPatchWithParsedIssue( - parsed_issue_arg, reject, nocommit, directory, False) + parsed_issue_arg, nocommit, False) def CMDUpload(self, options, git_diff_args, orig_args): """Uploads a change to codereview.""" - assert self.IsGerrit() custom_cl_base = None if git_diff_args: custom_cl_base = base_branch = git_diff_args[0] @@ -1803,18 +1747,12 @@ class _ChangelistCodereviewBase(object): """Returns the most recent patchset number from the codereview site.""" raise NotImplementedError() - def CMDPatchWithParsedIssue(self, parsed_issue_arg, reject, nocommit, - directory, force): + def CMDPatchWithParsedIssue(self, parsed_issue_arg, nocommit, force): """Fetches and applies the issue. Arguments: parsed_issue_arg: instance of _ParsedIssueNumberArgument. - reject: if True, reject the failed patch instead of switching to 3-way - merge. Rietveld only. - nocommit: do not commit the patch, thus leave the tree dirty. Rietveld - only. - directory: switch to directory before applying the patch. Rietveld only. - force: if true, overwrites existing local state. + nocommit: do not commit the patch, thus leave the tree dirty. """ raise NotImplementedError() @@ -2379,10 +2317,7 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): break return 0 - def CMDPatchWithParsedIssue(self, parsed_issue_arg, reject, nocommit, - directory, force): - assert not reject - assert not directory + def CMDPatchWithParsedIssue(self, parsed_issue_arg, nocommit, force): assert parsed_issue_arg.valid self._changelist.issue = parsed_issue_arg.issue @@ -2471,8 +2406,7 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): return _ParsedIssueNumberArgument( issue=int(match.group(3)), patchset=int(match.group(5)) if match.group(5) else None, - hostname=parsed_url.netloc, - codereview='gerrit') + hostname=parsed_url.netloc) return None def _GerritCommitMsgHookCheck(self, offer_removal): @@ -3039,28 +2973,17 @@ def _add_codereview_issue_select_options(parser, extra=""): parser.add_option('-i', '--issue', type=int, help=text) -def _process_codereview_issue_select_options(parser, options): - _process_codereview_select_options(parser, options) - if options.issue is not None and not options.forced_codereview: - parser.error('--issue must be specified with either --rietveld or --gerrit') - - def _add_codereview_select_options(parser): - """Appends --gerrit and --rietveld options to force specific codereview.""" + """Appends --gerrit option to force specific codereview.""" parser.codereview_group = optparse.OptionGroup( - parser, 'EXPERIMENTAL! Codereview override options') + parser, 'DEPRECATED! Codereview override options') parser.add_option_group(parser.codereview_group) parser.codereview_group.add_option( '--gerrit', action='store_true', - help='Force the use of Gerrit for codereview') - parser.codereview_group.add_option( - '--rietveld', action='store_true', - help='Force the use of Rietveld for codereview') + help='Deprecated. Noop. Do not use.') def _process_codereview_select_options(parser, options): - if options.rietveld: - parser.error('--rietveld is no longer supported.') options.forced_codereview = None if options.gerrit: options.forced_codereview = 'gerrit' @@ -4117,7 +4040,7 @@ def CMDstatus(parser, args): _add_codereview_issue_select_options( parser, 'Must be in conjunction with --field.') options, args = parser.parse_args(args) - _process_codereview_issue_select_options(parser, options) + _process_codereview_select_options(parser, options) if args: parser.error('Unsupported args: %s' % args) auth_config = auth.extract_auth_config_from_options(options) @@ -4126,8 +4049,7 @@ def CMDstatus(parser, args): parser.error('--field must be specified with --issue.') if options.field: - cl = Changelist(auth_config=auth_config, issue=options.issue, - codereview=options.forced_codereview) + cl = Changelist(auth_config=auth_config, issue=options.issue) if options.field.startswith('desc'): print(cl.GetDescription()) elif options.field == 'id': @@ -4297,15 +4219,15 @@ def CMDissue(parser, args): return 0 if len(args) > 0: - issue = ParseIssueNumberArgument(args[0], options.forced_codereview) + issue = ParseIssueNumberArgument(args[0]) if not issue.valid: DieWithError('Pass a url or number to set the issue, 0 to unset it, ' 'or no argument to list it.\n' 'Maybe you want to run git cl status?') - cl = Changelist(codereview=issue.codereview) + cl = Changelist() cl.SetIssue(issue.issue) else: - cl = Changelist(codereview=options.forced_codereview) + cl = Changelist() print('Issue number: %s (%s)' % (cl.GetIssue(), cl.GetIssueURL())) if options.json: write_json(options.json, { @@ -4323,8 +4245,7 @@ def CMDcomments(parser, args): parser.add_option('-p', '--publish', action='store_true', help='marks CL as ready and sends comment to reviewers') parser.add_option('-i', '--issue', dest='issue', - help='review issue id (defaults to current issue). ' - 'If given, requires --rietveld or --gerrit') + help='review issue id (defaults to current issue).') parser.add_option('-m', '--machine-readable', dest='readable', action='store_false', default=True, help='output comments in a format compatible with ' @@ -4344,10 +4265,7 @@ def CMDcomments(parser, args): except ValueError: DieWithError('A review issue ID is expected to be a number.') - cl = Changelist(issue=issue, codereview='gerrit', auth_config=auth_config) - - if not cl.IsGerrit(): - parser.error('Rietveld is not supported.') + cl = Changelist(issue=issue, auth_config=auth_config) if options.comment: cl.AddComment(options.comment, options.publish) @@ -4402,22 +4320,19 @@ def CMDdescription(parser, args): target_issue_arg = None if len(args) > 0: - target_issue_arg = ParseIssueNumberArgument(args[0], - options.forced_codereview) + target_issue_arg = ParseIssueNumberArgument(args[0]) if not target_issue_arg.valid: parser.error('Invalid issue ID or URL.') kwargs = { 'auth_config': auth.extract_auth_config_from_options(options), - 'codereview': options.forced_codereview, } detected_codereview_from_url = False if target_issue_arg: kwargs['issue'] = target_issue_arg.issue kwargs['codereview_host'] = target_issue_arg.hostname - if target_issue_arg.codereview and not options.forced_codereview: + if not args[0].isdigit() and not options.forced_codereview: detected_codereview_from_url = True - kwargs['codereview'] = target_issue_arg.codereview cl = Changelist(**kwargs) if not cl.GetIssue(): @@ -4425,8 +4340,7 @@ def CMDdescription(parser, args): DieWithError('This branch has no associated changelist.') if detected_codereview_from_url: - logging.info('canonical issue/change URL: %s (type: %s)\n', - cl.GetIssueURL(), target_issue_arg.codereview) + logging.info('canonical issue/change URL: %s\n', cl.GetIssueURL()) description = ChangeDescription(cl.GetDescription()) @@ -4445,8 +4359,7 @@ def CMDdescription(parser, args): description.set_description(text) else: - description.prompt(git_footer=cl.IsGerrit()) - + description.prompt() if cl.GetDescription().strip() != description.description: cl.UpdateDescription(description.description, force=options.force) return 0 @@ -4775,17 +4688,7 @@ def CMDupload(parser, args): # For sanity of test expectations, do this otherwise lazy-loading *now*. settings.GetIsGerrit() - cl = Changelist(auth_config=auth_config, codereview=options.forced_codereview) - if not cl.IsGerrit(): - # Error out with instructions for repos not yet configured for Gerrit. - print('=====================================') - print('NOTICE: Rietveld is no longer supported. ' - 'You can upload changes to Gerrit with') - print(' git cl upload --gerrit') - print('or set Gerrit to be your default code review tool with') - print(' git config gerrit.host true') - print('=====================================') - return 1 + cl = Changelist(auth_config=auth_config) return cl.CMDUpload(options, args, orig_args) @@ -4870,9 +4773,6 @@ def CMDland(parser, args): cl = Changelist(auth_config=auth_config) - if not cl.IsGerrit(): - parser.error('Rietveld is not supported.') - if not cl.GetIssue(): DieWithError('You must upload the change first to Gerrit.\n' ' If you would rather have `git cl land` upload ' @@ -4889,14 +4789,8 @@ def CMDpatch(parser, args): help='create a new branch off trunk for the patch') parser.add_option('-f', '--force', action='store_true', help='overwrite state on the current or chosen branch') - parser.add_option('-d', '--directory', action='store', metavar='DIR', - help='change to the directory DIR immediately, ' - 'before doing anything else. Rietveld only.') - parser.add_option('--reject', action='store_true', - help='failed patches spew .rej files rather than ' - 'attempting a 3-way merge. Rietveld only.') parser.add_option('-n', '--no-commit', action='store_true', dest='nocommit', - help='don\'t commit after patch applies. Rietveld only.') + help='don\'t commit after patch applies.') group = optparse.OptionGroup( parser, @@ -4925,8 +4819,7 @@ def CMDpatch(parser, args): if len(args) > 0: parser.error('--reapply implies no additional arguments.') - cl = Changelist(auth_config=auth_config, - codereview=options.forced_codereview) + cl = Changelist(auth_config=auth_config) if not cl.GetIssue(): parser.error('Current branch must have an associated issue.') @@ -4938,26 +4831,22 @@ def CMDpatch(parser, args): if options.pull: RunGit(['pull']) - return cl.CMDPatchIssue(cl.GetIssue(), options.reject, options.nocommit, - options.directory) + return cl.CMDPatchIssue(cl.GetIssue(), options.nocommit) if len(args) != 1 or not args[0]: parser.error('Must specify issue number or URL.') - target_issue_arg = ParseIssueNumberArgument(args[0], - options.forced_codereview) + target_issue_arg = ParseIssueNumberArgument(args[0]) if not target_issue_arg.valid: parser.error('Invalid issue ID or URL.') cl_kwargs = { 'auth_config': auth_config, 'codereview_host': target_issue_arg.hostname, - 'codereview': options.forced_codereview, } detected_codereview_from_url = False - if target_issue_arg.codereview and not options.forced_codereview: + if not args[0].isdigit() and not options.forced_codereview: detected_codereview_from_url = True - cl_kwargs['codereview'] = target_issue_arg.codereview cl_kwargs['issue'] = target_issue_arg.issue # We don't want uncommitted changes mixed up with the patch. @@ -4972,19 +4861,11 @@ def CMDpatch(parser, args): cl = Changelist(**cl_kwargs) - if cl.IsGerrit(): - if options.reject: - parser.error('--reject is not supported with Gerrit code review.') - if options.directory: - parser.error('--directory is not supported with Gerrit codereview.') - if detected_codereview_from_url: - print('canonical issue/change URL: %s (type: %s)\n' % - (cl.GetIssueURL(), target_issue_arg.codereview)) + print('canonical issue/change URL: %s\n' % cl.GetIssueURL()) - return cl.CMDPatchWithParsedIssue(target_issue_arg, options.reject, - options.nocommit, options.directory, - options.force) + return cl.CMDPatchWithParsedIssue( + target_issue_arg, options.nocommit, options.force) def GetTreeStatus(url=None): @@ -5076,7 +4957,7 @@ def CMDtry(parser, args): auth.add_auth_options(parser) _add_codereview_issue_select_options(parser) options, args = parser.parse_args(args) - _process_codereview_issue_select_options(parser, options) + _process_codereview_select_options(parser, options) auth_config = auth.extract_auth_config_from_options(options) if options.master and options.master.startswith('luci.'): @@ -5090,14 +4971,12 @@ def CMDtry(parser, args): if args: parser.error('Unknown arguments: %s' % args) - cl = Changelist(auth_config=auth_config, issue=options.issue, - codereview=options.forced_codereview) + cl = Changelist(auth_config=auth_config, issue=options.issue) if not cl.GetIssue(): parser.error('Need to upload first.') - if cl.IsGerrit(): - # HACK: warm up Gerrit change detail cache to save on RPCs. - cl._codereview_impl._GetChangeDetail(['DETAILED_ACCOUNTS', 'ALL_REVISIONS']) + # HACK: warm up Gerrit change detail cache to save on RPCs. + cl._codereview_impl._GetChangeDetail(['DETAILED_ACCOUNTS', 'ALL_REVISIONS']) error_message = cl.CannotTriggerTryJobReason() if error_message: @@ -5154,14 +5033,12 @@ def CMDtry_results(parser, args): auth.add_auth_options(parser) _add_codereview_issue_select_options(parser) options, args = parser.parse_args(args) - _process_codereview_issue_select_options(parser, options) + _process_codereview_select_options(parser, options) if args: parser.error('Unrecognized args: %s' % ' '.join(args)) auth_config = auth.extract_auth_config_from_options(options) - cl = Changelist( - issue=options.issue, codereview=options.forced_codereview, - auth_config=auth_config) + cl = Changelist(issue=options.issue, auth_config=auth_config) if not cl.GetIssue(): parser.error('Need to upload first.') @@ -5247,15 +5124,14 @@ def CMDset_commit(parser, args): auth.add_auth_options(parser) _add_codereview_issue_select_options(parser) options, args = parser.parse_args(args) - _process_codereview_issue_select_options(parser, options) + _process_codereview_select_options(parser, options) auth_config = auth.extract_auth_config_from_options(options) if args: parser.error('Unrecognized args: %s' % ' '.join(args)) if options.dry_run and options.clear: parser.error('Only one of --dry-run and --clear are allowed.') - cl = Changelist(auth_config=auth_config, issue=options.issue, - codereview=options.forced_codereview) + cl = Changelist(auth_config=auth_config, issue=options.issue) if options.clear: state = _CQState.NONE elif options.dry_run: @@ -5274,12 +5150,11 @@ def CMDset_close(parser, args): _add_codereview_issue_select_options(parser) auth.add_auth_options(parser) options, args = parser.parse_args(args) - _process_codereview_issue_select_options(parser, options) + _process_codereview_select_options(parser, options) auth_config = auth.extract_auth_config_from_options(options) if args: parser.error('Unrecognized args: %s' % ' '.join(args)) - cl = Changelist(auth_config=auth_config, issue=options.issue, - codereview=options.forced_codereview) + cl = Changelist(auth_config=auth_config, issue=options.issue) # Ensure there actually is an issue to close. if not cl.GetIssue(): DieWithError('ERROR: No issue to close.') @@ -5683,7 +5558,7 @@ def GetDirtyMetricsDirs(diff_files): @subcommand.usage('') @metrics.collector.collect_metrics('git cl checkout') def CMDcheckout(parser, args): - """Checks out a branch associated with a given Rietveld or Gerrit issue.""" + """Checks out a branch associated with a given Gerrit issue.""" _, args = parser.parse_args(args) if len(args) != 1: diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 0ddbad94a..82ef8c958 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -154,8 +154,7 @@ class SystemExitMock(Exception): class TestGitClBasic(unittest.TestCase): def test_get_description(self): - cl = git_cl.Changelist(issue=1, codereview='gerrit', - codereview_host='host') + cl = git_cl.Changelist(issue=1, codereview_host='host') cl.description = 'x' cl.has_description = True cl._codereview_impl.FetchDescription = lambda *a, **kw: 'y' @@ -164,8 +163,7 @@ class TestGitClBasic(unittest.TestCase): self.assertEquals(cl.GetDescription(), 'y') def test_description_footers(self): - cl = git_cl.Changelist(issue=1, codereview='gerrit', - codereview_host='host') + cl = git_cl.Changelist(issue=1, codereview_host='host') cl.description = '\n'.join([ 'This is some message', '', @@ -472,7 +470,7 @@ class TestGitClBasic(unittest.TestCase): class TestParseIssueURL(unittest.TestCase): def _validate(self, parsed, issue=None, patchset=None, hostname=None, - codereview=None, fail=False): + fail=False): self.assertIsNotNone(parsed) if fail: self.assertFalse(parsed.valid) @@ -481,7 +479,6 @@ class TestParseIssueURL(unittest.TestCase): self.assertEqual(parsed.issue, issue) self.assertEqual(parsed.patchset, patchset) self.assertEqual(parsed.hostname, hostname) - self.assertEqual(parsed.codereview, codereview) def _run_and_validate(self, func, url, *args, **kwargs): result = func(urlparse.urlparse(url)) @@ -493,7 +490,7 @@ class TestParseIssueURL(unittest.TestCase): def test_gerrit(self): def test(url, *args, **kwargs): self._run_and_validate(git_cl._GerritChangelistImpl.ParseIssueURL, url, - *args, codereview='gerrit', **kwargs) + *args, **kwargs) test('http://chrome-review.source.com/c/123', 123, None, 'chrome-review.source.com') @@ -516,9 +513,7 @@ class TestParseIssueURL(unittest.TestCase): def test_ParseIssueNumberArgument(self): def test(arg, *args, **kwargs): - codereview_hint = kwargs.pop('hint', None) - self._validate(git_cl.ParseIssueNumberArgument(arg, codereview_hint), - *args, **kwargs) + self._validate(git_cl.ParseIssueNumberArgument(arg), *args, **kwargs) test('123', 123) test('', fail=True) @@ -527,17 +522,12 @@ class TestParseIssueURL(unittest.TestCase): test('123a', fail=True) test('ssh://chrome-review.source.com/#/c/123/4/', fail=True) - # Looks like Rietveld and Gerrit, but we should select Gerrit now - # w/ or w/o hint. test('https://codereview.source.com/123', - 123, None, 'codereview.source.com', 'gerrit', - hint='gerrit') - test('https://codereview.source.com/123', - 123, None, 'codereview.source.com', 'gerrit') + 123, None, 'codereview.source.com') # Gerrrit. test('https://chrome-review.source.com/c/123/4', - 123, 4, 'chrome-review.source.com', 'gerrit') + 123, 4, 'chrome-review.source.com') test('https://chrome-review.source.com/bad/123/4', fail=True) @@ -807,18 +797,31 @@ class TestGitCl(TestCase): @classmethod def _gerrit_ensure_auth_calls( - cls, issue=None, skip_auth_check=False, short_hostname='chromium'): + cls, issue=None, skip_auth_check=False, short_hostname='chromium', + custom_cl_base=None): cmd = ['git', 'config', '--bool', 'gerrit.skip-ensure-authenticated'] if skip_auth_check: return [((cmd, ), 'true')] calls = [((cmd, ), CERR1)] + + if custom_cl_base: + calls += [ + ((['git', 'symbolic-ref', 'HEAD'],), 'master'), + ] + calls.extend([ ((['git', 'config', 'branch.master.merge'],), 'refs/heads/master'), ((['git', 'config', 'branch.master.remote'],), 'origin'), ((['git', 'config', 'remote.origin.url'],), 'https://%s.googlesource.com/my/repo' % short_hostname), ]) + + calls += [ + ((['git', 'config', 'branch.master.gerritissue'],), + CERR1 if issue is None else str(issue)), + ] + if issue: calls.extend([ ((['git', 'config', 'branch.master.gerritserver'],), CERR1), @@ -830,11 +833,10 @@ class TestGitCl(TestCase): fetched_status=None, other_cl_owner=None, custom_cl_base=None, short_hostname='chromium'): calls = cls._is_gerrit_calls(True) - calls += [ - ((['git', 'symbolic-ref', 'HEAD'],), 'master'), - ((['git', 'config', 'branch.master.gerritissue'],), - CERR1 if issue is None else str(issue)), - ] + if not custom_cl_base: + calls += [ + ((['git', 'symbolic-ref', 'HEAD'],), 'master'), + ] if custom_cl_base: ancestor_revision = custom_cl_base @@ -850,7 +852,8 @@ class TestGitCl(TestCase): # Calls to verify branch point is ancestor calls += cls._gerrit_ensure_auth_calls( - issue=issue, short_hostname=short_hostname) + issue=issue, short_hostname=short_hostname, + custom_cl_base=custom_cl_base) if issue: calls += [ @@ -1763,7 +1766,6 @@ class TestGitCl(TestCase): # These calls detect codereview to use. self.calls += [ ((['git', 'symbolic-ref', 'HEAD'],), 'master'), - ((['git', 'config', 'branch.master.gerritissue'],), CERR1), ] if detect_gerrit_server: self.calls += self._get_gerrit_codereview_server_calls( @@ -1914,7 +1916,6 @@ class TestGitCl(TestCase): self.calls = [ ((['git', 'symbolic-ref', 'HEAD'],), 'master'), - ((['git', 'config', 'branch.master.gerritissue'],), CERR1), ((['git', 'config', 'branch.master.gerritserver'],), CERR1), ((['git', 'config', 'branch.master.merge'],), 'refs/heads/master'), ((['git', 'config', 'branch.master.remote'],), 'origin'), @@ -1963,10 +1964,9 @@ class TestGitCl(TestCase): self.mock(git_cl, 'DieWithError', lambda msg, change=None: self._mocked_call(['DieWithError', msg])) self.calls = self._gerrit_ensure_auth_calls(skip_auth_check=skip_auth_check) - cl = git_cl.Changelist(codereview='gerrit') + cl = git_cl.Changelist() cl.branch = 'master' cl.branchref = 'refs/heads/master' - cl.lookedup_issue = True return cl def test_gerrit_ensure_authenticated_missing(self): @@ -2031,7 +2031,7 @@ class TestGitCl(TestCase): ] self.mock(git_cl.gerrit_util, 'CookiesAuthenticator', CookiesAuthenticatorMockFactory(hosts_with_creds={})) - cl = git_cl.Changelist(codereview='gerrit') + cl = git_cl.Changelist() cl.branch = 'master' cl.branchref = 'refs/heads/master' cl.lookedup_issue = True @@ -2086,7 +2086,7 @@ class TestGitCl(TestCase): self.assertEqual(git_cl.main(['status', '--issue', '1']), 0) except SystemExit as ex: self.assertEqual(ex.code, 2) - self.assertRegexpMatches(out.getvalue(), r'--issue must be specified') + self.assertRegexpMatches(out.getvalue(), r'--field must be specified') out = StringIO.StringIO() self.mock(git_cl.sys, 'stderr', out) @@ -2205,13 +2205,10 @@ class TestGitCl(TestCase): self.calls = [ ((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],), - 'refs/heads/master\nrefs/heads/foo\nrefs/heads/bar'), - ((['git', 'config', 'branch.master.gerritissue'],), '456'), - ((['git', 'config', 'branch.foo.gerritissue'],), CERR1), - ((['git', 'config', 'branch.bar.gerritissue'],), '789'), + 'refs/heads/master\nrefs/heads/foo\nrefs/heads/bar'), ((['git', 'symbolic-ref', 'HEAD'],), 'master'), ((['git', 'tag', 'git-cl-archived-456-foo', 'foo'],), ''), - ((['git', 'branch', '-D', 'foo'],), ''), + ((['git', 'branch', '-D', 'foo'],), '') ] self.mock(git_cl, 'get_cl_statuses', @@ -2227,7 +2224,6 @@ class TestGitCl(TestCase): self.calls = [ ((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],), 'refs/heads/master'), - ((['git', 'config', 'branch.master.gerritissue'],), '1'), ((['git', 'symbolic-ref', 'HEAD'],), 'master'), ] @@ -2243,9 +2239,6 @@ class TestGitCl(TestCase): self.calls = [ ((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],), 'refs/heads/master\nrefs/heads/foo\nrefs/heads/bar'), - ((['git', 'config', 'branch.master.gerritissue'],), '456'), - ((['git', 'config', 'branch.foo.gerritissue'],), CERR1), - ((['git', 'config', 'branch.bar.gerritissue'],), '789'), ((['git', 'symbolic-ref', 'HEAD'],), 'master') ] @@ -2263,9 +2256,6 @@ class TestGitCl(TestCase): self.calls = [ ((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],), 'refs/heads/master\nrefs/heads/foo\nrefs/heads/bar'), - ((['git', 'config', 'branch.master.gerritissue'],), '1'), - ((['git', 'config', 'branch.foo.gerritissue'],), '456'), - ((['git', 'config', 'branch.bar.gerritissue'],), CERR1), ((['git', 'symbolic-ref', 'HEAD'],), 'master'), ((['git', 'branch', '-D', 'foo'],), '') ] @@ -2283,7 +2273,6 @@ class TestGitCl(TestCase): self.mock(git_cl.sys, 'stdout', out) self.calls = [ ((['git', 'symbolic-ref', 'HEAD'],), 'feature'), - ((['git', 'config', 'branch.feature.gerritissue'],), '123'), # Let this command raise exception (retcode=1) - it should be ignored. ((['git', 'config', '--unset', 'branch.feature.last-upload-hash'],), CERR1), @@ -2303,7 +2292,6 @@ class TestGitCl(TestCase): lambda _: 'This is a description\n\nChange-Id: Ideadbeef') self.calls = [ ((['git', 'symbolic-ref', 'HEAD'],), 'feature'), - ((['git', 'config', 'branch.feature.gerritissue'],), '123'), # Let this command raise exception (retcode=1) - it should be ignored. ((['git', 'config', '--unset', 'branch.feature.last-upload-hash'],), CERR1), @@ -2484,7 +2472,7 @@ class TestGitCl(TestCase): ((['git', 'rev-parse', '--show-cdup'],), '../'), ((['abspath', '../'],), '/abs/git_repo_root'), ] - return git_cl.Changelist(codereview='gerrit', issue=123) + return git_cl.Changelist(issue=123) def test_GerritCommitMsgHookCheck_custom_hook(self): cl = self._common_GerritCommitMsgHookCheck() @@ -2523,7 +2511,7 @@ class TestGitCl(TestCase): ((['git', 'config', 'branch.feature.gerritserver'],), 'chromium-review.googlesource.com'), ] - cl = git_cl.Changelist(issue=123, codereview='gerrit') + cl = git_cl.Changelist(issue=123) cl._codereview_impl._GetChangeDetail = lambda _: { 'labels': {}, 'current_revision': 'deadbeaf', @@ -2690,10 +2678,10 @@ class TestGitCl(TestCase): (('GetChangeDetail', 'host', 'ab%2Frepo~2', []), 'b'), (('GetChangeDetail', 'host', 'ab%2Frepo~2', []), 'b2'), ] - cl1 = git_cl.Changelist(issue=1, codereview='gerrit') + cl1 = git_cl.Changelist(issue=1) cl1._cached_remote_url = ( True, 'https://chromium.googlesource.com/a/my/repo.git/') - cl2 = git_cl.Changelist(issue=2, codereview='gerrit') + cl2 = git_cl.Changelist(issue=2) cl2._cached_remote_url = ( True, 'https://chromium.googlesource.com/ab/repo') self.assertEqual(cl1._GetChangeDetail(), 'a') # Miss. @@ -2712,7 +2700,7 @@ class TestGitCl(TestCase): # no longer in cache. (('GetChangeDetail', 'host', 'repo~1', ['B']), 'b'), ] - cl = git_cl.Changelist(issue=1, codereview='gerrit') + cl = git_cl.Changelist(issue=1) cl._cached_remote_url = (True, 'https://chromium.googlesource.com/repo/') self.assertEqual(cl._GetChangeDetail(options=['C', 'A', 'B']), 'cab') self.assertEqual(cl._GetChangeDetail(options=['A', 'B', 'C']), 'cab') @@ -2746,7 +2734,7 @@ class TestGitCl(TestCase): ] self._mock_gerrit_changes_for_detail_cache() - cl = git_cl.Changelist(issue=1, codereview='gerrit') + cl = git_cl.Changelist(issue=1) cl._cached_remote_url = ( True, 'https://chromium.googlesource.com/a/my/repo.git/') self.assertEqual(cl.GetDescription(), 'desc1') @@ -3002,7 +2990,7 @@ class TestGitCl(TestCase): disapproval=False, approval=False, sender=u'reviewer@example.com'), ] cl = git_cl.Changelist( - codereview='gerrit', issue=1, branchref='refs/heads/foo') + issue=1, branchref='refs/heads/foo') self.assertEqual(cl.GetCommentsSummary(), expected_comments_summary) self.mock(git_cl.Changelist, 'GetBranch', lambda _: 'foo') self.assertEqual( @@ -3117,7 +3105,7 @@ class TestGitCl(TestCase): autogenerated=True, approval=False, disapproval=False) ] cl = git_cl.Changelist( - codereview='gerrit', issue=1, branchref='refs/heads/foo') + issue=1, branchref='refs/heads/foo') self.assertEqual(cl.GetCommentsSummary(), expected_comments_summary) def test_get_remote_url_with_mirror(self): @@ -3143,7 +3131,7 @@ class TestGitCl(TestCase): ((['git', 'config', 'remote.origin.url'],), url), ] - cl = git_cl.Changelist(codereview='gerrit', issue=1) + cl = git_cl.Changelist(issue=1) self.assertEqual(cl.GetRemoteUrl(), url) self.assertEqual(cl.GetRemoteUrl(), url) # Must be cached. @@ -3171,7 +3159,7 @@ class TestGitCl(TestCase): 'Remote "origin" for branch "/cache/this-dir-doesnt-exist" points to' ' "master", but it doesn\'t exist.'), None), ] - cl = git_cl.Changelist(codereview='gerrit', issue=1) + cl = git_cl.Changelist(issue=1) self.assertIsNone(cl.GetRemoteUrl()) def test_get_remote_url_misconfigured_mirror(self): @@ -3205,7 +3193,7 @@ class TestGitCl(TestCase): 'branch': 'master'} ), None), ] - cl = git_cl.Changelist(codereview='gerrit', issue=1) + cl = git_cl.Changelist(issue=1) self.assertIsNone(cl.GetRemoteUrl()) def test_gerrit_change_identifier_with_project(self): @@ -3216,7 +3204,7 @@ class TestGitCl(TestCase): ((['git', 'config', 'remote.origin.url'],), 'https://chromium.googlesource.com/a/my/repo.git/'), ] - cl = git_cl.Changelist(codereview='gerrit', issue=123456) + cl = git_cl.Changelist(issue=123456) self.assertEqual(cl._GerritChangeIdentifier(), 'my%2Frepo~123456') def test_gerrit_change_identifier_without_project(self): @@ -3226,7 +3214,7 @@ class TestGitCl(TestCase): ((['git', 'config', 'branch.master.remote'],), 'origin'), ((['git', 'config', 'remote.origin.url'],), CERR1), ] - cl = git_cl.Changelist(codereview='gerrit', issue=123456) + cl = git_cl.Changelist(issue=123456) self.assertEqual(cl._GerritChangeIdentifier(), '123456')