diff --git a/.github/workflows/line-count.yml b/.github/workflows/line-count.yml index f7d2ec96a..23c64e3fa 100644 --- a/.github/workflows/line-count.yml +++ b/.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 diff --git a/toolchain/mfc/args.py b/toolchain/mfc/args.py index c72e895f9..031e2db9b 100644 --- a/toolchain/mfc/args.py +++ b/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 diff --git a/toolchain/mfc/cli/commands.py b/toolchain/mfc/cli/commands.py index a96484809..e177ec81f 100644 --- a/toolchain/mfc/cli/commands.py +++ b/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( diff --git a/toolchain/mfc/count.py b/toolchain/mfc/count.py index 3e809c1eb..5a1b1b7a8 100644 --- a/toolchain/mfc/count.py +++ b/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,20 +13,18 @@ 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) @@ -34,7 +32,7 @@ def handle_dir(mfc_dir: str, srcdirname: str) -> typing.Tuple[typing.Dict[str, i 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)