Surface git errors in the build summary for presubmit builds

Occasionally someone will upload a CL whose diff was already landed in
another CL. In the past, the error message from the presubmit build
for their CL has led to confusion. So this tries to clarify it by:
- putting the git stdout from the git cmd in the build summary when it
  fails
- adding a hint about the identical diff in the build summary when it
  sees "nothing to commit" in git's stdout

eg: Before this CL, such builds fail like:
https://ci.chromium.org/ui/p/chromium/builders/try/chromium_presubmit/3035841/infra

With this CL, they'll fail like:
https://ci.chromium.org/ui/p/chromium/builders/try.shadow/chromium_presubmit/3883/infra

Bug: None
Change-Id: I8002e19efce3cae5a11d2e616e4db596afb3b50c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6039934
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Ben Pastene <bpastene@chromium.org>

recipes/README.recipes.md

@@ -825,7 +825,7 @@ Raises:
 — **def [initialize](/recipes/recipe_modules/osx_sdk/api.py#55)(self):**
 ### *recipe_modules* / [presubmit](/recipes/recipe_modules/presubmit)
 
-[DEPS](/recipes/recipe_modules/presubmit/__init__.py#13): [bot\_update](#recipe_modules-bot_update), [depot\_tools](#recipe_modules-depot_tools), [gclient](#recipe_modules-gclient), [git](#recipe_modules-git), [tryserver](#recipe_modules-tryserver), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/cv][recipe_engine/recipe_modules/cv], [recipe\_engine/json][recipe_engine/recipe_modules/json], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/resultdb][recipe_engine/recipe_modules/resultdb], [recipe\_engine/step][recipe_engine/recipe_modules/step]
+[DEPS](/recipes/recipe_modules/presubmit/__init__.py#13): [bot\_update](#recipe_modules-bot_update), [depot\_tools](#recipe_modules-depot_tools), [gclient](#recipe_modules-gclient), [git](#recipe_modules-git), [tryserver](#recipe_modules-tryserver), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/cv][recipe_engine/recipe_modules/cv], [recipe\_engine/json][recipe_engine/recipe_modules/json], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/raw\_io][recipe_engine/recipe_modules/raw_io], [recipe\_engine/resultdb][recipe_engine/recipe_modules/resultdb], [recipe\_engine/step][recipe_engine/recipe_modules/step]
 
 
 #### **class [PresubmitApi](/recipes/recipe_modules/presubmit/api.py#16)([RecipeApi][recipe_engine/wkt/RecipeApi]):**
@@ -834,7 +834,7 @@ Raises:
 
 Returns a presubmit step.
 
-— **def [execute](/recipes/recipe_modules/presubmit/api.py#103)(self, bot_update_step, skip_owners=False, run_all=False):**
+— **def [execute](/recipes/recipe_modules/presubmit/api.py#116)(self, bot_update_step, skip_owners=False, run_all=False):**
 
 Runs presubmit and sets summary markdown if applicable.
 
@@ -1212,10 +1212,10 @@ Move things around in a loop!
 — **def [RunSteps](/recipes/recipe_modules/presubmit/tests/execute.py#25)(api):**
 ### *recipes* / [presubmit:tests/prepare](/recipes/recipe_modules/presubmit/tests/prepare.py)
 
-[DEPS](/recipes/recipe_modules/presubmit/tests/prepare.py#11): [gclient](#recipe_modules-gclient), [presubmit](#recipe_modules-presubmit), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/runtime][recipe_engine/recipe_modules/runtime]
+[DEPS](/recipes/recipe_modules/presubmit/tests/prepare.py#11): [gclient](#recipe_modules-gclient), [presubmit](#recipe_modules-presubmit), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/raw\_io][recipe_engine/recipe_modules/raw_io], [recipe\_engine/runtime][recipe_engine/recipe_modules/runtime]
 
 
-— **def [RunSteps](/recipes/recipe_modules/presubmit/tests/prepare.py#28)(api, patch_project, patch_repository_url):**
+— **def [RunSteps](/recipes/recipe_modules/presubmit/tests/prepare.py#29)(api, patch_project, patch_repository_url):**
 ### *recipes* / [tryserver:examples/full](/recipes/recipe_modules/tryserver/examples/full.py)
 
 [DEPS](/recipes/recipe_modules/tryserver/examples/full.py#7): [gerrit](#recipe_modules-gerrit), [tryserver](#recipe_modules-tryserver), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [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/raw\_io][recipe_engine/recipe_modules/raw_io], [recipe\_engine/step][recipe_engine/recipe_modules/step]

recipes/recipe_modules/presubmit/__init__.py

@@ -11,19 +11,20 @@ from PB.recipe_modules.depot_tools.presubmit import properties
 PYTHON_VERSION_COMPATIBILITY = 'PY3'
 
 DEPS = [
-  'bot_update',
-  'depot_tools',
-  'gclient',
-  'git',
-  'recipe_engine/buildbucket',
-  'recipe_engine/context',
-  'recipe_engine/cv',
-  'recipe_engine/json',
-  'recipe_engine/path',
-  'recipe_engine/properties',
-  'recipe_engine/step',
-  'recipe_engine/resultdb',
-  'tryserver',
+    'bot_update',
+    'depot_tools',
+    'gclient',
+    'git',
+    'recipe_engine/buildbucket',
+    'recipe_engine/context',
+    'recipe_engine/cv',
+    'recipe_engine/json',
+    'recipe_engine/path',
+    'recipe_engine/properties',
+    'recipe_engine/raw_io',
+    'recipe_engine/resultdb',
+    'recipe_engine/step',
+    'tryserver',
 ]
 
 

recipes/recipe_modules/presubmit/api.py

@@ -81,18 +81,31 @@ class PresubmitApi(recipe_api.RecipeApi):
         # TODO(unowned): Consider either:
         #  - extracting user name & email address from the issue, or
         #  - using a dedicated and clearly nonexistent name/email address
-        self.m.git('-c',
-                   'user.email=commit-bot@chromium.org',
-                   '-c',
-                   'user.name=The Commit Bot',
-                   '-c',
-                   'diff.ignoreSubmodules=all',
-                   'commit',
-                   '-a',
-                   '-m',
-                   'Committed patch',
-                   name='commit-git-patch',
-                   infra_step=False)
+        step_result = self.m.git(
+            '-c',
+            'user.email=commit-bot@chromium.org',
+            '-c',
+            'user.name=The Commit Bot',
+            '-c',
+            'diff.ignoreSubmodules=all',
+            'commit',
+            '-a',
+            '-m',
+            'Committed patch',
+            name='commit-git-patch',
+            raise_on_failure=False,
+            stdout=self.m.raw_io.output_text('stdout',
+                                             add_output_log='on_failure'),
+            infra_step=False,
+        )
+        if step_result.retcode:
+          failure_md_lines = ['Failed to apply patch.']
+          if step_result.stdout:
+            failure_md_lines += step_result.stdout.splitlines() + ['']
+            if 'nothing to commit' in step_result.stdout:
+              failure_md_lines.append(
+                  'Was an identical diff already submitted elsewhere?')
+          raise self.m.step.StepFailure('<br/>'.join(failure_md_lines))
 
     if self._runhooks:
       with self.m.context(cwd=self.m.path.checkout_dir):

recipes/recipe_modules/presubmit/tests/prepare.py

@@ -9,13 +9,14 @@ from recipe_engine import recipe_api
 PYTHON_VERSION_COMPATIBILITY = 'PY3'
 
 DEPS = [
-  'gclient',
-  'presubmit',
-  'recipe_engine/buildbucket',
-  'recipe_engine/context',
-  'recipe_engine/path',
-  'recipe_engine/properties',
-  'recipe_engine/runtime',
+    'gclient',
+    'presubmit',
+    'recipe_engine/buildbucket',
+    'recipe_engine/context',
+    'recipe_engine/path',
+    'recipe_engine/properties',
+    'recipe_engine/raw_io',
+    'recipe_engine/runtime',
 ]
 
 
@@ -43,3 +44,21 @@ def GenTests(api):
          api.post_process(post_process.MustRun, 'gclient runhooks') +
          api.post_process(post_process.StatusSuccess) +
          api.post_process(post_process.DropExpectation))
+
+  yield api.test(
+      'failed_commit',
+      api.runtime(is_experimental=False),
+      api.buildbucket.try_build(project='infra'),
+      api.step_data(
+          'commit-git-patch',
+          retcode=1,
+          stdout=api.raw_io.output_text(
+              'nothing to commit, working tree clean'),
+      ),
+      api.expect_status('FAILURE'),
+      api.post_check(
+          post_process.SummaryMarkdownRE,
+          'Was an identical diff already submitted elsewhere?',
+      ),
+      api.post_process(post_process.DropExpectation),
+  )