From c14b4f1518dfd0bf57c27d42b28abdc9252145ef Mon Sep 17 00:00:00 2001 From: Michael Achenbach Date: Tue, 8 Aug 2017 14:17:20 +0200 Subject: [PATCH] Fix bot_update default value for empty revision Bug: 753297 Change-Id: I31c031ce1efa3a1a7d30fd7eeb71d3e4b3baa720 Reviewed-on: https://chromium-review.googlesource.com/605609 Reviewed-by: Andrii Shyshkalov Reviewed-by: Aaron Gable Commit-Queue: Michael Achenbach --- recipes/README.recipes.md | 4 +- recipes/recipe_modules/bot_update/api.py | 19 ++- .../full.expected/tryjob_empty_revision.json | 156 ++++++++++++++++++ .../bot_update/examples/full.py | 6 + 4 files changed, 176 insertions(+), 9 deletions(-) create mode 100644 recipes/recipe_modules/bot_update/examples/full.expected/tryjob_empty_revision.json diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index 6f2f42026..ae0dc6c66 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -50,7 +50,7 @@ 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 [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#429)(self, bot_update_step):** +— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#434)(self, bot_update_step):** Deapplies a patch, taking care of DEPS and solution revisions properly. @@ -72,7 +72,7 @@ Args: Needed as migration paths for recipes dealing with older revisions, such as bisect. -— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#406)(self, project_name, gclient_config=None):** +— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#411)(self, project_name, gclient_config=None):** Returns all property names used for storing the checked-out revision of a given project. diff --git a/recipes/recipe_modules/bot_update/api.py b/recipes/recipe_modules/bot_update/api.py index 5fae30e5b..3d7514560 100644 --- a/recipes/recipe_modules/bot_update/api.py +++ b/recipes/recipe_modules/bot_update/api.py @@ -196,28 +196,33 @@ class BotUpdateApi(recipe_api.RecipeApi): ['--output_json', self.m.json.output()], ] - - # Collect all fixed revisions to simulate them in the json output. - # Fixed revision are the explicit input revisions of bot_update.py, i.e. - # every command line parameter "--revision name@value". - fixed_revisions = {} - + # Compute requested revisions. revisions = {} for solution in cfg.solutions: if solution.revision: revisions[solution.name] = solution.revision elif solution == cfg.solutions[0]: + # TODO(machenbach): We should explicitly pass HEAD for ALL solutions + # that don't specify anything else. revisions[solution.name] = ( self._parent_got_revision or self._revision or 'HEAD') if self.m.gclient.c and self.m.gclient.c.revisions: - revisions.update(self.m.gclient.c.revisions) + # Only update with non-empty values. Some recipe might otherwise + # overwrite the HEAD default with an empty string. + revisions.update( + (k, v) for k, v in self.m.gclient.c.revisions.iteritems() if v) if cfg.solutions and root_solution_revision: revisions[cfg.solutions[0].name] = root_solution_revision # Allow for overrides required to bisect into rolls. revisions.update(self._deps_revision_overrides) + # Compute command-line parameters for requested revisions. + # Also collect all fixed revisions to simulate them in the json output. + # Fixed revision are the explicit input revisions of bot_update.py, i.e. + # every command line parameter "--revision name@value". + fixed_revisions = {} for name, revision in sorted(revisions.items()): fixed_revision = self.m.gclient.resolve_revision(revision) if fixed_revision: diff --git a/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_empty_revision.json b/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_empty_revision.json new file mode 100644 index 000000000..684f2e8ac --- /dev/null +++ b/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_empty_revision.json @@ -0,0 +1,156 @@ +[ + { + "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", + "--issue", + "12345", + "--patchset", + "654321", + "--rietveld_server", + "https://rietveld.example.com/", + "--output_json", + "/path/to/tmp/json", + "--revision", + "src@HEAD", + "--disable-syntax-validation" + ], + "env_prefixes": { + "PATH": [ + "RECIPE_PACKAGE_REPO[depot_tools]" + ] + }, + "infra_step": true, + "name": "bot_update", + "~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\": \"HEAD\"@@@", + "@@@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@ \"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}\"@@@" + ] + }, + { + "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", + "--rietveld_server", + "https://rietveld.example.com/", + "--output_json", + "/path/to/tmp/json", + "--revision", + "src@f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9" + ], + "env_prefixes": { + "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\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@", + "@@@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@ \"step_text\": \"Some step text\"@@@", + "@@@STEP_LOG_LINE@json.output@}@@@", + "@@@STEP_LOG_END@json.output@@@" + ] + }, + { + "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 3b51d5f10..93d25546d 100644 --- a/recipes/recipe_modules/bot_update/examples/full.py +++ b/recipes/recipe_modules/bot_update/examples/full.py @@ -101,6 +101,12 @@ def GenTests(api): patchset=654321, rietveld='https://rietveld.example.com/', ) + yield api.test('tryjob_empty_revision') + api.properties( + issue=12345, + patchset=654321, + rietveld='https://rietveld.example.com/', + revisions={'src': ''}, + ) yield api.test('deprecated_got_revision_mapping') + api.properties( deprecated_got_revision_mapping=True, issue=12345,