From efd15ecf30fa1ed2376e6a0ea3cf1b3659732f05 Mon Sep 17 00:00:00 2001 From: Garrett Beaty Date: Wed, 23 Oct 2019 20:13:21 +0000 Subject: [PATCH] Avoid accessing the None json output on gclient sync failure. Change-Id: Ib6bab87bf2b98b5122c56d37218c7f6db1bfdfe2 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1838116 Reviewed-by: Aaron Gable Commit-Queue: Garrett Beaty Auto-Submit: Garrett Beaty --- recipes/README.recipes.md | 20 +++++++++++------ recipes/recipe_modules/gclient/api.py | 15 +++++++------ .../gclient/tests/sync_failure.py | 22 +++++++++++++++++++ 3 files changed, 43 insertions(+), 14 deletions(-) create mode 100644 recipes/recipe_modules/gclient/tests/sync_failure.py diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index efb0b8373..a82071c8a 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -27,6 +27,7 @@ * [fetch_end_to_end_test](#recipes-fetch_end_to_end_test) * [gclient:examples/full](#recipes-gclient_examples_full) * [gclient:tests/patch_project](#recipes-gclient_tests_patch_project) + * [gclient:tests/sync_failure](#recipes-gclient_tests_sync_failure) * [gerrit:examples/full](#recipes-gerrit_examples_full) * [git:examples/full](#recipes-git_examples_full) * [git_cl:examples/full](#recipes-git_cl_examples_full) @@ -264,12 +265,12 @@ Returns (Path): The "depot_tools" root directory. Wrapper for easy calling of gclient steps. -— **def [break\_locks](/recipes/recipe_modules/gclient/api.py#270)(self):** +— **def [break\_locks](/recipes/recipe_modules/gclient/api.py#271)(self):** Remove all index.lock files. If a previous run of git crashed, bot was reset, etc... we might end up with leftover index.lock files. -— **def [checkout](/recipes/recipe_modules/gclient/api.py#230)(self, gclient_config=None, revert=RevertOnTryserver, inject_parent_got_revision=True, extra_sync_flags=None, \*\*kwargs):** +— **def [checkout](/recipes/recipe_modules/gclient/api.py#231)(self, gclient_config=None, revert=RevertOnTryserver, inject_parent_got_revision=True, extra_sync_flags=None, \*\*kwargs):** Return a step generator function for gclient checkouts. @@ -277,7 +278,7 @@ Return a step generator function for gclient checkouts. — **def [get\_config\_defaults](/recipes/recipe_modules/gclient/api.py#114)(self):** -— **def [get\_gerrit\_patch\_root](/recipes/recipe_modules/gclient/api.py#292)(self, gclient_config=None):** +— **def [get\_gerrit\_patch\_root](/recipes/recipe_modules/gclient/api.py#293)(self, gclient_config=None):** Returns local path to the repo where gerrit patch will be applied. @@ -290,7 +291,7 @@ Instead, properly map a repository to a local path using repo_path_map. TODO(nodir): remove this. Update all recipe tests to specify a git_repo matching the recipe. -— **def [get\_repo\_path](/recipes/recipe_modules/gclient/api.py#319)(self, repo_url, gclient_config=None):** +— **def [get\_repo\_path](/recipes/recipe_modules/gclient/api.py#320)(self, repo_url, gclient_config=None):** Returns local path to the repo checkout given its url. @@ -306,7 +307,7 @@ Returns (dict): A mapping from property name -> project name. It merges the values of the deprecated got_revision_mapping and the new got_revision_reverse_mapping. -— **def [inject\_parent\_got\_revision](/recipes/recipe_modules/gclient/api.py#205)(self, gclient_config=None, override=False):** +— **def [inject\_parent\_got\_revision](/recipes/recipe_modules/gclient/api.py#206)(self, gclient_config=None, override=False):** Match gclient config to build revisions obtained from build_properties. @@ -318,9 +319,9 @@ Args: — **def [resolve\_revision](/recipes/recipe_modules/gclient/api.py#142)(self, revision):** -— **def [runhooks](/recipes/recipe_modules/gclient/api.py#264)(self, args=None, name='runhooks', \*\*kwargs):** +— **def [runhooks](/recipes/recipe_modules/gclient/api.py#265)(self, args=None, name='runhooks', \*\*kwargs):** -— **def [set\_patch\_repo\_revision](/recipes/recipe_modules/gclient/api.py#349)(self, gclient_config=None):** +— **def [set\_patch\_repo\_revision](/recipes/recipe_modules/gclient/api.py#350)(self, gclient_config=None):** Updates config revision corresponding to patched project. @@ -987,6 +988,11 @@ Raises: [DEPS](/recipes/recipe_modules/gclient/tests/patch_project.py#9): [gclient](#recipe_modules-gclient), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/properties][recipe_engine/recipe_modules/properties] — **def [RunSteps](/recipes/recipe_modules/gclient/tests/patch_project.py#16)(api):** +### *recipes* / [gclient:tests/sync\_failure](/recipes/recipe_modules/gclient/tests/sync_failure.py) + +[DEPS](/recipes/recipe_modules/gclient/tests/sync_failure.py#7): [gclient](#recipe_modules-gclient) + +— **def [RunSteps](/recipes/recipe_modules/gclient/tests/sync_failure.py#9)(api):** ### *recipes* / [gerrit:examples/full](/recipes/recipe_modules/gerrit/examples/full.py) [DEPS](/recipes/recipe_modules/gerrit/examples/full.py#5): [gerrit](#recipe_modules-gerrit), [recipe\_engine/step][recipe_engine/recipe_modules/step] diff --git a/recipes/recipe_modules/gclient/api.py b/recipes/recipe_modules/gclient/api.py index 9fb0e1962..596c0b588 100644 --- a/recipes/recipe_modules/gclient/api.py +++ b/recipes/recipe_modules/gclient/api.py @@ -192,13 +192,14 @@ class GclientApi(recipe_api.RecipeApi): **kwargs) finally: result = self.m.step.active_result - solutions = result.json.output['solutions'] - for propname, path in sorted( - self.got_revision_reverse_mapping(cfg).items()): - # gclient json paths always end with a slash - info = solutions.get(path + '/') or solutions.get(path) - if info: - result.presentation.properties[propname] = info['revision'] + if result.json.output is not None: + solutions = result.json.output['solutions'] + for propname, path in sorted( + self.got_revision_reverse_mapping(cfg).items()): + # gclient json paths always end with a slash + info = solutions.get(path + '/') or solutions.get(path) + if info: + result.presentation.properties[propname] = info['revision'] return result diff --git a/recipes/recipe_modules/gclient/tests/sync_failure.py b/recipes/recipe_modules/gclient/tests/sync_failure.py new file mode 100644 index 000000000..5037aea1b --- /dev/null +++ b/recipes/recipe_modules/gclient/tests/sync_failure.py @@ -0,0 +1,22 @@ +# Copyright 2019 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +from recipe_engine import post_process + +DEPS = ['gclient'] + +def RunSteps(api): + src_cfg = api.gclient.make_config(CACHE_DIR='[ROOT]/git_cache') + api.gclient.sync(src_cfg) + +def GenTests(api): + yield api.test( + 'no-json', + api.override_step_data('gclient sync', retcode=1), + api.post_check( + lambda check, steps: + check(not steps['$result']['failure']['humanReason'] + .startswith('Uncaught Exception'))), + api.post_process(post_process.DropExpectation) + )