Add tails of logs of failed packages to github comments

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

@@ -114,6 +114,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), review.head_commit)

nixpkgs_review/report.py

@@ -1,6 +1,7 @@
 import functools
 import json
 import os
+import re
 import socket
 import subprocess
 from collections.abc import Callable
@@ -12,11 +13,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 +67,40 @@ 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 remove_ansi_escape_sequences(text: str) -> str:
+    """Remove ANSI escape sequences from a string."""
+    ansi_escape_pattern = re.compile(r"\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])")
+    return ansi_escape_pattern.sub("", text)
+
+
+def html_logs_section(logs_dir: Path, packages: list[Attr], system: str) -> str:
+    res = ""
+    for pkg in packages:
+        tail = remove_ansi_escape_sequences(
+            get_file_tail(logs_dir / get_log_filename(pkg, system))
+        )
+        if tail:
+            if not res:
+                res = "\n---\n"
+                res += f"<details>\n<summary>Error logs: `{system}`</summary>\n"
+            res += f"<details>\n<summary>{pkg.name}</summary>\n<pre>{tail}</pre>\n</details>\n"
+    res += "</details>\n"
+    return res
+
+
 class LazyDirectory:
     def __init__(self, path: Path) -> None:
         self.path = path
@@ -112,7 +154,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"))
 
@@ -243,12 +285,14 @@ 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.commit = commit
         self.show_header = show_header
+        self.show_logs = show_logs
         self.max_workers = max_workers
         self.attrs = attrs_per_system
         self.checkout = checkout
@@ -274,10 +318,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"""
@@ -303,7 +347,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"
@@ -350,6 +394,22 @@ def markdown(self, pr: int | None) -> str:
             )
             msg += html_pkgs_section(":white_check_mark:", report.built, "built")
 
+        if self.show_logs:
+            truncated_msg = (
+                "\n---\n"
+                "WARNING: Some logs were not included in this report: there were too many."
+            )
+            for system, report in self.system_reports.items():
+                if not report.failed:
+                    continue
+                full_msg = msg
+                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 - len(truncated_msg):
+                    msg += truncated_msg
+                    break
+                msg = full_msg
+
         return msg
 
     def print_console(self, root: Path, pr: int | None) -> None:
@@ -358,7 +418,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
         self.head_commit: str | None = None
 
     def _process_aliases_for_systems(self, system: str) -> set[str]:
@@ -412,6 +414,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
@@ -422,7 +425,7 @@ def start_review(
         success = report.succeeded()
 
         if pr and post_result:
-            self.github_client.comment_issue(pr, report.markdown(pr))
+            self.github_client.comment_issue(pr, report.markdown(path, pr))
 
         if pr and approve_pr and success:
             self.github_client.approve_pr(
@@ -431,7 +434,7 @@ def start_review(
             )
 
         if print_result:
-            print(report.markdown(pr))
+            print(report.markdown(path, pr))
 
         if not self.no_shell:
             nix_shell(