You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/12/07 16:40:03 UTC
[GitHub] [maven-mvnd] oehme opened a new pull request, #749: Add more discriminating properties to the daemon
oehme opened a new pull request, #749:
URL: https://github.com/apache/maven-mvnd/pull/749
The java home is an immutable property of the daemon and influences its behavior. It should thus be a discriminating property.
The local repo and maven settings are both used while resolving core extensions, which happens during daemon startup. Thus these two also need to be discriminating properties, as the core extensions themselves are discriminating.
Our concrete use case is that our extension's integration tests use an isolated repository which is deleted after the build, so we don't pile up garbage data on our CI agents. The lack of support for a custom local repository means we currently can't run the tests for our Maven extension against the Maven daemon. So technically we only care about the local repo property. But reading through the other properties I noticed that java home and maven settings ought to be changed as well.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-mvnd] oehme commented on a diff in pull request #749: Add more discriminating properties to the daemon
Posted by GitBox <gi...@apache.org>.
oehme commented on code in PR #749:
URL: https://github.com/apache/maven-mvnd/pull/749#discussion_r1044286021
##########
common/src/main/java/org/mvndaemon/mvnd/common/Environment.java:
##########
@@ -94,9 +94,16 @@ public enum Environment {
// Maven properties
//
/** The path to the Maven local repository */
- MAVEN_REPO_LOCAL("maven.repo.local", null, null, OptionType.PATH, Flags.NONE),
+ MAVEN_REPO_LOCAL("maven.repo.local", null, null, OptionType.PATH, Flags.DISCRIMINATING | Flags.OPTIONAL),
/** The location of the maven settings file */
- MAVEN_SETTINGS("maven.settings", null, null, OptionType.PATH, Flags.NONE, "mvn:-s", "mvn:--settings"),
+ MAVEN_SETTINGS(
+ "maven.settings",
+ null,
+ null,
+ OptionType.PATH,
+ Flags.DISCRIMINATING | Flags.OPTIONAL,
+ "mvn:-s",
+ "mvn:--settings"),
Review Comment:
Yes, it should, but from a cursory look at this class it's not yet supported at all. I also think that the Java Home should be discriminating and that the actual content of the settings.xml should be considered as well, but all of these seemed out of scope for this PR.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-mvnd] marcphilipp commented on a diff in pull request #749: Add more discriminating properties to the daemon
Posted by GitBox <gi...@apache.org>.
marcphilipp commented on code in PR #749:
URL: https://github.com/apache/maven-mvnd/pull/749#discussion_r1044217427
##########
common/src/main/java/org/mvndaemon/mvnd/common/Environment.java:
##########
@@ -94,9 +94,16 @@ public enum Environment {
// Maven properties
//
/** The path to the Maven local repository */
- MAVEN_REPO_LOCAL("maven.repo.local", null, null, OptionType.PATH, Flags.NONE),
+ MAVEN_REPO_LOCAL("maven.repo.local", null, null, OptionType.PATH, Flags.DISCRIMINATING | Flags.OPTIONAL),
/** The location of the maven settings file */
- MAVEN_SETTINGS("maven.settings", null, null, OptionType.PATH, Flags.NONE, "mvn:-s", "mvn:--settings"),
+ MAVEN_SETTINGS(
+ "maven.settings",
+ null,
+ null,
+ OptionType.PATH,
+ Flags.DISCRIMINATING | Flags.OPTIONAL,
+ "mvn:-s",
+ "mvn:--settings"),
Review Comment:
@oehme Should this also apply to `-gs` and `--global-settings`?
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-mvnd] ppalaga merged pull request #749: Add more discriminating properties to the daemon
Posted by GitBox <gi...@apache.org>.
ppalaga merged PR #749:
URL: https://github.com/apache/maven-mvnd/pull/749
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-mvnd] ppalaga commented on pull request #749: Add more discriminating properties to the daemon
Posted by GitBox <gi...@apache.org>.
ppalaga commented on PR #749:
URL: https://github.com/apache/maven-mvnd/pull/749#issuecomment-1343431187
Indeed, very useful, thanks @oehme!
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org