[depot_tools] retry git config if it lock-fails

If git config set is executed, it writes the new content into
a temp file (but called lock-file) and then replace .gitconfig
with it.

However, if it cannot create the lock file, it returns an error.
With this CL, if git config fails with the lock file error,
depot_tools will retry it at most 5 times with 0.2s interval.

It's found that there are applications, such as vscode extensions,
executes `git config set` frequently, and those could often cause
unexpected interruptions to ongoing `git rebase-updates`
by the lock failure.

Bug: 351950514
Change-Id: I985af0d8b7458dbf47cd6baa857dc5adccf15031
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5705561
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Scott Lee <ddoman@chromium.org>

git_common.py

@@ -30,12 +30,14 @@ import contextlib
 import functools
 import logging
 import os
+import random
 import re
 import setup_color
 import shutil
 import signal
 import tempfile
 import textwrap
+import time
 
 import scm
 import subprocess2
@@ -856,10 +858,41 @@ def run_stream_with_retcode(*cmd, **kwargs):
             raise subprocess2.CalledProcessError(retcode, cmd, os.getcwd(), b'',
                                                  b'')
 
-
 def run_with_stderr(*cmd, **kwargs):
     """Runs a git command.
 
+    Returns (stdout, stderr) as a pair of strings.
+    If the command is `config` and the execution fails due to a lock failure,
+    retry the execution at most 5 times with 0.2 interval.
+
+    kwargs
+        autostrip (bool) - Strip the output. Defaults to True.
+        indata (str) - Specifies stdin data for the process.
+        retry_lock (bool) - If true and the command is `config`,
+            retry on lock failures. Defaults to True.
+    """
+    retry_cnt = 0
+    if kwargs.pop('retry_lock', True) and len(cmd) > 0 and cmd[0] == 'config':
+        retry_cnt = 5
+
+    while True:
+        try:
+            return _run_with_stderr(*cmd, **kwargs)
+        except subprocess2.CalledProcessError as ex:
+            lock_err = 'could not lock config file .git/config: File exists'
+            if retry_cnt > 0 and lock_err in str(ex):
+                logging.error(ex)
+                jitter = random.uniform(0, 0.2)
+                time.sleep(0.1 + jitter)
+                retry_cnt -= 1
+                continue
+
+            raise ex
+
+
+def _run_with_stderr(*cmd, **kwargs):
+    """Runs a git command.
+
     Returns (stdout, stderr) as a pair of strings.
 
     kwargs

tests/git_common_test.py

@@ -22,6 +22,7 @@ from unittest import mock
 DEPOT_TOOLS_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
 sys.path.insert(0, DEPOT_TOOLS_ROOT)
 
+import subprocess2
 from testing_support import coverage_utils
 from testing_support import git_test_utils
 
@@ -258,7 +259,6 @@ class GitReadOnlyFunctionsTest(git_test_utils.GitRepoReadOnlyTestBase,
         self.repo.run(testfn)
 
     def testStreamWithRetcodeException(self):
-        import subprocess2
         with self.assertRaises(subprocess2.CalledProcessError):
             with self.gc.run_stream_with_retcode('checkout', 'unknown-branch'):
                 pass
@@ -1173,6 +1173,83 @@ class WarnSubmoduleTest(unittest.TestCase):
                         sys.stdout.getvalue())
 
 
+@mock.patch('time.sleep')
+@mock.patch('git_common._run_with_stderr')
+class RunWithStderr(GitCommonTestBase):
+
+    def setUp(self):
+        super(RunWithStderr, self).setUp()
+        msg = 'error: could not lock config file .git/config: File exists'
+        self.lock_failure = self.calledProcessError(msg)
+        msg = 'error: wrong number of arguments, should be 2'
+        self.wrong_param = self.calledProcessError(msg)
+
+    def calledProcessError(self, stderr):
+        return subprocess2.CalledProcessError(
+            2,
+            cmd=['a', 'b', 'c'],  # doesn't matter
+            cwd='/',
+            stdout='',
+            stderr=stderr.encode('utf-8'),
+        )
+
+    def runGitCheckout(self, ex, retry_lock):
+        with self.assertRaises(type(ex)):
+            self.gc.run_with_stderr('checkout', 'a', retry_lock=retry_lock)
+
+    def runGitConfig(self, ex, retry_lock):
+        with self.assertRaises(type(ex)):
+            self.gc.run_with_stderr('config', 'set', retry_lock=retry_lock)
+
+    def test_config_with_non_lock_failure(self, run_mock, _):
+        """Tests git-config with a non-lock-failure."""
+        ex = self.wrong_param
+        run_mock.side_effect = ex
+        # retry_lock == True
+        self.runGitConfig(ex, retry_lock=True)
+        self.assertEqual(run_mock.call_count, 1)  # 1 + 0 (retry)
+        # retry_lock == False
+        run_mock.reset_mock()
+        self.runGitConfig(ex, retry_lock=False)
+        self.assertEqual(run_mock.call_count, 1)  # 1 + 0 (retry)
+
+    def test_config_with_lock_failure(self, run_mock, _):
+        """Tests git-config with a lock-failure."""
+        ex = self.lock_failure
+        run_mock.side_effect = ex
+        # retry_lock == True
+        self.runGitConfig(ex, retry_lock=True)
+        self.assertEqual(run_mock.call_count, 6)  # 1 + 5 (retry)
+        # retry_lock == False
+        run_mock.reset_mock()
+        self.runGitConfig(ex, retry_lock=False)
+        self.assertEqual(run_mock.call_count, 1)  # 1 + 0 (retry)
+
+    def test_checkout_with_non_lock_failure(self, run_mock, _):
+        """Tests git-checkout with a non-lock-failure."""
+        ex = self.wrong_param
+        run_mock.side_effect = ex
+        # retry_lock == True
+        self.runGitCheckout(ex, retry_lock=True)
+        self.assertEqual(run_mock.call_count, 1)  # 1 + 0 (retry)
+        # retry_lock == False
+        run_mock.reset_mock()
+        self.runGitCheckout(ex, retry_lock=False)
+        self.assertEqual(run_mock.call_count, 1)  # 1 + 0 (retry)
+
+    def test_checkout_with_lock_failure(self, run_mock, _):
+        """Tests git-checkout with a lock-failure."""
+        ex = self.lock_failure
+        run_mock.side_effect = ex
+        # retry_lock == True
+        self.runGitCheckout(ex, retry_lock=True)
+        self.assertEqual(run_mock.call_count, 1)  # 1 + 0 (retry)
+        # retry_lock == False
+        run_mock.reset_mock()
+        self.runGitCheckout(ex, retry_lock=False)
+        self.assertEqual(run_mock.call_count, 1)  # 1 + 0 (retry)
+
+
 if __name__ == '__main__':
     sys.exit(
         coverage_utils.covered_main(