test(manifest): resolve the JVM compat fixtures' deps from generated stub artifacts - #1459
Open
John-David Dalton (jdalton) wants to merge 1 commit into
Open
test(manifest): resolve the JVM compat fixtures' deps from generated stub artifacts#1459John-David Dalton (jdalton) wants to merge 1 commit into
John-David Dalton (jdalton) wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit aa0a2ed. Configure here.
| ext = coord('demo.ext:tool:jar:') | ||
| harness = coord('demo.ext:harness:jar:') | ||
| harness_core = coord('demo.ext:harness-core:jar:') | ||
| lib = coord('demo:lib:') # internal module: bare id, no ext |
There was a problem hiding this comment.
Weakened reactor bare-id check
Medium Severity
lib = coord('demo:lib:') no longer enforces a bare reactor id. The prefix also matches typed forms like demo:lib:jar:…, so a regression that stops emitting the bare groupId:artifactId:version id can still pass while the error text and pass line claim bare-id coverage.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit aa0a2ed. Configure here.
John-David Dalton (jdalton)
force-pushed
the
test/maven-fixture-stub-artifacts
branch
from
August 1, 2026 03:58
aa0a2ed to
160fddc
Compare
…stub artifacts The maven-compat smoke fixture declared commons-io, junit and hamcrest as real artifacts from Maven Central purely to get a graph shape: an external prod dep, a test-scope dep and its transitive. No fixture code imports them, so they were never compiled against or executed - but they still aged into GHSA-78wr-2p64-hpwj, a Dependabot alert, and a version bump. gradle-compat already solved this: make-localrepo.sh generated a tiny file-based Maven repo of empty-jar stubs at test time. That generator moves up a directory to test/make-stub-repo.sh, gains the ability to give a stub its own dependency, and now serves all three fixtures. maven-compat resolves demo.ext:tool (prod), demo.ext:harness (test) and its transitive demo.ext:harness-core through a <repository> whose URL smoke-test.sh passes as -Dstub.repo.url; sbt-compat resolves demo.ext:tool and demo.ext:harness the same way. The suites also stop using the developer's home caches. compat-cache.sh resolves one cache root under the temp dir, overridable by SOCKET_COMPAT_CACHE, and each tool is pointed at it: Maven by -Dmaven.repo.local, Gradle by -g, sbt by -Dsbt.ivy.home and COURSIER_CACHE. The root is stable so each tool's own closure is downloaded once, but the stub artifacts are evicted before every run, so a run can never pass on a stale copy of the thing under test. Nothing is read from or written to ~/.m2, ~/.gradle or ~/.ivy2, and records.tsv no longer embeds a path under the developer's home. Assertions prove exactly what they proved before, still matching on groupId:artifactId with no version written in any test, and still keying on the .jar suffix rather than any path. The transitive check is now unconditional: with a stub graph it is deterministic, so its absence is a defect rather than a junit-version detail. Each build tool still fetches its own closure from Central - Maven's plugins pull commons-io themselves - so this is not a fully offline fixture. It declares no third-party dependency of its own.
John-David Dalton (jdalton)
force-pushed
the
test/maven-fixture-stub-artifacts
branch
from
August 1, 2026 18:20
160fddc to
b57ec06
Compare
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 JVM compat test fixtures pull three real, published artifacts down from Maven Central purely so the dependency graph has the right shape — one external production dependency, one test-scope dependency, and one transitive of that test dependency. No fixture code ever imports them. They are never compiled against and never executed. They are stage props.
Stage props still get CVEs. One of them,
commons-io:commons-io:2.11.0, fell into the range of GHSA-78wr-2p64-hpwj, Dependabot opened #1449, and #1457 / #1458 are open right now purely to move a version number in a test fixture. That will happen again with the next advisory against whatever the fixtures happen to pin. This PR replaces those three artifacts with stubs generated at test time, so no advisory can ever touch them again.The second thing it fixes: running these suites used to write into your own
~/.m2,~/.gradleand~/.ivy2. That is untidy, but the real cost is that a run could pass because an artifact happened to already be warm in your home cache — so a genuine resolution regression could hide on your machine and only appear in CI. Each tool is now pointed at one shared cache root instead.Builds on #1457 (
fix/commons-io-cve), so the version-agnostic assertions that PR introduced are inherited rather than fought with. Merge that one first.The problem — three real artifacts pulled in for their shape, not their code
The Maven smoke fixture declared
commons-io:commons-io,junit:junit, and junit's transitiveorg.hamcrest:hamcrest-core. It needed them only for their shape: one external prod dependency, one test-scope dependency, and one transitive of that test dependency.No fixture code imports any of them.
App.javacallsLib.greet()and nothing else, so they are never compiled against and never executed.They still cost real work. commons-io 2.11.0 fell into the range of GHSA-78wr-2p64-hpwj, Dependabot opened #1449, and #1457 / #1458 exist just to move a number.
The fix — a pattern
gradle-compathas always used, moved up one directory so all three fixtures share itgradle-compathas never had this problem. Itsmake-localrepo.shgenerates a tiny file-based Maven repository of stub artifacts — a generated.pomplus an empty-but-valid.jar— at test time, and the Gradle fixture resolvesdemo.lib:fooanddemo.test:barfrom it. Nothing binary is committed and no advisory can ever touch it.maven-compatwas the odd one out, not the place to invent something new.So the generator moves up one directory to
test/make-stub-repo.sh, next torun-compat.sh, and all three fixtures share it. It takes an output directory and a list of coordinates. The one new capability is that a stub can declare a dependency on another stub, which is what Maven needs for the transitive shape:maven-compatnow declares these instead:demo.ext:toolcommons-io:commons-iodemo.ext:harnessjunit:junitdemo.ext:harness-coreorg.hamcrest:hamcrest-coresbt-compathad the same exposure — it still pinned the vulnerablecommons-io:commons-io:2.11.0, invisible to Dependabot only because Dependabot does not readbuild.sbt— so it moves to the same stubs.How resolution is wired — one repository property on the root pom, overridden by the script
The root pom declares one repository whose URL comes from a property, and
smoke-test.shpasses the absolute path of the repo it just generated:Child modules inherit it, which is verified below. The property has a default of
file://${maven.multiModuleProjectDirectory}/localreposo a baremvn compileinsideproject/also works;-Dstub.repo.urlfrom the script overrides it.Alternative considered and rejected: seeding the artifacts straight into the run's local repository. That has to know the local repo path, which breaks the moment anyone passes
-Dmaven.repo.local— including the empty-local-repo check below.The generated repos are gitignored and rebuilt from scratch on every run, like
records.tsv.The caches — one shared root, stable so the tool closure stays warm, stubs evicted every run
The suites no longer touch your own caches.
compat-cache.shresolves a single cache root and each tool is pointed at it:-Dmaven.repo.local~/.m2-g-Dsbt.ivy.homeandCOURSIER_CACHEmktemp -d; the Ivy home and coursier cache were not coveredSOCKET_COMPAT_CACHEalready existed inrun-compat.shfor the downloaded build-tool distributions, so this reuses it rather than inventing a second knob;run-compat.shnow reads it from the shared file too.The most important reason is reproducibility, not tidiness — a run used to be able to pass off a warm home cache. Two lesser reasons:
records.tsvembeds the resolved artifact path, so it used to contain a path under the developer's home directory, and a test should not write into a developer's real Maven cache.Trade-off: a stable root, not a fresh one per run. A
mktemp -devery time would be maximally hermetic, but it would re-download Maven's whole plugin closure on every invocation, making the suite slower and more network-dependent than it is today. So the root is stable and each tool's own closure is cached across runs — while the stub artifacts are evicted from it before every run, so the thing actually under test is always resolved from scratch. That keeps the fresh-resolution property exactly where it matters. SetSOCKET_COMPAT_CACHEto a freshmktemp -dwhen you want a completely cold run.The first run after the cache root is cleared pays for the tool's closure download. Measured here: about 70 seconds cold, about 35 warm.
Coverage is not weakened — the same assertions, and one that gets stronger
The assertions prove exactly what they proved before: external prod dep in a prod root with a materialized jar, test-scope dep and its transitive kept out of prod roots, internal reactor module
demo:libemitted by its bare id and marked direct, and-Dsocket.populateFilesFormaterializing only the in-scope artifact. Matching is still ongroupId:artifactIdwith the version ignored, and the scoped run still derives its GAV from the emitted records, so no version is written anywhere in the test.One assertion gets stronger. The transitive check used to be conditional (
if hamcrest and in_prod(hamcrest)), tolerating hamcrest being absent because that depended on which junit version resolved. With a stub graph the transitive is deterministic, so its absence is now a failure.What this does not claim — the fixture is still not offline, and commons-io still lands in the local repo
This is not a fully offline fixture, and the change does not make it one. Maven fetches its own plugins from Central to run at all, and those plugins have their own dependencies.
maven-resources-plugin:3.3.1depends oncommons-io:commons-io:2.11.0— the very version this whole thread is about — so commons-io still lands in the local repository during a run.It is not in our dependency graph, we do not declare it, and Dependabot does not alert on it. What this PR removes is our declared externals, not Maven's build closure.
What I checked — Maven 3.9.10 and Gradle 8.14 end to end; sbt is reasoned, not run
Maven 3.9.10, Gradle 8.14, JDK 21.0.3 on macOS.
Ran — the Maven smoke test end to end. The brief assumed it could not run, because it needs the Coana extension jar as its second argument — but
maven-extension/build-jar.shbuilds that jar from this repo, so it was buildable locally. Both phases pass:Ran — the same thing again with an empty local repository (
MAVEN_ARGS=-Dmaven.repo.local=$(mktemp -d)), still exit 0. Counting where every artifact in that repository came from afterwards:socket-stubs— the three stubs' poms and jars, including both test-scope jars materialized under--with-filescentral— Maven's own plugin closurecommons-iopresent at exactly one version, 2.11.0,_remote.repositoriessayscentral=, pulled bymaven-resources-plugin-3.3.1.pomorg/junit/junit-bompoms present, alsocentral=, also plugin closurejunit:junitand nohamcrestanywheremvn --batch-mode -Dmaven.repo.local=$(mktemp -d) dependency:treeconfirms the shape and that the child module inherited the repository:Ran — cache isolation, verified rather than assumed. With the cache root pointed at a scratch path, the Maven smoke test passes both phases and the Gradle smoke test passes both phases. Full recursive listings of the real
~/.m2and~/.gradlehash identically before and after those runs. Neither suite'srecords.tsvcontains any path under the home directory; the Maven file record now points into the isolated repository and the Gradle one points straight at the stub repo.Ran — an A/B on the stub eviction. Leaving the previous run's stub copy in the local repository and then deleting the generated stub repo,
mvn compilestill succeeds — that is precisely the masking this guards against. Doing what the script does, evicting the stubs first, the same run fails withCould not find artifact demo.ext:tool:jar:1.0. So a broken stub repo cannot be hidden by a warm cache.The assertion blocks needed no changes for any of this; they key on the
.jarsuffix, not on any path.Ran — Gradle is not regressed.
gradle-compat/smoke-test.shruns end to end on Gradle 8.14 and passes both phases. The repo the shared generator produces for the Gradle coordinates is byte-identical to the old script's: same paths, same pom bytes, jars differing only in zip timestamps.Ran — the assertion blocks against synthetic
records.tsvinput, extracted from the committed script rather than a copy. 16 cases, all behaving as expected: well-formed records pass; the prod stub missing, its jar not materialized, the test-scope dep in a prod root, the transitive in a prod root, the transitive missing,demo:libabsent or not direct or dev-only, two versions of the same coordinate, and both scoped-run defects all fail with the right message. The sbt block got the same treatment on 4 cases.Ran —
bash -nandshellcheck, clean on all four scripts.Could not verify — the older matrix rows. The compat matrix covers Maven 3.6.3 / 3.8.8 / 3.9.9 and Gradle 1.12 through 9.2.1; only Maven 3.9.10 and Gradle 8.14 are installed here, so the older rows are unverified.
Did not run — sbt at all. sbt is not installed here and there is no CI for this matrix, so the sbt fixture changes are reasoned, not run — both the stub coordinates and the
-Dsbt.ivy.home/COURSIER_CACHErelocation. The reasoning:"Socket Stubs" at s"file://.../localrepo"is aMavenRepository, which coursier supports natively on sbt 1.x and Ivy resolves throughIBiblioResolveron 0.13. That form was chosen overResolver.file(...)(Resolver.mavenStylePatterns)precisely because the pattern-based form is the one with known coursier sharp edges. If you would rather not carry an unrun change, the sbt hunk is easy to split out.