Repository navigation
maven: wire the local repository to a session state dir (MAVEN_REPO_LOCAL) - #902
Conversation
…OCAL)
gradle declares its cache via env_state_wiring (GRADLE_USER_HOME);
maven declared nothing, so the maven/java stacks papered over it with
MAVEN_OPTS=-Dmaven.repo.local=/state/m2/repository and plain sessions
used ~/.m2. Declare env_state_wiring { MAVEN_REPO_LOCAL, prefix maven }
and have /usr/bin/mvn pass it as -Dmaven.repo.local (Maven has no env
var for it); a caller's own -Dmaven.repo.local still wins.
Test: an offline packaging=pom `validate` with MAVEN_REPO_LOCAL set
succeeds and Maven's debug output names the wired path.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/build |
|
Warning Review limit reached
This review includes 3 billable files and costs up to $0.75.
Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing. Or wait 13 minutes for your next included review. View limit details
Comment |
There was a problem hiding this comment.
The wiring is right and matches gradle's env_state_wiring shape. A -Dmaven.repo.local in the wrapper is the correct approach, since Maven has no environment variable for it, and a later caller -D still wins.
- It changes behaviour for the maven stack.
stacks/maven/stack.nclsetsMAVEN_OPTS=-Dmaven.repo.local=/state/m2/repository. With this PR, the wired variable is also set in stack builds, and the command-line-DbeatsMAVEN_OPTS, so stack builds move to a new, empty repository. "Unset means unchanged" doesn't hold there. Please drop the stack'sMAVEN_OPTSin this PR so there's one location. - The wired path also silently overrides a user's
settings.xml<localRepository>. That's probably intended, but worth a line in the comment. - mvnd (#904) ignores this variable, so the two will diverge unless #904's wrapper forwards it (commented there).
- Rebase needed: the branch predates #897 (
jdk→ Temurin 25). - Nit: the test at
build.ncl:81captures output without=$(...)underset -e, so a failedmvnexits silently. Echo$outon failure.
Generated by Claude Code
Review fixes: drop the maven stack's MAVEN_OPTS=-Dmaven.repo.local (the wired MAVEN_REPO_LOCAL already reaches stack builds and its -D beat MAVEN_OPTS, so stack builds were silently moving to the state dir anyway; the gradle stack likewise relies on its wiring alone). Note that the wired path overrides settings.xml <localRepository>. The repo_local_from_state_dir test now echoes mvn's output when it fails instead of exiting silently under set -e. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The JVM build tools get one convention: needs { dns, internet } as the
consumer-facing network dep (resolving from Maven Central is the tool's
job), the same as jbang, curl, go and uv. maven's own build stays offline.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks @norrietaylor. Addressed in 4d3f496 and c678fa9, on top of a merge of main (c4e8c15):
Also, for the Verified in a 🤖 Generated with Claude Code |
There was a problem hiding this comment.
All four points from my last review are addressed and verified in code:
- the stack's
MAVEN_OPTSis dropped; - the
settings.xmloverride is documented; - the branch is past #897;
- the ERR trap prints
$outon failure.
needs now matches jbang and the others.
Non-blocking:
- The comment at
build.sh:47-55could also mention that the wired-Dbeats a user'sMAVEN_OPTSand.mvn/maven.config. - Existing maven-stack users lose their
/state/m2/repositorycache once; worth a line in the merge notes. - The test's fixed
/tmp/m2-state-repo(build.ncl:92) could use$PWD, as mvnd's tests do.
Please merge this before #904.
Generated by Claude Code
Why
gradledeclares its cache as a session state directory (env_state_wiring:GRADLE_USER_HOME).mavendeclared nothing. So themavenstack, and thejavaloadout in #899, hardcodeMAVEN_OPTS=-Dmaven.repo.local=/state/m2/repository, and a session that uses maven outside those stacks falls back to~/.m2.What
attrs.env_state_wiring = { env_var = "MAVEN_REPO_LOCAL", prefix = "maven" }, the same mechanism gradle, go, node-lts, bun and deno use./usr/bin/mvnwrapper passes-Dmaven.repo.local="$MAVEN_REPO_LOCAL"when the variable is set. A-Dmaven.repo.localthe caller passes comes later on the command line and still wins. When set, it also overrides a<localRepository>in the user'ssettings.xml(a-Dproperty beats settings), which is intended.mavenstack drops itsMAVEN_OPTS=-Dmaven.repo.local=/state/m2/repository. The wired variable reaches stack builds too, and the wrapper's-DbeatsMAVEN_OPTS, so keeping both would have silently moved stack builds to a new, empty repository. The gradle stack likewise relies on its wiring alone.needs = { dns, internet }, the consumer-facing network dep (maven's own build stays offline). This is the one convention for the JVM build tools: jbang (jbang: add 0.142.0 #903) already declares it, and mvnd (mvnd: add 1.0.6 (Maven Daemon, embeds Maven 3.9.16) #904) does too.Test
repo_local_from_state_dir: a minimalpackaging=pomproject;validatebinds no plugins, so it runs offline (-o). WithMAVEN_REPO_LOCAL=/tmp/m2-state-repo, it assertsBUILD SUCCESSand that Maven's-Xoutput names that path, and it prints the mvn output when it fails. Verified in apkgbuildsession (min package build --rebuild maven+min check --packages maven): every check passes, includingstandalone tests. The package evaluates withmip dump.Follow-up
The
javaloadout in #899 (draft) still sets the sameMAVEN_OPTS; it should drop it once this lands.gradledoesn't declareneedsyet; it should get the same block in its own PR.🤖 Generated with Claude Code