From fdd25811154d75cca803137ce54df96853548f1d Mon Sep 17 00:00:00 2001 From: Nodir Turakulov Date: Tue, 2 Apr 2019 20:45:13 +0000 Subject: [PATCH] [bot_update] Set output commit of the build Add set_output_commit param to bot_update.ensure_checkout, false by default. If true, call api.buildbucket.set_output_gitiles_commit to set the output commit of the build. This generally supersedes got_revision and got_revision_cp. Bug: 940214 Change-Id: I5ffc790a6b12c8bf4a56469b3ecc567141c0d735 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1548256 Reviewed-by: Andrii Shyshkalov Commit-Queue: Nodir Turakulov --- recipes/README.recipes.md | 25 ++++--- recipes/recipe_modules/bot_update/api.py | 69 ++++++++++++++---- .../full.expected/with_manifest_name.json | 71 ++++--------------- .../bot_update/examples/full.py | 20 +++++- 4 files changed, 102 insertions(+), 83 deletions(-) diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index 4b0fa9818e..15bc9c7e96 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -46,18 +46,18 @@ Recipe module to ensure a checkout is consistent on a bot. -#### **class [BotUpdateApi](/recipes/recipe_modules/bot_update/api.py#10)([RecipeApi][recipe_engine/wkt/RecipeApi]):** +#### **class [BotUpdateApi](/recipes/recipe_modules/bot_update/api.py#12)([RecipeApi][recipe_engine/wkt/RecipeApi]):** -— **def [\_\_call\_\_](/recipes/recipe_modules/bot_update/api.py#25)(self, name, cmd, \*\*kwargs):** +— **def [\_\_call\_\_](/recipes/recipe_modules/bot_update/api.py#27)(self, name, cmd, \*\*kwargs):** Wrapper for easy calling of bot_update. -— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#411)(self, bot_update_step):** +— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#452)(self, bot_update_step):** Deapplies a patch, taking care of DEPS and solution revisions properly. -— **def [ensure\_checkout](/recipes/recipe_modules/bot_update/api.py#66)(self, gclient_config=None, suffix=None, patch=True, update_presentation=True, patch_root=None, with_branch_heads=False, with_tags=False, refs=None, patch_oauth2=None, oauth2_json=None, use_site_config_creds=None, clobber=False, root_solution_revision=None, rietveld=None, issue=None, patchset=None, gerrit_no_reset=False, gerrit_no_rebase_patch_ref=False, disable_syntax_validation=False, manifest_name=None, patch_refs=None, ignore_input_commit=False, \*\*kwargs):** +— **def [ensure\_checkout](/recipes/recipe_modules/bot_update/api.py#68)(self, gclient_config=None, suffix=None, patch=True, update_presentation=True, patch_root=None, with_branch_heads=False, with_tags=False, refs=None, patch_oauth2=None, oauth2_json=None, use_site_config_creds=None, clobber=False, root_solution_revision=None, rietveld=None, issue=None, patchset=None, gerrit_no_reset=False, gerrit_no_rebase_patch_ref=False, disable_syntax_validation=False, manifest_name=None, patch_refs=None, ignore_input_commit=False, set_output_commit=False, \*\*kwargs):** Args: gclient_config: The gclient configuration to use when running bot_update. @@ -67,8 +67,17 @@ Args: such as bisect. manifest_name: The name of the manifest to upload to LogDog. This must be unique for the whole build. - -— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#388)(self, project_name, gclient_config=None):** + ignore_input_commit: if True, ignore api.buildbucket.gitiles_commit. + Exists for historical reasons. Please do not use. + set_output_commit: if True, mark the checked out commit as the + primary output commit of this build, i.e. call + api.buildbucket.set_output_gitiles_commit. + In case of multiple repos, the repo is the one specified in + api.buildbucket.gitiles_commit or the first configured solution. + When sorting builds by commit position, this commit will be used. + Requires falsy ignore_input_commit. + +— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#429)(self, project_name, gclient_config=None):** Returns all property names used for storing the checked-out revision of a given project. @@ -82,9 +91,9 @@ Args: Returns (list of str): All properties that'll hold the checked-out revision of the given project. An empty list if no such properties exist. -— **def [initialize](/recipes/recipe_modules/bot_update/api.py#20)(self):** +— **def [initialize](/recipes/recipe_modules/bot_update/api.py#22)(self):** -  **@property**
— **def [last\_returned\_properties](/recipes/recipe_modules/bot_update/api.py#34)(self):** +  **@property**
— **def [last\_returned\_properties](/recipes/recipe_modules/bot_update/api.py#36)(self):** ### *recipe_modules* / [cipd](/recipes/recipe_modules/cipd) [DEPS](/recipes/recipe_modules/cipd/__init__.py#1): [infra\_paths](#recipe_modules-infra_paths), [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/python][recipe_engine/recipe_modules/python], [recipe\_engine/raw\_io][recipe_engine/recipe_modules/raw_io], [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 2bf154902c..3b13fef22c 100644 --- a/recipes/recipe_modules/bot_update/api.py +++ b/recipes/recipe_modules/bot_update/api.py @@ -6,6 +6,8 @@ from recipe_engine import recipe_api +from PB.go.chromium.org.luci.buildbucket.proto import common as common_pb2 + class BotUpdateApi(recipe_api.RecipeApi): @@ -74,6 +76,7 @@ class BotUpdateApi(recipe_api.RecipeApi): gerrit_no_rebase_patch_ref=False, disable_syntax_validation=False, manifest_name=None, patch_refs=None, ignore_input_commit=False, + set_output_commit=False, **kwargs): """ Args: @@ -84,6 +87,15 @@ class BotUpdateApi(recipe_api.RecipeApi): such as bisect. manifest_name: The name of the manifest to upload to LogDog. This must be unique for the whole build. + ignore_input_commit: if True, ignore api.buildbucket.gitiles_commit. + Exists for historical reasons. Please do not use. + set_output_commit: if True, mark the checked out commit as the + primary output commit of this build, i.e. call + api.buildbucket.set_output_gitiles_commit. + In case of multiple repos, the repo is the one specified in + api.buildbucket.gitiles_commit or the first configured solution. + When sorting builds by commit position, this commit will be used. + Requires falsy ignore_input_commit. """ assert use_site_config_creds is None, "use_site_config_creds is deprecated" assert rietveld is None, "rietveld is deprecated" @@ -91,6 +103,7 @@ class BotUpdateApi(recipe_api.RecipeApi): assert patchset is None, "patchset is deprecated" assert patch_oauth2 is None, "patch_oauth2 is deprecated" assert oauth2_json is None, "oauth2_json is deprecated" + assert not (ignore_input_commit and set_output_commit) refs = refs or [] # We can re-use the gclient spec from the gclient module, since all the @@ -143,18 +156,19 @@ class BotUpdateApi(recipe_api.RecipeApi): # HACK: ensure_checkout API must be redesigned so that we don't pass such # parameters. Existing semantics is too opiniated. + main_repo_path = None + input_commit = self.m.buildbucket.gitiles_commit + input_commit_rev = input_commit.id or input_commit.ref if not ignore_input_commit: - # Apply input gitiles_commit, if any. - input_commit = self.m.buildbucket.build.input.gitiles_commit - if input_commit.id or input_commit.ref: - repo_path = self._get_commit_repo_path(input_commit, cfg) - # Note: this is not entirely correct. build.input.gitiles_commit - # definition says "The Gitiles commit to run against.". - # However, here we ignore it if the config specified a revision. - # This is necessary because existing builders rely on this behavior, - # e.g. they want to force refs/heads/master at the config level. - revisions[repo_path] = ( - revisions.get(repo_path) or input_commit.id or input_commit.ref) + main_repo_path = self._get_commit_repo_path(input_commit, cfg) + # Note: this is not entirely correct. build.input.gitiles_commit + # definition says "The Gitiles commit to run against.". + # However, here we ignore it if the config specified a revision. + # This is necessary because existing builders rely on this behavior, + # e.g. they want to force refs/heads/master at the config level. + if input_commit_rev: + revisions[main_repo_path] = ( + revisions.get(main_repo_path) or input_commit_rev) # Guarantee that first solution has a revision. # TODO(machenbach): We should explicitly pass HEAD for ALL solutions @@ -253,27 +267,54 @@ class BotUpdateApi(recipe_api.RecipeApi): finally: if step_result and step_result.json.output: result = step_result.json.output - self._last_returned_properties = step_result.json.output.get( - 'properties', {}) + self._last_returned_properties = result.get('properties', {}) if update_presentation: # Set properties such as got_revision. for prop_name, prop_value in ( self.last_returned_properties.iteritems()): step_result.presentation.properties[prop_name] = prop_value + # Add helpful step description in the step UI. if 'step_text' in result: step_text = result['step_text'] step_result.presentation.step_text = step_text # Export the step results as a Source Manifest to LogDog. + source_manifest = result.get('source_manifest', {}) if manifest_name: if not patch: # The param "patched" is purely cosmetic to mean "if false, this # bot_update run exists purely to unpatch an existing patch". manifest_name += '_unpatched' self.m.source_manifest.set_json_manifest( - manifest_name, result.get('source_manifest', {})) + manifest_name, source_manifest) + + # Set output commit of the build. + git_checkout = ( + source_manifest + .get('directories', {}) + .get(main_repo_path, {}) + .get('git_checkout', {})) + if set_output_commit and git_checkout: + output_commit = common_pb2.GitilesCommit( + ref=revisions.get(main_repo_path) or '', + id=git_checkout['revision'], + ) + if not output_commit.ref.startswith('refs/'): + # Require that what was checked out is what was requested on + # input commit. + # If this assertion fails, this setup is unusual/unsupported. + # The caller should not pass set_output_commit=True and should call + # api.buildbucket.set_output_gitiles_commit themselves. + assert ( + input_commit.ref and + # Revision was not overridden. + revisions[main_repo_path] == input_commit_rev) + output_commit.ref = input_commit.ref + output_commit.host, output_commit.project = ( + self.m.gitiles.parse_repo_url(git_checkout['repo_url'])) + self.m.buildbucket.set_output_gitiles_commit(output_commit) # Set the "checkout" path for the main solution. # This is used by the Chromium module to figure out where to look for diff --git a/recipes/recipe_modules/bot_update/examples/full.expected/with_manifest_name.json b/recipes/recipe_modules/bot_update/examples/full.expected/with_manifest_name.json index 99414d07be..dda438b585 100644 --- a/recipes/recipe_modules/bot_update/examples/full.expected/with_manifest_name.json +++ b/recipes/recipe_modules/bot_update/examples/full.expected/with_manifest_name.json @@ -26,73 +26,26 @@ "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@ \"fixed_revisions\": {@@@", - "@@@STEP_LOG_LINE@json.output@ \"src\": \"2d72510e447ab60a9728aeea2362d8be2cbd7789\"@@@", - "@@@STEP_LOG_LINE@json.output@ }, @@@", - "@@@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@ \"src/third_party/angle\": {@@@", - "@@@STEP_LOG_LINE@json.output@ \"repository\": \"https://fake.org/src/third_party/angle.git\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"revision\": \"fac9503c46405f77757b9a728eb85b8d7bc6080c\"@@@", - "@@@STEP_LOG_LINE@json.output@ }, @@@", - "@@@STEP_LOG_LINE@json.output@ \"src/v8\": {@@@", - "@@@STEP_LOG_LINE@json.output@ \"repository\": \"https://fake.org/src/v8.git\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"revision\": \"801ada225ddc271c132c3a35f03975671d43e399\"@@@", - "@@@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_angle_revision\": \"fac9503c46405f77757b9a728eb85b8d7bc6080c\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"got_angle_revision_cp\": \"refs/heads/master@{#297276}\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"got_cr_revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"got_cr_revision_cp\": \"refs/heads/master@{#170242}\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"got_revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"got_revision_cp\": \"refs/heads/master@{#170242}\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"got_v8_revision\": \"801ada225ddc271c132c3a35f03975671d43e399\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"got_v8_revision_cp\": \"refs/heads/master@{#43426}\"@@@", - "@@@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@ \"src/third_party/angle\": {@@@", - "@@@STEP_LOG_LINE@json.output@ \"git_checkout\": {@@@", - "@@@STEP_LOG_LINE@json.output@ \"repo_url\": \"https://fake.org/src/third_party/angle.git\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"revision\": \"fac9503c46405f77757b9a728eb85b8d7bc6080c\"@@@", - "@@@STEP_LOG_LINE@json.output@ }@@@", - "@@@STEP_LOG_LINE@json.output@ }, @@@", - "@@@STEP_LOG_LINE@json.output@ \"src/v8\": {@@@", - "@@@STEP_LOG_LINE@json.output@ \"git_checkout\": {@@@", - "@@@STEP_LOG_LINE@json.output@ \"repo_url\": \"https://fake.org/src/v8.git\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"revision\": \"801ada225ddc271c132c3a35f03975671d43e399\"@@@", + "@@@STEP_LOG_LINE@json.output@ \"repo_url\": \"https://chromium.googlesource.com/chromium/src.git\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"revision\": \"ea17a292ecfb3dcdaa8dd226e67d6504fc13c15a\"@@@", "@@@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_LINE@json.output@ }@@@", "@@@STEP_LOG_LINE@json.output@}@@@", - "@@@STEP_LOG_END@json.output@@@", - "@@@SET_BUILD_PROPERTY@got_angle_revision@\"fac9503c46405f77757b9a728eb85b8d7bc6080c\"@@@", - "@@@SET_BUILD_PROPERTY@got_angle_revision_cp@\"refs/heads/master@{#297276}\"@@@", - "@@@SET_BUILD_PROPERTY@got_cr_revision@\"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@", - "@@@SET_BUILD_PROPERTY@got_cr_revision_cp@\"refs/heads/master@{#170242}\"@@@", - "@@@SET_BUILD_PROPERTY@got_revision@\"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@", - "@@@SET_BUILD_PROPERTY@got_revision_cp@\"refs/heads/master@{#170242}\"@@@", - "@@@SET_BUILD_PROPERTY@got_v8_revision@\"801ada225ddc271c132c3a35f03975671d43e399\"@@@", - "@@@SET_BUILD_PROPERTY@got_v8_revision_cp@\"refs/heads/master@{#43426}\"@@@" + "@@@STEP_LOG_END@json.output@@@" + ] + }, + { + "cmd": [], + "name": "set_output_gitiles_commit", + "~followup_annotations": [ + "@@@SET_BUILD_PROPERTY@$recipe_engine/buildbucket/output_gitiles_commit@{\"host\": \"chromium.googlesource.com\", \"id\": \"ea17a292ecfb3dcdaa8dd226e67d6504fc13c15a\", \"project\": \"chromium/src\", \"ref\": \"refs/heads/master\"}@@@" ] }, { diff --git a/recipes/recipe_modules/bot_update/examples/full.py b/recipes/recipe_modules/bot_update/examples/full.py index d4f7692cc6..65c5818736 100644 --- a/recipes/recipe_modules/bot_update/examples/full.py +++ b/recipes/recipe_modules/bot_update/examples/full.py @@ -55,6 +55,7 @@ def RunSteps(api): api.properties.get('gerrit_no_rebase_patch_ref')) manifest_name = api.properties.get('manifest_name') patch_refs = api.properties.get('patch_refs') + set_output_commit = api.properties.get('set_output_commit', False) bot_update_step = api.bot_update.ensure_checkout( patch=patch, @@ -68,7 +69,9 @@ def RunSteps(api): gerrit_no_rebase_patch_ref=gerrit_no_rebase_patch_ref, disable_syntax_validation=True, manifest_name=manifest_name, - patch_refs=patch_refs) + patch_refs=patch_refs, + set_output_commit=set_output_commit, + ) if patch: api.bot_update.deapply_patch(bot_update_step) @@ -115,7 +118,20 @@ def GenTests(api): yield ( api.test('with_manifest_name') + ci_build() + - api.properties(manifest_name='checkout') + api.properties(manifest_name='checkout', set_output_commit=True) + + api.step_data('bot_update (without patch)', api.json.output({ + 'source_manifest': { + 'directories': { + 'src': { + 'git_checkout': { + 'repo_url': ( + 'https://chromium.googlesource.com/chromium/src.git'), + 'revision': 'ea17a292ecfb3dcdaa8dd226e67d6504fc13c15a' + }, + }, + }, + }, + })) ) yield ( api.test('basic_with_branch_heads') +