From 56d4cf54ae6be8593e5827b0efee8dfdf2be898d Mon Sep 17 00:00:00 2001 From: Alex Kravchuk Date: Wed, 20 Nov 2024 09:59:57 +0000 Subject: [PATCH] Change BotUpdateApi.ensure_checkout to only have an option to skip commit position parsing for tags. Rename parse_commit_position parameter to parse_commit_position_for_tags. If input commit ref is not a tag, or parse_commit_position_for_tags is True, execute the old code path. This makes the changes backward compatible with the existing code. Bug: 366409421 Change-Id: I58405325e406a82c2e255fe5d3d4a2883c98a84a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6035030 Reviewed-by: Scott Lee Commit-Queue: Alex Kravchuk Reviewed-by: Garrett Beaty --- recipes/README.recipes.md | 14 ++++++------- recipes/recipe_modules/bot_update/api.py | 20 ++++++++++++------- .../tests/ensure_checkout_out_commit.py | 11 +++++----- 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index ed761fdba..8200747b7 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -65,12 +65,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#677)(self, bot_update_result):** +— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#683)(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=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, parse_commit_position_for_tags=True, \*\*kwargs):** Args: * gclient_config: The gclient configuration to use when running bot_update. @@ -106,10 +106,10 @@ 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: if True and got_revision_cp is set, parse output - commit ref and position from got_revision_cp. + * 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#654)(self, project_name, gclient_config=None):** +— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#660)(self, project_name, gclient_config=None):** Returns all property names used for storing the checked-out revision of a given project. @@ -125,12 +125,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#605)(self, bot_update_result, name):** +— **def [resolve\_fixed\_revision](/recipes/recipe_modules/bot_update/api.py#611)(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#694)(self, patch, suffix):** +— **def [step\_name](/recipes/recipe_modules/bot_update/api.py#700)(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] diff --git a/recipes/recipe_modules/bot_update/api.py b/recipes/recipe_modules/bot_update/api.py index b38c793f5..798f112c4 100644 --- a/recipes/recipe_modules/bot_update/api.py +++ b/recipes/recipe_modules/bot_update/api.py @@ -212,7 +212,7 @@ class BotUpdateApi(recipe_api.RecipeApi): download_topics=False, recipe_revision_overrides=None, step_tags=None, - parse_commit_position=True, + parse_commit_position_for_tags=True, **kwargs): """ Args: @@ -249,8 +249,8 @@ 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: if True and got_revision_cp is set, parse output - commit ref and position from got_revision_cp. + * 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: @@ -530,10 +530,16 @@ class BotUpdateApi(recipe_api.RecipeApi): in_rev = self.m.gclient.resolve_revision(revisions.get(out_solution)) if not in_rev: in_rev = 'HEAD' - if got_revision_cp and parse_commit_position: - # 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 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)) 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 index dc6a21a03..20168ed4b 100644 --- a/recipes/recipe_modules/bot_update/tests/ensure_checkout_out_commit.py +++ b/recipes/recipe_modules/bot_update/tests/ensure_checkout_out_commit.py @@ -22,9 +22,10 @@ def RunSteps(api): 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=api.properties.get( - 'parse_commit_position', True)) + 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): @@ -76,10 +77,10 @@ def GenTests(api): ) yield api.test( - 'got_revision_cp_do_not_parse_commit_position', + 'got_revision_cp_do_not_parse_commit_position_for_tags', api.properties( git_repo='https://chromium.googlesource.com/chromium/src.git', - parse_commit_position=False), + 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',