Skip to content

C
Chromium_Depot_tools
Commit da73999f authored by Xinan Lin's avatar Xinan Lin Committed by LUCI CQ
Browse files
Options

Make bot_update and tryserver module not crash for multiple changes 

Currently bot_update and tryserver crashes at initialization step if
the input contains multiple gerrit changes. This is not friendly for
CrOS CLs, e.g. crrev.com/c/2536761.

This CL moves the “crash” out of initialization, thus the consumer
of these modules could control which gerrit change to pass to bot_update
and tryserver.

This CL does NOT enable the modules here to support multiple patchset,
but allows consumers to choose one.

Bug:1146487
Change-Id: Iec46a8041189912e34da9bc5e8ff4611123ec9e0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2528670
Commit-Queue: Xinan Lin <linxinan@chromium.org>
Reviewed-by: 's avatarJosip Sokcevic <sokcevic@google.com>
parent fd5c1983
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 246 additions and 40 deletions
recipes/README.recipes.md
View file @ da73999f
This diff is collapsed.
recipes/recipe_modules/bot_update/api.py
View file @ da73999f
......@@ -19,11 +19,6 @@ class BotUpdateApi(recipe_api.RecipeApi):
self._last_returned_properties = {}
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')
def __call__(self, name, cmd, **kwargs):
"""Wrapper for easy calling of bot_update."""
assert isinstance(cmd, (list, tuple))
......@@ -93,6 +88,7 @@ class BotUpdateApi(recipe_api.RecipeApi):
patchset=None,
gerrit_no_reset=False,
gerrit_no_rebase_patch_ref=False,
accept_buildbucket_input=True,
disable_syntax_validation=False,
patch_refs=None,
ignore_input_commit=False,
......@@ -127,6 +123,11 @@ class BotUpdateApi(recipe_api.RecipeApi):
Use test_api.output_json to generate test data.
* enforce_fetch: Enforce a new fetch to refresh the git cache, even if the
solution revision passed in already exists in the current git cache.
* accept_buildbucket_input: should get the patchset from
buildbucket.build.input.gerrit_changes. If True, the input size is
asserted to be one, because bot_update module ONLY supports one CL.
Users may specify a CL via tryserver.set_change() and explicitly set
this flag False.
"""
assert use_site_config_creds is None, "use_site_config_creds is deprecated"
assert rietveld is None, "rietveld is deprecated"
......@@ -135,6 +136,10 @@ class BotUpdateApi(recipe_api.RecipeApi):
assert patch_oauth2 is None, "patch_oauth2 is deprecated"
assert oauth2_json is None, "oauth2_json is deprecated"
assert not (ignore_input_commit and set_output_commit)
if accept_buildbucket_input:
assert len(self.m.buildbucket.build.input.gerrit_changes) <= 1, (
'bot_update does not support more than one '
'buildbucket.build.input.gerrit_changes')
refs = refs or []
# We can re-use the gclient spec from the gclient module, since all the
......
recipes/recipe_modules/tryserver/api.py
View file @ da73999f
......@@ -22,13 +22,7 @@ class TryserverApi(recipe_api.RecipeApi):
def initialize(self):
changes = self.m.buildbucket.build.input.gerrit_changes
if len(changes) == 1:
cl = changes[0]
self._gerrit_change = cl
git_host = cl.host
gs_suffix = '-review.googlesource.com'
if git_host.endswith(gs_suffix):
git_host = '%s.googlesource.com' % git_host[:-len(gs_suffix)]
self._gerrit_change_repo_url = 'https://%s/%s' % (git_host, cl.project)
self.set_change(changes[0])
@property
def gerrit_change(self):
......@@ -262,3 +256,17 @@ class TryserverApi(recipe_api.RecipeApi):
def normalize_footer_name(self, footer):
return '-'.join([ word.title() for word in footer.strip().split('-') ])
def set_change(self, change):
"""Set the gerrit change for this module.
Args:
* cl: a GerritChange object.
"""
self._gerrit_info_initialized = False
self._gerrit_change = change
git_host = change.host
gs_suffix = '-review.googlesource.com'
if git_host.endswith(gs_suffix):
git_host = '%s.googlesource.com' % git_host[:-len(gs_suffix)]
self._gerrit_change_repo_url = 'https://%s/%s' % (git_host, change.project)
recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch.json
View file @ da73999f
......@@ -184,6 +184,49 @@
"@@@STEP_FAILURE@@@"
]
},
{
"cmd": [
"vpython",
"-u",
"RECIPE_REPO[depot_tools]/gerrit_client.py",
"changes",
"--host",
"https://chromium-review.googlesource.com",
"--json_file",
"/path/to/tmp/json",
"--limit",
"1",
"-p",
"change=1234567",
"-o",
"ALL_REVISIONS",
"-o",
"DOWNLOAD_COMMANDS"
],
"env": {
"PATH": "<PATH>:RECIPE_REPO[depot_tools]"
},
"infra_step": true,
"name": "gerrit fetch current CL info (2)",
"timeout": 600,
"~followup_annotations": [
"@@@STEP_LOG_LINE@json.output@[@@@",
"@@@STEP_LOG_LINE@json.output@ {@@@",
"@@@STEP_LOG_LINE@json.output@ \"branch\": \"master\", @@@",
"@@@STEP_LOG_LINE@json.output@ \"owner\": {@@@",
"@@@STEP_LOG_LINE@json.output@ \"name\": \"John Doe\"@@@",
"@@@STEP_LOG_LINE@json.output@ }, @@@",
"@@@STEP_LOG_LINE@json.output@ \"revisions\": {@@@",
"@@@STEP_LOG_LINE@json.output@ \"184ebe53805e102605d11f6b143486d15c23a09c\": {@@@",
"@@@STEP_LOG_LINE@json.output@ \"_number\": \"1\", @@@",
"@@@STEP_LOG_LINE@json.output@ \"ref\": \"refs/changes/67/1234567/1\"@@@",
"@@@STEP_LOG_LINE@json.output@ }@@@",
"@@@STEP_LOG_LINE@json.output@ }@@@",
"@@@STEP_LOG_LINE@json.output@ }@@@",
"@@@STEP_LOG_LINE@json.output@]@@@",
"@@@STEP_LOG_END@json.output@@@"
]
},
{
"name": "$result"
}
......
recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch_and_target_ref.json
View file @ da73999f
......@@ -184,6 +184,49 @@
"@@@STEP_FAILURE@@@"
]
},
{
"cmd": [
"vpython",
"-u",
"RECIPE_REPO[depot_tools]/gerrit_client.py",
"changes",
"--host",
"https://chromium-review.googlesource.com",
"--json_file",
"/path/to/tmp/json",
"--limit",
"1",
"-p",
"change=1234567",
"-o",
"ALL_REVISIONS",
"-o",
"DOWNLOAD_COMMANDS"
],
"env": {
"PATH": "<PATH>:RECIPE_REPO[depot_tools]"
},
"infra_step": true,
"name": "gerrit fetch current CL info (2)",
"timeout": 600,
"~followup_annotations": [
"@@@STEP_LOG_LINE@json.output@[@@@",
"@@@STEP_LOG_LINE@json.output@ {@@@",
"@@@STEP_LOG_LINE@json.output@ \"branch\": \"refs/heads/experiment\", @@@",
"@@@STEP_LOG_LINE@json.output@ \"owner\": {@@@",
"@@@STEP_LOG_LINE@json.output@ \"name\": \"John Doe\"@@@",
"@@@STEP_LOG_LINE@json.output@ }, @@@",
"@@@STEP_LOG_LINE@json.output@ \"revisions\": {@@@",
"@@@STEP_LOG_LINE@json.output@ \"184ebe53805e102605d11f6b143486d15c23a09c\": {@@@",
"@@@STEP_LOG_LINE@json.output@ \"_number\": \"1\", @@@",
"@@@STEP_LOG_LINE@json.output@ \"ref\": \"refs/changes/67/1234567/1\"@@@",
"@@@STEP_LOG_LINE@json.output@ }@@@",
"@@@STEP_LOG_LINE@json.output@ }@@@",
"@@@STEP_LOG_LINE@json.output@ }@@@",
"@@@STEP_LOG_LINE@json.output@]@@@",
"@@@STEP_LOG_END@json.output@@@"
]
},
{
"name": "$result"
}
......
recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch.json
View file @ da73999f
......@@ -15,6 +15,49 @@
"@@@STEP_LOG_END@files@@@"
]
},
{
"cmd": [
"vpython",
"-u",
"RECIPE_REPO[depot_tools]\\gerrit_client.py",
"changes",
"--host",
"https://chromium-review.googlesource.com",
"--json_file",
"/path/to/tmp/json",
"--limit",
"1",
"-p",
"change=1234567",
"-o",
"ALL_REVISIONS",
"-o",
"DOWNLOAD_COMMANDS"
],
"env": {
"PATH": "<PATH>;RECIPE_REPO[depot_tools]"
},
"infra_step": true,
"name": "gerrit fetch current CL info",
"timeout": 600,
"~followup_annotations": [
"@@@STEP_LOG_LINE@json.output@[@@@",
"@@@STEP_LOG_LINE@json.output@ {@@@",
"@@@STEP_LOG_LINE@json.output@ \"branch\": \"master\", @@@",
"@@@STEP_LOG_LINE@json.output@ \"owner\": {@@@",
"@@@STEP_LOG_LINE@json.output@ \"name\": \"John Doe\"@@@",
"@@@STEP_LOG_LINE@json.output@ }, @@@",
"@@@STEP_LOG_LINE@json.output@ \"revisions\": {@@@",
"@@@STEP_LOG_LINE@json.output@ \"184ebe53805e102605d11f6b143486d15c23a09c\": {@@@",
"@@@STEP_LOG_LINE@json.output@ \"_number\": \"1\", @@@",
"@@@STEP_LOG_LINE@json.output@ \"ref\": \"refs/changes/67/1234567/1\"@@@",
"@@@STEP_LOG_LINE@json.output@ }@@@",
"@@@STEP_LOG_LINE@json.output@ }@@@",
"@@@STEP_LOG_LINE@json.output@ }@@@",
"@@@STEP_LOG_LINE@json.output@]@@@",
"@@@STEP_LOG_END@json.output@@@"
]
},
{
"name": "$result"
}
......
recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch_new.json
View file @ da73999f
......@@ -16,6 +16,49 @@
"@@@STEP_LOG_END@files@@@"
]
},
{
"cmd": [
"vpython",
"-u",
"RECIPE_REPO[depot_tools]\\gerrit_client.py",
"changes",
"--host",
"https://chromium-review.googlesource.com",
"--json_file",
"/path/to/tmp/json",
"--limit",
"1",
"-p",
"change=1234567",
"-o",
"ALL_REVISIONS",
"-o",
"DOWNLOAD_COMMANDS"
],
"env": {
"PATH": "<PATH>;RECIPE_REPO[depot_tools]"
},
"infra_step": true,
"name": "gerrit fetch current CL info",
"timeout": 600,
"~followup_annotations": [
"@@@STEP_LOG_LINE@json.output@[@@@",
"@@@STEP_LOG_LINE@json.output@ {@@@",
"@@@STEP_LOG_LINE@json.output@ \"branch\": \"master\", @@@",
"@@@STEP_LOG_LINE@json.output@ \"owner\": {@@@",
"@@@STEP_LOG_LINE@json.output@ \"name\": \"John Doe\"@@@",
"@@@STEP_LOG_LINE@json.output@ }, @@@",
"@@@STEP_LOG_LINE@json.output@ \"revisions\": {@@@",
"@@@STEP_LOG_LINE@json.output@ \"184ebe53805e102605d11f6b143486d15c23a09c\": {@@@",
"@@@STEP_LOG_LINE@json.output@ \"_number\": \"1\", @@@",
"@@@STEP_LOG_LINE@json.output@ \"ref\": \"refs/changes/67/1234567/1\"@@@",
"@@@STEP_LOG_LINE@json.output@ }@@@",
"@@@STEP_LOG_LINE@json.output@ }@@@",
"@@@STEP_LOG_LINE@json.output@ }@@@",
"@@@STEP_LOG_LINE@json.output@]@@@",
"@@@STEP_LOG_END@json.output@@@"
]
},
{
"name": "$result"
}
......
recipes/recipe_modules/tryserver/examples/full.py
View file @ da73999f
......@@ -15,6 +15,8 @@ DEPS = [
'tryserver',
]
from PB.go.chromium.org.luci.buildbucket.proto.common import GerritChange
def RunSteps(api):
api.path['checkout'] = api.path['start_dir']
......@@ -51,6 +53,15 @@ def RunSteps(api):
api.tryserver.normalize_footer_name('Cr-Commit-Position')
api.tryserver.set_change(
GerritChange(host='chromium-review.googlesource.com',
project='infra/luci/recipes-py',
change=1234567,
patchset=1))
assert (api.tryserver.gerrit_change_repo_url ==
'https://chromium.googlesource.com/infra/luci/recipes-py')
assert api.tryserver.gerrit_change_fetch_ref == 'refs/changes/67/1234567/1'
def GenTests(api):
# The 'test_patch_root' property used below is just so that these
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment