diff --git a/README.md b/README.md index a4147ab..9c94308 100644 --- a/README.md +++ b/README.md @@ -127,10 +127,12 @@ in each. `test/packaging/lint-once.test.ts` asserts that count by walking the invocation graph, so a second pass cannot creep back in unnoticed. `script/fmt-check` remains as a standalone entrypoint for asking the formatting -question on its own, without docker and without the rest of lint. It is the one -formatting path that still runs on the host, and nothing reaches it: neither -`script/check` nor `script/precommit` calls it, so it never contributes to their -verdicts. Its own verdict cannot drift from the container's: prettier is pinned +question on its own, without docker and without the rest of lint. It is not the +last formatting path on the host — `script/fmt` writes with the host prettier, +and `package.json` exposes `fmt` and `fmt-check` as direct prettier calls — it is +the one of them that the check graph could plausibly have reached, and nothing +reaches it: neither `script/check` nor `script/precommit` calls it, so it never +contributes to their verdicts. Its own verdict cannot drift from the container's: prettier is pinned to an exact version, installed from `yarn.lock` under `--frozen-lockfile` in both places, and reads `.gitignore` as its default ignore file — which is why `.dockerignore` deliberately keeps `.gitignore` in the build context. diff --git a/test/packaging/lint-once.test.ts b/test/packaging/lint-once.test.ts index 895bb6b..7b2c4bf 100644 --- a/test/packaging/lint-once.test.ts +++ b/test/packaging/lint-once.test.ts @@ -40,12 +40,18 @@ // value built by a make function (`$(shell ...)`, `$(addprefix ...)`), and the // body of a `define`/`endef` block are not expanded and not followed. A // `docker build` resolves to the file named by `-f`, `-f=`, `-f` with -// the value attached to the flag, `--file` or `--file=`, wherever in the -// invocation that flag sits, and to `Dockerfile` when it names none; a +// the value attached to the flag, `--file` or `--file=`, wherever in that +// invocation the flag sits, and to `Dockerfile` when it names none; a // Dockerfile chosen some other way — a bundled short flag cluster // (`-qf `), a value this walk cannot expand to a literal — resolves to -// the default rather than to the real file. Within those edges, a -// prettier call is caught wherever it is added. +// the default rather than to the real file. Each `docker build` on a line is +// followed separately, and the flag search for one is bounded to the slice +// running from that command to the next `&&`, `||`, `;` or `|`, so a second +// build on the same line is not swallowed by the first and a later command's +// `-f` is not read as the build's. That bounding is a split on those four +// separators, not a shell parse: a separator appearing inside quotes or a +// command substitution still ends the slice. Within those edges, a prettier +// call is caught wherever it is added. // // Two entrypoints are walked, because they cover different graphs: `make check` // is what a developer runs, and `.gitea/workflows/check.yml` is what CI runs. @@ -58,8 +64,9 @@ // Undercounting is the failure mode that would make this test worthless. Three // things guard against it: the walk is asserted to have reached the nodes that // matter, an unresolvable or empty node is a thrown error rather than a quiet -// zero, and prettier is counted per occurrence rather than per line, so two -// invocations chained with `&&` cannot read as one. +// zero, and both the counting and the edge-following are per occurrence rather +// than per line, so two prettier calls or two `docker build`s chained with `&&` +// on one line cannot read as one. import { describe, expect, it } from "vitest"; import { readFileSync } from "node:fs"; import { fileURLToPath } from "node:url"; @@ -449,8 +456,24 @@ const edgesOf = (line: string): string[] => { // the same letters cannot supply the match, and the attached form is // allowed only for the short flag, so that `--force-rm` — a long flag that // merely starts with the same letters — still does not read as one. - if (/\bdocker\s+build\b/.test(line)) { - const file = /\s(?:--file[=\s]+|-f=?\s*)(\S+)/.exec(line); + // + // Every `docker build` on the line produces an edge, and each one looks for + // its file flag only within its own invocation — the slice from the command + // to the next `&&`, `||`, `;` or `|`. One `test` and one `exec` over the + // whole line got both halves of that wrong: `docker build -f Dockerfile.lint + // . && docker build -f Dockerfile.extra .` followed the first file and + // dropped the second, so a tree running prettier twice reported green, and a + // bare `docker build . && cp -f Dockerfile.lint /tmp/x` read `cp`'s `-f` as + // the build's, counting an invocation that never happens and losing the + // default `Dockerfile` edge. The bounding is a split on those four + // separators and nothing more: a separator inside quotes or a `$(...)` + // substitution ends the slice anyway, and a newline-separated command list + // is already one line per invocation by the time it gets here. + for (const match of line.matchAll(/\bdocker\s+build\b/g)) { + const invocation = line + .slice(match.index) + .split(/&&|\|\||;|\|/)[0] as string; + const file = /\s(?:--file[=\s]+|-f=?\s*)(\S+)/.exec(invocation); edges.push(`docker:${file === null ? "Dockerfile" : file[1]}`); }