diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index b0b0ab7fe..bc4daadb0 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -671,21 +671,17 @@ Returns: given is unknown. ### *recipe_modules* / [tryserver](/recipes/recipe_modules/tryserver) -[DEPS](/recipes/recipe_modules/tryserver/__init__.py#5): [gerrit](#recipe_modules-gerrit), [git](#recipe_modules-git), [git\_cl](#recipe_modules-git_cl), [rietveld](#recipe_modules-rietveld), [recipe\_engine/context][recipe_engine/recipe_modules/context], [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] +[DEPS](/recipes/recipe_modules/tryserver/__init__.py#5): [gerrit](#recipe_modules-gerrit), [git](#recipe_modules-git), [git\_cl](#recipe_modules-git_cl), [recipe\_engine/context][recipe_engine/recipe_modules/context], [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] #### **class [TryserverApi](/recipes/recipe_modules/tryserver/api.py#12)([RecipeApi][recipe_engine/wkt/RecipeApi]):** -— **def [add\_failure\_reason](/recipes/recipe_modules/tryserver/api.py#148)(self, reason):** +— **def [add\_failure\_reason](/recipes/recipe_modules/tryserver/api.py#108)(self, reason):** Records a more detailed reason why build is failing. The reason can be any JSON-serializable object. -  **@property**
— **def [can\_apply\_issue](/recipes/recipe_modules/tryserver/api.py#23)(self):** - -Returns true iff the properties exist to apply_issue from rietveld. - -— **def [get\_files\_affected\_by\_patch](/recipes/recipe_modules/tryserver/api.py#46)(self, patch_root=None, \*\*kwargs):** +— **def [get\_files\_affected\_by\_patch](/recipes/recipe_modules/tryserver/api.py#38)(self, patch_root, \*\*kwargs):** Returns list of paths to files affected by the patch. @@ -695,38 +691,34 @@ Argument: Returned paths will be relative to to patch_root. -TODO(tandrii): remove this doc. -Unless you use patch_root=None, in which case old behavior is used -which returns paths relative to checkout aka solution[0].name. - -— **def [get\_footer](/recipes/recipe_modules/tryserver/api.py#211)(self, tag, patch_text=None):** +— **def [get\_footer](/recipes/recipe_modules/tryserver/api.py#162)(self, tag, patch_text=None):** Gets a specific tag from a CL description -— **def [get\_footers](/recipes/recipe_modules/tryserver/api.py#184)(self, patch_text=None):** +— **def [get\_footers](/recipes/recipe_modules/tryserver/api.py#144)(self, patch_text=None):** Retrieves footers from the patch description. footers are machine readable tags embedded in commit messages. See git-footers documentation for more information. -  **@property**
— **def [is\_gerrit\_issue](/recipes/recipe_modules/tryserver/api.py#30)(self):** +  **@property**
— **def [is\_gerrit\_issue](/recipes/recipe_modules/tryserver/api.py#22)(self):** Returns true iff the properties exist to match a Gerrit issue. -  **@property**
— **def [is\_patch\_in\_git](/recipes/recipe_modules/tryserver/api.py#40)(self):** +  **@property**
— **def [is\_patch\_in\_git](/recipes/recipe_modules/tryserver/api.py#32)(self):**   **@property**
— **def [is\_tryserver](/recipes/recipe_modules/tryserver/api.py#17)(self):** -Returns true iff we can apply_issue or patch. +Returns true iff we have a change to check out. -— **def [normalize\_footer\_name](/recipes/recipe_modules/tryserver/api.py#215)(self, footer):** +— **def [normalize\_footer\_name](/recipes/recipe_modules/tryserver/api.py#166)(self, footer):** -— **def [set\_compile\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#127)(self):** +— **def [set\_compile\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#87)(self):** Mark the tryjob result as a compile failure. -  **@contextlib.contextmanager**
— **def [set\_failure\_hash](/recipes/recipe_modules/tryserver/api.py#157)(self):** +  **@contextlib.contextmanager**
— **def [set\_failure\_hash](/recipes/recipe_modules/tryserver/api.py#117)(self):** Context manager that sets a failure_hash build property on StepFailure. @@ -735,7 +727,7 @@ for the same reason. For example, if a patch is bad (breaks something), we'd expect it to always break in the same way. Different failures for the same patch are usually a sign of flakiness. -— **def [set\_invalid\_test\_results\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#139)(self):** +— **def [set\_invalid\_test\_results\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#99)(self):** Mark the tryjob result as having invalid test results. @@ -743,18 +735,18 @@ This means we run some tests, but the results were not valid (e.g. no list of specific test cases that failed, or too many tests failing, etc). -— **def [set\_patch\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#123)(self):** +— **def [set\_patch\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#83)(self):** Mark the tryjob result as failure to apply the patch. -— **def [set\_subproject\_tag](/recipes/recipe_modules/tryserver/api.py#105)(self, subproject_tag):** +— **def [set\_subproject\_tag](/recipes/recipe_modules/tryserver/api.py#65)(self, subproject_tag):** Adds a subproject tag to the build. This can be used to distinguish between builds that execute different steps depending on what was patched, e.g. blink vs. pure chromium patches. -— **def [set\_test\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#131)(self):** +— **def [set\_test\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#91)(self):** Mark the tryjob result as a test failure. diff --git a/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail_patch.json b/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail_patch.json index e14f8c35c..4907b791d 100644 --- a/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail_patch.json +++ b/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail_patch.json @@ -95,7 +95,6 @@ "@@@STEP_LOG_LINE@json.output@ \"step_text\": \"Some step text\"@@@", "@@@STEP_LOG_LINE@json.output@}@@@", "@@@STEP_LOG_END@json.output@@@", - "@@@SET_BUILD_PROPERTY@failure_type@\"PATCH_FAILURE\"@@@", "@@@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\"@@@", diff --git a/recipes/recipe_modules/tryserver/__init__.py b/recipes/recipe_modules/tryserver/__init__.py index 22cf172b2..06c9d0b11 100644 --- a/recipes/recipe_modules/tryserver/__init__.py +++ b/recipes/recipe_modules/tryserver/__init__.py @@ -14,5 +14,4 @@ DEPS = [ 'recipe_engine/python', 'recipe_engine/raw_io', 'recipe_engine/step', - 'rietveld', ] diff --git a/recipes/recipe_modules/tryserver/api.py b/recipes/recipe_modules/tryserver/api.py index e434d3033..a970e1ffc 100644 --- a/recipes/recipe_modules/tryserver/api.py +++ b/recipes/recipe_modules/tryserver/api.py @@ -16,16 +16,8 @@ class TryserverApi(recipe_api.RecipeApi): @property def is_tryserver(self): - """Returns true iff we can apply_issue or patch.""" - return ( - self.can_apply_issue or self.is_patch_in_git or self.is_gerrit_issue) - - @property - def can_apply_issue(self): - """Returns true iff the properties exist to apply_issue from rietveld.""" - return (self.m.properties.get('rietveld') - and 'issue' in self.m.properties - and 'patchset' in self.m.properties) + """Returns true iff we have a change to check out.""" + return (self.is_patch_in_git or self.is_gerrit_issue) @property def is_gerrit_issue(self): @@ -43,7 +35,7 @@ class TryserverApi(recipe_api.RecipeApi): self.m.properties.get('patch_repo_url') and self.m.properties.get('patch_ref')) - def get_files_affected_by_patch(self, patch_root=None, **kwargs): + def get_files_affected_by_patch(self, patch_root, **kwargs): """Returns list of paths to files affected by the patch. Argument: @@ -51,16 +43,7 @@ class TryserverApi(recipe_api.RecipeApi): api.gclient.calculate_patch_root(patch_project) Returned paths will be relative to to patch_root. - - TODO(tandrii): remove this doc. - Unless you use patch_root=None, in which case old behavior is used - which returns paths relative to checkout aka solution[0].name. """ - # patch_root must be set! None is for backwards compataibility and will be - # removed. - if patch_root is None: - return self._old_get_files_affected_by_patch() - cwd = self.m.context.cwd or self.m.path['start_dir'].join(patch_root) with self.m.context(cwd=cwd): step_result = self.m.git( @@ -79,29 +62,6 @@ class TryserverApi(recipe_api.RecipeApi): step_result.presentation.logs['files'] = paths return paths - - def _old_get_files_affected_by_patch(self): - issue_root = self.m.rietveld.calculate_issue_root() - cwd = self.m.path['checkout'].join(issue_root) if issue_root else None - - with self.m.context(cwd=cwd): - step_result = self.m.git( - '-c', 'core.quotePath=false', 'diff', '--cached', '--name-only', - name='git diff to analyze patch', - stdout=self.m.raw_io.output(), - step_test_data=lambda: - self.m.raw_io.test_api.stream_output('foo.cc')) - paths = step_result.stdout.split() - if issue_root: - paths = [self.m.path.join(issue_root, path) for path in paths] - if self.m.platform.is_win: - # Looks like "analyze" wants POSIX slashes even on Windows (since git - # uses that format even on Windows). - paths = [path.replace('\\', '/') for path in paths] - - step_result.presentation.logs['files'] = paths - return paths - def set_subproject_tag(self, subproject_tag): """Adds a subproject tag to the build. @@ -188,19 +148,10 @@ class TryserverApi(recipe_api.RecipeApi): git-footers documentation for more information. """ if patch_text is None: - if self.is_gerrit_issue: - patch_text = self.m.gerrit.get_change_description( - self.m.properties['patch_gerrit_url'], - self.m.properties['patch_issue'], - self.m.properties['patch_set']) - elif self.can_apply_issue: # pragma: no cover - patch_url = ( - self.m.properties['rietveld'].rstrip('/') + '/' + - str(self.m.properties['issue'])) - patch_text = self.m.git_cl.get_description( - patch_url=patch_url, codereview='rietveld').stdout - else: # pragma: no cover - raise recipe_api.StepFailure('Unknown patch storage.') + patch_text = self.m.gerrit.get_change_description( + self.m.properties['patch_gerrit_url'], + self.m.properties['patch_issue'], + self.m.properties['patch_set']) result = self.m.python( 'parse description', self.package_repo_resource('git_footers.py'), diff --git a/recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch.json b/recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch.json index ff8071886..f688c9aed 100644 --- a/recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch.json +++ b/recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch.json @@ -8,7 +8,6 @@ "--cached", "--name-only" ], - "cwd": "[START_DIR]", "infra_step": true, "name": "git diff to analyze patch", "stdout": "/path/to/tmp/", diff --git a/recipes/recipe_modules/tryserver/examples/full.py b/recipes/recipe_modules/tryserver/examples/full.py index 0a66a02de..d2548d7a4 100644 --- a/recipes/recipe_modules/tryserver/examples/full.py +++ b/recipes/recipe_modules/tryserver/examples/full.py @@ -28,7 +28,7 @@ def RunSteps(api): 'Foo', api.properties['patch_text']))]) return - if api.tryserver.can_apply_issue or api.tryserver.is_gerrit_issue: + if api.tryserver.is_gerrit_issue: api.tryserver.get_footers() api.tryserver.get_files_affected_by_patch( api.properties.get('test_patch_root')) @@ -50,22 +50,29 @@ def RunSteps(api): def GenTests(api): description_step = api.override_step_data( 'git_cl description', stdout=api.raw_io.output_text('foobar')) + # The 'test_patch_root' property used below is just so that these + # tests can avoid using the gclient module to calculate the + # patch root. Normal users would use gclient.calculate_patch_root(). yield (api.test('with_git_patch') + api.properties( - path_config='buildbot', - patch_storage='git', - patch_project='v8', - patch_repo_url='http://patch.url/', - patch_ref='johndoe#123.diff')) + path_config='buildbot', + patch_storage='git', + patch_project='v8', + patch_repo_url='http://patch.url/', + patch_ref='johndoe#123.diff', + test_patch_root='v8')) yield (api.test('with_git_patch_luci') + api.properties( patch_storage='git', patch_project='v8', patch_repo_url='http://patch.url/', - patch_ref='johndoe#123.diff')) + patch_ref='johndoe#123.diff', + test_patch_root='v8')) - yield (api.test('with_wrong_patch') + api.platform('win', 32)) + yield (api.test('with_wrong_patch') + + api.platform('win', 32) + + api.properties(test_patch_root='')) yield (api.test('with_gerrit_patch') + api.properties.tryserver(gerrit_project='infra/infra'))