From 916117078d8bfb07a1ceeb8c61db6fc635667108 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 8 Aug 2026 18:50:16 -0700 Subject: [PATCH 1/2] Match .gitattributes semantics when asking which files a pin covers The dead-pattern direction asked `git ls-files -- `, and pathspec does not share gitattributes glob semantics. It is wrong in both directions: under a slash-free pattern matches its basename at any depth in gitattributes, where bare pathspec reads it as a root-relative path. `Dockerfile` covers `Docker/Dockerfile`; bare pathspec returns nothing, so a live pin reports dead and reds CI. over `*` does not cross a `/` in gitattributes and does in bare pathspec, so `pkg/*.py` wrongly picks up `pkg/sub/nested.py`. Harmless here, since it can only hide a dead pin. The first is the one with teeth: a false failure on a correct pin. `:(glob)` gives `*` and `**` their gitattributes meaning, and a `**/` prefix supplies the any-depth match for a slash-free pattern. Verified against `git check-attr`, which is what git actually applies, rather than against the documentation. On a tree holding `Docker/Dockerfile`, `pkg/mod.py` and `pkg/sub/nested.py`, check-attr resolves eol=lf for exactly the first two, and the conversion selects exactly those two where the bare form selects the wrong set both times. Differentially: on that tree the old matcher reports `Dockerfile` dead and the new one does not. No pattern in this repository changes result, bare or converted, so this fixes a latent defect rather than a live one. Nothing here is slash-free-and-literal today. Found by Copilot review on #68. Its remedy was right and its evidence was not: it predicted `*.sh` would be flagged dead here for want of a root-level `.sh` file, and `git ls-files -- '*.sh'` returns all five nested ones, because bare pathspec lets `*` cross a `/`. That is the over-match above, and the reason CI was green rather than failing as the comment predicted. Co-Authored-By: Claude Opus 5 (1M context) --- checks/check-eol-pins.py | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/checks/check-eol-pins.py b/checks/check-eol-pins.py index e329b52..5bd0b43 100755 --- a/checks/check-eol-pins.py +++ b/checks/check-eol-pins.py @@ -76,6 +76,34 @@ def has_shebang(path: Path) -> bool: return False +def pathspec_for(pattern: str) -> str: + """Convert a .gitattributes pattern into a pathspec matching the same files. + + A plain `git ls-files -- ` is the obvious way to ask which files a pin covers + and it is wrong in both directions, because pathspec and gitattributes do not share + glob semantics: + + under a pattern with no slash matches its basename at any depth in gitattributes, + where bare pathspec reads it as a root-relative path. `Dockerfile` covers + `Docker/Dockerfile`, and bare pathspec returns nothing, so a live pin reports + dead and reds CI. This is the direction that produces a false failure. + over `*` does not cross a `/` in gitattributes, and does in bare pathspec, so + `pkg/*.py` wrongly picks up `pkg/sub/nested.py`. Harmless for this check, + since it can only hide a dead pin, never invent one. + + `:(glob)` gives `*` and `**` their gitattributes meaning, and the `**/` prefix supplies + the any-depth match for a slash-free pattern. Verified against `git check-attr`, which + is what git actually applies: on a tree holding `Docker/Dockerfile`, `pkg/mod.py` and + `pkg/sub/nested.py`, check-attr resolves eol=lf for exactly the first two, and this + conversion selects exactly those two where the bare form selects the wrong set both + times. + """ + body = pattern[1:] if pattern.startswith("/") else pattern + # A trailing slash marks a directory, and is not part of the name being matched. + anchored = "/" in body.rstrip("/") + return f":(glob){body}" if anchored else f":(glob)**/{body}" + + def eol_attribute(paths: list[str]) -> dict[str, str]: """The resolved `eol` attribute per path, from git rather than by re-implementing the match rules, because a hand-rolled matcher is a second source of truth that can differ @@ -116,7 +144,7 @@ def main() -> int: for number, pattern in patterns(): if pattern in BASELINE: continue - if not git("ls-files", "--", pattern).strip(): + if not git("ls-files", "--", pathspec_for(pattern)).strip(): findings.append( f"dead: .gitattributes:{number} pattern {pattern!r} matches no tracked " f"file, so it binds nothing while reading as coverage." From 62fd48dbab23bfa893dc8bf3d16fff506de69e1f Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 8 Aug 2026 18:56:40 -0700 Subject: [PATCH 2/2] Anchor a leading-slash pattern, and check the matcher against git itself `/Dockerfile` is root-anchored and covers only the root file, where a bare `Dockerfile` matches at any depth. `pathspec_for` stripped the leading slash and then tested the remainder for one, which destroys the evidence the decision needs, so it built `:(glob)**/Dockerfile` and matched `Docker/Dockerfile` too. Verified against check-attr, which resolves eol=lf for the root file alone. Anchoring is now decided before the strip. That is the second conversion defect in this function, one under-matching and one over-matching, and both were found by review rather than by the gate. Fixing each shape as it surfaces leaves the next one to be found the same way, so the matcher is now checked against `git check-attr`, which is the thing it exists to predict: every file resolving to eol=lf is matched by some converted pattern, and every file a converted pattern matches resolves to eol=lf. A missing file means the conversion under-matched, which is what produces a false dead report. An extra file means it over-matched, which hides a real one. Both are reported as matcher findings rather than as pin findings, because they are defects in this script and not in the pins. Verified by reintroducing each defect against a tree holding `Docker/Dockerfile`, a root `Dockerfile`, `pkg/mod.py` and `pkg/sub/nested.py`. The bare-pathspec version is caught in both directions at once, the leading-slash version as an over-match, and with the matcher correct the third direction reports nothing on the same tree. This holds because no pattern here clears `eol` once another has set it, which the comment records, since a future pattern that unsets it would need the comparison to account for precedence. Found by Copilot review on #70. Co-Authored-By: Claude Opus 5 (1M context) --- checks/check-eol-pins.py | 63 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 60 insertions(+), 3 deletions(-) diff --git a/checks/check-eol-pins.py b/checks/check-eol-pins.py index 5bd0b43..8ea0a22 100755 --- a/checks/check-eol-pins.py +++ b/checks/check-eol-pins.py @@ -24,6 +24,13 @@ the same reason and are not detectable by any property of their contents, so they stay a named list and only the dead-pattern direction covers them. +A third direction checks this script's own pattern matching against `git check-attr`, and +it exists because the first two are only as trustworthy as that matching. Two conversion +defects were found by review rather than by this gate, one under-matching and one +over-matching, and each would have made a direction above report the wrong answer while +looking healthy. Comparing against git rather than fixing each shape as it appears is what +stops the next shape being found the same way. + Read-only. Exit 1 on any finding. """ from __future__ import annotations @@ -67,6 +74,19 @@ def patterns() -> list[tuple[int, str]]: return found +def lf_patterns() -> list[str]: + """Just the patterns that pin LF, which are the ones with a checkable consequence.""" + found = [] + for raw in ATTRIBUTES.read_text(encoding="utf-8").splitlines(): + line = raw.strip() + if not line or line.startswith("#"): + continue + fields = line.split() + if "eol=lf" in fields[1:]: + found.append(fields[0]) + return found + + def has_shebang(path: Path) -> bool: """True if the file opens `#!`, read as bytes so a binary file cannot raise.""" try: @@ -98,9 +118,12 @@ def pathspec_for(pattern: str) -> str: conversion selects exactly those two where the bare form selects the wrong set both times. """ + # Anchoring is decided before the leading slash is removed, because removing it first + # destroys the evidence: `/Dockerfile` is root-anchored and covers only the root file, + # while a bare `Dockerfile` matches at any depth, and the two differ by exactly the + # character being stripped. A trailing slash marks a directory and never anchors. + anchored = pattern.startswith("/") or "/" in pattern.rstrip("/") body = pattern[1:] if pattern.startswith("/") else pattern - # A trailing slash marks a directory, and is not part of the name being matched. - anchored = "/" in body.rstrip("/") return f":(glob){body}" if anchored else f":(glob)**/{body}" @@ -150,6 +173,39 @@ def main() -> int: f"file, so it binds nothing while reading as coverage." ) + # Direction three: the matcher above, checked against what git actually applies. + # + # Directions one and two disagree about nothing when the pattern conversion is right, + # and silently report the wrong thing when it is not. Two conversion defects were found + # by review rather than by this gate, one in each direction: a slash-free `Dockerfile` + # under-matched and reported a live pin dead, and a root-anchored `/Dockerfile` + # over-matched into subdirectories. Fixing each shape as it surfaced would leave the + # next shape to be found the same way, so the matcher is checked against `git + # check-attr` instead, which is the thing it is trying to predict. + # + # Both directions are needed and they catch different defects. A missing file means the + # conversion under-matched, which is what produces a false dead report. An extra file + # means it over-matched, which hides a real one. + # + # This holds because no pattern here clears `eol` once another has set it. A future + # pattern that unsets it would need this comparison to account for precedence. + predicted = set() + for pattern in lf_patterns(): + matched = git("ls-files", "-z", "--", pathspec_for(pattern)).split("\0") + predicted.update(path for path in matched if path) + resolved_all = eol_attribute(files) + actual = {path for path in files if resolved_all.get(path) == "lf"} + for path in sorted(actual - predicted): + findings.append( + f"matcher: {path} resolves to eol=lf, and no converted pattern matches it. " + f"The pattern conversion under-matches, so a live pin can report dead." + ) + for path in sorted(predicted - actual): + findings.append( + f"matcher: {path} is matched by a converted eol=lf pattern and resolves to " + f"eol={resolved_all.get(path, 'unspecified')}. The conversion over-matches." + ) + if findings: for finding in findings: print(f"error: {finding}") @@ -158,7 +214,8 @@ def main() -> int: print( f"PASS - {len(shebangs)} shebang files pinned to LF, " - f"{len(patterns()) - len(BASELINE)} patterns all matching tracked files" + f"{len(patterns()) - len(BASELINE)} patterns all matching tracked files, " + f"{len(actual)} LF-pinned files agreeing with git check-attr" ) return 0