From f022f91453f7c9caef7b7c7519d2e311dc0e72bd Mon Sep 17 00:00:00 2001 From: Nodir Turakulov Date: Sat, 8 Sep 2018 00:50:54 +0000 Subject: [PATCH] Revert "[bot_update] Stop using repository properties" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 4d2b901804e0319d56594502f038d018b40afb97. Reason for revert: https://chromium-review.googlesource.com/c/chromium/tools/build/+/1214622 Original change's description: > [bot_update] Stop using repository properties > > Stop using repository and patch_repository_url properties. > Use self.m.buildbucket.build.input.gerrit_changes[0].{host, project} > instead. > > R=​ehmaldonado@chromium.org, tandrii@chromium.org > > Recipe-Nontrivial-Roll: build > Bug: 877161, 694348 > Change-Id: Ideb069c6c8f8b6e6cfb5217f8a04ea18f16d3056 > Reviewed-on: https://chromium-review.googlesource.com/1208368 > Commit-Queue: Nodir Turakulov > Reviewed-by: Andrii Shyshkalov TBR=nodir@chromium.org,tandrii@chromium.org,ehmaldonado@chromium.org Change-Id: I825242124382e9f1789d4a3da0a3bdd2a5fcab37 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 877161, 694348 Reviewed-on: https://chromium-review.googlesource.com/1214703 Reviewed-by: Nodir Turakulov Commit-Queue: Nodir Turakulov --- recipes/README.recipes.md | 14 +++++----- recipes/recipe_modules/bot_update/__init__.py | 6 +++++ recipes/recipe_modules/bot_update/api.py | 27 ++++++++++--------- .../full.expected/tryjob_gerrit_webrtc.json | 10 +++---- .../bot_update/examples/full.py | 2 +- 5 files changed, 34 insertions(+), 25 deletions(-) diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index 4278d618b..83b95e0d1 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#32)(self, name, cmd, \*\*kwargs):** +— **def [\_\_call\_\_](/recipes/recipe_modules/bot_update/api.py#37)(self, name, cmd, \*\*kwargs):** Wrapper for easy calling of bot_update. -— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#424)(self, bot_update_step):** +— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#427)(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#73)(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, \*\*kwargs):** +— **def [ensure\_checkout](/recipes/recipe_modules/bot_update/api.py#78)(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, \*\*kwargs):** Args: gclient_config: The gclient configuration to use when running bot_update. @@ -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#401)(self, project_name, gclient_config=None):** +— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#404)(self, project_name, gclient_config=None):** Returns all property names used for storing the checked-out revision of a given project. @@ -82,9 +82,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#27)(self):** +— **def [initialize](/recipes/recipe_modules/bot_update/api.py#30)(self):** -  **@property**
— **def [last\_returned\_properties](/recipes/recipe_modules/bot_update/api.py#41)(self):** +  **@property**
— **def [last\_returned\_properties](/recipes/recipe_modules/bot_update/api.py#46)(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/__init__.py b/recipes/recipe_modules/bot_update/__init__.py index 1526ed3b2..7927b97ee 100644 --- a/recipes/recipe_modules/bot_update/__init__.py +++ b/recipes/recipe_modules/bot_update/__init__.py @@ -25,7 +25,13 @@ PROPERTIES = { 'patch_issue': Property(default=None), # TODO(tandrii): add kind=int. 'patch_set': Property(default=None), # TODO(tandrii): add kind=int. 'patch_gerrit_url': Property(default=None), + 'patch_repository_url': Property(default=None), 'patch_ref': Property(default=None), + + # Rietveld-only (?) fields. + 'repository': Property(default=None), + + # Common fields for both systems. 'deps_revision_overrides': Property(default={}), 'fail_patch': Property(default=None, kind=str), diff --git a/recipes/recipe_modules/bot_update/api.py b/recipes/recipe_modules/bot_update/api.py index e4c57ba4e..f08c9944d 100644 --- a/recipes/recipe_modules/bot_update/api.py +++ b/recipes/recipe_modules/bot_update/api.py @@ -4,18 +4,21 @@ """Recipe module to ensure a checkout is consistent on a bot.""" +import re + from recipe_engine import recipe_api class BotUpdateApi(recipe_api.RecipeApi): - def __init__(self, properties, patch_issue, patch_set, patch_ref, - patch_gerrit_url, deps_revision_overrides, fail_patch, *args, - **kwargs): + def __init__(self, properties, patch_issue, patch_set, repository, + patch_repository_url, patch_ref, patch_gerrit_url, + deps_revision_overrides, fail_patch, *args, **kwargs): self._apply_patch_on_gclient = properties.get( 'apply_patch_on_gclient', True) self._issue = patch_issue self._patchset = patch_set + self._repository = repository or patch_repository_url self._gerrit_ref = patch_ref self._gerrit = patch_gerrit_url self._deps_revision_overrides = deps_revision_overrides @@ -25,9 +28,11 @@ class BotUpdateApi(recipe_api.RecipeApi): super(BotUpdateApi, self).__init__(*args, **kwargs) def initialize(self): - assert len(self.m.buildbucket.build.input.gerrit_changes) <= 1, ( - 'bot_update does not support more than one ' - 'buildbucket.build.input.gerrit_changes') + changes = self.m.buildbucket.build.input.gerrit_changes + if self._repository is None and len(changes) == 1: + cl = changes[0] + host = re.sub(r'([^\.]+)-review(\.googlesource\.com)', r'\1\2', cl.host) + self._repository = 'https://%s/%s' % (host, cl.project) def __call__(self, name, cmd, **kwargs): """Wrapper for easy calling of bot_update.""" @@ -106,14 +111,13 @@ class BotUpdateApi(recipe_api.RecipeApi): assert cfg is not None, ( 'missing gclient_config or forgot api.gclient.set_config(...) before?') + # Construct our bot_update command. This basically be inclusive of # everything required for bot_update to know: root = patch_root if root is None: - # TODO(nodir): use m.gclient.get_repo_path instead. root = self.m.gclient.calculate_patch_root( - self.m.properties.get('patch_project'), cfg, - self.m.tryserver.gerrit_change_repo_url) + self.m.properties.get('patch_project'), cfg, self._repository) # Allow patch_project's revision if necessary. # This is important for projects which are checked out as DEPS of the @@ -138,10 +142,9 @@ class BotUpdateApi(recipe_api.RecipeApi): # How to find the patch, if any if patch: - if self.m.tryserver.gerrit_change_repo_url and self._gerrit_ref: + if self._repository and self._gerrit_ref: flags.append( - ['--patch_ref', '%s@%s' % ( - self.m.tryserver.gerrit_change_repo_url, self._gerrit_ref)]) + ['--patch_ref', '%s@%s' % (self._repository, self._gerrit_ref)]) if patch_refs: flags.extend( ['--patch_ref', patch_ref] 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 index 714458fb7..3e0c8a1c8 100644 --- 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 @@ -51,7 +51,7 @@ "--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", + "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", @@ -61,7 +61,7 @@ "--output_json", "/path/to/tmp/json", "--patch_ref", - "https://webrtc.googlesource.com/src@refs/changes/11/338811/3", + "https://chromium.googlesource.com/chromium/src@refs/changes/11/338811/3", "--revision", "src@HEAD", "--disable-syntax-validation" @@ -95,7 +95,7 @@ "@@@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@ \"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}\", @@@", @@ -151,7 +151,7 @@ "--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", + "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", @@ -192,7 +192,7 @@ "@@@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@ \"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}\", @@@", diff --git a/recipes/recipe_modules/bot_update/examples/full.py b/recipes/recipe_modules/bot_update/examples/full.py index d90841673..e0d1d43e2 100644 --- a/recipes/recipe_modules/bot_update/examples/full.py +++ b/recipes/recipe_modules/bot_update/examples/full.py @@ -158,7 +158,7 @@ def GenTests(api): ) yield api.test('tryjob_v8_head_by_default') + api.properties.tryserver( repository='https://chromium.googlesource.com/v8/v8', - patch_project='v8/v8', + patch_project='v8', ) yield api.test('tryjob_gerrit_angle') + api.properties.tryserver( repository='https://chromium.googlesource.com/angle/angle',