diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index a7d135a01..50c2e1dbc 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -58,7 +58,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#539)(self, bot_update_step):** +— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#529)(self, bot_update_step):** Deapplies a patch, taking care of DEPS and solution revisions properly. @@ -95,7 +95,7 @@ Args: bot_update module ONLY supports one change. Users may specify a change via tryserver.set_change() and explicitly set this flag False. -— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#516)(self, project_name, gclient_config=None):** +— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#506)(self, project_name, gclient_config=None):** Returns all property names used for storing the checked-out revision of a given project. @@ -111,7 +111,7 @@ Returns (list of str): All properties that'll hold the checked-out revision   **@property**
— **def [last\_returned\_properties](/recipes/recipe_modules/bot_update/api.py#45)(self):** -— **def [resolve\_fixed\_revision](/recipes/recipe_modules/bot_update/api.py#467)(self, bot_update_json, name):** +— **def [resolve\_fixed\_revision](/recipes/recipe_modules/bot_update/api.py#457)(self, bot_update_json, name):** Sets a fixed revision for a single dependency using project revision properties. diff --git a/recipes/recipe_modules/bot_update/api.py b/recipes/recipe_modules/bot_update/api.py index 932e75108..e212b0c38 100644 --- a/recipes/recipe_modules/bot_update/api.py +++ b/recipes/recipe_modules/bot_update/api.py @@ -313,12 +313,7 @@ class BotUpdateApi(recipe_api.RecipeApi): # Ah hah! Now that everything is in place, lets run bot_update! step_result = None try: - # 87 and 88 are the 'patch failure' codes for patch download and patch - # apply, respectively. We don't actually use the error codes, and instead - # rely on emitted json to determine cause of failure. - step_result = self( - name, cmd, step_test_data=step_test_data, - ok_ret=(0, 87, 88), **kwargs) + step_result = self(name, cmd, step_test_data=step_test_data, **kwargs) except self.m.step.StepFailure as f: step_result = f.result raise @@ -338,6 +333,24 @@ class BotUpdateApi(recipe_api.RecipeApi): step_text = result['step_text'] step_result.presentation.step_text = step_text + if result.get('patch_failure'): + patch_body = result.get('failed_patch_body') + if patch_body: + step_result.presentation.logs['patch error'] = ( + patch_body.splitlines()) + + if result.get('patch_apply_return_code') == 3: + # This is download failure, hence an infra failure. + raise self.m.step.InfraFailure( + 'Patch failure: Git reported a download failure') + else: + # This is actual patch failure. + self.m.tryserver.set_patch_failure_tryjob_result() + self.m.cq.set_do_not_retry_build() + raise self.m.step.StepFailure( + 'Patch failure: See patch error log attached to bot_update. ' + 'Try rebasing?') + if add_blamelists and 'manifest' in result: blamelist_pins = [] for name in revisions: @@ -399,29 +412,6 @@ class BotUpdateApi(recipe_api.RecipeApi): # Set the "checkout" path for the main solution. # This is used by the Chromium module to figure out where to look for # the checkout. - # If there is a patch failure, emit another step that said things - # failed. - if result.get('patch_failure'): - return_code = result.get('patch_apply_return_code') - patch_body = result.get('failed_patch_body') - try: - if return_code == 3: - # This is download failure, hence an infra failure. - with self.m.context(infra_steps=True): - self.m.python.failing_step( - 'Patch failure', 'Git reported a download failure') - else: - # This is actual patch failure. - self.m.tryserver.set_patch_failure_tryjob_result() - self.m.cq.set_do_not_retry_build() - self.m.python.failing_step( - 'Patch failure', 'See attached log. Try rebasing?') - except self.m.step.StepFailure as e: - if patch_body: - e.result.presentation.logs['patch error'] = ( - patch_body.splitlines()) - raise e - # bot_update actually just sets root to be the folder name of the # first solution. if (result.get('did_run') 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 09f5540c4..610897618 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 @@ -142,6 +142,19 @@ "@@@STEP_LOG_LINE@json.output@ \"step_text\": \"Some step text\"@@@", "@@@STEP_LOG_LINE@json.output@}@@@", "@@@STEP_LOG_END@json.output@@@", + "@@@STEP_LOG_LINE@patch error@Downloading patch...@@@", + "@@@STEP_LOG_LINE@patch error@Applying the patch...@@@", + "@@@STEP_LOG_LINE@patch error@Patch: foo/bar.py@@@", + "@@@STEP_LOG_LINE@patch error@Index: foo/bar.py@@@", + "@@@STEP_LOG_LINE@patch error@diff --git a/foo/bar.py b/foo/bar.py@@@", + "@@@STEP_LOG_LINE@patch error@index HASH..HASH MODE@@@", + "@@@STEP_LOG_LINE@patch error@--- a/foo/bar.py@@@", + "@@@STEP_LOG_LINE@patch error@+++ b/foo/bar.py@@@", + "@@@STEP_LOG_LINE@patch error@context@@@", + "@@@STEP_LOG_LINE@patch error@+something@@@", + "@@@STEP_LOG_LINE@patch error@-something@@@", + "@@@STEP_LOG_LINE@patch error@more context@@@", + "@@@STEP_LOG_END@patch error@@@", "@@@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\"@@@", @@ -149,14 +162,8 @@ "@@@SET_BUILD_PROPERTY@got_revision@\"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@", "@@@SET_BUILD_PROPERTY@got_revision_cp@\"refs/heads/master@{#170242}\"@@@", "@@@SET_BUILD_PROPERTY@got_v8_revision@\"801ada225ddc271c132c3a35f03975671d43e399\"@@@", - "@@@SET_BUILD_PROPERTY@got_v8_revision_cp@\"refs/heads/master@{#43426}\"@@@" - ] - }, - { - "cmd": [], - "name": "set_output_gitiles_commit", - "~followup_annotations": [ - "@@@SET_BUILD_PROPERTY@$recipe_engine/buildbucket/output_gitiles_commit@{\"host\": \"fake.org\", \"id\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\", \"position\": 170242, \"project\": \"src\", \"ref\": \"refs/heads/master\"}@@@" + "@@@SET_BUILD_PROPERTY@got_v8_revision_cp@\"refs/heads/master@{#43426}\"@@@", + "@@@STEP_EXCEPTION@@@" ] }, { @@ -176,35 +183,10 @@ "@@@SET_BUILD_PROPERTY@do_not_retry@true@@@" ] }, - { - "cmd": [ - "python", - "-u", - "import sys; sys.exit(1)" - ], - "name": "Patch failure", - "~followup_annotations": [ - "@@@STEP_TEXT@See attached log. Try rebasing?@@@", - "@@@STEP_LOG_LINE@patch error@Downloading patch...@@@", - "@@@STEP_LOG_LINE@patch error@Applying the patch...@@@", - "@@@STEP_LOG_LINE@patch error@Patch: foo/bar.py@@@", - "@@@STEP_LOG_LINE@patch error@Index: foo/bar.py@@@", - "@@@STEP_LOG_LINE@patch error@diff --git a/foo/bar.py b/foo/bar.py@@@", - "@@@STEP_LOG_LINE@patch error@index HASH..HASH MODE@@@", - "@@@STEP_LOG_LINE@patch error@--- a/foo/bar.py@@@", - "@@@STEP_LOG_LINE@patch error@+++ b/foo/bar.py@@@", - "@@@STEP_LOG_LINE@patch error@context@@@", - "@@@STEP_LOG_LINE@patch error@+something@@@", - "@@@STEP_LOG_LINE@patch error@-something@@@", - "@@@STEP_LOG_LINE@patch error@more context@@@", - "@@@STEP_LOG_END@patch error@@@", - "@@@STEP_FAILURE@@@" - ] - }, { "failure": { "failure": {}, - "humanReason": "Step('Patch failure') (retcode: 1)" + "humanReason": "Patch failure: See patch error log attached to bot_update. Try rebasing?" }, "name": "$result" } diff --git a/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail_patch_download.json b/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail_patch_download.json index 7754d803b..2f7028abd 100644 --- a/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail_patch_download.json +++ b/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail_patch_download.json @@ -142,33 +142,6 @@ "@@@STEP_LOG_LINE@json.output@ \"step_text\": \"Some step text\"@@@", "@@@STEP_LOG_LINE@json.output@}@@@", "@@@STEP_LOG_END@json.output@@@", - "@@@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\"@@@", - "@@@SET_BUILD_PROPERTY@got_cr_revision_cp@\"refs/heads/master@{#170242}\"@@@", - "@@@SET_BUILD_PROPERTY@got_revision@\"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@", - "@@@SET_BUILD_PROPERTY@got_revision_cp@\"refs/heads/master@{#170242}\"@@@", - "@@@SET_BUILD_PROPERTY@got_v8_revision@\"801ada225ddc271c132c3a35f03975671d43e399\"@@@", - "@@@SET_BUILD_PROPERTY@got_v8_revision_cp@\"refs/heads/master@{#43426}\"@@@" - ] - }, - { - "cmd": [], - "name": "set_output_gitiles_commit", - "~followup_annotations": [ - "@@@SET_BUILD_PROPERTY@$recipe_engine/buildbucket/output_gitiles_commit@{\"host\": \"fake.org\", \"id\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\", \"position\": 170242, \"project\": \"src\", \"ref\": \"refs/heads/master\"}@@@" - ] - }, - { - "cmd": [ - "python", - "-u", - "import sys; sys.exit(1)" - ], - "infra_step": true, - "name": "Patch failure", - "~followup_annotations": [ - "@@@STEP_TEXT@Git reported a download failure@@@", "@@@STEP_LOG_LINE@patch error@Downloading patch...@@@", "@@@STEP_LOG_LINE@patch error@Applying the patch...@@@", "@@@STEP_LOG_LINE@patch error@Patch: foo/bar.py@@@", @@ -182,12 +155,20 @@ "@@@STEP_LOG_LINE@patch error@-something@@@", "@@@STEP_LOG_LINE@patch error@more context@@@", "@@@STEP_LOG_END@patch error@@@", + "@@@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\"@@@", + "@@@SET_BUILD_PROPERTY@got_cr_revision_cp@\"refs/heads/master@{#170242}\"@@@", + "@@@SET_BUILD_PROPERTY@got_revision@\"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@", + "@@@SET_BUILD_PROPERTY@got_revision_cp@\"refs/heads/master@{#170242}\"@@@", + "@@@SET_BUILD_PROPERTY@got_v8_revision@\"801ada225ddc271c132c3a35f03975671d43e399\"@@@", + "@@@SET_BUILD_PROPERTY@got_v8_revision_cp@\"refs/heads/master@{#43426}\"@@@", "@@@STEP_EXCEPTION@@@" ] }, { "failure": { - "humanReason": "Infra Failure: Step('Patch failure') (retcode: 1)" + "humanReason": "Patch failure: Git reported a download failure" }, "name": "$result" }