diff --git a/git_cl.py b/git_cl.py index 98a2f01b4..59e412123 100755 --- a/git_cl.py +++ b/git_cl.py @@ -5,7 +5,7 @@ # Copyright (C) 2008 Evan Martin -"""A git-command for integrating reviews on Rietveld and Gerrit.""" +"""A git-command for integrating reviews on Rietveld.""" from distutils.version import LooseVersion from multiprocessing.pool import ThreadPool @@ -47,7 +47,6 @@ import commit_queue import dart_format import fix_encoding import gclient_utils -import gerrit_util import git_cache import git_common import git_footers @@ -157,7 +156,7 @@ def ask_for_data(prompt): def git_set_branch_value(key, value): - branch = GetCurrentBranch() + branch = Changelist().GetBranch() if not branch: return @@ -169,7 +168,7 @@ def git_set_branch_value(key, value): def git_get_branch_default(key, default): - branch = GetCurrentBranch() + branch = Changelist().GetBranch() if branch: git_key = 'branch.%s.%s' % (branch, key) (_, stdout) = RunGitWithCode(['config', '--int', '--get', git_key]) @@ -814,61 +813,23 @@ class Settings(object): def ShortBranchName(branch): """Convert a name like 'refs/heads/foo' to just 'foo'.""" - return branch.replace('refs/heads/', '', 1) - - -def GetCurrentBranchRef(): - """Returns branch ref (e.g., refs/heads/master) or None.""" - return RunGit(['symbolic-ref', 'HEAD'], - stderr=subprocess2.VOID, error_ok=True).strip() or None - - -def GetCurrentBranch(): - """Returns current branch or None. - - For refs/heads/* branches, returns just last part. For others, full ref. - """ - branchref = GetCurrentBranchRef() - if branchref: - return ShortBranchName(branchref) - return None + return branch.replace('refs/heads/', '') class Changelist(object): - """Changelist works with one changelist in local branch. - - Supports two codereview backends: Rietveld or Gerrit, selected at object - creation. - - Not safe for concurrent multi-{thread,process} use. - """ - - def __init__(self, branchref=None, issue=None, codereview=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. - """ + def __init__(self, branchref=None, issue=None, auth_config=None): # Poke settings so we get the "configure your server" message if necessary. global settings if not settings: # 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' - + settings.GetDefaultServerUrl() self.branchref = branchref if self.branchref: self.branch = ShortBranchName(self.branchref) else: self.branch = None + self.rietveld_server = None self.upstream_branch = None self.lookedup_issue = False self.issue = issue or None @@ -878,44 +839,14 @@ class Changelist(object): self.patchset = None self.cc = None self.watchers = () + self._auth_config = auth_config + self._props = None self._remote = None + self._rpc_server = None - self._codereview_impl = None - self._load_codereview_impl(codereview, **kwargs) - - def _load_codereview_impl(self, codereview=None, **kwargs): - if codereview: - codereview = codereview.lower() - if codereview == 'gerrit': - self._codereview_impl = _GerritChangelistImpl(self, **kwargs) - elif codereview == 'rietveld': - self._codereview_impl = _RietveldChangelistImpl(self, **kwargs) - else: - assert codereview in ('rietveld', 'gerrit') - return - - # Automatic selection. - assert not self.issue - # Check if this branch is associated with Rietveld => Rieveld. - self._codereview_impl = _RietveldChangelistImpl(self, **kwargs) - if self.GetIssue(force_lookup=True): - return - - tmp_rietveld = self._codereview_impl # Save Rietveld object. - - # Check if this branch has Gerrit issue associated => Gerrit. - self._codereview_impl = _GerritChangelistImpl(self, **kwargs) - if self.GetIssue(force_lookup=True): - return - - # OK, no issue is set for this branch. - # If Gerrit is set repo-wide => Gerrit. - if settings.GetIsGerrit(): - return - - self._codereview_impl = tmp_rietveld - return - + @property + def auth_config(self): + return self._auth_config def GetCCList(self): """Return the users cc'd on this CL. @@ -943,7 +874,8 @@ class Changelist(object): def GetBranch(self): """Returns the short branch name, e.g. 'master'.""" if not self.branch: - branchref = GetCurrentBranchRef() + branchref = RunGit(['symbolic-ref', 'HEAD'], + stderr=subprocess2.VOID, error_ok=True).strip() if not branchref: return None self.branchref = branchref @@ -987,12 +919,11 @@ class Changelist(object): remote = 'origin' upstream_branch = 'refs/heads/trunk' else: - DieWithError( - 'Unable to determine default branch to diff against.\n' - 'Either pass complete "git diff"-style arguments, like\n' - ' git cl upload origin/master\n' - 'or verify this branch is set up to track another \n' - '(via the --track argument to "git checkout -b ...").') + DieWithError("""Unable to determine default branch to diff against. +Either pass complete "git diff"-style arguments, like + git cl upload origin/master +or verify this branch is set up to track another (via the --track argument to +"git checkout -b ...").""") return remote, upstream_branch @@ -1131,26 +1062,68 @@ class Changelist(object): cwd=url).strip() return url - def GetIssue(self, force_lookup=False): + def GetIssue(self): """Returns the issue number as a int or None if not set.""" - if force_lookup or (self.issue is None and not self.lookedup_issue): - issue = RunGit(['config', self._codereview_impl.IssueSetting()], - error_ok=True).strip() + if self.issue is None and not self.lookedup_issue: + issue = RunGit(['config', self._IssueSetting()], error_ok=True).strip() self.issue = int(issue) or None if issue else None self.lookedup_issue = True return self.issue + def GetRietveldServer(self): + if not self.rietveld_server: + # If we're on a branch then get the server potentially associated + # with that branch. + if self.GetIssue(): + rietveld_server_config = self._RietveldServer() + if rietveld_server_config: + self.rietveld_server = gclient_utils.UpgradeToHttps(RunGit( + ['config', rietveld_server_config], error_ok=True).strip()) + if not self.rietveld_server: + self.rietveld_server = settings.GetDefaultServerUrl() + return self.rietveld_server + + def GetGerritServer(self): + # We don't support multiple Gerrit servers, and assume it to be same as + # origin, except with a '-review' suffix for first subdomain. + parts = urlparse.urlparse(self.GetRemoteUrl()).netloc.split('.') + parts[0] = parts[0] + '-review' + return 'https://%s' % '.'.join(parts) + def GetIssueURL(self): """Get the URL for a particular issue.""" - issue = self.GetIssue() - if not issue: + if not self.GetIssue(): return None - return '%s/%s' % (self._codereview_impl.GetCodereviewServer(), issue) + if settings.GetIsGerrit(): + return '%s/%s' % (self.GetGerritServer(), self.GetIssue()) + return '%s/%s' % (self.GetRietveldServer(), self.GetIssue()) def GetDescription(self, pretty=False): if not self.has_description: if self.GetIssue(): - self.description = self._codereview_impl.FetchDescription() + issue = self.GetIssue() + try: + self.description = self.RpcServer().get_description(issue).strip() + except urllib2.HTTPError as e: + if e.code == 404: + DieWithError( + ('\nWhile fetching the description for issue %d, received a ' + '404 (not found)\n' + 'error. It is likely that you deleted this ' + 'issue on the server. If this is the\n' + 'case, please run\n\n' + ' git cl issue 0\n\n' + 'to clear the association with the deleted issue. Then run ' + 'this command again.') % issue) + else: + DieWithError( + '\nFailed to fetch issue description. HTTP error %d' % e.code) + except urllib2.URLError as e: + print >> sys.stderr, ( + 'Warning: Failed to retrieve CL description due to network ' + 'failure.') + self.description = '' + self.has_description = True if pretty: wrapper = textwrap.TextWrapper() @@ -1161,7 +1134,7 @@ class Changelist(object): def GetPatchset(self): """Returns the patchset number as a int or None if not set.""" if self.patchset is None and not self.lookedup_patchset: - patchset = RunGit(['config', self._codereview_impl.PatchsetSetting()], + patchset = RunGit(['config', self._PatchsetSetting()], error_ok=True).strip() self.patchset = int(patchset) or None if patchset else None self.lookedup_patchset = True @@ -1169,29 +1142,47 @@ class Changelist(object): def SetPatchset(self, patchset): """Set this branch's patchset. If patchset=0, clears the patchset.""" - patchset_setting = self._codereview_impl.PatchsetSetting() if patchset: - RunGit(['config', patchset_setting, str(patchset)]) + RunGit(['config', self._PatchsetSetting(), str(patchset)]) self.patchset = patchset else: - RunGit(['config', '--unset', patchset_setting], + RunGit(['config', '--unset', self._PatchsetSetting()], stderr=subprocess2.PIPE, error_ok=True) self.patchset = None + def GetMostRecentPatchset(self): + return self.GetIssueProperties()['patchsets'][-1] + + def GetPatchSetDiff(self, issue, patchset): + return self.RpcServer().get( + '/download/issue%s_%s.diff' % (issue, patchset)) + + def GetIssueProperties(self): + if self._props is None: + issue = self.GetIssue() + if not issue: + self._props = {} + else: + self._props = self.RpcServer().get_issue_properties(issue, True) + return self._props + + def GetApprovingReviewers(self): + return get_approving_reviewers(self.GetIssueProperties()) + + def AddComment(self, message): + return self.RpcServer().add_comment(self.GetIssue(), message) + def SetIssue(self, issue=None): """Set this branch's issue. If issue isn't given, clears the issue.""" - issue_setting = self._codereview_impl.IssueSetting() - codereview_setting = self._codereview_impl.GetCodereviewServerSetting() if issue: self.issue = issue - RunGit(['config', issue_setting, str(issue)]) - codereview_server = self._codereview_impl.GetCodereviewServer() - if codereview_server: - RunGit(['config', codereview_setting, codereview_server]) + RunGit(['config', self._IssueSetting(), str(issue)]) + if not settings.GetIsGerrit() and self.rietveld_server: + RunGit(['config', self._RietveldServer(), self.rietveld_server]) else: current_issue = self.GetIssue() if current_issue: - RunGit(['config', '--unset', issue_setting]) + RunGit(['config', '--unset', self._IssueSetting()]) self.issue = None self.SetPatchset(None) @@ -1241,184 +1232,6 @@ class Changelist(object): author, upstream=upstream_branch) - def UpdateDescription(self, description): - self.description = description - return self._codereview_impl.UpdateDescriptionRemote(description) - - def RunHook(self, committing, may_prompt, verbose, change): - """Calls sys.exit() if the hook fails; returns a HookResults otherwise.""" - try: - return presubmit_support.DoPresubmitChecks(change, committing, - verbose=verbose, output_stream=sys.stdout, input_stream=sys.stdin, - default_presubmit=None, may_prompt=may_prompt, - rietveld_obj=self._codereview_impl.GetRieveldObjForPresubmit()) - except presubmit_support.PresubmitFailure, e: - DieWithError( - ('%s\nMaybe your depot_tools is out of date?\n' - 'If all fails, contact maruel@') % e) - - # Forward methods to codereview specific implementation. - - def CloseIssue(self): - return self._codereview_impl.CloseIssue() - - def GetStatus(self): - return self._codereview_impl.GetStatus() - - def GetCodereviewServer(self): - return self._codereview_impl.GetCodereviewServer() - - def GetApprovingReviewers(self): - return self._codereview_impl.GetApprovingReviewers() - - def GetMostRecentPatchset(self): - return self._codereview_impl.GetMostRecentPatchset() - - def __getattr__(self, attr): - # This is because lots of untested code accesses Rietveld-specific stuff - # directly, and it's hard to fix for sure. So, just let it work, and fix - # on a cases by case basis. - return getattr(self._codereview_impl, attr) - - -class _ChangelistCodereviewBase(object): - """Abstract base class encapsulating codereview specifics of a changelist.""" - def __init__(self, changelist): - self._changelist = changelist # instance of Changelist - - def __getattr__(self, attr): - # Forward methods to changelist. - # TODO(tandrii): maybe clean up _GerritChangelistImpl and - # _RietveldChangelistImpl to avoid this hack? - return getattr(self._changelist, attr) - - def GetStatus(self): - """Apply a rough heuristic to give a simple summary of an issue's review - or CQ status, assuming adherence to a common workflow. - - Returns None if no issue for this branch, or specific string keywords. - """ - raise NotImplementedError() - - def GetCodereviewServer(self): - """Returns server URL without end slash, like "https://codereview.com".""" - raise NotImplementedError() - - def FetchDescription(self): - """Fetches and returns description from the codereview server.""" - raise NotImplementedError() - - def GetCodereviewServerSetting(self): - """Returns git config setting for the codereview server.""" - raise NotImplementedError() - - def IssueSetting(self): - """Returns name of git config setting which stores issue number.""" - raise NotImplementedError() - - def PatchsetSetting(self): - """Returns name of git config setting which stores issue number.""" - raise NotImplementedError() - - def GetRieveldObjForPresubmit(self): - # This is an unfortunate Rietveld-embeddedness in presubmit. - # For non-Rietveld codereviews, this probably should return a dummy object. - raise NotImplementedError() - - def UpdateDescriptionRemote(self, description): - """Update the description on codereview site.""" - raise NotImplementedError() - - def CloseIssue(self): - """Closes the issue.""" - raise NotImplementedError() - - def GetApprovingReviewers(self): - """Returns a list of reviewers approving the change. - - Note: not necessarily committers. - """ - raise NotImplementedError() - - def GetMostRecentPatchset(self): - """Returns the most recent patchset number from the codereview site.""" - raise NotImplementedError() - - -class _RietveldChangelistImpl(_ChangelistCodereviewBase): - def __init__(self, changelist, auth_config=None, rietveld_server=None): - super(_RietveldChangelistImpl, self).__init__(changelist) - assert settings, 'must be initialized in _ChangelistCodereviewBase' - settings.GetDefaultServerUrl() - - self._rietveld_server = rietveld_server - self._auth_config = auth_config - self._props = None - self._rpc_server = None - - def GetAuthConfig(self): - return self._auth_config - - def GetCodereviewServer(self): - if not self._rietveld_server: - # If we're on a branch then get the server potentially associated - # with that branch. - if self.GetIssue(): - rietveld_server_setting = self.GetCodereviewServerSetting() - if rietveld_server_setting: - self._rietveld_server = gclient_utils.UpgradeToHttps(RunGit( - ['config', rietveld_server_setting], error_ok=True).strip()) - if not self._rietveld_server: - self._rietveld_server = settings.GetDefaultServerUrl() - return self._rietveld_server - - def FetchDescription(self): - issue = self.GetIssue() - assert issue - try: - return self.RpcServer().get_description(issue).strip() - except urllib2.HTTPError as e: - if e.code == 404: - DieWithError( - ('\nWhile fetching the description for issue %d, received a ' - '404 (not found)\n' - 'error. It is likely that you deleted this ' - 'issue on the server. If this is the\n' - 'case, please run\n\n' - ' git cl issue 0\n\n' - 'to clear the association with the deleted issue. Then run ' - 'this command again.') % issue) - else: - DieWithError( - '\nFailed to fetch issue description. HTTP error %d' % e.code) - except urllib2.URLError as e: - print >> sys.stderr, ( - 'Warning: Failed to retrieve CL description due to network ' - 'failure.') - return '' - - def GetMostRecentPatchset(self): - return self.GetIssueProperties()['patchsets'][-1] - - def GetPatchSetDiff(self, issue, patchset): - return self.RpcServer().get( - '/download/issue%s_%s.diff' % (issue, patchset)) - - def GetIssueProperties(self): - if self._props is None: - issue = self.GetIssue() - if not issue: - self._props = {} - else: - self._props = self.RpcServer().get_issue_properties(issue, True) - return self._props - - def GetApprovingReviewers(self): - return get_approving_reviewers(self.GetIssueProperties()) - - def AddComment(self, message): - return self.RpcServer().add_comment(self.GetIssue(), message) - def GetStatus(self): """Apply a rough heuristic to give a simple summary of an issue's review or CQ status, assuming adherence to a common workflow. @@ -1466,11 +1279,26 @@ class _RietveldChangelistImpl(_ChangelistCodereviewBase): return 'reply' return 'waiting' - def UpdateDescriptionRemote(self, description): + def RunHook(self, committing, may_prompt, verbose, change): + """Calls sys.exit() if the hook fails; returns a HookResults otherwise.""" + + try: + return presubmit_support.DoPresubmitChecks(change, committing, + verbose=verbose, output_stream=sys.stdout, input_stream=sys.stdin, + default_presubmit=None, may_prompt=may_prompt, + rietveld_obj=self.RpcServer()) + except presubmit_support.PresubmitFailure, e: + DieWithError( + ('%s\nMaybe your depot_tools is out of date?\n' + 'If all fails, contact maruel@') % e) + + def UpdateDescription(self, description): + self.description = description return self.RpcServer().update_description( self.GetIssue(), self.description) def CloseIssue(self): + """Updates the description and closes the issue.""" return self.RpcServer().close_issue(self.GetIssue()) def SetFlag(self, flag, value): @@ -1494,110 +1322,25 @@ class _RietveldChangelistImpl(_ChangelistCodereviewBase): """ if not self._rpc_server: self._rpc_server = rietveld.CachingRietveld( - self.GetCodereviewServer(), + self.GetRietveldServer(), self._auth_config or auth.make_auth_config()) return self._rpc_server - def IssueSetting(self): + def _IssueSetting(self): """Return the git setting that stores this change's issue.""" return 'branch.%s.rietveldissue' % self.GetBranch() - def PatchsetSetting(self): + def _PatchsetSetting(self): """Return the git setting that stores this change's most recent patchset.""" return 'branch.%s.rietveldpatchset' % self.GetBranch() - def GetCodereviewServerSetting(self): + def _RietveldServer(self): """Returns the git setting that stores this change's rietveld server.""" branch = self.GetBranch() if branch: return 'branch.%s.rietveldserver' % branch return None - def GetRieveldObjForPresubmit(self): - return self.RpcServer() - - -class _GerritChangelistImpl(_ChangelistCodereviewBase): - def __init__(self, changelist, auth_config=None): - # auth_config is Rietveld thing, kept here to preserve interface only. - super(_GerritChangelistImpl, self).__init__(changelist) - self._change_id = None - self._gerrit_server = None # e.g. https://chromium-review.googlesource.com - self._gerrit_host = None # e.g. chromium-review.googlesource.com - - def _GetGerritHost(self): - # Lazy load of configs. - self.GetCodereviewServer() - return self._gerrit_host - - def GetCodereviewServer(self): - if not self._gerrit_server: - # If we're on a branch then get the server potentially associated - # with that branch. - if self.GetIssue(): - gerrit_server_setting = self.GetCodereviewServerSetting() - if gerrit_server_setting: - self._gerrit_server = RunGit(['config', gerrit_server_setting], - error_ok=True).strip() - if self._gerrit_server: - self._gerrit_host = urlparse.urlparse(self._gerrit_server).netloc - if not self._gerrit_server: - # We assume repo to be hosted on Gerrit, and hence Gerrit server - # has "-review" suffix for lowest level subdomain. - parts = urlparse.urlparse(self.GetRemoteUrl()).netloc.split('.') - parts[0] = parts[0] + '-review' - self._gerrit_host = '.'.join(parts) - self._gerrit_server = 'https://%s' % self._gerrit_host - return self._gerrit_server - - def IssueSetting(self): - """Return the git setting that stores this change's issue.""" - return 'branch.%s.gerritissue' % self.GetBranch() - - def PatchsetSetting(self): - """Return the git setting that stores this change's most recent patchset.""" - return 'branch.%s.gerritpatchset' % self.GetBranch() - - def GetCodereviewServerSetting(self): - """Returns the git setting that stores this change's Gerrit server.""" - branch = self.GetBranch() - if branch: - return 'branch.%s.gerritserver' % branch - return None - - def GetRieveldObjForPresubmit(self): - class ThisIsNotRietveldIssue(object): - def __getattr__(self, attr): - print( - 'You aren\'t using Rietveld at the moment, but Gerrit.\n' - 'Using Rietveld in your PRESUBMIT scripts won\'t work.\n' - 'Please, either change your PRESUBIT to not use rietveld_obj.%s,\n' - 'or use Rietveld for codereview.\n' % attr) - raise NotImplementedError() - return ThisIsNotRietveldIssue() - - def GetStatus(self): - # TODO(tandrii) - raise NotImplementedError() - - def GetMostRecentPatchset(self): - data = gerrit_util.GetChangeDetail(self._GetGerritHost(), self.GetIssue(), - ['CURRENT_REVISION']) - return data['revisions'][data['current_revision']]['_number'] - - def FetchDescription(self): - data = gerrit_util.GetChangeDetail(self._GetGerritHost(), self.GetIssue(), - ['COMMIT_FOOTERS', 'CURRENT_REVISION']) - return data['revisions'][data['current_revision']]['commit_with_footers'] - - def UpdateDescriptionRemote(self, description): - # TODO(tandrii) - raise NotImplementedError() - - def CloseIssue(self): - # TODO(tandrii) - raise NotImplementedError() - class ChangeDescription(object): """Contains a parsed form of the change description.""" @@ -2143,7 +1886,6 @@ def CMDstatus(parser, args): changes = ( Changelist(branchref=b, auth_config=auth_config) for b in branches.splitlines()) - # TODO(tandrii): refactor to use CLs list instead of branches list. branches = [c.GetBranch() for c in changes] alignment = max(5, max(len(b) for b in branches)) print 'Branches associated with reviews:' @@ -2259,7 +2001,7 @@ def CMDcomments(parser, args): except ValueError: DieWithError('A review issue id is expected to be a number') - cl = Changelist(issue=issue, codereview='rietveld', auth_config=auth_config) + cl = Changelist(issue=issue, auth_config=auth_config) if options.comment: cl.AddComment(options.comment) @@ -2638,10 +2380,8 @@ def GetTargetRef(remote, remote_branch, target_branch, pending_prefix): def RietveldUpload(options, args, cl, change): """upload the patch to rietveld.""" upload_args = ['--assume_yes'] # Don't ask about untracked files. - upload_args.extend(['--server', cl.GetCodereviewServer()]) - # TODO(tandrii): refactor this ugliness into _RietveldChangelistImpl. - upload_args.extend(auth.auth_config_to_command_options( - cl._codereview_impl.GetAuthConfig())) + upload_args.extend(['--server', cl.GetRietveldServer()]) + upload_args.extend(auth.auth_config_to_command_options(cl.auth_config)) if options.emulate_svn_auto_props: upload_args.append('--emulate_svn_auto_props') @@ -2884,9 +2624,9 @@ def CMDupload(parser, args): # during the actual upload. if not settings.GetIsGerrit() and auth_config.use_oauth2: authenticator = auth.get_authenticator_for_host( - cl.GetCodereviewServer(), auth_config) + cl.GetRietveldServer(), auth_config) if not authenticator.has_cached_credentials(): - raise auth.LoginRequiredError(cl.GetCodereviewServer()) + raise auth.LoginRequiredError(cl.GetRietveldServer()) # Apply watchlists on upload. change = cl.GetChange(base_branch, None) @@ -3480,13 +3220,12 @@ def PatchIssue(issue_arg, reject, nocommit, directory, auth_config): # consequences of the caller not checking this could be dire. assert(not git_common.is_dirty_git_tree('apply')) - # TODO(tandrii): implement for Gerrit. if type(issue_arg) is int or issue_arg.isdigit(): # Input is an issue id. Figure out the URL. issue = int(issue_arg) - cl = Changelist(issue=issue, codereview='rietveld', auth_config=auth_config) + cl = Changelist(issue=issue, auth_config=auth_config) patchset = cl.GetMostRecentPatchset() - patch_data = cl._codereview_impl.GetPatchSetDiff(issue, patchset) + patch_data = cl.GetPatchSetDiff(issue, patchset) else: # Assume it's a URL to the patch. Default to https. issue_url = gclient_utils.UpgradeToHttps(issue_arg) @@ -3495,8 +3234,8 @@ def PatchIssue(issue_arg, reject, nocommit, directory, auth_config): DieWithError('Must pass an issue ID or full URL for ' '\'Download raw patch set\'') issue = int(match.group(2)) - cl = Changelist(issue=issue, codereview='rietveld', - rietvled_server=match.group(1), auth_config=auth_config) + cl = Changelist(issue=issue, auth_config=auth_config) + cl.rietveld_server = match.group(1) patchset = int(match.group(3)) patch_data = urllib2.urlopen(issue_arg).read() @@ -3540,8 +3279,7 @@ def PatchIssue(issue_arg, reject, nocommit, directory, auth_config): 'patch from issue %(i)s at patchset ' '%(p)s (http://crrev.com/%(i)s#ps%(p)s)' % {'i': issue, 'p': patchset})]) - cl = Changelist(codereview='rietveld', auth_config=auth_config, - rietveld_server=cl.GetCodereviewServer()) + cl = Changelist(auth_config=auth_config) cl.SetIssue(issue) cl.SetPatchset(patchset) print "Committed patch locally." diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 5c116e6ad..7da5aaefc 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -163,21 +163,20 @@ class TestGitCl(TestCase): '-M'+similarity, 'fake_ancestor_sha', 'HEAD'],), '+dat') return [ + ((['git', 'config', 'rietveld.autoupdate'],), ''), + ((['git', 'config', 'rietveld.server'],), + 'codereview.example.com'), ((['git', 'symbolic-ref', 'HEAD'],), 'master'), similarity_call, ((['git', 'symbolic-ref', 'HEAD'],), 'master'), find_copies_call, - ((['git', 'config', 'rietveld.autoupdate'],), ''), - ((['git', 'config', 'rietveld.server'],), - 'codereview.example.com'), ((['git', 'symbolic-ref', 'HEAD'],), 'master'), - ((['git', 'config', 'branch.master.rietveldissue'],), ''), - ((['git', 'config', 'branch.master.gerritissue'],), ''), - ((['git', 'config', 'gerrit.host'],), ''), ((['git', 'config', 'branch.master.merge'],), 'master'), ((['git', 'config', 'branch.master.remote'],), 'origin'), ((['get_or_create_merge_base', 'master', 'master'],), 'fake_ancestor_sha'), + ((['git', 'config', 'gerrit.host'],), ''), + ((['git', 'config', 'branch.master.rietveldissue'],), ''), ] + cls._git_sanity_checks('fake_ancestor_sha', 'master') + [ ((['git', 'rev-parse', '--show-cdup'],), ''), ((['git', 'rev-parse', 'HEAD'],), '12345'), @@ -280,17 +279,15 @@ class TestGitCl(TestCase): 'svn-remote.svn.fetch trunk/src:refs/remotes/origin/master'), None), 0)), + ((['git', + 'config', 'rietveld.server'],), 'codereview.example.com'), ((['git', 'symbolic-ref', 'HEAD'],), 'refs/heads/working'), ((['git', 'config', '--int', '--get', 'branch.working.git-cl-similarity'],), ''), ((['git', 'symbolic-ref', 'HEAD'],), 'refs/heads/working'), ((['git', 'config', '--int', '--get', 'branch.working.git-find-copies'],), ''), - ((['git', - 'config', 'rietveld.server'],), 'codereview.example.com'), ((['git', 'symbolic-ref', 'HEAD'],), 'refs/heads/working'), - ((['git', - 'config', 'branch.working.rietveldissue'],), '12345'), ((['git', 'config', 'branch.working.merge'],), 'refs/heads/master'), ((['git', 'config', 'branch.working.remote'],), 'origin'), @@ -324,6 +321,8 @@ class TestGitCl(TestCase): 'diff', '--name-status', '--no-renames', '-r', 'fake_ancestor_sha...', '.'],), 'M\tPRESUBMIT.py'), + ((['git', + 'config', 'branch.working.rietveldissue'],), '12345'), ((['git', 'config', 'branch.working.rietveldpatchset'],), '31137'), ((['git', 'config', 'branch.working.rietveldserver'],), @@ -335,6 +334,8 @@ class TestGitCl(TestCase): @classmethod def _dcommit_calls_bypassed(cls): return [ + ((['git', + 'config', 'branch.working.rietveldissue'],), '12345'), ((['git', 'config', 'branch.working.rietveldserver'],), 'codereview.example.com'), ] @@ -342,6 +343,7 @@ class TestGitCl(TestCase): @classmethod def _dcommit_calls_3(cls): return [ + ((['git', 'config', 'gerrit.host'],), ''), ((['git', 'diff', '--no-ext-diff', '--stat', '--find-copies-harder', '-l100000', '-C50', 'fake_ancestor_sha', @@ -544,23 +546,22 @@ class TestGitCl(TestCase): @classmethod def _gerrit_base_calls(cls): return [ + ((['git', 'config', 'rietveld.autoupdate'],), + ''), + ((['git', + 'config', 'rietveld.server'],), 'codereview.example.com'), ((['git', 'symbolic-ref', 'HEAD'],), 'master'), ((['git', 'config', '--int', '--get', 'branch.master.git-cl-similarity'],), ''), ((['git', 'symbolic-ref', 'HEAD'],), 'master'), ((['git', 'config', '--int', '--get', 'branch.master.git-find-copies'],), ''), - ((['git', 'config', 'rietveld.autoupdate'],), ''), - ((['git', 'config', 'rietveld.server'],), ''), - ((['git', 'config', 'rietveld.server'],), ''), ((['git', 'symbolic-ref', 'HEAD'],), 'master'), - ((['git', 'config', 'branch.master.rietveldissue'],), ''), - ((['git', 'config', 'branch.master.gerritissue'],), ''), - ((['git', 'config', 'gerrit.host'],), 'True'), ((['git', 'config', 'branch.master.merge'],), 'master'), ((['git', 'config', 'branch.master.remote'],), 'origin'), ((['get_or_create_merge_base', 'master', 'master'],), 'fake_ancestor_sha'), + ((['git', 'config', 'gerrit.host'],), 'True'), ] + cls._git_sanity_checks('fake_ancestor_sha', 'master') + [ ((['git', 'rev-parse', '--show-cdup'],), ''), ((['git', 'rev-parse', 'HEAD'],), '12345'), @@ -568,7 +569,9 @@ class TestGitCl(TestCase): 'diff', '--name-status', '--no-renames', '-r', 'fake_ancestor_sha...', '.'],), 'M\t.gitignore\n'), - ((['git', 'config', 'branch.master.gerritpatchset'],), ''), + ((['git', 'config', 'branch.master.rietveldissue'],), ''), + ((['git', + 'config', 'branch.master.rietveldpatchset'],), ''), ((['git', 'log', '--pretty=format:%s%n%n%b', 'fake_ancestor_sha...'],), 'foo'), @@ -659,12 +662,7 @@ class TestGitCl(TestCase): ] if squash: calls += [ - ((['git', 'config', 'branch.master.gerritissue', '123456'],), ''), - ((['git', 'config', 'branch.master.gerritserver'],), ''), - ((['git', 'config', 'remote.origin.url'],), - 'https://chromium.googlesource.com/my/repo.git'), - ((['git', 'config', 'branch.master.gerritserver', - 'https://chromium-review.googlesource.com'],), ''), + ((['git', 'config', 'branch.master.rietveldissue', '123456'],), ''), ((['git', 'rev-parse', 'HEAD'],), 'abcdef0123456789'), ((['git', 'update-ref', '-m', 'Uploaded abcdef0123456789', 'refs/heads/git_cl_uploads/master', 'abcdef0123456789'],), @@ -888,12 +886,9 @@ class TestGitCl(TestCase): self.assertNotEqual(git_cl.main(['diff']), 0) def _patch_common(self): - self.mock(git_cl._RietveldChangelistImpl, 'GetMostRecentPatchset', - lambda x: '60001') - self.mock(git_cl._RietveldChangelistImpl, 'GetPatchSetDiff', - lambda *args: None) - self.mock(git_cl.Changelist, 'GetDescription', - lambda *args: 'Description') + self.mock(git_cl.Changelist, 'GetMostRecentPatchset', lambda x: '60001') + self.mock(git_cl.Changelist, 'GetPatchSetDiff', lambda *args: None) + self.mock(git_cl.Changelist, 'GetDescription', lambda *args: 'Description') self.mock(git_cl.Changelist, 'SetIssue', lambda *args: None) self.mock(git_cl.Changelist, 'SetPatchset', lambda *args: None) self.mock(git_cl, 'IsGitVersionAtLeast', lambda *args: True) @@ -913,8 +908,6 @@ class TestGitCl(TestCase): 'Description\n\n' + 'patch from issue 123456 at patchset 60001 ' + '(http://crrev.com/123456#ps60001)'],), ''), - ((['git', 'symbolic-ref', 'HEAD'],), 'master'), - ((['git', 'config', 'branch.master.rietveldserver'],), ''), ] self.assertEqual(git_cl.main(['patch', '123456']), 0)