Add tails of logs of failed packages to github comments

Snippets will be hidden under dropdown sections so as not to clutter UI.
Snippets are limited to 20 lines, and if too many logs are generated,
the report is truncated so that it fits into a single github comment.

Signed-off-by: Ihar Hrachyshka <[email protected]>

Author: booxter

README.md

@@ -119,6 +119,13 @@ pass the `--post-result` flag:
 $ nixpkgs-review pr --post-result 37242
 ```
 
+If you'd like to exclude log snippets for failed builds, add the `--no-logs`
+flag:
+
+```console
+$ nixpkgs-review pr --post-result --no-logs 37242
+```
+
 Instead of posting a PR comment, nixpkgs-review can also print the report to the
 terminal using the `--print-result` flag. This flag will work for the `rev` and
 `wip` command..

nixpkgs_review/cli/__init__.py

@@ -71,6 +71,11 @@ def pr_flags(
         action="store_true",
         help="Do not render the header in the markdown report",
     )
+    pr_parser.add_argument(
+        "--no-logs",
+        action="store_true",
+        help="Do not include build error log snippets in the markdown report",
+    )
     pr_parser.set_defaults(func=pr_command)
     return pr_parser
 

nixpkgs_review/cli/pr.py

@@ -112,6 +112,7 @@ def pr_command(args: argparse.Namespace) -> str:
                     extra_nixpkgs_config=args.extra_nixpkgs_config,
                     num_parallel_evals=args.num_parallel_evals,
                     show_header=not args.no_headers,
+                    show_logs=not args.no_logs,
                 )
                 contexts.append((pr, builddir.path, review.build_pr(pr)))
             except NixpkgsReviewError as e:

nixpkgs_review/report.py

@@ -12,11 +12,18 @@
 from .nix import Attr
 from .utils import System, info, link, skipped, system_order_key, to_link, warn
 
+# https://github.com/orgs/community/discussions/27190
+MAX_GITHUB_COMMENT_LENGTH = 65536
+
 
 def get_log_filename(a: Attr, system: str) -> str:
     return f"{a.name}-{system}.log"
 
 
+def get_log_dir(root: Path) -> Path:
+    return root / "logs"
+
+
 def print_number(
     logs_dir: Path,
     system: str,
@@ -59,6 +66,28 @@ def html_pkgs_section(
     return res
 
 
+def get_file_tail(file: Path, lines: int = 20) -> str:
+    try:
+        with file.open("rb") as f:
+            f.seek(0, os.SEEK_END)
+            end = f.tell()
+            f.seek(max(end - lines * 1024, 0), os.SEEK_SET)
+            return "\n".join(
+                f.read().decode("utf-8", errors="replace").splitlines()[-lines:]
+            )
+    except OSError:
+        return ""
+
+
+def html_logs_section(logs_dir: Path, packages: list[Attr], system: str) -> str:
+    res = ""
+    for pkg in packages:
+        tail = get_file_tail(logs_dir / get_log_filename(pkg, system))
+        if tail:
+            res += f"<details>\n<summary>{pkg.name}</summary>\n<pre>{tail}</pre>\n</details>\n"
+    return res
+
+
 class LazyDirectory:
     def __init__(self, path: Path) -> None:
         self.path = path
@@ -112,7 +141,7 @@ def write_error_logs(
     *,
     max_workers: int | None = 1,
 ) -> None:
-    logs = LazyDirectory(directory.joinpath("logs"))
+    logs = LazyDirectory(get_log_dir(directory))
     results = LazyDirectory(directory.joinpath("results"))
     failed_results = LazyDirectory(directory.joinpath("failed_results"))
 
@@ -242,11 +271,13 @@ def __init__(
         skip_packages: set[str],
         skip_packages_regex: list[Pattern[str]],
         show_header: bool = True,
+        show_logs: bool = False,
         max_workers: int | None = 1,
         *,
         checkout: Literal["merge", "commit"] = "merge",
     ) -> None:
         self.show_header = show_header
+        self.show_logs = show_logs
         self.max_workers = max_workers
         self.attrs = attrs_per_system
         self.checkout = checkout
@@ -272,10 +303,10 @@ def built_packages(self) -> dict[System, list[str]]:
         }
 
     def write(self, directory: Path, pr: int | None) -> None:
-        directory.joinpath("report.md").write_text(self.markdown(pr))
-        directory.joinpath("report.json").write_text(self.json(pr))
-
+        # write logs first because snippets from them may be needed for the report
         write_error_logs(self.attrs, directory, max_workers=self.max_workers)
+        directory.joinpath("report.md").write_text(self.markdown(directory, pr))
+        directory.joinpath("report.json").write_text(self.json(pr))
 
     def succeeded(self) -> bool:
         """Whether the report is considered a success or a failure"""
@@ -301,7 +332,7 @@ def json(self, pr: int | None) -> str:
             indent=4,
         )
 
-    def markdown(self, pr: int | None) -> str:
+    def markdown(self, root: Path, pr: int | None) -> str:
         msg = ""
         if self.show_header:
             msg += "## `nixpkgs-review` result\n\n"
@@ -346,6 +377,19 @@ def markdown(self, pr: int | None) -> str:
             )
             msg += html_pkgs_section(":white_check_mark:", report.built, "built")
 
+        if self.show_logs:
+            for system, report in self.system_reports.items():
+                if not report.failed:
+                    continue
+                full_msg = msg
+                full_msg += "\n---\n"
+                full_msg += f"### Error logs: `{system}`\n"
+                full_msg += html_logs_section(get_log_dir(root), report.failed, system)
+                # if the final message won't fit a single github comment, stop
+                if len(full_msg) > MAX_GITHUB_COMMENT_LENGTH:
+                    break
+                msg = full_msg
+
         return msg
 
     def print_console(self, root: Path, pr: int | None) -> None:
@@ -354,7 +398,7 @@ def print_console(self, root: Path, pr: int | None) -> None:
             info("\nLink to currently reviewing PR:")
             link(to_link(pr_url, pr_url))
 
-        logs_dir = root / "logs"
+        logs_dir = get_log_dir(root)
         for system, report in self.system_reports.items():
             info(f"--------- Report for '{system}' ---------")
             p = functools.partial(print_number, logs_dir, system)

nixpkgs_review/review.py

@@ -112,6 +112,7 @@ def __init__(
         sandbox: bool = False,
         num_parallel_evals: int = 1,
         show_header: bool = True,
+        show_logs: bool = False,
     ) -> None:
         if skip_packages_regex is None:
             skip_packages_regex = []
@@ -149,6 +150,7 @@ def __init__(
         self.extra_nixpkgs_config = extra_nixpkgs_config
         self.num_parallel_evals = num_parallel_evals
         self.show_header = show_header
+        self.show_logs = show_logs
 
     def _process_aliases_for_systems(self, system: str) -> set[str]:
         match system:
@@ -408,6 +410,7 @@ def start_review(
             skip_packages=self.skip_packages,
             skip_packages_regex=self.skip_packages_regex,
             show_header=self.show_header,
+            show_logs=self.show_logs,
             # we don't use self.num_parallel_evals here since its choice
             # is mainly capped by available RAM
             max_workers=min(32, os.cpu_count() or 1),  # 'None' assumes IO tasks