diff --git a/checks/check-eol-pins.py b/checks/check-eol-pins.py index e329b52..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: @@ -76,6 +96,37 @@ 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. + """ + # 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 + 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,12 +167,45 @@ 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." ) + # 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}") @@ -130,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