From 069dd4fa6b0571029d7889b3ce63a3b41b504f5e Mon Sep 17 00:00:00 2001 From: Robert Iannucci Date: Thu, 14 Dec 2017 17:44:20 -0800 Subject: [PATCH] [bot_update] Put depot_tools at the end of $PATH on LUCI. This should prevent bot_update from overriding vpython by accident. R=tandrii@chromium.org Recipe-Nontrivial-Roll: build Bug: 789808 Change-Id: Ia574204439588862f29f94a80e978b2ff4b57586 Reviewed-on: https://chromium-review.googlesource.com/828069 Commit-Queue: Robbie Iannucci Reviewed-by: Andrii Shyshkalov --- recipes/README.recipes.md | 17 +-- recipes/recipe_modules/bot_update/__init__.py | 1 + recipes/recipe_modules/bot_update/api.py | 13 ++- .../examples/full.expected/basic_luci.json | 105 ++++++++++++++++++ .../bot_update/examples/full.py | 5 + 5 files changed, 129 insertions(+), 12 deletions(-) create mode 100644 recipes/recipe_modules/bot_update/examples/full.expected/basic_luci.json diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index 44beeec78..85f3cec92 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -38,7 +38,7 @@ ### *recipe_modules* / [bot\_update](/recipes/recipe_modules/bot_update) -[DEPS](/recipes/recipe_modules/bot_update/__init__.py#1): [depot\_tools](#recipe_modules-depot_tools), [gclient](#recipe_modules-gclient), [gerrit](#recipe_modules-gerrit), [rietveld](#recipe_modules-rietveld), [tryserver](#recipe_modules-tryserver), [recipe\_engine/context][recipe_engine/recipe_modules/context], [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/source\_manifest][recipe_engine/recipe_modules/source_manifest], [recipe\_engine/step][recipe_engine/recipe_modules/step] +[DEPS](/recipes/recipe_modules/bot_update/__init__.py#1): [depot\_tools](#recipe_modules-depot_tools), [gclient](#recipe_modules-gclient), [gerrit](#recipe_modules-gerrit), [rietveld](#recipe_modules-rietveld), [tryserver](#recipe_modules-tryserver), [recipe\_engine/context][recipe_engine/recipe_modules/context], [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/runtime][recipe_engine/recipe_modules/runtime], [recipe\_engine/source\_manifest][recipe_engine/recipe_modules/source_manifest], [recipe\_engine/step][recipe_engine/recipe_modules/step] Recipe module to ensure a checkout is consistent on a bot. @@ -48,14 +48,14 @@ Recipe module to ensure a checkout is consistent on a bot. Wrapper for easy calling of bot_update. -— **def [apply\_gerrit\_ref](/recipes/recipe_modules/bot_update/api.py#49)(self, root, gerrit_no_reset=False, gerrit_no_rebase_patch_ref=False, gerrit_repo=None, gerrit_ref=None, step_name='apply_gerrit', \*\*kwargs):** +— **def [apply\_gerrit\_ref](/recipes/recipe_modules/bot_update/api.py#54)(self, root, gerrit_no_reset=False, gerrit_no_rebase_patch_ref=False, gerrit_repo=None, gerrit_ref=None, step_name='apply_gerrit', \*\*kwargs):** -— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#445)(self, bot_update_step):** +— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#450)(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#71)(self, gclient_config=None, suffix=None, patch=True, update_presentation=True, patch_root=None, no_shallow=False, with_branch_heads=False, with_tags=False, refs=None, patch_oauth2=False, oauth2_json=False, use_site_config_creds=True, 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, \*\*kwargs):** +— **def [ensure\_checkout](/recipes/recipe_modules/bot_update/api.py#76)(self, gclient_config=None, suffix=None, patch=True, update_presentation=True, patch_root=None, no_shallow=False, with_branch_heads=False, with_tags=False, refs=None, patch_oauth2=False, oauth2_json=False, use_site_config_creds=True, 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, \*\*kwargs):** Args: use_site_config_creds: If the oauth2 credentials are in the buildbot @@ -74,7 +74,7 @@ Args: 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#422)(self, project_name, gclient_config=None):** +— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#427)(self, project_name, gclient_config=None):** Returns all property names used for storing the checked-out revision of a given project. @@ -88,7 +88,7 @@ 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. -  **@property**
— **def [last\_returned\_properties](/recipes/recipe_modules/bot_update/api.py#43)(self):** +  **@property**
— **def [last\_returned\_properties](/recipes/recipe_modules/bot_update/api.py#48)(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] @@ -752,9 +752,9 @@ like checkout or compile), and some of these tests have failed. ### *recipes* / [bot\_update:examples/full](/recipes/recipe_modules/bot_update/examples/full.py) -[DEPS](/recipes/recipe_modules/bot_update/examples/full.py#5): [bot\_update](#recipe_modules-bot_update), [gclient](#recipe_modules-gclient), [gerrit](#recipe_modules-gerrit), [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] +[DEPS](/recipes/recipe_modules/bot_update/examples/full.py#5): [bot\_update](#recipe_modules-bot_update), [gclient](#recipe_modules-gclient), [gerrit](#recipe_modules-gerrit), [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#15)(api):** +— **def [RunSteps](/recipes/recipe_modules/bot_update/examples/full.py#16)(api):** ### *recipes* / [cipd:examples/full](/recipes/recipe_modules/cipd/examples/full.py) [DEPS](/recipes/recipe_modules/cipd/examples/full.py#8): [cipd](#recipe_modules-cipd), [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/step][recipe_engine/recipe_modules/step] @@ -841,6 +841,7 @@ Move things around in a loop! [recipe_engine/recipe_modules/properties]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/2dd87ee7779e4f9cb39619a7d0b235495ed12dda/README.recipes.md#recipe_modules-properties [recipe_engine/recipe_modules/python]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/2dd87ee7779e4f9cb39619a7d0b235495ed12dda/README.recipes.md#recipe_modules-python [recipe_engine/recipe_modules/raw_io]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/2dd87ee7779e4f9cb39619a7d0b235495ed12dda/README.recipes.md#recipe_modules-raw_io +[recipe_engine/recipe_modules/runtime]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/2dd87ee7779e4f9cb39619a7d0b235495ed12dda/README.recipes.md#recipe_modules-runtime [recipe_engine/recipe_modules/source_manifest]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/2dd87ee7779e4f9cb39619a7d0b235495ed12dda/README.recipes.md#recipe_modules-source_manifest [recipe_engine/recipe_modules/step]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/2dd87ee7779e4f9cb39619a7d0b235495ed12dda/README.recipes.md#recipe_modules-step [recipe_engine/recipe_modules/url]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/2dd87ee7779e4f9cb39619a7d0b235495ed12dda/README.recipes.md#recipe_modules-url diff --git a/recipes/recipe_modules/bot_update/__init__.py b/recipes/recipe_modules/bot_update/__init__.py index e6a257881..deaece127 100644 --- a/recipes/recipe_modules/bot_update/__init__.py +++ b/recipes/recipe_modules/bot_update/__init__.py @@ -9,6 +9,7 @@ DEPS = [ 'recipe_engine/properties', 'recipe_engine/python', 'recipe_engine/raw_io', + 'recipe_engine/runtime', 'recipe_engine/source_manifest', 'recipe_engine/step', 'rietveld', diff --git a/recipes/recipe_modules/bot_update/api.py b/recipes/recipe_modules/bot_update/api.py index 7454c7e03..b6412de14 100644 --- a/recipes/recipe_modules/bot_update/api.py +++ b/recipes/recipe_modules/bot_update/api.py @@ -34,10 +34,15 @@ class BotUpdateApi(recipe_api.RecipeApi): bot_update_path = self.resource('bot_update.py') kwargs.setdefault('infra_step', True) - env_prefixes = { - 'PATH': [self.m.depot_tools.root], - } - with self.m.context(env_prefixes=env_prefixes): + # On buildbot we have to put this on the FRONT of path, to combat the + # 'automatic' depot_tools. However, on LUCI, there is no automatic + # depot_tools, so it's safer to put it at the END of path, where it won't + # accidentally override e.g. python, vpython, etc. + key = 'env_prefixes' + if self.m.runtime.is_luci: + key = 'env_suffixes' + + with self.m.context(**{key: {'PATH': [self.m.depot_tools.root]}}): return self.m.python(name, bot_update_path, cmd, **kwargs) @property diff --git a/recipes/recipe_modules/bot_update/examples/full.expected/basic_luci.json b/recipes/recipe_modules/bot_update/examples/full.expected/basic_luci.json new file mode 100644 index 000000000..a618700bd --- /dev/null +++ b/recipes/recipe_modules/bot_update/examples/full.expected/basic_luci.json @@ -0,0 +1,105 @@ +[ + { + "cmd": [ + "python", + "-u", + "RECIPE_MODULE[depot_tools::bot_update]/resources/bot_update.py", + "--spec-path", + "cache_dir = '[GIT_CACHE]'\nsolutions = [{'deps_file': '.DEPS.git', 'managed': True, 'name': 'src', 'url': 'https://chromium.googlesource.com/chromium/src.git'}]", + "--patch_root", + "src", + "--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", + "[GIT_CACHE]", + "--cleanup-dir", + "[CLEANUP]/bot_update", + "--output_json", + "/path/to/tmp/json", + "--revision", + "src@abc", + "--disable-syntax-validation" + ], + "env_suffixes": { + "PATH": [ + "RECIPE_PACKAGE_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@ \"fixed_revisions\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"src\": \"abc\"@@@", + "@@@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@ }@@@", + "@@@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_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}\"@@@" + ] + }, + { + "name": "$result", + "recipe_result": null, + "status_code": 0 + } +] \ 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 a17242583..dc216eefb 100644 --- a/recipes/recipe_modules/bot_update/examples/full.py +++ b/recipes/recipe_modules/bot_update/examples/full.py @@ -10,6 +10,7 @@ DEPS = [ 'recipe_engine/path', 'recipe_engine/platform', 'recipe_engine/properties', + 'recipe_engine/runtime', ] def RunSteps(api): @@ -92,6 +93,10 @@ def GenTests(api): patch=False, revision='abc' ) + yield api.test('basic_luci') + api.properties( + patch=False, + revision='abc' + ) + api.runtime(is_experimental=False, is_luci=True) yield api.test('with_manifest_name_no_patch') + api.properties( manifest_name='checkout', patch=False