From a0382d39be0d7bf0f0766633185f20dcdd32a459 Mon Sep 17 00:00:00 2001 From: Josip Sokcevic Date: Fri, 8 Apr 2022 18:28:26 +0000 Subject: [PATCH] Revert "Set a default got_revision property in the bot_update json output." This reverts commit 053817260a00cac70d8e5a2105d94b93b3f81e6f. Reason for revert: we reverted got_revision change, and this change itself may be causing some other failues. Original change's description: > 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 Change-Id: Ie975dfe39e7b8997336761e12f53f3046359d825 Recipe-Nontrivial-Roll: build Recipe-Nontrivial-Roll: build_limited Recipe-Nontrivial-Roll: chromiumos Recipe-Nontrivial-Roll: infra No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3578820 Auto-Submit: Josip Sokcevic Commit-Queue: Rubber Stamper Bot-Commit: Rubber Stamper --- 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, 7 insertions(+), 12 deletions(-) diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index 04f293aa83..b47e16d247 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#523)(self, bot_update_step):** +— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#522)(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#500)(self, project_name, gclient_config=None):** +— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#499)(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#451)(self, bot_update_json, name):** +— **def [resolve\_fixed\_revision](/recipes/recipe_modules/bot_update/api.py#450)(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 503c15627e..4a3cfbd505 100644 --- a/recipes/recipe_modules/bot_update/api.py +++ b/recipes/recipe_modules/bot_update/api.py @@ -362,8 +362,7 @@ class BotUpdateApi(recipe_api.RecipeApi): # Set output commit of the build. if (set_output_commit and - 'got_revision' in self._last_returned_properties and - 'got_revision' in reverse_rev_map): + 'got_revision' in self._last_returned_properties): # 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 4bd1aa879a..b6346ca30a 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,8 +122,7 @@ "@@@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_revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@", + "@@@STEP_LOG_LINE@json.output@ \"got_cr_revision_cp\": \"refs/heads/main@{#170242}\"@@@", "@@@STEP_LOG_LINE@json.output@ }, @@@", "@@@STEP_LOG_LINE@json.output@ \"root\": \"src\", @@@", "@@@STEP_LOG_LINE@json.output@ \"source_manifest\": {@@@", @@ -141,8 +140,7 @@ "@@@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_revision@\"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@" + "@@@SET_BUILD_PROPERTY@got_cr_revision_cp@\"refs/heads/main@{#170242}\"@@@" ] }, { @@ -210,8 +208,7 @@ "@@@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_revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@", + "@@@STEP_LOG_LINE@json.output@ \"got_cr_revision_cp\": \"refs/heads/main@{#170242}\"@@@", "@@@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 edea176fd5..e849d3f6ea 100644 --- a/recipes/recipe_modules/bot_update/test_api.py +++ b/recipes/recipe_modules/bot_update/test_api.py @@ -34,7 +34,6 @@ 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):