Skip to content

Skip compiled dependences by default - #120

Merged
fredbi merged 17 commits into
go-openapi:masterfrom
fredbi:skip-compiled-dependences-by-default
Aug 17, 2026
Merged

fredbi merged 17 commits into
go-openapi:masterfrom
fredbi:skip-compiled-dependences-by-default

Conversation

@fredbi

@fredbi fredbi commented Aug 17, 2026

Copy link
Copy Markdown
Member

Change type

Please select: 🆕 New feature or enhancement|🔧 Bug fix'|📃 Documentation update

Short description

Fixes

Full description

Checklist

  • I have signed all my commits with my name and email (see DCO. This does not require a PGP-signed commit
  • I have rebased and squashed my work, so only one commit remains
  • I have added tests to cover my changes.
  • I have properly enriched go doc comments in code.
  • I have properly documented any breaking change.

@codecov

codecov Bot commented Aug 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.90909% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.21%. Comparing base (5e4cf04) to head (995e47c).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...nal/benchmarks/loader-benchmark/options_current.go 0.00% 1 Missing ⚠️
internal/scanner/scan_context.go 94.44% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #120   +/-   ##
=======================================
  Coverage   84.20%   84.21%           
=======================================
  Files         176      176           
  Lines       15187    15202   +15     
=======================================
+ Hits        12788    12802   +14     
+ Misses       1789     1788    -1     
- Partials      610      612    +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

fredbi and others added 6 commits August 17, 2026 13:56
Taking dependency types from the compiler's export data became the default in an
unreleased commit. It goes back to being asked for: SkipCompiledDependencies is
now CompiledDependencies, off unless set, and the flag behind it is
-compiled-dependencies.

No configuration wins in every state of the build cache, so the default should be
the one with no bad case rather than the one with the best case. Reading
dependencies from source is never an order of magnitude off, writes 7 MB of build
cache instead of 231, and does not care whether the cache was there. Export data
is 35-55% faster and several times smaller warm, and 6x slower cold -- and a CI
runner is cold by construction, which is the shape the windows lane was reporting.

The two better answers stay one flag away, and each is conditional: compiled
dependencies where the cache is warm by construction, and ToolchainFreeLoader
where memory binds or the cost has to be predictable. That loader is the better
trade of the two on the current figures and would be the natural default; it is
experimental, which is the only reason it is not.

The option becoming askable again brings back the pair that reports on it:
a Hint when the load took the shortcut, a Warning when the resolved loader could
not honour the request. Both were deleted as noise while nobody could ask.

The measurements are unchanged and stay where they are -- only which side of them
the default sits on. Benchmark tables, the loader-choice guidance, the options
reference and the three command flag listings are updated with it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
The three entries covering these were anchored at the repository root, which is
where "go build ./hack/..." drops them. A "go build ./..." run from inside
cmd/genspec-wasi puts the binary beside its main package instead, and nothing
was covering that.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
The load-flag table skipped it, though the TUI accepts it like every other
command. It is the one place worth calling out: a session rescans on every
save, so the build cache is warm after the first scan -- which is the state
the option is for.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
The three-points-in-time series on the performance page took its last point from
a scan with compiled dependencies while labelling it the default configuration.
The third bar therefore differed from the second by six months of work AND by a
flag, which credited the calendar for the flag: 7.3x faster and 10x less
allocated where the default reaches 4.7x and 3.7x.

The series now holds the configuration constant and says so, and the opt-in
figures are quoted beside it rather than mixed into it. The peak-RSS paragraph
followed the same error -- nothing since v0.35.1 lowers that floor by default
(-56 MB); asking for a different loader is what does, and is worth -395 to
-501 MB.

Two figures were wrong independently of this. The page is kubeapi throughout but
quoted dockerctl's 19 packages-read-from-source against kubeapi's 296; the
kubeapi pair is 31 and 296. And the cold-cache penalty is 6.3x and 6.6x on the
two corpora, which both documents rounded down to 6x.

The measurements are unchanged; every figure on the page now reconciles with the
tables in internal/benchmarks.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
The suites emit the same document under any loader -- TestLoaderAgreement holds
that over the whole corpus -- so which one runs is a question about what the run
costs. This harness is the shape compiled dependencies were made for: several
hundred scans of small trees over one dependency closure, so the closure can be
compiled once and read back by every scan after the first.

scantest.ApplyLoader is now the one place that decides, reading
CODESCAN_TEST_LOADER or the build tag the binary was compiled with. The
environment variable is for a person at a terminal; the build tags exist because
CI reaches go test through a shared workflow that forwards flags and not the
environment. An unrecognised value panics rather than silently running the
default under another name.

It reaches the integration suite through a runScan wrapper, and the builders and
scanner suites through the cached fixture loaders they already share. Over the
whole repository, compiled dependencies cost 4m06s of CPU against 5m34s; on the
integration suite alone it is 1m35s against 2m55s, and a runner with two cores
spends most of that difference on the wall.

The default is unchanged, so an ordinary run still exercises what ships. Tests
that pin a loader themselves -- the agreement A/B, the virtual-filesystem and
export-data suites -- keep calling codescan.Run directly, since a field left at
its zero value cannot be told from one set to it on purpose. The runnable
examples under docs/ are left alone too: they show a caller how to call the API.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
The suites emit the same document under any loader, so which one runs is a
question about what the run costs -- and this harness is the shape compiled
dependencies were made for: several hundred scans of small trees over one
dependency closure, compiled once and read back by every scan after the first.
Over the repository that is 4m06s of CPU against 5m34s, and a runner with two
cores spends most of the difference on the wall.

A build tag rather than the environment variable, because the shared workflow
forwards flags to `go test` and not the environment.

The toolchain-independence matrix is untouched: it compiles its own binaries
outside the shared workflow, and its go-less job could not run `go list` anyway.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
@fredbi
fredbi force-pushed the skip-compiled-dependences-by-default branch from 995e47c to 169503c Compare August 17, 2026 12:05
fredbi and others added 11 commits August 17, 2026 15:04
The setting covered internal/integration and, through the shared fixture
loaders, the builders. It missed internal/scanner entirely -- 21 sites building
Options directly -- which is the second most expensive package in the run.

It missed it for a structural reason: most of those tests are in-package, and
scantest imports the scanner, so the policy could not be read from where the
scans are. It now lives in internal/testloader, which imports nothing, and both
sides reach it: scantest re-exports it, and the in-package tests apply it
through a local three-line helper.

Measured over the workspace, which is what CI runs rather than the root module
alone: 42.7s against 1m12s wall, 5m07s against 7m41s of CPU. The scanner package
by itself goes from 14.3s to 5.5s. Earlier figures understated this because they
were taken with ok  	github.com/go-openapi/codescan	(cached)
ok  	github.com/go-openapi/codescan/cmd/genspec-wasi	(cached)
ok  	github.com/go-openapi/codescan/cmd/internal/cliconf	(cached)
ok  	github.com/go-openapi/codescan/cmd/internal/cliopts	(cached)
?   	github.com/go-openapi/codescan/hack/genexportdata	[no test files]
?   	github.com/go-openapi/codescan/hack/go-loader	[no test files]
ok  	github.com/go-openapi/codescan/internal/benchmarks	(cached)
ok  	github.com/go-openapi/codescan/internal/benchmarks/corpus	(cached)
?   	github.com/go-openapi/codescan/internal/benchmarks/corpus/unpack	[no test files]
?   	github.com/go-openapi/codescan/internal/benchmarks/loader-benchmark	[no test files]
ok  	github.com/go-openapi/codescan/internal/builders/common	(cached)
ok  	github.com/go-openapi/codescan/internal/builders/godoclink	(cached)
ok  	github.com/go-openapi/codescan/internal/builders/handlers	(cached)
ok  	github.com/go-openapi/codescan/internal/builders/operations	(cached)
ok  	github.com/go-openapi/codescan/internal/builders/parameters	(cached)
ok  	github.com/go-openapi/codescan/internal/builders/resolvers	(cached)
ok  	github.com/go-openapi/codescan/internal/builders/responses	(cached)
ok  	github.com/go-openapi/codescan/internal/builders/routes	(cached)
ok  	github.com/go-openapi/codescan/internal/builders/schema	(cached)
ok  	github.com/go-openapi/codescan/internal/builders/spec	(cached)
ok  	github.com/go-openapi/codescan/internal/builders/validations	(cached)
?   	github.com/go-openapi/codescan/internal/exportdata	[no test files]
?   	github.com/go-openapi/codescan/internal/ifaces	[no test files]
ok  	github.com/go-openapi/codescan/internal/integration	(cached)
ok  	github.com/go-openapi/codescan/internal/packages	(cached)
ok  	github.com/go-openapi/codescan/internal/packages/list	(cached)
ok  	github.com/go-openapi/codescan/internal/packages/vfs	(cached)
ok  	github.com/go-openapi/codescan/internal/parsers	(cached)
ok  	github.com/go-openapi/codescan/internal/parsers/grammar	(cached)
?   	github.com/go-openapi/codescan/internal/parsers/routebody	[no test files]
ok  	github.com/go-openapi/codescan/internal/parsers/security	(cached)
ok  	github.com/go-openapi/codescan/internal/parsers/yaml	(cached)
ok  	github.com/go-openapi/codescan/internal/scanner	(cached)
ok  	github.com/go-openapi/codescan/internal/scanner/classify	(cached)
?   	github.com/go-openapi/codescan/internal/scantest	[no test files]
?   	github.com/go-openapi/codescan/internal/scantest/classification	[no test files]
?   	github.com/go-openapi/codescan/internal/scantest/mocks	[no test files]
?   	github.com/go-openapi/codescan/internal/testloader	[no test files] from the root module, which leaves out the
command modules, the examples and the fixtures.

Tests that pin a loader are untouched, in the scanner suite as elsewhere.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
The tag that selects it is set in a workflow file, far from any test, so a run
was only distinguishable from a differently-configured one by its wall-clock
time -- and two rounds of CI debugging went on inferring exactly that.

The integration and scanner suites now announce it. Written to stdout rather
than through testing.T, because a passing package's test logs are not shown by
the console formatter CI uses; package-level output reaches the gotestsum JSON
report whatever the console shows, so the answer survives in the artifact.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
The echo went in TestMain, which also runs under `go test -list`. The fuzz
matrix builds itself from that listing and treats every line that is not an
`ok` or a `?` as a target name, so both echoes became fuzz tests that do not
exist and the job tried to run them.

It is now an ordinary test, which `-list` names but does not execute, and it
reports through testing.T rather than stdout. `go test -json` captures a passing
test's log, so the line still reaches the report CI keeps.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
The echo was a passing test's log, and CI runs gotestsum, whose default format
prints one line per package and nothing else for a package that passed. Checked
against gotestsum directly this time rather than reasoned about: a test's stdout
and stderr are both swallowed too, and a SKIP is printed with its message.

So the two marker tests skip, carrying the loader in the skip reason. There was
never anything to assert in them -- they exist to state a configuration, not to
check one -- and the run now shows two skips whose message answers which loader
was in force.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Extends the setting to the builders packages, which built their own Options and
so ran the shipped default whatever the run asked for, and adds a guard for the
mistake that surfaced while doing it.

A test that pins a loader must not also be routed: there is no telling a field
left at its zero value from one set on purpose, so the setting writes over the
pin. Under the default that is invisible, because the setting writes nothing --
which is why four sites ended up wrapped twice, passing every ordinary run and
failing only under a tag, with an arithmetic assertion far from the cause.
ApplyLoader now refuses options that already pin one, so an ordinary run names
the call site instead.

TestReadBackOnDemand_PaysPerDeclarationWanted is one of the genuinely pinned
ones and is no longer routed: it measures an economy that exists only under
compiled dependencies, since the toolchain-free loader reads every dependency
during the load and leaves nothing to read back.

The workspace now passes under all three loaders. Over it: 37.9s wall and 252s
of CPU with compiled dependencies, 52.3s and 316s with the toolchain-free
loader, against 80.8s and 452s reading dependencies from source.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Sweeps the two remaining modules that build Options of their own, so the setting
now covers every test that names one rather than the root module's share of them.

The TUI answers through its existing shared test helpers; the examples get a
small package of their own, because each of the 38 sites lives in a package of
its own and a helper per package would be 38 copies. Both refuse options that
already pin a loader, as the library-side helpers do.

cmd/genspec is deliberately left out. It has no Options to write onto -- it
builds one from flags, which is the point of it -- and routing it through a flag
turned out to change the thing its tests are about: two of them assert silence
on a clean scan, and asking for compiled dependencies emits the Hint that says
the request was met.

Over the workspace, which is what CI runs: 36.0s wall and 225s of CPU with
compiled dependencies, 48.1s and 293s with the toolchain-free loader, against
80.8s and 452s reading dependencies from source. All three pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
The loaders are meant to produce the same document by different means, and the
agreement A/B asserts that over the fixture corpus. It says nothing about the
SUITE, where a test can depend on how its graph was loaded without saying so --
which is how one measuring the compiled-dependency read-back economy came to
fail under the toolchain-free loader, and how four sites wrapped twice in the
run-wide loader setting passed every ordinary run.

Three jobs, one per loader, and deliberately none of what the shared go-test
workflow does: no OS matrix, no -race, no coverage. On this question those add
wall clock and variance and answer nothing, and windows in particular drowns the
signal. What it buys is a tick per loader and three durations that can be read
side by side.

Selected through CODESCAN_TEST_LOADER rather than the build tags, because a
reusable workflow does not inherit the caller's environment and this job is our
own -- which also gets the variable exercised somewhere.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
The help defined "auto" as picking "own" where there is no exec, and left the
reader to chain that with the default to work out what a plain run gets. It
reads as though "own" were the default, which it is on no native build.

It now leads with the answer, and stays true of the WebAssembly build that
prints the same string: "auto" runs go list wherever a subprocess can be
started, which is every native build, and uses "own" only where one cannot.

The same two-hop wording sat in three documented flag tables. The WASI one now
says the thing that is peculiar to it -- there, "auto" and "own" are the same
choice, and "go" is refused up front rather than failing inside the go command.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
crypto/tls imports golang.org/x/crypto/chacha20poly1305, which the Go
distribution vendors at GOROOT/src/vendor. The toolchain-free loader could not
find it: the stdlib branch is gated on an import path whose first segment has no
dot, and every other branch answers for the module under scan. So a scan of any
tree reaching crypto/tls synthesized three x/crypto packages and said so, where
the go/packages loader synthesized none.

The resolver now carries a second vendor root. That is not a special case so
much as the second half of a rule it already had: go mod vendor flattens the
whole graph into the MAIN module's vendor directory and a dependency's own is
never read, so one root answers for module mode entirely -- and std is the
exception because std never goes through modload. The go command resolves its
imports with the older walk that climbs from the importing package to the source
root, which is how a package in GOROOT/src reaches GOROOT/src/vendor and nothing
outside it does.

That bound is why resolution now takes the importing directory. Both call sites
already had it. The package path carries the vendor/ prefix go list reports, so
the two loaders agree on identity.

The stub memo needed the same: it keyed on the import path alone, and the same
path has two honest answers now -- resolvable from std, not from anywhere else
-- so the first asker's was handed to everybody.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Two of them assert that a clean scan says nothing, and both non-default loaders
have something to say on one, for unrelated reasons: compiled dependencies
announce that the request was met, and the toolchain-free loader says it did not
run the cgo tool.

Neither is a finding about the scanned code, which is what those assertions are
about, so the exception is the loader having spoken rather than any particular
thing it said -- otherwise each new loader edits these tests again.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
@fredbi
fredbi merged commit 9a12c4e into go-openapi:master Aug 17, 2026
37 of 49 checks passed
@fredbi
fredbi deleted the skip-compiled-dependences-by-default branch August 17, 2026 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant