diff --git a/recipe_modules/bot_update/__init__.py b/recipe_modules/bot_update/__init__.py index 7846a68ad..8e44beee1 100644 --- a/recipe_modules/bot_update/__init__.py +++ b/recipe_modules/bot_update/__init__.py @@ -15,14 +15,27 @@ from recipe_engine.recipe_api import Property from recipe_engine.types import freeze PROPERTIES = { + # Gerrit patches will have all properties about them prefixed with patch_. + 'patch_issue': Property(default=None), # TODO(tandrii): add kind=int. + 'patch_set': Property(default=None), # TODO(tandrii): add kind=int. + 'patch_project': Property(default=None), # Also used by Rietveld. + 'patch_gerrit_url': Property(default=None), + 'patch_repository_url': Property(default=None), + 'patch_ref': Property(default=None), + + # TODO(tAndrii): remove legacy Gerrit fields. + # Legacy Gerrit fields. + 'event.patchSet.ref': Property(default=None, param_name='gerrit_ref'), + + # Rietveld-only fields. + 'rietveld': Property(default=None), # Stores Url of Rietveld server. 'issue': Property(default=None), 'patchset': Property(default=None), - 'patch_project': Property(default=None), 'repository': Property(default=None), - 'event.patchSet.ref': Property(default=None, param_name="gerrit_ref"), - 'rietveld': Property(default=None), - 'revision': Property(default=None), - 'parent_got_revision': Property(default=None), + + # Common fields for both systems. 'deps_revision_overrides': Property(default=freeze({})), 'fail_patch': Property(default=None, kind=str), + 'parent_got_revision': Property(default=None), + 'revision': Property(default=None), } diff --git a/recipe_modules/bot_update/api.py b/recipe_modules/bot_update/api.py index be231c85d..2b179cbca 100644 --- a/recipe_modules/bot_update/api.py +++ b/recipe_modules/bot_update/api.py @@ -10,13 +10,15 @@ from recipe_engine import recipe_api class BotUpdateApi(recipe_api.RecipeApi): - def __init__(self, issue, patchset, repository, gerrit_ref, rietveld, - revision, parent_got_revision, deps_revision_overrides, - fail_patch, *args, **kwargs): - self._issue = issue - self._patchset = patchset - self._repository = repository - self._gerrit_ref = gerrit_ref + def __init__(self, issue, patch_issue, patchset, patch_set, patch_project, + repository, patch_repository_url, gerrit_ref, patch_ref, + patch_gerrit_url, rietveld, revision, parent_got_revision, + deps_revision_overrides, fail_patch, *args, **kwargs): + self._issue = issue or patch_issue + self._patchset = patchset or patch_set + self._repository = repository or patch_repository_url + self._gerrit_ref = gerrit_ref or patch_ref + self._gerrit = patch_gerrit_url self._rietveld = rietveld self._revision = revision self._parent_got_revision = parent_got_revision @@ -124,6 +126,12 @@ class BotUpdateApi(recipe_api.RecipeApi): if not gerrit_ref or not gerrit_repo: gerrit_repo = gerrit_ref = None assert (gerrit_ref != None) == (gerrit_repo != None) + if gerrit_ref: + # Gerrit patches have historically not specified issue and patchset. + # resourece/bot_update has as a result implicit assumption that set issue + # implies Rietveld patch. + # TODO(tandrii): fix this madness. + issue = patchset = None # Point to the oauth2 auth files if specified. # These paths are where the bots put their credential files. diff --git a/recipe_modules/bot_update/example.expected/tryjob_gerrit_angle.json b/recipe_modules/bot_update/example.expected/tryjob_gerrit_angle.json index 4d1d7b89a..390447172 100644 --- a/recipe_modules/bot_update/example.expected/tryjob_gerrit_angle.json +++ b/recipe_modules/bot_update/example.expected/tryjob_gerrit_angle.json @@ -15,7 +15,7 @@ "--gerrit_repo", "https://chromium.googlesource.com/angle/angle", "--gerrit_ref", - "refs/changes/11/338811/3", + "refs/changes/89/456789/12", "--output_json", "/path/to/tmp/json", "--revision", diff --git a/recipe_modules/bot_update/example.expected/tryjob_gerrit_angle_deprecated.json b/recipe_modules/bot_update/example.expected/tryjob_gerrit_angle_deprecated.json new file mode 100644 index 000000000..4d1d7b89a --- /dev/null +++ b/recipe_modules/bot_update/example.expected/tryjob_gerrit_angle_deprecated.json @@ -0,0 +1,57 @@ +[ + { + "cmd": [ + "python", + "-u", + "RECIPE_MODULE[depot_tools::bot_update]/resources/bot_update.py", + "--spec", + "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/angle", + "--revision_mapping_file", + "{\"src\": \"got_cr_revision\"}", + "--git-cache-dir", + "[GIT_CACHE]", + "--gerrit_repo", + "https://chromium.googlesource.com/angle/angle", + "--gerrit_ref", + "refs/changes/11/338811/3", + "--output_json", + "/path/to/tmp/json", + "--revision", + "src@HEAD", + "--revision", + "src/third_party/angle@HEAD" + ], + "env": { + "PATH": "%(PATH)s:RECIPE_PACKAGE_REPO[depot_tools]" + }, + "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@ \"src/third_party/angle\": \"HEAD\"@@@", + "@@@STEP_LOG_LINE@json.output@ }, @@@", + "@@@STEP_LOG_LINE@json.output@ \"patch_failure\": false, @@@", + "@@@STEP_LOG_LINE@json.output@ \"patch_root\": \"src/third_party/angle\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"properties\": {@@@", + "@@@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@ }, @@@", + "@@@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_cr_revision@\"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@", + "@@@SET_BUILD_PROPERTY@got_cr_revision_cp@\"refs/heads/master@{#170242}\"@@@" + ] + }, + { + "name": "$result", + "recipe_result": null, + "status_code": 0 + } +] \ No newline at end of file diff --git a/recipe_modules/bot_update/example.py b/recipe_modules/bot_update/example.py index d95fc4d48..0dc27c293 100644 --- a/recipe_modules/bot_update/example.py +++ b/recipe_modules/bot_update/example.py @@ -141,6 +141,11 @@ def GenTests(api): yield api.test('tryjob_v8_head_by_default') + api.properties.tryserver( patch_project='v8', ) - yield api.test('tryjob_gerrit_angle') + api.properties.tryserver_gerrit( - full_project_name='angle/angle', + yield api.test('tryjob_gerrit_angle') + api.properties.tryserver( + gerrit_project='angle/angle', + patch_issue=338811, + patch_set=3, + ) + yield (api.test('tryjob_gerrit_angle_deprecated') + + api.properties.tryserver_gerrit(full_project_name='angle/angle') ) diff --git a/recipe_modules/tryserver/api.py b/recipe_modules/tryserver/api.py index f18c2b5d9..05aad08ab 100644 --- a/recipe_modules/tryserver/api.py +++ b/recipe_modules/tryserver/api.py @@ -41,6 +41,9 @@ class TryserverApi(recipe_api.RecipeApi): @property def is_gerrit_issue(self): """Returns true iff the properties exist to match a Gerrit issue.""" + if self.m.properties.get('patch_storage') == 'gerrit': + return True + # TODO(tandrii): remove this, once nobody is using buildbot Gerrit Poller. return ('event.patchSet.ref' in self.m.properties and 'event.change.url' in self.m.properties and 'event.change.id' in self.m.properties) diff --git a/recipe_modules/tryserver/example.expected/with_gerrit_patch.json b/recipe_modules/tryserver/example.expected/with_gerrit_patch.json new file mode 100644 index 000000000..39cba07f3 --- /dev/null +++ b/recipe_modules/tryserver/example.expected/with_gerrit_patch.json @@ -0,0 +1,39 @@ +[ + { + "cmd": [ + "git", + "diff", + "--cached", + "--name-only" + ], + "cwd": "[SLAVE_BUILD]", + "name": "git diff to analyze patch", + "stdout": "/path/to/tmp/", + "~followup_annotations": [ + "@@@STEP_LOG_LINE@files@foo.cc@@@", + "@@@STEP_LOG_END@files@@@", + "@@@SET_BUILD_PROPERTY@failure_type@\"INVALID_TEST_RESULTS\"@@@", + "@@@SET_BUILD_PROPERTY@subproject_tag@\"v8\"@@@" + ] + }, + { + "cmd": [ + "python", + "-u", + "import sys; sys.exit(1)" + ], + "name": "fail", + "~followup_annotations": [ + "step returned non-zero exit code: 1", + "@@@STEP_TEXT@foo@@@", + "@@@STEP_FAILURE@@@", + "@@@SET_BUILD_PROPERTY@failure_hash@\"c2311ad770732eade3d2f48247abd147e40a70e7\"@@@" + ] + }, + { + "name": "$result", + "reason": "Step('fail') failed with return_code 1", + "recipe_result": null, + "status_code": 1 + } +] \ No newline at end of file diff --git a/recipe_modules/tryserver/example.expected/with_gerrit_patch_deprecated.json b/recipe_modules/tryserver/example.expected/with_gerrit_patch_deprecated.json new file mode 100644 index 000000000..39cba07f3 --- /dev/null +++ b/recipe_modules/tryserver/example.expected/with_gerrit_patch_deprecated.json @@ -0,0 +1,39 @@ +[ + { + "cmd": [ + "git", + "diff", + "--cached", + "--name-only" + ], + "cwd": "[SLAVE_BUILD]", + "name": "git diff to analyze patch", + "stdout": "/path/to/tmp/", + "~followup_annotations": [ + "@@@STEP_LOG_LINE@files@foo.cc@@@", + "@@@STEP_LOG_END@files@@@", + "@@@SET_BUILD_PROPERTY@failure_type@\"INVALID_TEST_RESULTS\"@@@", + "@@@SET_BUILD_PROPERTY@subproject_tag@\"v8\"@@@" + ] + }, + { + "cmd": [ + "python", + "-u", + "import sys; sys.exit(1)" + ], + "name": "fail", + "~followup_annotations": [ + "step returned non-zero exit code: 1", + "@@@STEP_TEXT@foo@@@", + "@@@STEP_FAILURE@@@", + "@@@SET_BUILD_PROPERTY@failure_hash@\"c2311ad770732eade3d2f48247abd147e40a70e7\"@@@" + ] + }, + { + "name": "$result", + "reason": "Step('fail') failed with return_code 1", + "recipe_result": null, + "status_code": 1 + } +] \ No newline at end of file diff --git a/recipe_modules/tryserver/example.py b/recipe_modules/tryserver/example.py index 96b9e0bfa..025f25855 100644 --- a/recipe_modules/tryserver/example.py +++ b/recipe_modules/tryserver/example.py @@ -65,6 +65,13 @@ def GenTests(api): api.properties.tryserver(test_patch_root='sub/project') + description_step) + yield (api.test('with_gerrit_patch_deprecated') + + api.properties.tryserver_gerrit( + full_project_name='infra/infra')) + + yield (api.test('with_gerrit_patch') + + api.properties.tryserver(gerrit_project='infra/infra')) + yield (api.test('with_wrong_patch_new') + api.platform('win', 32) + api.properties(test_patch_root='sub\\project'))