lint-once: follow every spelling of docker build's file flag
All checks were successful
check / check (push) Successful in 1m17s
All checks were successful
check / check (push) Successful in 1m17s
The edge resolver matched `-f <file>` only. `docker build --file=X` fell
through to the default `Dockerfile` edge, so a second prettier pass wired in
that way was followed into the wrong file, counted nothing, and left the suite
green -- the exact false green this test exists to prevent, reachable by
writing the flag the long way.
Mutation, adding one line to the `check` recipe, before this commit:
@docker build -f Dockerfile.lint . 1 failed / 47 caught
@docker build --file=Dockerfile.lint . 48 passed / 48 MISSED
After, all four spellings fail with `expected 2 to be 1`:
`-f X`, `-f=X`, `--file X`, `--file=X`.
Docker takes the value either way for both the short and long flag, so the
resolver now reads `(?:-f|--file)[=\s]+`, still searched anywhere in the
invocation rather than at a fixed position. A leading \s keeps a longer flag
ending in the same letters (`--force-rm`) from supplying the match.
The header claimed `docker build -f <file>` coverage without qualification,
which a reader could take to include the long form it did not follow; it now
names all four forms and the fallback. The one limitation it asserts -- a
bundled cluster like `-qf X` resolving to the default -- is pinned by a test,
since an unpinned limitation is how the header drifts back into overclaiming.
This commit is contained in:
@@ -18,8 +18,8 @@
|
|||||||
// `"$SCRIPT_DIR/<name>"` and `script/<name>` into other scripts, `make <target>`
|
// `"$SCRIPT_DIR/<name>"` and `script/<name>` into other scripts, `make <target>`
|
||||||
// and `$(MAKE) <target>` through the Makefile shims, the prerequisites of a
|
// and `$(MAKE) <target>` through the Makefile shims, the prerequisites of a
|
||||||
// Makefile target, `yarn run <name>` through the `package.json` scripts, and
|
// Makefile target, `yarn run <name>` through the `package.json` scripts, and
|
||||||
// `docker build -f <file>` into that Dockerfile's `RUN` steps, heredoc bodies
|
// `docker build` into the `RUN` steps of the Dockerfile it names, heredoc
|
||||||
// included — and counts the prettier invocations it finds.
|
// bodies included — and counts the prettier invocations it finds.
|
||||||
//
|
//
|
||||||
// What that covers is worth stating exactly rather than as "anywhere", because
|
// What that covers is worth stating exactly rather than as "anywhere", because
|
||||||
// a comment claiming more than the code delivers is the same defect as an
|
// a comment claiming more than the code delivers is the same defect as an
|
||||||
@@ -38,8 +38,13 @@
|
|||||||
// writes its recipes that way (`@$(YARN) tsc --watch`); the substitution is
|
// writes its recipes that way (`@$(YARN) tsc --watch`); the substitution is
|
||||||
// one pass over literals, so a value that itself names another variable, a
|
// one pass over literals, so a value that itself names another variable, a
|
||||||
// value built by a make function (`$(shell ...)`, `$(addprefix ...)`), and the
|
// value built by a make function (`$(shell ...)`, `$(addprefix ...)`), and the
|
||||||
// body of a `define`/`endef` block are not expanded and not followed. Within
|
// body of a `define`/`endef` block are not expanded and not followed. A
|
||||||
// those edges, a prettier call is caught wherever it is added.
|
// `docker build` resolves to the file named by `-f`, `-f=`, `--file` or
|
||||||
|
// `--file=`, wherever in the invocation that flag sits, and to `Dockerfile`
|
||||||
|
// when it names none; a Dockerfile chosen some other way — a bundled short
|
||||||
|
// flag cluster (`-qf <file>`), 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.
|
||||||
//
|
//
|
||||||
// Two entrypoints are walked, because they cover different graphs: `make check`
|
// 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.
|
// is what a developer runs, and `.gitea/workflows/check.yml` is what CI runs.
|
||||||
@@ -430,8 +435,18 @@ const edgesOf = (line: string): string[] => {
|
|||||||
|
|
||||||
// The container lint pass lives behind a `docker build`; without following
|
// The container lint pass lives behind a `docker build`; without following
|
||||||
// it the count would miss the one invocation that is supposed to survive.
|
// it the count would miss the one invocation that is supposed to survive.
|
||||||
|
//
|
||||||
|
// Every spelling of the file flag is read, not just `-f <file>`. Docker
|
||||||
|
// accepts `--file <file>`, `--file=<file>` and `-f=<file>` for the same
|
||||||
|
// thing, and matching `-f ` alone resolved all of them to the default
|
||||||
|
// `Dockerfile` edge — so `docker build --file=Dockerfile.lint .` in a
|
||||||
|
// recipe was followed into the wrong file, counted nothing, and reported
|
||||||
|
// green. The flag is searched for anywhere in the invocation, as `-f`
|
||||||
|
// already was, because a real build line wraps it in `--build-arg` and
|
||||||
|
// other flags. A leading `\s` is required so that a longer flag ending in
|
||||||
|
// the same letters cannot supply the match.
|
||||||
if (/\bdocker\s+build\b/.test(line)) {
|
if (/\bdocker\s+build\b/.test(line)) {
|
||||||
const file = /\s-f\s+(\S+)/.exec(line);
|
const file = /\s(?:-f|--file)[=\s]+(\S+)/.exec(line);
|
||||||
edges.push(`docker:${file === null ? "Dockerfile" : file[1]}`);
|
edges.push(`docker:${file === null ? "Dockerfile" : file[1]}`);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -982,6 +997,61 @@ describe("the resolver reads what the shell would run", () => {
|
|||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// `-f` is not the only spelling of the flag. `--file=Dockerfile.lint` and
|
||||||
|
// `--file Dockerfile.lint` mean exactly what `-f Dockerfile.lint` means,
|
||||||
|
// and matching only `-f` sent both to the default `Dockerfile` edge
|
||||||
|
// instead. That is the false-green shape this whole file exists to
|
||||||
|
// prevent: a second prettier pass added as
|
||||||
|
// `docker build --file=Dockerfile.lint .` was followed into the wrong
|
||||||
|
// Dockerfile, counted nothing, and left every build green. The long form
|
||||||
|
// is an ordinary thing for a human to write, so it is followed rather than
|
||||||
|
// merely documented as unfollowed.
|
||||||
|
//
|
||||||
|
// The equality rather than containment matters: resolving to the right
|
||||||
|
// file is only half of it, the default edge must not also be emitted.
|
||||||
|
it("follows --file in all three spellings as it does -f", () => {
|
||||||
|
expect(edgesOf("docker build --file=Dockerfile.lint .")).toEqual([
|
||||||
|
"docker:Dockerfile.lint",
|
||||||
|
]);
|
||||||
|
expect(edgesOf("docker build --file Dockerfile.lint .")).toEqual([
|
||||||
|
"docker:Dockerfile.lint",
|
||||||
|
]);
|
||||||
|
expect(edgesOf("docker build -f=Dockerfile.lint .")).toEqual([
|
||||||
|
"docker:Dockerfile.lint",
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
// The flag is found anywhere in the invocation rather than at a fixed
|
||||||
|
// position, matching how `-f` was already read, since a real build line
|
||||||
|
// carries `--build-arg` and friends around it.
|
||||||
|
it("finds the file flag after other flags and build args", () => {
|
||||||
|
expect(
|
||||||
|
edgesOf(
|
||||||
|
'docker build --build-arg LINT_EPOCH="1" --file=Dockerfile.lint --progress=plain .',
|
||||||
|
),
|
||||||
|
).toEqual(["docker:Dockerfile.lint"]);
|
||||||
|
});
|
||||||
|
|
||||||
|
// A different flag that merely begins with the same letters is not the
|
||||||
|
// file flag: `--force-rm` must not read as one, or its next token would be
|
||||||
|
// resolved as a Dockerfile and thrown on as unreadable.
|
||||||
|
it("does not mistake --force-rm for the file flag", () => {
|
||||||
|
expect(edgesOf("docker build --force-rm .")).toEqual([
|
||||||
|
"docker:Dockerfile",
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
// The header says a bundled short flag cluster resolves to the default
|
||||||
|
// rather than to the file it names, which is a limitation and so has to be
|
||||||
|
// pinned: an unpinned limitation is how the header starts overclaiming
|
||||||
|
// again. If someone teaches the resolver to split clusters, this test is
|
||||||
|
// the one that tells them to correct the header too.
|
||||||
|
it("resolves a bundled -qf cluster to the default Dockerfile", () => {
|
||||||
|
expect(edgesOf("docker build -qf Dockerfile.lint .")).toEqual([
|
||||||
|
"docker:Dockerfile",
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
// This exact-equality assertion is also the block-scalar guard, which is
|
// This exact-equality assertion is also the block-scalar guard, which is
|
||||||
// not obvious from its name: a `run: |` step resolves to the bare `|`,
|
// not obvious from its name: a `run: |` step resolves to the bare `|`,
|
||||||
// which reaches nothing, so the prettier count would stay 1 no matter what
|
// which reaches nothing, so the prettier count would stay 1 no matter what
|
||||||
|
|||||||
Reference in New Issue
Block a user