Refactor CMDUpload further to avoid checks IsGerrit().

git_cl.py

 #!/usr/bin/env python
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 # Copyright (C) 2008 Evan Martin <martine@danga.com>
 
 """A git-command for integrating reviews on Rietveld and Gerrit."""
 
 from distutils.version import LooseVersion
@@ skipping 1295 matching lines @@
     else:
       # Assume url.
       parsed_issue_arg = self._codereview_impl.ParseIssueURL(
           urlparse.urlparse(issue_arg))
     if not parsed_issue_arg or not parsed_issue_arg.valid:
       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)
 
+  def CMDUpload(self, options, git_diff_args, orig_args):
+    """Uploads a change to codereview."""
+    if git_diff_args:
+      # TODO(ukai): is it ok for gerrit case?
+      base_branch = git_diff_args[0]
+    else:
+      if self.GetBranch() is None:
+        DieWithError('Can\'t upload from detached HEAD state. Get on a branch!')
+
+      # Default to diffing against common ancestor of upstream branch
+      base_branch = self.GetCommonAncestorWithUpstream()
+      git_diff_args = [base_branch, 'HEAD']
+
+    # Make sure authenticated to codereview before running potentially expensive
+    # hooks.  It is a fast, best efforts check. Codereview still can reject the
+    # authentication during the actual upload.
+    self._codereview_impl.EnsureAuthenticated()
+
+    # Apply watchlists on upload.
+    change = self.GetChange(base_branch, None)
+    watchlist = watchlists.Watchlists(change.RepositoryRoot())
+    files = [f.LocalPath() for f in change.AffectedFiles()]
+    if not options.bypass_watchlists:
+      self.SetWatchers(watchlist.GetWatchersForPaths(files))
+
+    if not options.bypass_hooks:
+      if options.reviewers or options.tbr_owners:
+        # Set the reviewer list now so that presubmit checks can access it.
+        change_description = ChangeDescription(change.FullDescriptionText())
+        change_description.update_reviewers(options.reviewers,
+                                            options.tbr_owners,
+                                            change)
+        change.SetDescriptionText(change_description.description)
+      hook_results = self.RunHook(committing=False,
+                                may_prompt=not options.force,
+                                verbose=options.verbose,
+                                change=change)
+      if not hook_results.should_continue():
+        return 1
+      if not options.reviewers and hook_results.reviewers:
+        options.reviewers = hook_results.reviewers.split(',')
+
+    if self.GetIssue():
+      latest_patchset = self.GetMostRecentPatchset()
+      local_patchset = self.GetPatchset()
+      if (latest_patchset and local_patchset and
+          local_patchset != latest_patchset):
+        print ('The last upload made from this repository was patchset #%d but '
+               'the most recent patchset on the server is #%d.'
+               % (local_patchset, latest_patchset))
+        print ('Uploading will still work, but if you\'ve uploaded to this '
+               'issue from another machine or branch the patch you\'re '
+               'uploading now might not include those changes.')
+        ask_for_data('About to upload; enter to confirm.')
+
+    print_stats(options.similarity, options.find_copies, git_diff_args)
+    ret = self.CMDUploadChange(options, git_diff_args, change)
+    if not ret:
+      git_set_branch_value('last-upload-hash',
+                           RunGit(['rev-parse', 'HEAD']).strip())
+      # Run post upload hooks, if specified.
+      if settings.GetRunPostUploadHook():
+        presubmit_support.DoPostUploadExecuter(
+            change,
+            self,
+            settings.GetRoot(),
+            options.verbose,
+            sys.stdout)
+
+      # Upload all dependencies if specified.
+      if options.dependencies:
+        print
+        print '--dependencies has been specified.'
+        print 'All dependent local branches will be re-uploaded.'
+        print
+        # Remove the dependencies flag from args so that we do not end up in a
+        # loop.
+        orig_args.remove('--dependencies')
+        ret = upload_branch_deps(self, orig_args)
+    return ret
+
   # 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()
@@ skipping 93 matching lines @@
       directory: switch to directory before applying the patch. Rietveld only.
     """
     raise NotImplementedError()
 
   @staticmethod
   def ParseIssueURL(parsed_url):
     """Parses url and returns instance of _ParsedIssueNumberArgument or None if
     failed."""
     raise NotImplementedError()
 
+  def EnsureAuthenticated(self):
+    """Best effort check that user is authenticated with codereview server."""
+    raise NotImplementedError()
+
   def CMDUploadChange(self, options, args, change):
     """Uploads a change to codereview."""
     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 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 EnsureAuthenticated(self):
+    """Best effort check that user is authenticated with Rietveld server."""
+    if self._auth_config.use_oauth2:
+      authenticator = auth.get_authenticator_for_host(
+          self.GetCodereviewServer(), self._auth_config)
+      if not authenticator.has_cached_credentials():
+        raise auth.LoginRequiredError(self.GetCodereviewServer())
+
   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'
@@ skipping 322 matching lines @@
           auth_config = auth.extract_auth_config_from_options(options)
           branch_cl = Changelist(branchref=local_branch,
                                  auth_config=auth_config)
           branch_cl_issue_url = branch_cl.GetIssueURL()
           branch_cl_issue = branch_cl.GetIssue()
           branch_cl_patchset = branch_cl.GetPatchset()
           if branch_cl_issue_url and branch_cl_issue and branch_cl_patchset:
             upload_args.extend(
                 ['--depends_on_patchset', '%s:%s' % (
                      branch_cl_issue, branch_cl_patchset)])
-            print (
+            print(
                 '\n'
                 'The current branch (%s) is tracking a local branch (%s) with '
                 'an associated CL.\n'
                 'Adding %s/#ps%s as a dependency patchset.\n'
                 '\n' % (self.GetBranch(), local_branch, branch_cl_issue_url,
                         branch_cl_patchset))
 
     project = settings.GetProject()
     if project:
       upload_args.extend(['--project', project])
@@ skipping 60 matching lines @@
         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
 
   @classmethod
   def IssueSettingPrefix(cls):
     return 'gerritissue'
 
+  def EnsureAuthenticated(self):
+    """Best effort check that user is authenticated with Gerrit server."""
+    #TODO(tandrii): implement per bug http://crbug.com/583153.
+
   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
@@ skipping 181 matching lines @@
     match = re.match('(/c)?/(\d+)(/(\d+)?/?)?$', part)
     if match:
       return _ParsedIssueNumberArgument(
           issue=int(match.group(2)),
           patchset=int(match.group(4)) if match.group(4) else None,
           hostname=parsed_url.netloc)
     return None
 
   def CMDUploadChange(self, options, args, change):
     """Upload the current branch to Gerrit."""
+    if options.squash and options.no_squash:
+      DieWithError('Can only use one of --squash or --no-squash')
+    options.squash = ((settings.GetSquashGerritUploads() or options.squash) and
+                      not options.no_squash)
     # We assume the remote called "origin" is the one we want.
     # It is probably not worthwhile to support different workflows.
     gerrit_remote = 'origin'
 
     remote, remote_branch = self.GetRemoteBranch()
     branch = GetTargetRef(remote, remote_branch, options.target_branch,
                           pending_prefix='')
 
     if options.title:
       # TODO(tandrii): it's now supported by Gerrit, implement!
@@ skipping 1191 matching lines @@
   if git_common.is_dirty_git_tree('upload'):
     return 1
 
   options.reviewers = cleanup_list(options.reviewers)
   options.cc = cleanup_list(options.cc)
 
   # For sanity of test expectations, do this otherwise lazy-loading *now*.
   settings.GetIsGerrit()
 
   cl = Changelist(auth_config=auth_config)
-  if args:
-    # TODO(ukai): is it ok for gerrit case?
-    base_branch = args[0]
-  else:
-    if cl.GetBranch() is None:
-      DieWithError('Can\'t upload from detached HEAD state. Get on a branch!')
-
-    # Default to diffing against common ancestor of upstream branch
-    base_branch = cl.GetCommonAncestorWithUpstream()
-    args = [base_branch, 'HEAD']
-
-  # Make sure authenticated to Rietveld before running expensive hooks. It is
-  # a fast, best efforts check. Rietveld still can reject the authentication
-  # during the actual upload.
-  if not cl.IsGerrit() and auth_config.use_oauth2:
-    authenticator = auth.get_authenticator_for_host(
-        cl.GetCodereviewServer(), auth_config)
-    if not authenticator.has_cached_credentials():
-      raise auth.LoginRequiredError(cl.GetCodereviewServer())
-
-  # Apply watchlists on upload.
-  change = cl.GetChange(base_branch, None)
-  watchlist = watchlists.Watchlists(change.RepositoryRoot())
-  files = [f.LocalPath() for f in change.AffectedFiles()]
-  if not options.bypass_watchlists:
-    cl.SetWatchers(watchlist.GetWatchersForPaths(files))
-
-  if not options.bypass_hooks:
-    if options.reviewers or options.tbr_owners:
-      # Set the reviewer list now so that presubmit checks can access it.
-      change_description = ChangeDescription(change.FullDescriptionText())
-      change_description.update_reviewers(options.reviewers,
-                                          options.tbr_owners,
-                                          change)
-      change.SetDescriptionText(change_description.description)
-    hook_results = cl.RunHook(committing=False,
-                              may_prompt=not options.force,
-                              verbose=options.verbose,
-                              change=change)
-    if not hook_results.should_continue():
-      return 1
-    if not options.reviewers and hook_results.reviewers:
-      options.reviewers = hook_results.reviewers.split(',')
-
-  if cl.GetIssue():
-    latest_patchset = cl.GetMostRecentPatchset()
-    local_patchset = cl.GetPatchset()
-    if latest_patchset and local_patchset and local_patchset != latest_patchset:
-      print ('The last upload made from this repository was patchset #%d but '
-            'the most recent patchset on the server is #%d.'
-            % (local_patchset, latest_patchset))
-      print ('Uploading will still work, but if you\'ve uploaded to this issue '
-            'from another machine or branch the patch you\'re uploading now '
-            'might not include those changes.')
-      ask_for_data('About to upload; enter to confirm.')
-
-  print_stats(options.similarity, options.find_copies, args)
-  if cl.IsGerrit():
-    if options.squash and options.no_squash:
-      DieWithError('Can only use one of --squash or --no-squash')
-
-    options.squash = ((settings.GetSquashGerritUploads() or options.squash) and
-                      not options.no_squash)
-
-  ret = cl.CMDUploadChange(options, args, change)
-  if not ret:
-    git_set_branch_value('last-upload-hash',
-                         RunGit(['rev-parse', 'HEAD']).strip())
-    # Run post upload hooks, if specified.
-    if settings.GetRunPostUploadHook():
-      presubmit_support.DoPostUploadExecuter(
-          change,
-          cl,
-          settings.GetRoot(),
-          options.verbose,
-          sys.stdout)
-
-    # Upload all dependencies if specified.
-    if options.dependencies:
-      print
-      print '--dependencies has been specified.'
-      print 'All dependent local branches will be re-uploaded.'
-      print
-      # Remove the dependencies flag from args so that we do not end up in a
-      # loop.
-      orig_args.remove('--dependencies')
-      upload_branch_deps(cl, orig_args)
-  return ret
+  return cl.CMDUpload(options, args, orig_args)
 
 
 def IsSubmoduleMergeCommit(ref):
   # When submodules are added to the repo, we expect there to be a single
   # non-git-svn merge commit at remote HEAD with a signature comment.
   pattern = '^SVN changes up to revision [0-9]*$'
   cmd = ['rev-list', '--merges', '--grep=%s' % pattern, '%s^!' % ref]
   return RunGit(cmd) != ''
 
 
@@ skipping 1246 matching lines @@
 if __name__ == '__main__':
   # These affect sys.stdout so do it outside of main() to simplify mocks in
   # unit testing.
   fix_encoding.fix_encoding()
   setup_color.init()
   try:
     sys.exit(main(sys.argv[1:]))
   except KeyboardInterrupt:
     sys.stderr.write('interrupted\n')
     sys.exit(1)