From e856b6bba8266df0634a365b0a51b6097bb01d4b Mon Sep 17 00:00:00 2001 From: Garrett Beaty Date: Thu, 7 Jan 2021 17:57:24 +0000 Subject: [PATCH] Enable set_output_commit to be used with RevisionFallbackChain. When set_output_commit is True and the repo associated with the got_revision property does not use commit positions, buiders whose gclient config uses RevisionFallbackChain for that repo will raise an exception when attempting set the output commit because a string is expected for the revision value, not a RevisionFallbackChain object. This changes the set_output_commit code to resolve the revision so that the output commit can be set in those cases. Bug: 1159749, 1141886 Change-Id: Iddf1d49871028d9500eefb0eed07e36ce7e6b0d6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2612223 Auto-Submit: Garrett Beaty Reviewed-by: Josip Sokcevic Commit-Queue: Josip Sokcevic --- recipes/README.recipes.md | 8 +- recipes/recipe_modules/bot_update/api.py | 6 +- ...sion_fallback_chain_set_output_commit.json | 78 +++++++++++++++++++ .../bot_update/examples/full.py | 20 ++++- recipes/recipe_modules/bot_update/test_api.py | 13 ++-- 5 files changed, 112 insertions(+), 13 deletions(-) create mode 100644 recipes/recipe_modules/bot_update/examples/full.expected/revision_fallback_chain_set_output_commit.json diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index 07baa09ec..ac173b5a1 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -58,7 +58,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#528)(self, bot_update_step):** +— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#530)(self, bot_update_step):** Deapplies a patch, taking care of DEPS and solution revisions properly. @@ -95,7 +95,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#505)(self, project_name, gclient_config=None):** +— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#507)(self, project_name, gclient_config=None):** Returns all property names used for storing the checked-out revision of a given project. @@ -111,7 +111,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#39)(self):** -— **def [resolve\_fixed\_revision](/recipes/recipe_modules/bot_update/api.py#456)(self, bot_update_json, name):** +— **def [resolve\_fixed\_revision](/recipes/recipe_modules/bot_update/api.py#458)(self, bot_update_json, name):** Sets a fixed revision for a single dependency using project revision properties. @@ -955,7 +955,7 @@ Raises: [DEPS](/recipes/recipe_modules/bot_update/examples/full.py#5): [bot\_update](#recipe_modules-bot_update), [gclient](#recipe_modules-gclient), [gerrit](#recipe_modules-gerrit), [tryserver](#recipe_modules-tryserver), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/json][recipe_engine/recipe_modules/json], [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/runtime][recipe_engine/recipe_modules/runtime] -— **def [RunSteps](/recipes/recipe_modules/bot_update/examples/full.py#22)(api):** +— **def [RunSteps](/recipes/recipe_modules/bot_update/examples/full.py#23)(api):** ### *recipes* / [bot\_update:tests/do\_not\_retry\_patch\_failures\_in\_cq](/recipes/recipe_modules/bot_update/tests/do_not_retry_patch_failures_in_cq.py) [DEPS](/recipes/recipe_modules/bot_update/tests/do_not_retry_patch_failures_in_cq.py#5): [bot\_update](#recipe_modules-bot_update), [gclient](#recipe_modules-gclient), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/cq][recipe_engine/recipe_modules/cq], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/step][recipe_engine/recipe_modules/step] diff --git a/recipes/recipe_modules/bot_update/api.py b/recipes/recipe_modules/bot_update/api.py index e1a45ca90..bef7e1ea4 100644 --- a/recipes/recipe_modules/bot_update/api.py +++ b/recipes/recipe_modules/bot_update/api.py @@ -363,7 +363,9 @@ class BotUpdateApi(recipe_api.RecipeApi): # Determine the output ref. got_revision_cp = self._last_returned_properties.get('got_revision_cp') - in_rev = revisions.get(out_solution) + in_rev = self.m.gclient.resolve_revision(revisions.get(out_solution)) + if not in_rev: + in_rev = 'HEAD' if got_revision_cp: # If commit position string is available, read the ref from there. out_commit.ref, out_commit.position = ( @@ -380,7 +382,7 @@ class BotUpdateApi(recipe_api.RecipeApi): out_commit.ref = in_commit.ref else: # pragma: no cover assert False, ( - 'Unsupposed case. ' + 'Unsupported case. ' 'Call buildbucket.set_output_gitiles_commit directly.' ) self.m.buildbucket.set_output_gitiles_commit(out_commit) diff --git a/recipes/recipe_modules/bot_update/examples/full.expected/revision_fallback_chain_set_output_commit.json b/recipes/recipe_modules/bot_update/examples/full.expected/revision_fallback_chain_set_output_commit.json new file mode 100644 index 000000000..8d66cc5da --- /dev/null +++ b/recipes/recipe_modules/bot_update/examples/full.expected/revision_fallback_chain_set_output_commit.json @@ -0,0 +1,78 @@ +[ + { + "cmd": [ + "python", + "-u", + "RECIPE_MODULE[depot_tools::bot_update]/resources/bot_update.py", + "--spec-path", + "cache_dir = '[CACHE]/git'\nsolutions = [{'deps_file': '.DEPS.git', 'managed': True, 'name': 'src', 'url': 'https://chromium.googlesource.com/chromium/src.git'}]", + "--revision_mapping_file", + "{\"got_angle_revision\": \"src/third_party/angle\", \"got_cr_revision\": \"src\", \"got_revision\": \"src\", \"got_v8_revision\": \"src/v8\"}", + "--git-cache-dir", + "[CACHE]/git", + "--cleanup-dir", + "[CLEANUP]/bot_update", + "--output_json", + "/path/to/tmp/json", + "--refs", + "refs/heads/master", + "--disable-syntax-validation" + ], + "env": { + "GIT_HTTP_LOW_SPEED_LIMIT": "102400", + "GIT_HTTP_LOW_SPEED_TIME": "300" + }, + "env_suffixes": { + "DEPOT_TOOLS_UPDATE": [ + "0" + ], + "PATH": [ + "RECIPE_REPO[depot_tools]" + ] + }, + "infra_step": true, + "name": "bot_update (without patch)", + "~followup_annotations": [ + "@@@STEP_TEXT@Some step text@@@", + "@@@STEP_LOG_LINE@json.output@{@@@", + "@@@STEP_LOG_LINE@json.output@ \"did_run\": true, @@@", + "@@@STEP_LOG_LINE@json.output@ \"manifest\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"src\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"repository\": \"https://fake.org/src.git\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@", + "@@@STEP_LOG_LINE@json.output@ }@@@", + "@@@STEP_LOG_LINE@json.output@ }, @@@", + "@@@STEP_LOG_LINE@json.output@ \"patch_failure\": false, @@@", + "@@@STEP_LOG_LINE@json.output@ \"patch_root\": \"src\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"properties\": {@@@", + "@@@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\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"directories\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"src\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"git_checkout\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"repo_url\": \"https://fake.org/src.git\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@", + "@@@STEP_LOG_LINE@json.output@ }@@@", + "@@@STEP_LOG_LINE@json.output@ }@@@", + "@@@STEP_LOG_LINE@json.output@ }, @@@", + "@@@STEP_LOG_LINE@json.output@ \"version\": 0@@@", + "@@@STEP_LOG_LINE@json.output@ }, @@@", + "@@@STEP_LOG_LINE@json.output@ \"step_text\": \"Some step text\"@@@", + "@@@STEP_LOG_LINE@json.output@}@@@", + "@@@STEP_LOG_END@json.output@@@", + "@@@SET_BUILD_PROPERTY@got_revision@\"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@" + ] + }, + { + "cmd": [], + "name": "set_output_gitiles_commit", + "~followup_annotations": [ + "@@@SET_BUILD_PROPERTY@$recipe_engine/buildbucket/output_gitiles_commit@{\"host\": \"fake.org\", \"id\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\", \"project\": \"src\", \"ref\": \"refs/heads/master\"}@@@" + ] + }, + { + "name": "$result" + } +] \ No newline at end of file diff --git a/recipes/recipe_modules/bot_update/examples/full.py b/recipes/recipe_modules/bot_update/examples/full.py index 5503a42ba..f36388d09 100644 --- a/recipes/recipe_modules/bot_update/examples/full.py +++ b/recipes/recipe_modules/bot_update/examples/full.py @@ -17,6 +17,7 @@ DEPS = [ from recipe_engine import types +from RECIPE_MODULES.depot_tools import gclient from PB.go.chromium.org.luci.buildbucket.proto.build import Build def RunSteps(api): @@ -27,7 +28,10 @@ def RunSteps(api): soln = src_cfg.solutions.add() soln.name = 'src' soln.url = 'https://chromium.googlesource.com/chromium/src.git' - soln.revision = commit.id or commit.ref or None + if api.properties.get('revision_fallback_chain'): + soln.revision = gclient.api.RevisionFallbackChain() + else: + soln.revision = commit.id or commit.ref or None api.gclient.c = src_cfg api.gclient.c.revisions.update(api.properties.get('revisions', {})) if api.properties.get('deprecated_got_revision_mapping'): @@ -238,6 +242,20 @@ def GenTests(api): ci_build(revision='origin/master') ) + yield ( + api.test('revision_fallback_chain_set_output_commit') + + ci_build() + + api.properties( + set_output_commit=True, + revision_fallback_chain=True, + ) + + # Don't set commit position properties so that the set_output_commit code + # attempts to do comparisons on the revision value + api.step_data('bot_update (without patch)', api.bot_update.output_json( + root='src', first_sln='src', revision_mapping={'got_revision': 'src'}, + commit_positions=False)) + ) + yield ( api.test('add_blamelists') + ci_build() + diff --git a/recipes/recipe_modules/bot_update/test_api.py b/recipes/recipe_modules/bot_update/test_api.py index 7f44a0b68..e709fbece 100644 --- a/recipes/recipe_modules/bot_update/test_api.py +++ b/recipes/recipe_modules/bot_update/test_api.py @@ -9,7 +9,7 @@ from recipe_engine import recipe_test_api class BotUpdateTestApi(recipe_test_api.RecipeTestApi): def output_json(self, root, first_sln, revision_mapping, fail_patch=False, - fixed_revisions=None): + fixed_revisions=None, commit_positions=True): """Deterministically synthesize json.output test data for gclient's --output-json option. """ @@ -34,11 +34,12 @@ class BotUpdateTestApi(recipe_test_api.RecipeTestApi): property_name: revisions[project_name] for property_name, project_name in revision_mapping.items() } - properties.update({ - '%s_cp' % property_name: ('refs/heads/master@{#%s}' % - self.gen_commit_position(project_name)) - for property_name, project_name in revision_mapping.items() - }) + if commit_positions: + properties.update({ + '%s_cp' % property_name: ('refs/heads/master@{#%s}' % + self.gen_commit_position(project_name)) + for property_name, project_name in revision_mapping.items() + }) output.update({ 'patch_root': root or first_sln,