Add retry on git push failure

This adds retry if target is old default branch. This may happen only if
user didn't fetch remote for extended period of time and old default
branch no longer exists.

R=ehmaldonado@chromium.org

Change-Id: I81981049ee006f68a073718180b454c230d055e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2473757
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>

gerrit_util.py

@@ -926,6 +926,12 @@ def GetGerritBranch(host, project, branch):
   raise GerritError(200, 'Unable to get gerrit branch')
 
 
+def GetProjectHead(host, project):
+  conn = CreateHttpConn(host,
+                        '/projects/%s/HEAD' % urllib.parse.quote(project, ''))
+  return ReadHttpJsonResponse(conn, accept_statuses=[200])
+
+
 def GetAccountDetails(host, account_id='self'):
   """Returns details of the account.
 

git_cl.py

@@ -145,6 +145,10 @@ assert len(_KNOWN_GERRIT_TO_SHORT_URLS) == len(
     set(_KNOWN_GERRIT_TO_SHORT_URLS.values())), 'must have unique values'
 
 
+class GitPushError(Exception):
+  pass
+
+
 def DieWithError(message, change_desc=None):
   if change_desc:
     SaveDescriptionBackup(change_desc)
@@ -2080,8 +2084,7 @@ class Changelist(object):
 
     gclient_utils.rmtree(git_info_dir)
 
-  def _RunGitPushWithTraces(
-      self, change_desc, refspec, refspec_opts, git_push_metadata):
+  def _RunGitPushWithTraces(self, refspec, refspec_opts, git_push_metadata):
     """Run git push and collect the traces resulting from the execution."""
     # Create a temporary directory to store traces in. Traces will be compressed
     # and stored in a 'traces' dir inside depot_tools.
@@ -2111,20 +2114,19 @@ class Changelist(object):
       push_stdout = push_stdout.decode('utf-8', 'replace')
     except subprocess2.CalledProcessError as e:
       push_returncode = e.returncode
-      DieWithError('Failed to create a change. Please examine output above '
-                   'for the reason of the failure.\n'
-                   'Hint: run command below to diagnose common Git/Gerrit '
-                   'credential problems:\n'
-                   '  git cl creds-check\n'
-                   '\n'
-                   'If git-cl is not working correctly, file a bug under the '
-                   'Infra>SDK component including the files below.\n'
-                   'Review the files before upload, since they might contain '
-                   'sensitive information.\n'
-                   'Set the Restrict-View-Google label so that they are not '
-                   'publicly accessible.\n'
-                   + TRACES_MESSAGE % {'trace_name': trace_name},
-                   change_desc)
+      raise GitPushError(
+          'Failed to create a change. Please examine output above for the '
+          'reason of the failure.\n'
+          'Hint: run command below to diagnose common Git/Gerrit '
+          'credential problems:\n'
+          '  git cl creds-check\n'
+          '\n'
+          'If git-cl is not working correctly, file a bug under the Infra>SDK '
+          'component including the files below.\n'
+          'Review the files before upload, since they might contain sensitive '
+          'information.\n'
+          'Set the Restrict-View-Google label so that they are not publicly '
+          'accessible.\n' + TRACES_MESSAGE % {'trace_name': trace_name})
     finally:
       execution_time = time_time() - before_push
       metrics.collector.add_repeated('sub_commands', {
@@ -2143,8 +2145,32 @@ class Changelist(object):
 
     return push_stdout
 
-  def CMDUploadChange(
-      self, options, git_diff_args, custom_cl_base, change_desc):
+  def CMDUploadChange(self, options, git_diff_args, custom_cl_base,
+                      change_desc):
+    """Upload the current branch to Gerrit, retry if new remote HEAD is
+    found. options and change_desc may be mutated."""
+    try:
+      return self._CMDUploadChange(options, git_diff_args, custom_cl_base,
+                                   change_desc)
+    except GitPushError as e:
+      remote, remote_branch = self.GetRemoteBranch()
+      should_retry = remote_branch == DEFAULT_OLD_BRANCH and \
+          gerrit_util.GetProjectHead(
+              self._gerrit_host, self._GetGerritProject()) == 'refs/heads/main'
+      if not should_retry:
+        DieWithError(str(e), change_desc)
+
+    print("WARNING: Detected HEAD change in upstream, fetching remote state")
+    RunGit(['fetch', remote])
+    options.edit_description = False
+    options.force = True
+    try:
+      self._CMDUploadChange(options, git_diff_args, custom_cl_base, change_desc)
+    except GitPushError as e:
+      DieWithError(str(e), change_desc)
+
+  def _CMDUploadChange(self, options, git_diff_args, custom_cl_base,
+                       change_desc):
     """Upload the current branch to Gerrit."""
     remote, remote_branch = self.GetRemoteBranch()
     branch = GetTargetRef(remote, remote_branch, options.target_branch)
@@ -2242,10 +2268,13 @@ class Changelist(object):
     # TODO(tandrii): options.message should be posted as a comment
     # if --send-mail is set on non-initial upload as Rietveld used to do it.
 
-    title = self._GetTitleForUpload(options)
-    if title:
+    # Set options.title in case user was prompted in _GetTitleForUpload and
+    # _CMDUploadChange needs to be called again.
+    options.title = self._GetTitleForUpload(options)
+    if options.title:
       # Punctuation and whitespace in |title| must be percent-encoded.
-      refspec_opts.append('m=' + gerrit_util.PercentEncodeForGitRef(title))
+      refspec_opts.append(
+          'm=' + gerrit_util.PercentEncodeForGitRef(options.title))
 
     if options.private:
       refspec_opts.append('private')
@@ -2298,12 +2327,12 @@ class Changelist(object):
 
     git_push_metadata = {
         'gerrit_host': self._GetGerritHost(),
-        'title': title or '<untitled>',
+        'title': options.title or '<untitled>',
         'change_id': change_id,
         'description': change_desc.description,
     }
-    push_stdout = self._RunGitPushWithTraces(
-        change_desc, refspec, refspec_opts, git_push_metadata)
+    push_stdout = self._RunGitPushWithTraces(refspec, refspec_opts,
+                                             git_push_metadata)
 
     if options.squash:
       regex = re.compile(r'remote:\s+https?://[\w\-\.\+\/#]*/(\d+)\s.*')

tests/git_cl_test.py

@@ -226,6 +226,70 @@ class TestGitClBasic(unittest.TestCase):
     actual = set(git_cl.get_cl_statuses(changes, True))
     self.assertEqual(set(zip(changes, statuses)), actual)
 
+  def test_upload_to_non_default_branch_no_retry(self):
+    m = mock.patch('git_cl.Changelist._CMDUploadChange',
+                   side_effect=[git_cl.GitPushError(), None]).start()
+    mock.patch('git_cl.Changelist.GetRemoteBranch',
+               return_value=('foo', 'bar')).start()
+    mock.patch('git_cl.Changelist._GetGerritProject',
+               return_value='foo').start()
+    mock.patch('git_cl.gerrit_util.GetProjectHead',
+               return_value='refs/heads/main').start()
+
+    cl = git_cl.Changelist()
+    options = optparse.Values()
+    with self.assertRaises(SystemExitMock):
+      cl.CMDUploadChange(options, [], 'foo', git_cl.ChangeDescription('bar'))
+
+    # ensure upload is called once
+    self.assertEqual(len(m.mock_calls), 1)
+    sys.exit.assert_called_once_with(1)
+    # option not set as retry didn't happen
+    self.assertFalse(hasattr(options, 'force'))
+    self.assertFalse(hasattr(options, 'edit_description'))
+
+  def test_upload_to_old_default_still_active(self):
+    m = mock.patch('git_cl.Changelist._CMDUploadChange',
+                   side_effect=[git_cl.GitPushError(), None]).start()
+    mock.patch('git_cl.Changelist.GetRemoteBranch',
+               return_value=('foo', git_cl.DEFAULT_OLD_BRANCH)).start()
+    mock.patch('git_cl.Changelist._GetGerritProject',
+               return_value='foo').start()
+    mock.patch('git_cl.gerrit_util.GetProjectHead',
+               return_value='refs/heads/old_default').start()
+
+    cl = git_cl.Changelist()
+    options = optparse.Values()
+    with self.assertRaises(SystemExitMock):
+      cl.CMDUploadChange(options, [], 'foo', git_cl.ChangeDescription('bar'))
+
+    # ensure upload is called once
+    self.assertEqual(len(m.mock_calls), 1)
+    sys.exit.assert_called_once_with(1)
+    # option not set as retry didn't happen
+    self.assertFalse(hasattr(options, 'force'))
+    self.assertFalse(hasattr(options, 'edit_description'))
+
+  def test_upload_to_old_default_retry_on_failure(self):
+    m = mock.patch('git_cl.Changelist._CMDUploadChange',
+                   side_effect=[git_cl.GitPushError(), None]).start()
+    mock.patch('git_cl.Changelist.GetRemoteBranch',
+               return_value=('foo', git_cl.DEFAULT_OLD_BRANCH)).start()
+    mock.patch('git_cl.Changelist._GetGerritProject',
+               return_value='foo').start()
+    mock.patch('git_cl.gerrit_util.GetProjectHead',
+               return_value='refs/heads/main').start()
+    mock.patch('git_cl.RunGit').start()
+
+    cl = git_cl.Changelist()
+    options = optparse.Values()
+    cl.CMDUploadChange(options, [], 'foo', git_cl.ChangeDescription('bar'))
+    # ensure upload is called twice
+    self.assertEqual(len(m.mock_calls), 2)
+    # option overrides on retry
+    self.assertEqual(options.force, True)
+    self.assertEqual(options.edit_description, False)
+
   def test_get_cl_statuses_no_changes(self):
     self.assertEqual([], list(git_cl.get_cl_statuses([], True)))