diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index a3d644692..7eb3a1615 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -52,7 +52,7 @@ 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#423)(self, bot_update_step):** +— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#418)(self, bot_update_step):** Deapplies a patch, taking care of DEPS and solution revisions properly. @@ -68,7 +68,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#400)(self, project_name, gclient_config=None):** +— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#395)(self, project_name, gclient_config=None):** Returns all property names used for storing the checked-out revision of a given project. @@ -248,7 +248,7 @@ 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#339)(self, patch_project, gclient_config=None, patch_repo=None):** +— **def [calculate\_patch\_root](/recipes/recipe_modules/gclient/api.py#360)(self, patch_project, gclient_config=None, patch_repo=None):** Returns path where a patch should be applied to based patch_project. @@ -277,7 +277,18 @@ Return a step generator function for gclient checkouts. — **def [get\_config\_defaults](/recipes/recipe_modules/gclient/api.py#114)(self):** -— **def [get\_repo\_path](/recipes/recipe_modules/gclient/api.py#309)(self, repo_url, gclient_config=None):** +— **def [get\_gerrit\_patch\_root](/recipes/recipe_modules/gclient/api.py#305)(self, gclient_config=None):** + +Returns local path to the repo where gerrit patch will be applied. + +If there is no patch, returns None. +If patch is specified, but such repo is not found among configured solutions +or repo_path_map, returns name of the first solution. This is done solely +for backward compatibility with existing tests. +Please do not rely on this logic in new code. +Instead, properly map a repository to a local path using repo_path_map. + +— **def [get\_repo\_path](/recipes/recipe_modules/gclient/api.py#330)(self, repo_url, gclient_config=None):** Returns local path to the repo checkout given its url. @@ -316,7 +327,7 @@ Chromium config. This may happen for one of two reasons: — **def [runhooks](/recipes/recipe_modules/gclient/api.py#264)(self, args=None, name='runhooks', \*\*kwargs):** -— **def [set\_patch\_project\_revision](/recipes/recipe_modules/gclient/api.py#376)(self, patch_project, gclient_config=None):** +— **def [set\_patch\_project\_revision](/recipes/recipe_modules/gclient/api.py#397)(self, patch_project, gclient_config=None):** Updates config revision corresponding to patch_project. @@ -933,9 +944,9 @@ Raises: — **def [RunSteps](/recipes/recipe_modules/gclient/examples/full.py#50)(api):** ### *recipes* / [gclient:tests/patch\_project](/recipes/recipe_modules/gclient/tests/patch_project.py) -[DEPS](/recipes/recipe_modules/gclient/tests/patch_project.py#9): [gclient](#recipe_modules-gclient), [recipe\_engine/properties][recipe_engine/recipe_modules/properties] +[DEPS](/recipes/recipe_modules/gclient/tests/patch_project.py#9): [gclient](#recipe_modules-gclient), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/properties][recipe_engine/recipe_modules/properties] -— **def [RunSteps](/recipes/recipe_modules/gclient/tests/patch_project.py#21)(api, patch_project, patch_repository_url):** +— **def [RunSteps](/recipes/recipe_modules/gclient/tests/patch_project.py#22)(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 454adfd60..3d5879bc0 100644 --- a/recipes/recipe_modules/bot_update/api.py +++ b/recipes/recipe_modules/bot_update/api.py @@ -60,7 +60,7 @@ class BotUpdateApi(recipe_api.RecipeApi): 'invalid (host, project) pair in ' 'buildbucket.build.input.gitiles_commit: ' '(%r, %r) does not match any of configured gclient solutions ' - 'and not present in gclient.c.repo_path_map' % ( + 'and not present in gclient_config.repo_path_map' % ( commit.host, commit.project)) return repo_path @@ -335,14 +335,9 @@ class BotUpdateApi(recipe_api.RecipeApi): A destination branch as understood by bot_update.py if available and if different from master, returns 'HEAD' otherwise. """ - # Bail out if this is not a gerrit change. - if not self.m.tryserver.gerrit_change: - return 'HEAD' - - # Ignore other project paths than the one belonging to the CL. - if path != cfg.patch_projects.get( - self.m.properties.get('patch_project'), - (cfg.solutions[0].name, None))[0]: + # Ignore project paths other than the one belonging to the current CL. + patch_path = self.m.gclient.get_gerrit_patch_root(gclient_config=cfg) + if not patch_path or path != patch_path: return 'HEAD' target_ref = self.m.tryserver.gerrit_change_target_ref diff --git a/recipes/recipe_modules/bot_update/examples/buildbucket.expected/ci with invalid repo.json b/recipes/recipe_modules/bot_update/examples/buildbucket.expected/ci with invalid repo.json index 75f6e7cad..9beabfafc 100644 --- a/recipes/recipe_modules/bot_update/examples/buildbucket.expected/ci with invalid repo.json +++ b/recipes/recipe_modules/bot_update/examples/buildbucket.expected/ci with invalid repo.json @@ -1,7 +1,7 @@ [ { "name": "$result", - "reason": "invalid (host, project) pair in buildbucket.build.input.gitiles_commit: (u'chromium.googlesource.com', u'trash') does not match any of configured gclient solutions and not present in gclient.c.repo_path_map", + "reason": "invalid (host, project) pair in buildbucket.build.input.gitiles_commit: (u'chromium.googlesource.com', u'trash') does not match any of configured gclient solutions and not present in gclient_config.repo_path_map", "recipe_result": null, "status_code": 1 } diff --git a/recipes/recipe_modules/gclient/api.py b/recipes/recipe_modules/gclient/api.py index 7d82b28b1..f8f137d8e 100644 --- a/recipes/recipe_modules/gclient/api.py +++ b/recipes/recipe_modules/gclient/api.py @@ -302,6 +302,29 @@ class GclientApi(recipe_api.RecipeApi): infra_step=True, ) + def get_gerrit_patch_root(self, gclient_config=None): + """Returns local path to the repo where gerrit patch will be applied. + + If there is no patch, returns None. + If patch is specified, but such repo is not found among configured solutions + or repo_path_map, returns name of the first solution. This is done solely + for backward compatibility with existing tests. + Please do not rely on this logic in new code. + Instead, properly map a repository to a local path using repo_path_map. + TODO(nodir): remove this. Update all recipe tests to specify a git_repo + matching the recipe. + """ + cfg = gclient_config or self.c + repo_url = self.m.tryserver.gerrit_change_repo_url + if not repo_url: + return None + root = self.get_repo_path(repo_url, gclient_config=cfg) + + # This is wrong, but that's what a ton of recipe tests expect today + root = root or cfg.solutions[0] + + return root + def _canonicalize_repo_url(self, repo_url): """Attempts to make repo_url canonical. Supports Gitiles URL.""" return self.m.gitiles.canonicalize_repo_url(repo_url) @@ -337,7 +360,7 @@ class GclientApi(recipe_api.RecipeApi): return None def calculate_patch_root(self, patch_project, gclient_config=None, - patch_repo=None): + patch_repo=None): # pragma: no cover """Returns path where a patch should be applied to based patch_project. TODO(nodir): delete this function in favor of get_repo_path. diff --git a/recipes/recipe_modules/gclient/tests/patch_project.py b/recipes/recipe_modules/gclient/tests/patch_project.py index 6aea2e7a9..467d2f0c7 100644 --- a/recipes/recipe_modules/gclient/tests/patch_project.py +++ b/recipes/recipe_modules/gclient/tests/patch_project.py @@ -8,6 +8,7 @@ from recipe_engine import recipe_api DEPS = [ 'gclient', + 'recipe_engine/buildbucket', 'recipe_engine/properties', ] @@ -26,6 +27,7 @@ def RunSteps(api, patch_project, patch_repository_url): src_cfg.patch_projects['v8'] = ('src/v8', 'HEAD') src_cfg.patch_projects['v8/v8'] = ('src/v8', 'HEAD') src_cfg.repo_path_map.update({ + 'https://chromium.googlesource.com/src': ('src', 'HEAD'), 'https://chromium.googlesource.com/v8/v8': ('src/v8', 'HEAD'), # non-canonical URL 'https://webrtc.googlesource.com/src.git': ( @@ -48,6 +50,8 @@ def RunSteps(api, patch_project, patch_repository_url): gclient_config=src_cfg) is None api.gclient.c = src_cfg + patch_root = api.gclient.get_gerrit_patch_root(gclient_config=src_cfg) + assert patch_root == api.properties['expected_patch_root'], patch_root api.gclient.calculate_patch_root( patch_project, None, patch_repository_url) @@ -57,20 +61,49 @@ def RunSteps(api, patch_project, patch_repository_url): def GenTests(api): yield ( - api.test('chromium') + - api.properties(patch_project='chromium') + + api.test('chromium_ci') + + api.buildbucket.ci_build( + project='chromium', + builder='linux', + git_repo='https://chromium.googlesource.com/src') + + api.properties( + expected_patch_root=None, + patch_project='chromium') + api.post_process(post_process.DropExpectation) ) yield ( - api.test('v8') + - api.properties(patch_project='v8') + + api.test('chromium_try') + + api.buildbucket.try_build( + project='chromium', + builder='linux', + git_repo='https://chromium.googlesource.com/src') + + api.properties( + expected_patch_root='src', + patch_project='chromium') + + api.post_process(post_process.DropExpectation) + ) + + yield ( + api.test('v8_try') + + api.buildbucket.try_build( + project='chromium', + builder='linux', + git_repo='https://chromium.googlesource.com/v8/v8') + + api.properties( + expected_patch_root='src/v8', + patch_project='v8') + api.post_process(post_process.DropExpectation) ) yield ( - api.test('webrtc') + + api.test('webrtc_try') + + api.buildbucket.try_build( + project='chromium', + builder='linux', + git_repo='https://webrtc.googlesource.com/src') + api.properties( - patch_repository_url='https://webrtc.googlesource.com/src') + + expected_patch_root='src/third_party/webrtc', + patch_project='webrtc') + api.post_process(post_process.DropExpectation) )