From 98f1e59b41c6c580cd168ac4456bf27d78c12a95 Mon Sep 17 00:00:00 2001 From: Robert Iannucci Date: Fri, 19 Oct 2018 22:39:57 +0000 Subject: [PATCH] [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 Reviewed-by: Ryan Tseng --- recipes/README.recipes.md | 26 +- recipes/recipe_modules/git/__init__.py | 4 +- recipes/recipe_modules/git/api.py | 17 +- .../examples/full.expected/basic_luci.json | 223 ++++++++++++++++++ recipes/recipe_modules/git/examples/full.py | 8 +- 5 files changed, 259 insertions(+), 19 deletions(-) create mode 100644 recipes/recipe_modules/git/examples/full.expected/basic_luci.json diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index 1b119765b5..ec295460be 100644 --- a/recipes/README.recipes.md +++ b/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] diff --git a/recipes/recipe_modules/git/__init__.py b/recipes/recipe_modules/git/__init__.py index 0d7a27f53b..f8faf36727 100644 --- a/recipes/recipe_modules/git/__init__.py +++ b/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', ] diff --git a/recipes/recipe_modules/git/api.py b/recipes/recipe_modules/git/api.py index adc250753c..6b86546ccf 100644 --- a/recipes/recipe_modules/git/api.py +++ b/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, diff --git a/recipes/recipe_modules/git/examples/full.expected/basic_luci.json b/recipes/recipe_modules/git/examples/full.expected/basic_luci.json new file mode 100644 index 0000000000..df3d041873 --- /dev/null +++ b/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]:" + }, + "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@
checked out 'deadbeef'
@@@" + ] + }, + { + "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]:" + }, + "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]:" + }, + "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 + } +] \ No newline at end of file diff --git a/recipes/recipe_modules/git/examples/full.py b/recipes/recipe_modules/git/examples/full.py index c01712751b..6461ef5bb8 100644 --- a/recipes/recipe_modules/git/examples/full.py +++ b/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')