[Workflow] Expand code-format-helper.py error reporting

 * Consider the darker/clang-format command to have failed if the
   exit code is non-zero, regardless of the stdout/stderr output.
 * Allow stderr from the formatter command to propagate to the script's
   caller (and into the GitHub log).
 * On success, dump stdout to the caller, so it ends up in GitHub's
   log. I'm not sure what this would ever be, but if it exists, it
   should be preserved.
 * Just before the script exits, if any formatter failed, print a line
   showing which formatters failed.

Replace [str] with list[str] to satisfy mypy/pytype.

Author: rprichard

llvm/utils/git/code-format-helper.py

@@ -36,10 +36,8 @@ def format_run(
     ) -> str | None:
         raise NotImplementedError()
 
-    def pr_comment_text(self, diff: str) -> str:
+    def pr_comment_text_for_diff(self, diff: str) -> str:
         return f"""
-{self.comment_tag}
-
 :warning: {self.friendly_name}, {self.name} found issues in your code. :warning:
 
 <details>
@@ -73,39 +71,41 @@ def find_comment(
                 return comment
         return None
 
-    def update_pr(self, diff: str, args: argparse.Namespace) -> None:
+    def update_pr(
+        self, comment_text: str, args: argparse.Namespace, create_new: bool
+    ) -> None:
         repo = github.Github(args.token).get_repo(args.repo)
         pr = repo.get_issue(args.issue_number).as_pull_request()
 
-        existing_comment = self.find_comment(pr)
-        pr_text = self.pr_comment_text(diff)
-
-        if existing_comment:
-            existing_comment.edit(pr_text)
-        else:
-            pr.as_issue().create_comment(pr_text)
-
-    def update_pr_success(self, args: argparse.Namespace) -> None:
-        repo = github.Github(args.token).get_repo(args.repo)
-        pr = repo.get_issue(args.issue_number).as_pull_request()
+        comment_text = self.comment_tag + "\n\n" + comment_text
 
         existing_comment = self.find_comment(pr)
         if existing_comment:
-            existing_comment.edit(
-                f"""
-{self.comment_tag}
-:white_check_mark: With the latest revision this PR passed the {self.friendly_name}.
-"""
-            )
+            existing_comment.edit(comment_text)
+        elif create_new:
+            pr.as_issue().create_comment(comment_text)
 
     def run(self, changed_files: list[str], args: argparse.Namespace) -> bool:
+        print(f"Formatter {self.name} ({self.friendly_name}):")
         diff = self.format_run(changed_files, args)
-        if diff:
-            self.update_pr(diff, args)
+        if diff is None:
+            comment_text = f"""
+:white_check_mark: With the latest revision this PR passed the {self.friendly_name}.
+"""
+            self.update_pr(comment_text, args, create_new=False)
+            return True
+        elif len(diff) > 0:
+            comment_text = self.pr_comment_text_for_diff(diff)
+            self.update_pr(comment_text, args, create_new=True)
             return False
         else:
-            self.update_pr_success(args)
-            return True
+            # The formatter failed but didn't output a diff (e.g. some sort of
+            # infrastructure failure).
+            comment_text = f"""
+:warning: The {self.friendly_name} failed without printing a diff. Check the logs for stderr output. :warning:
+"""
+            self.update_pr(comment_text, args, create_new=False)
+            return False
 
 
 class ClangFormatHelper(FormatHelper):
@@ -151,21 +151,23 @@ def format_run(
         ] + cpp_files
         print(f"Running: {' '.join(cf_cmd)}")
         self.cf_cmd = cf_cmd
-        proc = subprocess.run(cf_cmd, capture_output=True)
+        proc = subprocess.run(cf_cmd, stdout=subprocess.PIPE)
 
-        # formatting needed
-        if proc.returncode == 1:
+        if proc.returncode != 0:
+            # formatting needed, or the command otherwise failed
+            print(f"error: {self.name} exited with code {proc.returncode}")
             return proc.stdout.decode("utf-8")
-
-        return None
+        else:
+            sys.stdout.write(proc.stdout.decode("utf-8"))
+            return None
 
 
 class DarkerFormatHelper(FormatHelper):
     name = "darker"
     friendly_name = "Python code formatter"
 
     @property
-    def instructions(self):
+    def instructions(self) -> str:
         return " ".join(self.darker_cmd)
 
     def filter_changed_files(self, changed_files: list[str]) -> list[str]:
@@ -192,13 +194,15 @@ def format_run(
         ] + py_files
         print(f"Running: {' '.join(darker_cmd)}")
         self.darker_cmd = darker_cmd
-        proc = subprocess.run(darker_cmd, capture_output=True)
+        proc = subprocess.run(darker_cmd, stdout=subprocess.PIPE)
 
-        # formatting needed
-        if proc.returncode == 1:
+        if proc.returncode != 0:
+            # formatting needed, or the command otherwise failed
+            print(f"error: {self.name} exited with code {proc.returncode}")
             return proc.stdout.decode("utf-8")
-
-        return None
+        else:
+            sys.stdout.write(proc.stdout.decode("utf-8"))
+            return None
 
 
 ALL_FORMATTERS = (DarkerFormatHelper(), ClangFormatHelper())
@@ -236,9 +240,11 @@ def format_run(
     if args.changed_files:
         changed_files = args.changed_files.split(",")
 
-    exit_code = 0
+    failed_formatters = []
     for fmt in ALL_FORMATTERS:
         if not fmt.run(changed_files, args):
-            exit_code = 1
+            failed_formatters.append(fmt.name)
 
-    sys.exit(exit_code)
+    if len(failed_formatters) > 0:
+        print(f"error: some formatters failed: {' '.join(failed_formatters)}")
+        sys.exit(1)