From 053817260a00cac70d8e5a2105d94b93b3f81e6f Mon Sep 17 00:00:00 2001 From: Garrett Beaty Date: Thu, 7 Apr 2022 20:01:05 +0000 Subject: [PATCH] Set a default got_revision property in the bot_update json output. The bot_update.py script sets a got_revision property by default even if it is not present in the reverse revision mapping. This results in an uncaught exception when set_output_commit is set to True if got_revision isn't present in the reverse revision mapping, but it isn't caught until production because the test API doesn't match that behavior. This change updates the test API method to match the behavior of the script. Recipe-Nontrivial-Roll: build Recipe-Nontrivial-Roll: build_limited Recipe-Nontrivial-Roll: chromiumos Recipe-Nontrivial-Roll: infra Change-Id: Ideefa9d77d2a816ae66a2bb52737264ed3f5bcee Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3575361 Reviewed-by: Robbie Iannucci Reviewed-by: Josip Sokcevic Commit-Queue: Garrett Beaty Auto-Submit: Garrett Beaty --- recipes/README.recipes.md | 6 +++--- recipes/recipe_modules/bot_update/api.py | 3 ++- .../full.expected/deprecated_got_revision_mapping.json | 9 ++++++--- recipes/recipe_modules/bot_update/test_api.py | 1 + 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index b47e16d24..04f293aa8 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -59,7 +59,7 @@ Recipe module to ensure a checkout is consistent on a bot. Wrapper for easy calling of bot_update. -— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#522)(self, bot_update_step):** +— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#523)(self, bot_update_step):** Deapplies a patch, taking care of DEPS and solution revisions properly. @@ -93,7 +93,7 @@ Args: bot_update module ONLY supports one change. Users may specify a change via tryserver.set_change() and explicitly set this flag False. -— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#499)(self, project_name, gclient_config=None):** +— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#500)(self, project_name, gclient_config=None):** Returns all property names used for storing the checked-out revision of a given project. @@ -109,7 +109,7 @@ Returns (list of str): All properties that'll hold the checked-out revision   **@property**
— **def [last\_returned\_properties](/recipes/recipe_modules/bot_update/api.py#49)(self):** -— **def [resolve\_fixed\_revision](/recipes/recipe_modules/bot_update/api.py#450)(self, bot_update_json, name):** +— **def [resolve\_fixed\_revision](/recipes/recipe_modules/bot_update/api.py#451)(self, bot_update_json, name):** Sets a fixed revision for a single dependency using project revision properties. diff --git a/recipes/recipe_modules/bot_update/api.py b/recipes/recipe_modules/bot_update/api.py index 4a3cfbd50..503c15627 100644 --- a/recipes/recipe_modules/bot_update/api.py +++ b/recipes/recipe_modules/bot_update/api.py @@ -362,7 +362,8 @@ class BotUpdateApi(recipe_api.RecipeApi): # Set output commit of the build. if (set_output_commit and - 'got_revision' in self._last_returned_properties): + 'got_revision' in self._last_returned_properties and + 'got_revision' in reverse_rev_map): # As of April 2019, got_revision describes the output commit, # the same commit that Build.output.gitiles_commit describes. # In particular, users tend to set got_revision to make Milo display diff --git a/recipes/recipe_modules/bot_update/examples/full.expected/deprecated_got_revision_mapping.json b/recipes/recipe_modules/bot_update/examples/full.expected/deprecated_got_revision_mapping.json index b6346ca30..4bd1aa879 100644 --- a/recipes/recipe_modules/bot_update/examples/full.expected/deprecated_got_revision_mapping.json +++ b/recipes/recipe_modules/bot_update/examples/full.expected/deprecated_got_revision_mapping.json @@ -122,7 +122,8 @@ "@@@STEP_LOG_LINE@json.output@ \"patch_root\": \"src\", @@@", "@@@STEP_LOG_LINE@json.output@ \"properties\": {@@@", "@@@STEP_LOG_LINE@json.output@ \"got_cr_revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"got_cr_revision_cp\": \"refs/heads/main@{#170242}\"@@@", + "@@@STEP_LOG_LINE@json.output@ \"got_cr_revision_cp\": \"refs/heads/main@{#170242}\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"got_revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@", "@@@STEP_LOG_LINE@json.output@ }, @@@", "@@@STEP_LOG_LINE@json.output@ \"root\": \"src\", @@@", "@@@STEP_LOG_LINE@json.output@ \"source_manifest\": {@@@", @@ -140,7 +141,8 @@ "@@@STEP_LOG_LINE@json.output@}@@@", "@@@STEP_LOG_END@json.output@@@", "@@@SET_BUILD_PROPERTY@got_cr_revision@\"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@", - "@@@SET_BUILD_PROPERTY@got_cr_revision_cp@\"refs/heads/main@{#170242}\"@@@" + "@@@SET_BUILD_PROPERTY@got_cr_revision_cp@\"refs/heads/main@{#170242}\"@@@", + "@@@SET_BUILD_PROPERTY@got_revision@\"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@" ] }, { @@ -208,7 +210,8 @@ "@@@STEP_LOG_LINE@json.output@ \"patch_root\": \"src\", @@@", "@@@STEP_LOG_LINE@json.output@ \"properties\": {@@@", "@@@STEP_LOG_LINE@json.output@ \"got_cr_revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"got_cr_revision_cp\": \"refs/heads/main@{#170242}\"@@@", + "@@@STEP_LOG_LINE@json.output@ \"got_cr_revision_cp\": \"refs/heads/main@{#170242}\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"got_revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@", "@@@STEP_LOG_LINE@json.output@ }, @@@", "@@@STEP_LOG_LINE@json.output@ \"root\": \"src\", @@@", "@@@STEP_LOG_LINE@json.output@ \"source_manifest\": {@@@", diff --git a/recipes/recipe_modules/bot_update/test_api.py b/recipes/recipe_modules/bot_update/test_api.py index e849d3f6e..edea176fd 100644 --- a/recipes/recipe_modules/bot_update/test_api.py +++ b/recipes/recipe_modules/bot_update/test_api.py @@ -34,6 +34,7 @@ class BotUpdateTestApi(recipe_test_api.RecipeTestApi): property_name: revisions[project_name] for property_name, project_name in revision_mapping.items() } + properties.setdefault('got_revision', self.gen_revision(first_sln)) if commit_positions: def get_ref(project_name):