Adding retry logic (recursive) to run_command().

When setting remote URL on slow hosts such as AppVeyor, sometimes
test_bad_git_config() runs into a race condition. Adding retry.

Handling git remote set-url errors.

Author: Robpol86

sphinxcontrib/versioning/git.py

@@ -7,6 +7,7 @@
 import re
 import sys
 import tarfile
+import time
 from datetime import datetime
 from subprocess import CalledProcessError, PIPE, Popen, STDOUT
 
@@ -111,7 +112,7 @@ def chunk(iterator, max_size):
         yield chunked
 
 
-def run_command(local_root, command, env_var=True, pipeto=None):
+def run_command(local_root, command, env_var=True, pipeto=None, retry=0):
     """Run a command and return the output.
 
     :raise CalledProcessError: Command exits non-zero.
@@ -120,6 +121,7 @@ def run_command(local_root, command, env_var=True, pipeto=None):
     :param iter command: Command to run.
     :param bool env_var: Define GIT_DIR environment variable (on non-Windows).
     :param function pipeto: Pipe `command`'s stdout to this function (only parameter given).
+    :param int retry: Retry this many times on CalledProcessError after 0.1 seconds.
 
     :return: Command output.
     :rtype: str
@@ -145,7 +147,10 @@ def run_command(local_root, command, env_var=True, pipeto=None):
 
     # Verify success.
     if main.poll() != 0:
-        raise CalledProcessError(main.poll(), command, output=main_output)
+        if retry < 1:
+            raise CalledProcessError(main.poll(), command, output=main_output)
+        time.sleep(0.1)
+        return run_command(local_root, command, env_var, pipeto, retry - 1)
 
     return main_output
 
@@ -345,8 +350,11 @@ def clone(local_root, new_root, remote, branch, rel_dest, exclude):
 
     # Copy all remotes from original repo.
     for name, (fetch, push) in remotes.items():
-        run_command(new_root, ['git', 'remote', 'set-url' if name == 'origin' else 'add', name, fetch])
-        run_command(new_root, ['git', 'remote', 'set-url', '--push', name, push])
+        try:
+            run_command(new_root, ['git', 'remote', 'set-url' if name == 'origin' else 'add', name, fetch], retry=3)
+            run_command(new_root, ['git', 'remote', 'set-url', '--push', name, push], retry=3)
+        except CalledProcessError as exc:
+            raise GitError('Failed to set git remote URL.', exc.output)
 
     # Done if no exclude.
     if not exclude:

tests/conftest.py

@@ -29,7 +29,7 @@ def config(monkeypatch):
 @pytest.fixture
 def run():
     """run_command() wrapper returned from a pytest fixture."""
-    return lambda d, c: run_command(str(d), [str(i) for i in c])
+    return lambda d, c, *args, **kwargs: run_command(str(d), [str(i) for i in c], *args, **kwargs)
 
 
 @pytest.fixture

tests/test__main__/test_main_push_scenarios.py

@@ -332,13 +332,8 @@ def test_bad_git_config(local_docs_ghp, run):
                 # Invalidate lock file.
                 tmp_repo = py.path.local(re.findall(r'"cwd": "([^"]+)"', line.decode('utf-8'))[0])
                 assert tmp_repo.check(dir=True)
-                for _ in range(3):
-                    try:
-                        run(tmp_repo, ['git', 'config', 'user.useConfigOnly', 'true'])
-                        run(tmp_repo, ['git', 'config', 'user.email', '(none)'])
-                    except CalledProcessError:
-                        continue
-                    break
+                run(tmp_repo, ['git', 'config', 'user.useConfigOnly', 'true'], retry=3)
+                run(tmp_repo, ['git', 'config', 'user.email', '(none)'], retry=3)
                 caused = True
     output_lines.append(proc.communicate()[0])
     output = b''.join(output_lines).decode('utf-8')
@@ -348,4 +343,4 @@ def test_bad_git_config(local_docs_ghp, run):
     # Verify.
     assert 'Traceback' not in output
     assert 'Failed to commit locally.' in output
-    assert 'Please tell me who you are.' in output
+    assert 'Please tell me who you are.' in output or 'user.useConfigOnly set but no name given' in output