From a16ccaf11a5a0b145b166161b80d99eaef294f4b Mon Sep 17 00:00:00 2001 From: Aaron Gable Date: Thu, 28 Sep 2017 11:58:29 -0700 Subject: [PATCH] bot_update: use patch repo instead of project if it is mapped This is a step away from patch_project, which was a holdover from Rietveld. Instead of mapping "projects" (which are poorly-defined and not guaranteed to be unique) to subpaths, instead map repository urls (which are at least unique). Eventually we may be able to compute this directly from DEPS instead of hardcoding the mapping here, but at least this is a step in the right direction. Bug: 765633 Change-Id: Idd65984fc6edefcbedb0438d38c2338b10b7e8e5 Reviewed-on: https://chromium-review.googlesource.com/690776 Commit-Queue: Aaron Gable Reviewed-by: Robbie Iannucci --- recipes/README.recipes.md | 14 +- recipes/recipe_modules/bot_update/api.py | 2 +- .../full.expected/tryjob_gerrit_webrtc.json | 196 ++++++++++++++++++ .../bot_update/examples/full.py | 9 + recipes/recipe_modules/gclient/api.py | 31 +-- recipes/recipe_modules/gclient/config.py | 2 + .../gclient/tests/patch_project.py | 17 +- 7 files changed, 248 insertions(+), 23 deletions(-) create mode 100644 recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_webrtc.json diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index 056ac81f0..4b298edfc 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -227,17 +227,19 @@ Wrapper for easy calling of gclient steps. Remove all index.lock files. If a previous run of git crashed, bot was reset, etc... we might end up with leftover index.lock files. -— **def [calculate\_patch\_root](/recipes/recipe_modules/gclient/api.py#309)(self, patch_project, gclient_config=None):** +— **def [calculate\_patch\_root](/recipes/recipe_modules/gclient/api.py#309)(self, patch_project, gclient_config=None, patch_repo=None):** Returns path where a patch should be applied to based patch_project. -Maps "patch_project" to a path of directories relative to checkout's root, -which describe where to place the patch. +Maps the patch's repo to a path of directories relative to checkout's root, +which describe where to place the patch. If no mapping is found for the +repo url, falls back to trying to find a mapping for the old-style +"patch_project". For now, considers only first solution (c.solutions[0]), but in theory can be extended to all of them. -See patch_projects solution config property. +See patch_projects and repo_path_map solution config property. Returns: Relative path, including solution's root. @@ -283,7 +285,7 @@ Chromium config. This may happen for one of two reasons: — **def [runhooks](/recipes/recipe_modules/gclient/api.py#268)(self, args=None, name='runhooks', \*\*kwargs):** -— **def [set\_patch\_project\_revision](/recipes/recipe_modules/gclient/api.py#336)(self, patch_project, gclient_config=None):** +— **def [set\_patch\_project\_revision](/recipes/recipe_modules/gclient/api.py#341)(self, patch_project, gclient_config=None):** Updates config revision corresponding to patch_project. @@ -780,7 +782,7 @@ like checkout or compile), and some of these tests have failed. [DEPS](/recipes/recipe_modules/gclient/tests/patch_project.py#9): [gclient](#recipe_modules-gclient), [recipe\_engine/properties][recipe_engine/recipe_modules/properties] -— **def [RunSteps](/recipes/recipe_modules/gclient/tests/patch_project.py#20)(api, patch_project):** +— **def [RunSteps](/recipes/recipe_modules/gclient/tests/patch_project.py#21)(api, patch_project, patch_repository_url):** ### *recipes* / [gerrit:examples/full](/recipes/recipe_modules/gerrit/examples/full.py) [DEPS](/recipes/recipe_modules/gerrit/examples/full.py#5): [gerrit](#recipe_modules-gerrit), [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 3d7514560..cc0c4c1f2 100644 --- a/recipes/recipe_modules/bot_update/api.py +++ b/recipes/recipe_modules/bot_update/api.py @@ -109,7 +109,7 @@ class BotUpdateApi(recipe_api.RecipeApi): root = patch_root if root is None: root = self.m.gclient.calculate_patch_root( - self.m.properties.get('patch_project'), cfg) + self.m.properties.get('patch_project'), cfg, self._repository) if patch: issue = issue or self._issue diff --git a/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_webrtc.json b/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_webrtc.json new file mode 100644 index 000000000..df56ea4fe --- /dev/null +++ b/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_webrtc.json @@ -0,0 +1,196 @@ +[ + { + "cmd": [ + "python", + "-u", + "RECIPE_PACKAGE_REPO[depot_tools]/gerrit_client.py", + "changes", + "--host", + "https://webrtc-review.googlesource.com", + "--json_file", + "/path/to/tmp/json", + "--limit", + "1", + "-p", + "change=338811" + ], + "env": { + "PATH": ":RECIPE_PACKAGE_REPO[depot_tools]" + }, + "infra_step": true, + "name": "gerrit get_patch_destination_branch", + "~followup_annotations": [ + "@@@STEP_LOG_LINE@json.output@[@@@", + "@@@STEP_LOG_LINE@json.output@ {@@@", + "@@@STEP_LOG_LINE@json.output@ \"_number\": \"91827\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"branch\": \"master\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"change_id\": \"Ideadbeef\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"created\": \"2017-01-30 13:11:20.000000000\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"has_review_started\": false, @@@", + "@@@STEP_LOG_LINE@json.output@ \"project\": \"chromium/src\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"revisions\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"184ebe53805e102605d11f6b143486d15c23a09c\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"_number\": \"1\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"commit\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"message\": \"Change commit message\"@@@", + "@@@STEP_LOG_LINE@json.output@ }@@@", + "@@@STEP_LOG_LINE@json.output@ }@@@", + "@@@STEP_LOG_LINE@json.output@ }, @@@", + "@@@STEP_LOG_LINE@json.output@ \"status\": \"NEW\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"subject\": \"Change title\"@@@", + "@@@STEP_LOG_LINE@json.output@ }@@@", + "@@@STEP_LOG_LINE@json.output@]@@@", + "@@@STEP_LOG_END@json.output@@@" + ] + }, + { + "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/third_party/webrtc", + "--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", + "--gerrit_repo", + "https://webrtc.googlesource.com/src", + "--gerrit_ref", + "refs/changes/11/338811/3", + "--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/third_party/webrtc\", @@@", + "@@@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/third_party/webrtc", + "--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@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/third_party/webrtc\", @@@", + "@@@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 93d25546d..ecc1c3059 100644 --- a/recipes/recipe_modules/bot_update/examples/full.py +++ b/recipes/recipe_modules/bot_update/examples/full.py @@ -33,6 +33,9 @@ def RunSteps(api): api.gclient.c.patch_projects['v8/v8'] = ('src/v8', 'HEAD') api.gclient.c.patch_projects['angle/angle'] = ('src/third_party/angle', 'HEAD') + api.gclient.c.repo_path_map['https://webrtc.googlesource.com/src'] = ( + 'src/third_party/webrtc', 'HEAD') + patch = api.properties.get('patch', True) clobber = True if api.properties.get('clobber') else False no_shallow = True if api.properties.get('no_shallow') else False @@ -233,3 +236,9 @@ def GenTests(api): 'event.patchSet.ref': 'refs/changes/11/338811/3', } ) + yield api.test('tryjob_gerrit_webrtc') + api.properties.tryserver( + gerrit_project='src', + git_url='https://webrtc.googlesource.com/src', + patch_issue=338811, + patch_set=3, + ) diff --git a/recipes/recipe_modules/gclient/api.py b/recipes/recipe_modules/gclient/api.py index c595fee02..9aa1c4279 100644 --- a/recipes/recipe_modules/gclient/api.py +++ b/recipes/recipe_modules/gclient/api.py @@ -306,16 +306,19 @@ class GclientApi(recipe_api.RecipeApi): infra_step=True, ) - def calculate_patch_root(self, patch_project, gclient_config=None): + def calculate_patch_root(self, patch_project, gclient_config=None, + patch_repo=None): """Returns path where a patch should be applied to based patch_project. - Maps "patch_project" to a path of directories relative to checkout's root, - which describe where to place the patch. + Maps the patch's repo to a path of directories relative to checkout's root, + which describe where to place the patch. If no mapping is found for the + repo url, falls back to trying to find a mapping for the old-style + "patch_project". For now, considers only first solution (c.solutions[0]), but in theory can be extended to all of them. - See patch_projects solution config property. + See patch_projects and repo_path_map solution config property. Returns: Relative path, including solution's root. @@ -323,15 +326,17 @@ class GclientApi(recipe_api.RecipeApi): solution root. """ cfg = gclient_config or self.c - root, _ = cfg.patch_projects.get(patch_project, ('', '')) - if root: - # Note, that c.patch_projects contains patch roots as - # slash(/)-separated path, which are roots of the respective project repos - # and include actual solution name in them. - return self.m.path.join(*root.split('/')) - # Default case - assume patch is for first solution, as this is what most - # projects rely on. - return cfg.solutions[0].name + root, _ = cfg.repo_path_map.get(patch_repo, ('', '')) + if not root: + root, _ = cfg.patch_projects.get(patch_project, ('', '')) + if not root: + # Failure case - assume patch is for first solution, as this is what most + # projects rely on. + return cfg.solutions[0].name + # Note, that c.patch_projects contains patch roots as + # slash(/)-separated path, which are roots of the respective project repos + # and include actual solution name in them. + return self.m.path.join(*root.split('/')) def set_patch_project_revision(self, patch_project, gclient_config=None): """Updates config revision corresponding to patch_project. diff --git a/recipes/recipe_modules/gclient/config.py b/recipes/recipe_modules/gclient/config.py index 5bdd924c8..5b1e55737 100644 --- a/recipes/recipe_modules/gclient/config.py +++ b/recipes/recipe_modules/gclient/config.py @@ -81,6 +81,8 @@ def BaseConfig(USE_MIRROR=True, CACHE_DIR=None, # then a patch to Angle project can be applied to a chromium src's # checkout after first updating Angle's repo to its master's HEAD. patch_projects = Dict(value_type=tuple, hidden=True), + # Same as the above, except the keys are full repo URLs. + repo_path_map = Dict(value_type=tuple, hidden=True), # Check out refs/branch-heads. # TODO (machenbach): Only implemented for bot_update atm. diff --git a/recipes/recipe_modules/gclient/tests/patch_project.py b/recipes/recipe_modules/gclient/tests/patch_project.py index 316e1283e..f144ceae9 100644 --- a/recipes/recipe_modules/gclient/tests/patch_project.py +++ b/recipes/recipe_modules/gclient/tests/patch_project.py @@ -13,20 +13,24 @@ DEPS = [ PROPERTIES = { - 'patch_project': recipe_api.Property(), + 'patch_project': recipe_api.Property(None), + 'patch_repository_url': recipe_api.Property(None), } -def RunSteps(api, patch_project): +def RunSteps(api, patch_project, patch_repository_url): src_cfg = api.gclient.make_config(CACHE_DIR='[ROOT]/git_cache') soln = src_cfg.solutions.add() soln.name = 'src' soln.url = 'https://chromium.googlesource.com/chromium/src.git' src_cfg.patch_projects['v8'] = ('src/v8', 'HEAD') src_cfg.patch_projects['v8/v8'] = ('src/v8', 'HEAD') + src_cfg.repo_path_map['https://webrtc.googlesource.com'] = ( + 'src/third_party/webrtc', 'HEAD') api.gclient.c = src_cfg - patch_root = api.gclient.calculate_patch_root(patch_project) + patch_root = api.gclient.calculate_patch_root( + patch_project, None, patch_repository_url) api.gclient.set_patch_project_revision(patch_project) @@ -43,3 +47,10 @@ def GenTests(api): api.properties(patch_project='v8') + api.post_process(post_process.DropExpectation) ) + + yield ( + api.test('webrtc') + + api.properties( + patch_repository_url='https://webrtc.googlesource.com/src') + + api.post_process(post_process.DropExpectation) + )