From 4d581d7b3f63cac3dc2f51ebdb8c598269412ce8 Mon Sep 17 00:00:00 2001 From: Alex Kravchuk Date: Wed, 27 Nov 2024 08:50:27 +0000 Subject: [PATCH] Revert "Add parse_commit_position parameter to BotUpdateApi.ensure_checkout." This reverts commit 66b3972fc51760c006ff1f7f05236f6a7acf345e. Reason for revert: This won't work because chromium_checkout relies on the position being set in the output commit in order to set information for RDB. We'll have to come up with a different solution. Original change's description: > Add parse_commit_position parameter to BotUpdateApi.ensure_checkout. > > When true and got_revision_cp is present, output commit ref and position are set from got_revision_cp. Default value is true. > > For Chrome builds that build from tags we will set parse_commit_position to false to show tags instead of branches on Buildbucket UI, e.g. refs/tags/132.0.6824.0 instead of refs/branch-heads/6824@{#1}. > > Bug: 366409421 > Change-Id: I77d01615edb6b791445a06469f80c673c97ad8d6 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6000539 > Reviewed-by: Garrett Beaty > Reviewed-by: Scott Lee > Commit-Queue: Alex Kravchuk Bug: 366409421 Change-Id: I21a5fc6e011a64fffd1d2ef0689d5a3f3843482c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6037603 Reviewed-by: Garrett Beaty Commit-Queue: Alex Kravchuk Reviewed-by: Scott Lee --- recipes/README.recipes.md | 19 +- recipes/recipe_modules/bot_update/api.py | 15 +- .../tests/ensure_checkout_out_commit.py | 178 ------------------ 3 files changed, 8 insertions(+), 204 deletions(-) delete mode 100644 recipes/recipe_modules/bot_update/tests/ensure_checkout_out_commit.py diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index 5826a1e88..5e774c9b9 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -21,7 +21,6 @@ * [bot_update:tests/do_not_retry_patch_failures_in_cq](#recipes-bot_update_tests_do_not_retry_patch_failures_in_cq) * [bot_update:tests/download_topics](#recipes-bot_update_tests_download_topics) * [bot_update:tests/ensure_checkout](#recipes-bot_update_tests_ensure_checkout) - * [bot_update:tests/ensure_checkout_out_commit](#recipes-bot_update_tests_ensure_checkout_out_commit) * [bot_update:tests/ensure_checkout_return_custom_result](#recipes-bot_update_tests_ensure_checkout_return_custom_result) * [depot_tools:examples/full](#recipes-depot_tools_examples_full) * [gclient:examples/full](#recipes-gclient_examples_full) @@ -65,12 +64,12 @@ 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#683)(self, bot_update_result):** +— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#674)(self, bot_update_result):** Deapplies a patch, taking care of DEPS and solution revisions properly. -— **def [ensure\_checkout](/recipes/recipe_modules/bot_update/api.py#191)(self, gclient_config=None, suffix=None, patch=True, update_presentation=True, patch_root=None, with_branch_heads=False, with_tags=False, no_fetch_tags=False, refs=None, clobber=False, root_solution_revision=None, gerrit_no_reset=False, gerrit_no_rebase_patch_ref=False, assert_one_gerrit_change=True, patch_refs=None, ignore_input_commit=False, add_blamelists=False, set_output_commit=False, step_test_data=None, enforce_fetch=False, download_topics=False, recipe_revision_overrides=None, step_tags=None, parse_commit_position_for_tags=True, \*\*kwargs):** +— **def [ensure\_checkout](/recipes/recipe_modules/bot_update/api.py#191)(self, gclient_config=None, suffix=None, patch=True, update_presentation=True, patch_root=None, with_branch_heads=False, with_tags=False, no_fetch_tags=False, refs=None, clobber=False, root_solution_revision=None, gerrit_no_reset=False, gerrit_no_rebase_patch_ref=False, assert_one_gerrit_change=True, patch_refs=None, ignore_input_commit=False, add_blamelists=False, set_output_commit=False, step_test_data=None, enforce_fetch=False, download_topics=False, recipe_revision_overrides=None, step_tags=None, \*\*kwargs):** Args: * gclient_config: The gclient configuration to use when running bot_update. @@ -106,10 +105,8 @@ Args: change's commit message to get this revision override requested by the author. * step_tags: a dict {tag name: tag value} of tags to add to the step - * parse_commit_position_for_tags: if True and got_revision_cp is set, parse output - commit ref and position from got_revision_cp when input ref is a tag. -— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#660)(self, project_name, gclient_config=None):** +— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#651)(self, project_name, gclient_config=None):** Returns all property names used for storing the checked-out revision of a given project. @@ -125,12 +122,12 @@ 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#99)(self):** -— **def [resolve\_fixed\_revision](/recipes/recipe_modules/bot_update/api.py#611)(self, bot_update_result, name):** +— **def [resolve\_fixed\_revision](/recipes/recipe_modules/bot_update/api.py#602)(self, bot_update_result, name):** Sets a fixed revision for a single dependency using project revision properties. -— **def [step\_name](/recipes/recipe_modules/bot_update/api.py#700)(self, patch, suffix):** +— **def [step\_name](/recipes/recipe_modules/bot_update/api.py#691)(self, patch, suffix):** ### *recipe_modules* / [depot\_tools](/recipes/recipe_modules/depot_tools) [DEPS](/recipes/recipe_modules/depot_tools/__init__.py#7): [recipe\_engine/cipd][recipe_engine/recipe_modules/cipd], [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/platform][recipe_engine/recipe_modules/platform], [recipe\_engine/runtime][recipe_engine/recipe_modules/runtime] @@ -1094,12 +1091,6 @@ Raises: — **def [RunSteps](/recipes/recipe_modules/bot_update/tests/ensure_checkout.py#16)(api):** -### *recipes* / [bot\_update:tests/ensure\_checkout\_out\_commit](/recipes/recipe_modules/bot_update/tests/ensure_checkout_out_commit.py) - -[DEPS](/recipes/recipe_modules/bot_update/tests/ensure_checkout_out_commit.py#9): [bot\_update](#recipe_modules-bot_update), [gclient](#recipe_modules-gclient), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/json][recipe_engine/recipe_modules/json], [recipe\_engine/properties][recipe_engine/recipe_modules/properties] - - -— **def [RunSteps](/recipes/recipe_modules/bot_update/tests/ensure_checkout_out_commit.py#18)(api):** ### *recipes* / [bot\_update:tests/ensure\_checkout\_return\_custom\_result](/recipes/recipe_modules/bot_update/tests/ensure_checkout_return_custom_result.py) [DEPS](/recipes/recipe_modules/bot_update/tests/ensure_checkout_return_custom_result.py#10): [bot\_update](#recipe_modules-bot_update), [gclient](#recipe_modules-gclient), [recipe\_engine/assertions][recipe_engine/recipe_modules/assertions], [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/properties][recipe_engine/recipe_modules/properties] diff --git a/recipes/recipe_modules/bot_update/api.py b/recipes/recipe_modules/bot_update/api.py index 798f112c4..8e768cef4 100644 --- a/recipes/recipe_modules/bot_update/api.py +++ b/recipes/recipe_modules/bot_update/api.py @@ -212,7 +212,6 @@ class BotUpdateApi(recipe_api.RecipeApi): download_topics=False, recipe_revision_overrides=None, step_tags=None, - parse_commit_position_for_tags=True, **kwargs): """ Args: @@ -249,8 +248,6 @@ class BotUpdateApi(recipe_api.RecipeApi): change's commit message to get this revision override requested by the author. * step_tags: a dict {tag name: tag value} of tags to add to the step - * parse_commit_position_for_tags: if True and got_revision_cp is set, parse output - commit ref and position from got_revision_cp when input ref is a tag. """ assert not (ignore_input_commit and set_output_commit) if assert_one_gerrit_change: @@ -531,15 +528,9 @@ class BotUpdateApi(recipe_api.RecipeApi): if not in_rev: in_rev = 'HEAD' if got_revision_cp: - if in_commit and in_commit.ref.startswith( - 'refs/tags/') and not parse_commit_position_for_tags: - # If we don't want to parse commit position for tags, use input - # ref as output. - out_commit.ref = in_commit.ref - else: - # If commit position string is available, read the ref from there. - out_commit.ref, out_commit.position = ( - self.m.commit_position.parse(got_revision_cp)) + # If commit position string is available, read the ref from there. + out_commit.ref, out_commit.position = ( + self.m.commit_position.parse(got_revision_cp)) elif in_rev.startswith('refs/'): # If we were asked to check out a specific ref, use it as output # ref. diff --git a/recipes/recipe_modules/bot_update/tests/ensure_checkout_out_commit.py b/recipes/recipe_modules/bot_update/tests/ensure_checkout_out_commit.py deleted file mode 100644 index 20168ed4b..000000000 --- a/recipes/recipe_modules/bot_update/tests/ensure_checkout_out_commit.py +++ /dev/null @@ -1,178 +0,0 @@ -# Copyright 2024 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 - -PYTHON_VERSION_COMPATIBILITY = 'PY3' - -DEPS = [ - 'bot_update', - 'gclient', - 'recipe_engine/buildbucket', - 'recipe_engine/json', - 'recipe_engine/properties', -] - - -def RunSteps(api): - config = api.gclient.make_config() - config.got_revision_reverse_mapping['got_revision'] = 'src' - sln = config.solutions.add() - sln.name = 'src' - sln.url = api.properties.get('git_repo') - api.gclient.c = config - api.bot_update.ensure_checkout( - set_output_commit=True, - parse_commit_position_for_tags=api.properties.get( - 'parse_commit_position_for_tags', True)) - - -def GenTests(api): - - def json_output(repository, revision, got_revision_cp=None): - output = { - 'did_run': True, - "manifest": { - "src": { - "repository": repository, - "revision": revision - } - }, - "patch_root": None, - "properties": { - "got_revision": revision, - }, - "root": "src", - "step_text": "text" - } - if got_revision_cp: - output['properties']['got_revision_cp'] = got_revision_cp - return output - - yield api.test( - 'got_revision_cp', - api.properties(git_repo='https://fake.org/repo.git'), - api.buildbucket.ci_build(git_repo='https://fake.org/repo.git', - git_ref='refs/tags/100.0.0000.0', - revision='1234567890'), - api.step_data( - 'bot_update', - api.json.output( - json_output(repository='https://fake.org/repo.git', - revision='1234567890', - got_revision_cp='refs/branch-heads/1000@{#1}'))), - api.post_process( - post_process.PropertyEquals, - '$recipe_engine/buildbucket/output_gitiles_commit', - { - 'host': 'fake.org', - 'id': '1234567890', - 'position': 1, - 'project': 'repo', - 'ref': 'refs/branch-heads/1000' - }, - ), - api.post_process(post_process.DropExpectation), - ) - - yield api.test( - 'got_revision_cp_do_not_parse_commit_position_for_tags', - api.properties( - git_repo='https://chromium.googlesource.com/chromium/src.git', - parse_commit_position_for_tags=False), - api.buildbucket.ci_build( - git_repo='https://chromium.googlesource.com/chromium/src.git', - git_ref='refs/tags/100.0.0000.0', - revision='1234567890'), - api.step_data( - 'bot_update', - api.json.output( - json_output(repository= - "https://chromium.googlesource.com/chromium/src.git", - revision='1234567890', - got_revision_cp='got_revision_cp'))), - api.post_process( - post_process.PropertyEquals, - '$recipe_engine/buildbucket/output_gitiles_commit', - { - 'host': 'chromium.googlesource.com', - 'id': '1234567890', - 'project': 'chromium/src', - 'ref': 'refs/tags/100.0.0000.0' - }, - ), - api.post_process(post_process.DropExpectation), - ) - - yield api.test( - 'no_got_revision_cp_ref_revision', - api.properties(git_repo='https://fake.org/repo.git'), - api.buildbucket.ci_build(git_repo='https://fake.org/repo.git', - git_ref='refs/branch-heads/1000', - revision='refs/refname'), - api.step_data( - 'bot_update', - api.json.output( - json_output(repository='https://fake.org/repo.git', - revision='1234567890'))), - api.post_process( - post_process.PropertyEquals, - '$recipe_engine/buildbucket/output_gitiles_commit', - { - 'host': 'fake.org', - 'id': '1234567890', - 'project': 'repo', - 'ref': 'refs/refname' - }, - ), - api.post_process(post_process.DropExpectation), - ) - - yield api.test( - 'no_got_revision_cp_head_revision', - api.properties(git_repo='https://fake.org/repo.git'), - api.buildbucket.ci_build(git_repo='https://fake.org/repo.git', - git_ref='refs/branch-heads/1000', - revision='HEAD'), - api.step_data( - 'bot_update', - api.json.output( - json_output(repository='https://fake.org/repo.git', - revision='1234567890'))), - api.post_process( - post_process.PropertyEquals, - '$recipe_engine/buildbucket/output_gitiles_commit', - { - 'host': 'fake.org', - 'id': '1234567890', - 'project': 'repo', - 'ref': 'refs/heads/main' - }, - ), - api.post_process(post_process.DropExpectation), - ) - - yield api.test( - 'out_commit_id_equals_in_commit_id', - api.properties(git_repo='https://fake.org/repo.git'), - api.buildbucket.ci_build(git_repo='https://fake.org/repo.git', - git_ref='refs/branch-heads/1000', - revision='1234567890'), - api.step_data( - 'bot_update', - api.json.output( - json_output(repository='https://fake.org/repo.git', - revision='1234567890'))), - api.post_process( - post_process.PropertyEquals, - '$recipe_engine/buildbucket/output_gitiles_commit', - { - 'host': 'fake.org', - 'id': '1234567890', - 'project': 'repo', - 'ref': 'refs/branch-heads/1000' - }, - ), - api.post_process(post_process.DropExpectation), - )