gnodet-bot commented on code in PR #13368:
URL: https://github.com/apache/maven/pull/13368#discussion_r4203572904
##########
apache-maven/src/assembly/maven/bin/m2.conf:
##########
@@ -20,12 +20,13 @@ main is org.apache.maven.cling.MavenCling from plexus.core
set maven.conf default ${maven.home}/conf
set maven.installation.conf default ${maven.conf}
+set maven.user.conf default ${user.home}/.m2
Review Comment:
**Precedence edge case with `${user.maven-system.properties}`**
Classworlds processes `m2.conf` before `BaseParser` loads
`maven-system.properties`. In `MavenPropertiesLoader.loadProperties()`, the
file is loaded first into a `MavenProperties` map, then existing JVM system
properties are written *on top* (the `properties.forEach(sp::put)` call
overwrites file values). That means a user who sets `maven.user.conf` via their
personal `${user.home}/.m2/maven-system.properties` override (`${includes}`
mechanism) will find that the classworlds-set system property **beats** their
override — the file-level override never wins because the system property is
already in place.
This is consistent with how `maven.installation.conf` is handled (same
pattern), but it is worth noting in the PR description or a comment here, since
the PR body already acknowledges the command-line `-D` limitation. Users who
expect `~/.m2/maven-system.properties` to relocate ext jars will be surprised.
##########
apache-maven/src/assembly/maven/bin/m2.conf:
##########
@@ -20,12 +20,13 @@ main is org.apache.maven.cling.MavenCling from plexus.core
set maven.conf default ${maven.home}/conf
set maven.installation.conf default ${maven.conf}
+set maven.user.conf default ${user.home}/.m2
[plexus.core]
load ${maven.conf}/logging
optionally ${maven.home}/lib/ext/redisson/*.jar
optionally ${maven.home}/lib/ext/hazelcast/*.jar
-optionally ${user.home}/.m2/ext/*.jar
+optionally ${maven.user.conf}/ext/*.jar
Review Comment:
**`maven.project.conf` not covered**
`maven-system.properties` defines three path roots:
```
maven.installation.conf = ${maven.home}/conf
maven.user.conf = ${user.home}/.m2
maven.project.conf = ${session.rootDirectory}/.mvn
```
This PR covers `maven.user.conf`. There is no `optionally
${maven.project.conf}/ext/*.jar` entry. That may be intentional (project-level
ext jars already go through `extensions.xml`), but it is asymmetric — worth a
comment explaining why project-level ext jars are excluded here, or adding the
entry if it should be symmetric.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
