ci: post the LOC diff as a sticky pull request comment

.github/workflows/line-count.yml

@@ -1,58 +1,55 @@
 name: Lines of Code
 
+# pull_request_target: fork PRs are read-only under pull_request and cannot be
+# commented on. Nothing in pr/ may ever be executed.
+# Unfiltered by path: a PR that stops touching src/ must still run to clean up.
 on:
-  push:
-    branches: [master]
-  pull_request:
-  workflow_dispatch:
+  pull_request_target:
 
-jobs:
-  file-changes:
-    name: Detect File Changes
-    runs-on: 'ubuntu-latest'
-    outputs: 
-      checkall: ${{ steps.changes.outputs.checkall }}
-    steps:
-      - name: Clone
-        uses: actions/checkout@v5
-
-      - name: Detect Changes
-        uses: dorny/paths-filter@v4
-        id: changes
-        with: 
-          filters: ".github/file-filter.yml"
+concurrency:
+  group: ${{ github.workflow }}-${{ github.event.number }}
+  cancel-in-progress: true
 
+jobs:
   sz:
     name: Core MFC Line Difference
-    if: needs.file-changes.outputs.checkall == 'true'
-    needs: file-changes
     permissions:
       contents: read
       pull-requests: write
     runs-on: ubuntu-latest
     steps:
-      - name: Checkout code from PR branch
+      - name: Checkout the base branch
         uses: actions/checkout@v5
         with:
-          path: pr
-
-      - name: Checkout code from MFC master
-        uses: actions/checkout@v5
-        with:
-          repository: ${{ github.event.pull_request.repository }}
-          ref:        ${{ github.event.pull_request.base.ref }}
-          path:       base
-          # repository: MFlowCode/MFC
-          # ref:        master
-          # path:       base
+          path: base
+          fetch-depth: 0
 
+      # git worktree, not actions/checkout: no untrusted checkout in a
+      # privileged workflow. Merge base, not branch tip: a stale branch is not
+      # charged for other people's lines.
       - name: Get Line Diff
+        id: diff
+        working-directory: base
         run: |
-          BASE="$GITHUB_WORKSPACE/base"
-          PR="$GITHUB_WORKSPACE/pr"
-          cd $BASE
-          export MFC_PR=$PR
-          pwd
-          ./mfc.sh init &> tmp.txt
-          ./mfc.sh count_diff
+          git fetch --no-tags origin "+refs/pull/${{ github.event.number }}/head:refs/mfc/pr"
+          git worktree add ../pr refs/mfc/pr
+          git worktree add ../mb "$(git merge-base HEAD refs/mfc/pr)"
+          ./mfc.sh count_diff ../mb ../pr --markdown "$GITHUB_WORKSPACE/loc.md"
+          test -s "$GITHUB_WORKSPACE/loc.md" && echo "changed=true" >> "$GITHUB_OUTPUT" || echo "changed=false" >> "$GITHUB_OUTPUT"
 
+      - name: Comment Line Diff
+        if: steps.diff.outputs.changed == 'true'
+        uses: marocchino/sticky-pull-request-comment@v3
+        with:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          skip_unchanged: true
+          recreate: true
+          path: loc.md
+
+      # No diff: take down any comment an earlier push left.
+      - name: Remove Line Diff Comment
+        if: steps.diff.outputs.changed == 'false'
+        uses: marocchino/sticky-pull-request-comment@v3
+        with:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          delete: true

toolchain/mfc/args.py

@@ -101,7 +101,7 @@ def custom_error(message):
     # Add default arguments of other subparsers
     # This ensures all argument keys exist even for commands that don't define them
     # Only process subparsers that have common arguments we need
-    relevant_subparsers = ["run", "test", "build", "clean", "count", "count_diff", "validate", "viz"]
+    relevant_subparsers = ["run", "test", "build", "clean", "count", "validate", "viz"]
     for name in relevant_subparsers:
         if args["command"] == name:
             continue

toolchain/mfc/cli/commands.py

@@ -876,8 +876,24 @@
 
 COUNT_DIFF_COMMAND = Command(
     name="count_diff",
-    help="Compare LOC between branches.",
+    help="Compare LOC between two MFC source trees.",
     include_common=["targets", "mfc_config", "jobs", "verbose", "debug_log"],
+    positionals=[
+        Positional(name="base", help="Path to the baseline MFC source tree.", completion=Completion(type=CompletionType.DIRECTORIES)),
+        Positional(name="pr", help="Path to the MFC source tree to compare against the baseline.", completion=Completion(type=CompletionType.DIRECTORIES)),
+    ],
+    arguments=[
+        Argument(
+            name="markdown",
+            help="Also write the diff as GitHub-flavored Markdown to PATH (empty if nothing changed).",
+            type=str,
+            metavar="PATH",
+            completion=Completion(type=CompletionType.FILES),
+        ),
+    ],
+    examples=[
+        Example("./mfc.sh count_diff ../master .", "Compare this tree's LOC against another checkout"),
+    ],
 )
 
 FP_STABILITY_COMMAND = Command(

toolchain/mfc/count.py

@@ -4,7 +4,7 @@
 
 import rich.table
 
-from .common import MFC_ROOT_DIR, MFCException, format_list_to_string
+from .common import MFC_ROOT_DIR, file_write, format_list_to_string
 from .printer import cons
 from .state import ARG
 
@@ -13,28 +13,26 @@ def handle_dir(mfc_dir: str, srcdirname: str) -> typing.Tuple[typing.Dict[str, i
     files = {}
     total = 0
 
-    for filepath in glob.glob(os.path.join(mfc_dir, "src", srcdirname, "*.*f*")):
-        with open(filepath) as f:
-            counter = 0
-            for line in f.read().split("\n"):
-                # Skip whitespace
-                if line.isspace() or len(line) == 0:
-                    continue
-                # Skip comments but not !$acc ones!
-                if line.lstrip().startswith("!") and not line.lstrip().startswith("!$acc"):
-                    continue
-                counter += 1
-
-            files[os.path.relpath(filepath, mfc_dir)] = counter
-            total += counter
+    # Untrusted tree in CI: a symlink must not reach off it.
+    root = os.path.realpath(os.path.join(mfc_dir, "src")) + os.sep
+
+    for filepath in glob.glob(os.path.join(mfc_dir, "src", srcdirname, "**", "*.*f*"), recursive=True):
+        if not os.path.isfile(filepath) or not os.path.realpath(filepath).startswith(root):
+            continue
+
+        with open(filepath, errors="replace") as f:
+            counter = sum(1 for line in f if line.strip() and not line.lstrip().startswith("!"))
+
+        files[os.path.relpath(filepath, mfc_dir)] = counter
+        total += counter
 
     return (files, total)
 
 
 def count():
     target_str_list = format_list_to_string(ARG("targets"), "magenta")
 
-    cons.print(f"[bold]Counting lines of code in {target_str_list}[/bold] (excluding whitespace lines)")
+    cons.print(f"[bold]Counting lines of code in {target_str_list}[/bold] (excluding whitespace and comment lines)")
     cons.indent()
 
     total = 0
@@ -44,8 +42,8 @@ def count():
         table.add_column(f"File (in [magenta]{codedir}[/magenta])", justify="left")
         table.add_column(f"Lines ([cyan]{dircount}[/cyan])", justify="right")
 
-        for filepath, n in dirfiles.items():
-            table.add_row(os.path.basename(filepath), f"[bold cyan]{n}[/bold cyan]")
+        for filepath, n in sorted(dirfiles.items()):
+            table.add_row(filepath, f"[bold cyan]{n}[/bold cyan]")
 
         total += dircount
 
@@ -56,51 +54,54 @@ def count():
     cons.unindent()
 
 
+def _write_markdown(filepath: str, files: list, dirs: list, total: int, total_diff: int):
+    # Empty file: the CI commenter posts nothing.
+    if not files:
+        file_write(filepath, "")
+        return
+
+    lines = ["### Lines of Code", "", "| File | Lines | Diff |", "| :--- | ---: | ---: |"]
+    lines += [f"| `{path}` | {n} | {diff:+d} |" for path, n, diff in files]
+    lines += ["", "| Directory | Lines | Diff |", "| :--- | ---: | ---: |"]
+    lines += [f"| {codedir} | {n} | {diff:+d} |" for codedir, n, diff in dirs if diff != 0]
+    lines += [f"| **total** | **{total}** | **{total_diff:+d}** |", ""]
+
+    file_write(filepath, "\n".join(lines))
+
+
 def count_diff():
+    base_dir, pr_dir = ARG("base"), ARG("pr")
     target_str_list = format_list_to_string(ARG("targets"), "magenta")
-    cons.print(f"[bold]Counting lines of code in {target_str_list}[/bold] (excluding whitespace lines)")
+
+    cons.print(f"[bold]Counting lines of code in {target_str_list}[/bold] (excluding whitespace and comment lines)")
     cons.indent()
 
-    total = 0
-    MFC_COMPARE_DIR = os.getenv("MFC_PR")
-    if MFC_COMPARE_DIR is None:
-        raise MFCException("MFC_PR is not in your environment.")
+    files, dirs, total, total_diff = [], [], 0, 0
+    for codedir in ["common"] + ARG("targets"):
+        base_files, base_count = handle_dir(base_dir, codedir)
+        pr_files, pr_count = handle_dir(pr_dir, codedir)
 
-    print("compare dir", MFC_COMPARE_DIR)
+        for filepath in set(base_files) | set(pr_files):
+            diff = pr_files.get(filepath, 0) - base_files.get(filepath, 0)
+            if diff != 0:
+                files.append((filepath, pr_files.get(filepath, 0), diff))
 
-    # MFC_COMPARE_DIR="/Users/spencer/Downloads/MFC-shbfork"
-    for codedir in ["common"] + ARG("targets"):
-        dirfiles_root, dircount_root = handle_dir(MFC_ROOT_DIR, codedir)
-        dirfiles_pr, dircount_pr = handle_dir(MFC_COMPARE_DIR, codedir)
-        table = rich.table.Table(show_header=True, box=rich.table.box.SIMPLE)
-        table.add_column(f"File (in [magenta]{codedir}[/magenta])", justify="left")
-        table.add_column(f"Lines [HEAD] ([cyan]{dircount_root}[/cyan])", justify="right")
-        table.add_column(f"Lines [PR] ([cyan]{dircount_pr}[/cyan])", justify="right")
-        table.add_column("", justify="right")
-        table.add_column("Diff", justify="right")
-
-        for filepath in set(dirfiles_root.keys()) | set(dirfiles_pr.keys()):
-            dirfiles_root[filepath] = dirfiles_root.get(filepath, 0)
-            dirfiles_pr[filepath] = dirfiles_pr.get(filepath, 0)
-
-            PLUS = "++ "
-            MINUS = "-- "
-
-            diff_count = dirfiles_pr[filepath] - dirfiles_root[filepath]
-            mycolor = "red" if diff_count > 0 else "green"
-            mysymbol = PLUS if diff_count > 0 else MINUS
-            table.add_row(
-                os.path.basename(filepath),
-                f"[bold cyan]{dirfiles_root[filepath]}[/bold cyan]",
-                f"[bold cyan]{dirfiles_pr[filepath]}[/bold cyan]",
-                mysymbol,
-                f"[bold {mycolor}]{diff_count}[/bold {mycolor}]",
-            )
-
-        total += dircount_root
+        dirs.append((codedir, pr_count, pr_count - base_count))
+        total, total_diff = total + pr_count, total_diff + pr_count - base_count
 
-        cons.raw.print(table)
+    files.sort(key=lambda entry: (-abs(entry[2]), entry[0]))
 
-    cons.print(f"[bold]Total {target_str_list} lines: [bold cyan]{total}[/bold cyan].[/bold]")
+    table = rich.table.Table(show_header=True, box=rich.table.box.SIMPLE)
+    table.add_column("Changed file", justify="left")
+    table.add_column("Lines", justify="right")
+    table.add_column("Diff", justify="right")
+    for path, n, diff in files:
+        table.add_row(path, f"[bold cyan]{n}[/bold cyan]", f"[bold {'red' if diff > 0 else 'green'}]{diff:+d}[/bold {'red' if diff > 0 else 'green'}]")
+    cons.raw.print(table)
+
+    cons.print(f"[bold]Total {target_str_list} lines: [bold cyan]{total}[/bold cyan] ([bold cyan]{total_diff:+d}[/bold cyan]).[/bold]")
     cons.print()
     cons.unindent()
+
+    if ARG("markdown") is not None:
+        _write_markdown(ARG("markdown"), files, dirs, total, total_diff)