[git] Stop using git-retry wrapper on LUCI.

The LUCI environment alreadly does `git retry` style retries by virtue
of it's Go git-wrapper program. No need to put an extra python
interpreter in the middle.

R=hinoka@chromium.org, nodir@chromium.org

Recipe-Nontrivial-Roll: infra
Recipe-Nontrivial-Roll: build
Change-Id: I2918d7f413dde667fccd45c83ad6f4b96c2afe2e
Reviewed-on: https://chromium-review.googlesource.com/c/1292236
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>
Reviewed-by: Ryan Tseng <hinoka@chromium.org>

recipes/README.recipes.md

@@ -394,7 +394,7 @@ Returns:
       https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#list-changes
 ### *recipe_modules* / [git](/recipes/recipe_modules/git)
 
-[DEPS](/recipes/recipe_modules/git/__init__.py#1): [infra\_paths](#recipe_modules-infra_paths), [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/platform][recipe_engine/recipe_modules/platform], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/python][recipe_engine/recipe_modules/python], [recipe\_engine/raw\_io][recipe_engine/recipe_modules/raw_io], [recipe\_engine/step][recipe_engine/recipe_modules/step]
+[DEPS](/recipes/recipe_modules/git/__init__.py#1): [infra\_paths](#recipe_modules-infra_paths), [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/platform][recipe_engine/recipe_modules/platform], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/python][recipe_engine/recipe_modules/python], [recipe\_engine/raw\_io][recipe_engine/recipe_modules/raw_io], [recipe\_engine/runtime][recipe_engine/recipe_modules/runtime], [recipe\_engine/step][recipe_engine/recipe_modules/step]
 
 #### **class [GitApi](/recipes/recipe_modules/git/api.py#10)([RecipeApi][recipe_engine/wkt/RecipeApi]):**
 
@@ -402,7 +402,7 @@ Returns:
 
 Return a git command step.
 
-— **def [bundle\_create](/recipes/recipe_modules/git/api.py#374)(self, bundle_path, rev_list_args=None, \*\*kwargs):**
+— **def [bundle\_create](/recipes/recipe_modules/git/api.py#381)(self, bundle_path, rev_list_args=None, \*\*kwargs):**
 
 Run 'git bundle create' on a Git repository.
 
@@ -412,11 +412,11 @@ Args:
       refs in the Git checkout will be bundled.
   kwargs: Forwarded to '__call__'.
 
-— **def [cat\_file\_at\_commit](/recipes/recipe_modules/git/api.py#38)(self, file_path, commit_hash, remote_name=None, \*\*kwargs):**
+— **def [cat\_file\_at\_commit](/recipes/recipe_modules/git/api.py#43)(self, file_path, commit_hash, remote_name=None, \*\*kwargs):**
 
 Outputs the contents of a file at a given revision.
 
-— **def [checkout](/recipes/recipe_modules/git/api.py#110)(self, url, ref=None, dir_path=None, recursive=False, submodules=True, submodule_update_force=False, keep_paths=None, step_suffix=None, curl_trace_file=None, can_fail_build=True, set_got_revision=False, remote_name=None, display_fetch_size=None, file_name=None, submodule_update_recursive=True, use_git_cache=False, progress=True, tags=False):**
+— **def [checkout](/recipes/recipe_modules/git/api.py#115)(self, url, ref=None, dir_path=None, recursive=False, submodules=True, submodule_update_force=False, keep_paths=None, step_suffix=None, curl_trace_file=None, can_fail_build=True, set_got_revision=False, remote_name=None, display_fetch_size=None, file_name=None, submodule_update_recursive=True, use_git_cache=False, progress=True, tags=False):**
 
 Performs a full git checkout and returns sha1 of checked out revision.
 
@@ -455,7 +455,7 @@ Args:
 Returns: If the checkout was successful, this returns the commit hash of
   the checked-out-repo. Otherwise this returns None.
 
-— **def [config\_get](/recipes/recipe_modules/git/api.py#347)(self, prop_name, \*\*kwargs):**
+— **def [config\_get](/recipes/recipe_modules/git/api.py#354)(self, prop_name, \*\*kwargs):**
 
 Returns: (str) The Git config output, or None if no output was generated.
 
@@ -463,7 +463,7 @@ Args:
   prop_name: (str) The name of the config property to query.
   kwargs: Forwarded to '__call__'.
 
-— **def [count\_objects](/recipes/recipe_modules/git/api.py#46)(self, previous_result=None, can_fail_build=False, \*\*kwargs):**
+— **def [count\_objects](/recipes/recipe_modules/git/api.py#51)(self, previous_result=None, can_fail_build=False, \*\*kwargs):**
 
 Returns `git count-objects` result as a dict.
 
@@ -476,11 +476,11 @@ Args:
 Returns:
   A dict of count-object values, or None if count-object run failed.
 
-— **def [fetch\_tags](/recipes/recipe_modules/git/api.py#32)(self, remote_name=None, \*\*kwargs):**
+— **def [fetch\_tags](/recipes/recipe_modules/git/api.py#37)(self, remote_name=None, \*\*kwargs):**
 
 Fetches all tags from the remote.
 
-— **def [get\_remote\_url](/recipes/recipe_modules/git/api.py#364)(self, remote_name=None, \*\*kwargs):**
+— **def [get\_remote\_url](/recipes/recipe_modules/git/api.py#371)(self, remote_name=None, \*\*kwargs):**
 
 Returns: (str) The URL of the remote Git repository, or None.
 
@@ -488,11 +488,11 @@ Args:
   remote_name: (str) The name of the remote to query, defaults to 'origin'.
   kwargs: Forwarded to '__call__'.
 
-— **def [get\_timestamp](/recipes/recipe_modules/git/api.py#319)(self, commit='HEAD', test_data=None, \*\*kwargs):**
+— **def [get\_timestamp](/recipes/recipe_modules/git/api.py#326)(self, commit='HEAD', test_data=None, \*\*kwargs):**
 
 Find and return the timestamp of the given commit.
 
-— **def [new\_branch](/recipes/recipe_modules/git/api.py#387)(self, branch, name=None, upstream=None, \*\*kwargs):**
+— **def [new\_branch](/recipes/recipe_modules/git/api.py#394)(self, branch, name=None, upstream=None, \*\*kwargs):**
 
 Runs git new-branch on a Git repository, to be used before git cl upload.
 
@@ -502,7 +502,7 @@ Args:
   upstream (str): to origin/master.
   kwargs: Forwarded to '__call__'.
 
-— **def [rebase](/recipes/recipe_modules/git/api.py#328)(self, name_prefix, branch, dir_path, remote_name=None, \*\*kwargs):**
+— **def [rebase](/recipes/recipe_modules/git/api.py#335)(self, name_prefix, branch, dir_path, remote_name=None, \*\*kwargs):**
 
 Run rebase HEAD onto branch
 Args:
@@ -928,9 +928,9 @@ Raises:
 — **def [RunSteps](/recipes/recipe_modules/gerrit/examples/full.py#11)(api):**
 ### *recipes* / [git:examples/full](/recipes/recipe_modules/git/examples/full.py)
 
-[DEPS](/recipes/recipe_modules/git/examples/full.py#5): [git](#recipe_modules-git), [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/platform][recipe_engine/recipe_modules/platform], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/raw\_io][recipe_engine/recipe_modules/raw_io], [recipe\_engine/step][recipe_engine/recipe_modules/step]
+[DEPS](/recipes/recipe_modules/git/examples/full.py#5): [git](#recipe_modules-git), [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/platform][recipe_engine/recipe_modules/platform], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/raw\_io][recipe_engine/recipe_modules/raw_io], [recipe\_engine/runtime][recipe_engine/recipe_modules/runtime], [recipe\_engine/step][recipe_engine/recipe_modules/step]
 
-— **def [RunSteps](/recipes/recipe_modules/git/examples/full.py#16)(api):**
+— **def [RunSteps](/recipes/recipe_modules/git/examples/full.py#18)(api):**
 ### *recipes* / [git\_cl:examples/full](/recipes/recipe_modules/git_cl/examples/full.py)
 
 [DEPS](/recipes/recipe_modules/git_cl/examples/full.py#9): [git\_cl](#recipe_modules-git_cl), [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/raw\_io][recipe_engine/recipe_modules/raw_io], [recipe\_engine/step][recipe_engine/recipe_modules/step]

recipes/recipe_modules/git/__init__.py

@@ -1,10 +1,12 @@
 DEPS = [
-  'infra_paths',
   'recipe_engine/context',
+  'recipe_engine/runtime',
   'recipe_engine/path',
   'recipe_engine/platform',
   'recipe_engine/properties',
   'recipe_engine/python',
   'recipe_engine/raw_io',
   'recipe_engine/step',
+
+  'infra_paths',
 ]

recipes/recipe_modules/git/api.py

@@ -13,8 +13,15 @@ class GitApi(recipe_api.RecipeApi):
   def __call__(self, *args, **kwargs):
     """Return a git command step."""
     name = kwargs.pop('name', 'git ' + args[0])
+
     infra_step = kwargs.pop('infra_step', True)
     git_cmd = ['git']
+
+    if kwargs.pop('add_retry', False) and not self.m.runtime.is_luci:
+      # On LUCI, the `git` binary is a go wrapper which already implements all
+      # the git-retry logic.
+      git_cmd += ['retry']
+
     options = kwargs.pop('git_config_options', {})
     for k, v in sorted(options.iteritems()):
       git_cmd.extend(['-c', '%s=%s' % (k, v)])
@@ -192,11 +199,12 @@ class GitApi(recipe_api.RecipeApi):
     with self.m.context(cwd=dir_path):
       if use_git_cache:
         with self.m.context(env={'PATH': path}):
-          self('retry', 'cache', 'populate', '-c',
+          self('cache', 'populate', '-c',
                self.m.infra_paths.default_git_cache_dir, url,
 
                name='populate cache',
-               can_fail_build=can_fail_build)
+               can_fail_build=can_fail_build,
+               add_retry=True)
           dir_cmd = self(
               'cache', 'exists', '--quiet',
               '--cache-dir', self.m.infra_paths.default_git_cache_dir, url,
@@ -259,10 +267,11 @@ class GitApi(recipe_api.RecipeApi):
             step_test_data=lambda: self.m.raw_io.test_api.stream_output(
                 self.test_api.count_objects_output(1000)))
       with self.m.context(env=fetch_env):
-        self('retry', 'fetch', *fetch_args,
+        self('fetch', *fetch_args,
           name=fetch_step_name,
           stderr=fetch_stderr,
-          can_fail_build=can_fail_build)
+          can_fail_build=can_fail_build,
+          add_retry=True)
       if display_fetch_size:
         self.count_objects(
             name='count-objects after %s' % fetch_step_name,

recipes/recipe_modules/git/examples/full.expected/basic_luci.json

@@ -0,0 +1,223 @@
+[
+  {
+    "cmd": [
+      "python",
+      "-u",
+      "RECIPE_MODULE[depot_tools::git]/resources/git_setup.py",
+      "--path",
+      "[START_DIR]/src",
+      "--url",
+      "https://chromium.googlesource.com/chromium/src.git"
+    ],
+    "name": "git setup"
+  },
+  {
+    "cmd": [
+      "git",
+      "fetch",
+      "origin",
+      "master",
+      "--recurse-submodules",
+      "--progress"
+    ],
+    "cwd": "[START_DIR]/src",
+    "env": {
+      "PATH": "RECIPE_PACKAGE_REPO[depot_tools]:<PATH>"
+    },
+    "infra_step": true,
+    "name": "git fetch"
+  },
+  {
+    "cmd": [
+      "git",
+      "checkout",
+      "-f",
+      "FETCH_HEAD"
+    ],
+    "cwd": "[START_DIR]/src",
+    "infra_step": true,
+    "name": "git checkout"
+  },
+  {
+    "cmd": [
+      "git",
+      "rev-parse",
+      "HEAD"
+    ],
+    "cwd": "[START_DIR]/src",
+    "infra_step": true,
+    "name": "read revision",
+    "stdout": "/path/to/tmp/",
+    "~followup_annotations": [
+      "@@@STEP_TEXT@<br/>checked out 'deadbeef'<br/>@@@"
+    ]
+  },
+  {
+    "cmd": [
+      "git",
+      "clean",
+      "-f",
+      "-d",
+      "-x"
+    ],
+    "cwd": "[START_DIR]/src",
+    "infra_step": true,
+    "name": "git clean"
+  },
+  {
+    "cmd": [
+      "git",
+      "submodule",
+      "sync"
+    ],
+    "cwd": "[START_DIR]/src",
+    "infra_step": true,
+    "name": "submodule sync"
+  },
+  {
+    "cmd": [
+      "git",
+      "submodule",
+      "update",
+      "--init",
+      "--recursive"
+    ],
+    "cwd": "[START_DIR]/src",
+    "infra_step": true,
+    "name": "submodule update"
+  },
+  {
+    "cmd": [
+      "git",
+      "-c",
+      "foo=bar",
+      "count-objects",
+      "-v"
+    ],
+    "cwd": "[START_DIR]/src",
+    "infra_step": true,
+    "name": "count-objects",
+    "stdout": "/path/to/tmp/"
+  },
+  {
+    "cmd": [
+      "git",
+      "config",
+      "--get",
+      "remote.origin.url"
+    ],
+    "cwd": "[START_DIR]/src",
+    "infra_step": true,
+    "name": "git config remote.origin.url",
+    "stdout": "/path/to/tmp/",
+    "~followup_annotations": [
+      "@@@STEP_TEXT@foo@@@"
+    ]
+  },
+  {
+    "cmd": [
+      "git",
+      "show",
+      "HEAD",
+      "--format=%at",
+      "-s"
+    ],
+    "cwd": "[START_DIR]/src",
+    "infra_step": true,
+    "name": "git show",
+    "stdout": "/path/to/tmp/"
+  },
+  {
+    "cmd": [
+      "git",
+      "fetch",
+      "origin",
+      "--tags"
+    ],
+    "cwd": "[START_DIR]/src",
+    "infra_step": true,
+    "name": "git fetch tags"
+  },
+  {
+    "cmd": [
+      "git",
+      "status"
+    ],
+    "cwd": "[START_DIR]/src",
+    "infra_step": true,
+    "name": "git status"
+  },
+  {
+    "cmd": [
+      "git",
+      "status"
+    ],
+    "cwd": "[START_DIR]/src",
+    "infra_step": true,
+    "name": "git status can_fail_build"
+  },
+  {
+    "cmd": [
+      "git",
+      "status"
+    ],
+    "cwd": "[START_DIR]/src",
+    "infra_step": true,
+    "name": "git status cannot_fail_build"
+  },
+  {
+    "cmd": [
+      "git",
+      "new-branch",
+      "refactor"
+    ],
+    "cwd": "[START_DIR]/src",
+    "env": {
+      "PATH": "RECIPE_PACKAGE_REPO[depot_tools]:<PATH>"
+    },
+    "infra_step": true,
+    "name": "git new-branch refactor"
+  },
+  {
+    "cmd": [
+      "git",
+      "new-branch",
+      "feature",
+      "--upstream",
+      "refactor"
+    ],
+    "cwd": "[START_DIR]/src",
+    "env": {
+      "PATH": "RECIPE_PACKAGE_REPO[depot_tools]:<PATH>"
+    },
+    "infra_step": true,
+    "name": "git new-branch feature"
+  },
+  {
+    "cmd": [
+      "git",
+      "rebase",
+      "origin/master"
+    ],
+    "cwd": "[START_DIR]/src",
+    "infra_step": true,
+    "name": "my repo rebase"
+  },
+  {
+    "cmd": [
+      "git",
+      "bundle",
+      "create",
+      "[START_DIR]/all.bundle",
+      "--all"
+    ],
+    "cwd": "[START_DIR]/src",
+    "infra_step": true,
+    "name": "git bundle"
+  },
+  {
+    "name": "$result",
+    "recipe_result": null,
+    "status_code": 0
+  }
+]

recipes/recipe_modules/git/examples/full.py

@@ -3,13 +3,15 @@
 # found in the LICENSE file.
 
 DEPS = [
-  'git',
   'recipe_engine/context',
   'recipe_engine/path',
   'recipe_engine/platform',
   'recipe_engine/properties',
   'recipe_engine/raw_io',
+  'recipe_engine/runtime',
   'recipe_engine/step',
+
+  'git',
 ]
 
 
@@ -94,6 +96,10 @@ def RunSteps(api):
 
 def GenTests(api):
   yield api.test('basic')
+  yield (
+    api.test('basic_luci') +
+    api.runtime(is_luci=True, is_experimental=False)
+  )
   yield api.test('basic_tags') + api.properties(tags=True)
   yield api.test('basic_ref') + api.properties(revision='refs/foo/bar')
   yield api.test('basic_branch') + api.properties(revision='refs/heads/testing')