Replace can_fail_build with raise_on_failure.

The step API now supports the same functionality that can_fail_build
provides, so standardize on that argument rather than maintaining a
duplicate mechanism.

Bug: 1274644
Change-Id: Idcc3c0dacbab8d2d786b27ec277713063da6d334
Recipe-Manual-Change: build_limited
Recipe-Manual-Change: infra
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3301439Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Garrett Beaty <gbeaty@google.com>

recipes/README.recipes.md

@@ -384,7 +384,7 @@ PYTHON_VERSION_COMPATIBILITY: PY2+3
 
 Returns a git command step.
 
-— **def [bundle\_create](/recipes/recipe_modules/git/api.py#390)(self, bundle_path, rev_list_args=None, \*\*kwargs):**
+— **def [bundle\_create](/recipes/recipe_modules/git/api.py#383)(self, bundle_path, rev_list_args=None, \*\*kwargs):**
 
 Runs 'git bundle create' on a Git repository.
 
@@ -394,11 +394,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#49)(self, file_path, commit_hash, remote_name=None, \*\*kwargs):**
+— **def [cat\_file\_at\_commit](/recipes/recipe_modules/git/api.py#42)(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#123)(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#116)(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, raise_on_failure=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.
 
@@ -415,7 +415,7 @@ Args:
   * curl_trace_file (Path): if not None, dump GIT_CURL_VERBOSE=1 trace to that
       file. Useful for debugging git issue reproducible only on bots. It has
       a side effect of all stderr output of 'git fetch' going to that file.
-  * can_fail_build (bool): if False, ignore errors during fetch or checkout.
+  * raise_on_failure (bool): if False, ignore errors during fetch or checkout.
   * set_got_revision (bool): if True, resolves HEAD and sets got_revision
       property.
   * remote_name (str): name of the git remote to use
@@ -437,7 +437,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#359)(self, prop_name, \*\*kwargs):**
+— **def [config\_get](/recipes/recipe_modules/git/api.py#352)(self, prop_name, \*\*kwargs):**
 
 Returns git config output.
 
@@ -447,24 +447,24 @@ Args:
 
 Returns: (str) The Git config output, or None if no output was generated.
 
-— **def [count\_objects](/recipes/recipe_modules/git/api.py#57)(self, previous_result=None, can_fail_build=False, \*\*kwargs):**
+— **def [count\_objects](/recipes/recipe_modules/git/api.py#50)(self, previous_result=None, raise_on_failure=False, \*\*kwargs):**
 
 Returns `git count-objects` result as a dict.
 
 Args:
   * previous_result (dict): the result of previous count_objects call.
     If passed, delta is reported in the log and step text.
-  * can_fail_build (bool): if True, may fail the build and/or raise an
-    exception. Defaults to False.
+  * raise_on_failure (bool): if True, an exception will be raised if the
+    operation fails. Defaults to False.
 
 Returns:
   A dict of count-object values, or None if count-object run failed.
 
-— **def [fetch\_tags](/recipes/recipe_modules/git/api.py#43)(self, remote_name=None, \*\*kwargs):**
+— **def [fetch\_tags](/recipes/recipe_modules/git/api.py#36)(self, remote_name=None, \*\*kwargs):**
 
 Fetches all tags from the remote.
 
-— **def [get\_remote\_url](/recipes/recipe_modules/git/api.py#378)(self, remote_name=None, \*\*kwargs):**
+— **def [get\_remote\_url](/recipes/recipe_modules/git/api.py#371)(self, remote_name=None, \*\*kwargs):**
 
 Returns the remote Git repository URL, or None.
 
@@ -474,11 +474,11 @@ Args:
 
 Returns: (str) The URL of the remote Git repository, or None.
 
-— **def [get\_timestamp](/recipes/recipe_modules/git/api.py#330)(self, commit='HEAD', test_data=None, \*\*kwargs):**
+— **def [get\_timestamp](/recipes/recipe_modules/git/api.py#323)(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#403)(self, branch, name=None, upstream=None, upstream_current=False, \*\*kwargs):**
+— **def [new\_branch](/recipes/recipe_modules/git/api.py#396)(self, branch, name=None, upstream=None, upstream_current=False, \*\*kwargs):**
 
 Runs git new-branch on a Git repository, to be used before git cl
 upload.
@@ -490,7 +490,7 @@ Args:
   * upstream_current (bool): whether to use '--upstream_current'.
   * kwargs: Forwarded to '__call__'.
 
-— **def [rebase](/recipes/recipe_modules/git/api.py#339)(self, name_prefix, branch, dir_path, remote_name=None, \*\*kwargs):**
+— **def [rebase](/recipes/recipe_modules/git/api.py#332)(self, name_prefix, branch, dir_path, remote_name=None, \*\*kwargs):**
 
 Runs rebase HEAD onto branch
 

recipes/recipe_modules/git/api.py

@@ -29,16 +29,9 @@ class GitApi(recipe_api.RecipeApi):
     options = kwargs.pop('git_config_options', {})
     for k, v in sorted(options.items()):
       git_cmd.extend(['-c', '%s=%s' % (k, v)])
-    can_fail_build = kwargs.pop('can_fail_build', True)
-    try:
-      with self.m.context(cwd=(self.m.context.cwd or self.m.path['checkout'])):
-        return self.m.step(name, git_cmd + list(args), infra_step=infra_step,
-                           **kwargs)
-    except self.m.step.StepFailure as f:
-      if can_fail_build:
-        raise
-      else:
-        return f.result
+    with self.m.context(cwd=(self.m.context.cwd or self.m.path['checkout'])):
+      return self.m.step(name, git_cmd + list(args), infra_step=infra_step,
+                          **kwargs)
 
   def fetch_tags(self, remote_name=None, **kwargs):
     """Fetches all tags from the remote."""
@@ -54,14 +47,14 @@ class GitApi(recipe_api.RecipeApi):
     return self('cat-file', 'blob', '%s:%s' % (commit_hash, file_path),
                 **kwargs)
 
-  def count_objects(self, previous_result=None, can_fail_build=False, **kwargs):
+  def count_objects(self, previous_result=None, raise_on_failure=False, **kwargs):
     """Returns `git count-objects` result as a dict.
 
     Args:
       * previous_result (dict): the result of previous count_objects call.
         If passed, delta is reported in the log and step text.
-      * can_fail_build (bool): if True, may fail the build and/or raise an
-        exception. Defaults to False.
+      * raise_on_failure (bool): if True, an exception will be raised if the
+        operation fails. Defaults to False.
 
     Returns:
       A dict of count-object values, or None if count-object run failed.
@@ -77,7 +70,7 @@ class GitApi(recipe_api.RecipeApi):
     try:
       step_result = self(
           'count-objects', '-v', stdout=self.m.raw_io.output(),
-          can_fail_build=can_fail_build, **kwargs)
+          raise_on_failure=raise_on_failure, **kwargs)
 
       if not step_result.stdout:
         return None
@@ -116,14 +109,14 @@ class GitApi(recipe_api.RecipeApi):
       if step_result:
         step_result.presentation.logs['exception'] = recipe_util.format_ex(ex)
         step_result.presentation.status = self.m.step.WARNING
-      if can_fail_build:
+      if raise_on_failure:
         raise recipe_api.InfraFailure('count-objects failed: %s' % ex)
       return None
 
   def checkout(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,
+               curl_trace_file=None, raise_on_failure=True,
                set_got_revision=False, remote_name=None,
                display_fetch_size=None, file_name=None,
                submodule_update_recursive=True,
@@ -143,7 +136,7 @@ class GitApi(recipe_api.RecipeApi):
       * curl_trace_file (Path): if not None, dump GIT_CURL_VERBOSE=1 trace to that
           file. Useful for debugging git issue reproducible only on bots. It has
           a side effect of all stderr output of 'git fetch' going to that file.
-      * can_fail_build (bool): if False, ignore errors during fetch or checkout.
+      * raise_on_failure (bool): if False, ignore errors during fetch or checkout.
       * set_got_revision (bool): if True, resolves HEAD and sets got_revision
           property.
       * remote_name (str): name of the git remote to use
@@ -208,17 +201,17 @@ class GitApi(recipe_api.RecipeApi):
           self('cache', 'populate', '-c',
                self.m.path['cache'].join('git'), url,
                name='populate cache',
-               can_fail_build=can_fail_build)
+               raise_on_failure=raise_on_failure)
           dir_cmd = self(
               'cache', 'exists', '--quiet',
               '--cache-dir', self.m.path['cache'].join('git'), url,
-              can_fail_build=can_fail_build,
+              raise_on_failure=raise_on_failure,
               stdout=self.m.raw_io.output(),
               step_test_data=lambda:
                   self.m.raw_io.test_api.stream_output('mirror_dir'))
           mirror_dir = dir_cmd.stdout.strip().decode('utf-8')
           self('remote', 'set-url', 'origin', mirror_dir,
-               can_fail_build=can_fail_build)
+               raise_on_failure=raise_on_failure)
 
       # There are five kinds of refs we can be handed:
       # 0) None. In this case, we default to api.buildbucket.gitiles_commit.ref.
@@ -273,7 +266,7 @@ class GitApi(recipe_api.RecipeApi):
         self('fetch', *fetch_args,
           name=fetch_step_name,
           stderr=fetch_stderr,
-          can_fail_build=can_fail_build)
+          raise_on_failure=raise_on_failure)
       if display_fetch_size:
         self.count_objects(
             name='count-objects after %s' % fetch_step_name,
@@ -284,17 +277,17 @@ class GitApi(recipe_api.RecipeApi):
       if file_name:
         self('checkout', '-f', checkout_ref, '--', file_name,
           name='git checkout%s' % step_suffix,
-          can_fail_build=can_fail_build)
+          raise_on_failure=raise_on_failure)
 
       else:
         self('checkout', '-f', checkout_ref,
           name='git checkout%s' % step_suffix,
-          can_fail_build=can_fail_build)
+          raise_on_failure=raise_on_failure)
 
       rev_parse_step = self('rev-parse', 'HEAD',
                            name='read revision',
                            stdout=self.m.raw_io.output_text(),
-                           can_fail_build=False,
+                           raise_on_failure=False,
                            step_test_data=lambda:
                               self.m.raw_io.test_api.stream_output_text('deadbeef'))
 
@@ -310,12 +303,12 @@ class GitApi(recipe_api.RecipeApi):
 
       self('clean', '-f', '-d', '-x', *clean_args,
         name='git clean%s' % step_suffix,
-        can_fail_build=can_fail_build)
+        raise_on_failure=raise_on_failure)
 
       if submodules:
         self('submodule', 'sync',
           name='submodule sync%s' % step_suffix,
-          can_fail_build=can_fail_build)
+          raise_on_failure=raise_on_failure)
         submodule_update = ['submodule', 'update', '--init']
         if submodule_update_recursive:
           submodule_update.append('--recursive')
@@ -323,7 +316,7 @@ class GitApi(recipe_api.RecipeApi):
           submodule_update.append('--force')
         self(*submodule_update,
           name='submodule update%s' % step_suffix,
-          can_fail_build=can_fail_build)
+          raise_on_failure=raise_on_failure)
 
     return retVal
 

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

@@ -182,9 +182,9 @@
       "The recipe has crashed at point 'Uncaught exception'!",
       "",
       "Traceback (most recent call last):",
-      "  File \"RECIPE_REPO[depot_tools]/recipes/recipe_modules/git/examples/full.py\", line 83, in RunSteps",
+      "  File \"RECIPE_REPO[depot_tools]/recipes/recipe_modules/git/examples/full.py\", line 81, in RunSteps",
       "    api.git.new_branch('failed_new_branch', upstream='will_fail', upstream_current=True) #pylint: disable = line-too-long",
-      "  File \"RECIPE_REPO[depot_tools]/recipes/recipe_modules/git/api.py\", line 420, in new_branch",
+      "  File \"RECIPE_REPO[depot_tools]/recipes/recipe_modules/git/api.py\", line 413, in new_branch",
       "    raise ValueError('Can not define both upstream and upstream_current')",
       "ValueError('Can not define both upstream and upstream_current')"
     ]

recipes/recipe_modules/git/examples/full.py

@@ -53,7 +53,7 @@ def RunSteps(api):
   # count_objects shows number and size of objects in .git dir.
   api.git.count_objects(
       name='count-objects',
-      can_fail_build=api.properties.get('count_objects_can_fail_build'),
+      raise_on_failure=api.properties.get('count_objects_can_fail_build', False),
       git_config_options={'foo': 'bar'})
 
   # Get the remote URL.
@@ -70,11 +70,9 @@ def RunSteps(api):
   with api.context(cwd=api.path['checkout']):
     api.git('status')
 
-  api.git('status', name='git status can_fail_build',
-          can_fail_build=True)
+  api.git('status', name='git status can_fail_build', raise_on_failure=True)
 
-  api.git('status', name='git status cannot_fail_build',
-          can_fail_build=False)
+  api.git('status', name='git status cannot_fail_build', raise_on_failure=False)
 
   # You should run git new-branch before you upload something with git cl.
   api.git.new_branch('refactor')  # Upstream is origin/main by default.