Repository navigation
Ask the toolchain which files are ours, in one place - #3
Merged
verygoodsoftwarenotvirus merged 1 commit intoAug 15, 2026
Merged
Conversation
The tools that walk the filesystem each decided for themselves which Go
files belong to this module, and they disagreed three ways:
format_golang.sh and format_imports.sh carried their own
`-not -path '*/vendor/*'`, the formatting workflow filtered gofmt's
output with `grep -Ev '^vendor\/'`, and goimports.sh ran
`goimports -w .` with no exclusion at all.
That last one is a bug rather than a duplication. With a vendor tree
present, `make format` rewrote vendored third-party source: on this
repo's own dependencies it had reformatted 171 of 2681 vendored files
before it was interrupted, and the run took over six minutes.
scripts/go_files.sh answers the question once, by asking the Go
toolchain: `go list -e -f '{{.Dir}}' ./...`, then find -maxdepth 1 over
those directories. `./...` does not descend into vendor, testdata, or
any _ or . prefixed directory, which is why every wildcard-driven target
here — test, lint, go fix, fieldalignment, tagalign — has never needed an
exclusion. The walkers now inherit the same answer instead of
maintaining their own.
Two details are load-bearing, both found while testing this:
An out-of-sync vendor/modules.txt makes `go list` exit non-zero. The
first version of this returned an empty list in that case, and every
formatter silently formatted nothing while the CI check reported clean —
silence reading as success. go_files.sh now refuses to emit an empty
list.
Its callers read it through a file rather than `< <(go_files.sh)`,
because process substitution discards the exit status of what it runs,
which is precisely how that empty list went unnoticed. The workflow sets
pipefail for the same reason: xargs exits 0 on no input and would
otherwise mask the failure.
Verified against a real `go mod vendor` of this module's dependencies:
0 of 2681 vendored files touched, `make format` a no-op on a clean tree
and down to 19 seconds, the formatting check clean, and every path
exiting non-zero when the list cannot be produced.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TGxR7rXoXGdHXaUxNe346q
verygoodsoftwarenotvirus
left a comment
Contributor
Author
There was a problem hiding this comment.
LGTM
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
The tools that walk the filesystem each decided for themselves which Go files belong to this module, and they disagreed three ways:
scripts/format_golang.shfind … -not -path '*/vendor/*'scripts/format_imports.shfind … -not -path '*/vendor/*'.github/workflows/formatting.yamlgofmt -l . | grep -Ev '^vendor\/'scripts/goimports.shgo tool goimports -w .That last row is a bug rather than a duplication.
goimports -w .walks from the working directory, so with a vendor tree presentmake formatrewrote vendored third-party source. Measured against this repo's own dependencies: 171 of 2681 vendored files reformatted before I interrupted it, and the run was still going after six minutes.The fix
scripts/go_files.shanswers the question once, by asking the Go toolchain:./...does not descend intovendor/,testdata/, or any_- or.-prefixed directory — which is exactly why every wildcard-driven target here (test,lint,go fix,fieldalignment,tagalign) has never needed an exclusion at all. Only the filesystem walkers ever did, and now they inherit the same answer instead of each maintaining their own.-ekeeps a package that does not compile in the list, because formatting a file is most useful exactly when it is still broken.-maxdepth 1because whatgo listnames are package directories, and a package is entitled to atestdatadirectory of its own.Two details worth reviewing closely
Both were found while testing this, and both are the sort of thing that hides:
It fails loudly rather than emitting an empty list. An out-of-sync
vendor/modules.txtmakesgo listexit non-zero — not exotic, it's what you get from editinggo.modwithout re-vendoring. The first version of this returned an empty list in that case, and every formatter silently formatted nothing while the CI check reported "clean". Silence read as success.Callers read it through a file, not
< <(go_files.sh). Process substitution discards the exit status of what it runs, which is precisely how that empty list went unnoticed. The workflow setspipefailfor the same reason:xargsexits 0 on no input and would otherwise mask the failure.Verification
Against a real
go mod vendorof this module's dependencies (2681 vendored Go files):make formatmake formatwall clockmake formaton a clean treego listfailsshellcheckpasses on all ofscripts/, andgo test -race ./...is unchanged.Provenance
This came out of
primandproper/tarpaulin, which had drifted its own copies of the same exclusion (*/vendor/*in the scripts,./vendor/*in the workflow) and has the equivalent change applied. Nothing here is tarpaulin-specific.