Re: [PR] Default value must be manually handled [maven-mvnd]
cstamas merged PR #953: URL: https://github.com/apache/maven-mvnd/pull/953 -- 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Default value must be manually handled [maven-mvnd]
gnodet commented on code in PR #953:
URL: https://github.com/apache/maven-mvnd/pull/953#discussion_r1576253478
##
common/src/main/java/org/mvndaemon/mvnd/common/Environment.java:
##
@@ -224,7 +224,7 @@ public enum Environment {
null,
"io.takari.maven:takari-smart-builder",
OptionType.STRING,
-Flags.OPTIONAL),
+Flags.DISCRIMINATING | Flags.OPTIONAL),
Review Comment:
I don't think this one is discriminating. What is discriminating is the
list of extensions to load which may actually be affected by this property, but
not necessarily. If you add an exclusion which is not supposed to be loaded,
this won't affect the result. In addition, that property is not really
interesting on the daemon side, as the extensions list is loaded by the
client...
--
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-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Default value must be manually handled [maven-mvnd]
cstamas commented on PR #953: URL: https://github.com/apache/maven-mvnd/pull/953#issuecomment-2071802629 Unsure why ubuntu failed, win passed locally also passed w/ same CLI params as CI invoked :thinking: -- 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Default value must be manually handled [maven-mvnd]
cstamas commented on code in PR #953: URL: https://github.com/apache/maven-mvnd/pull/953#discussion_r1575828803 ## integration-tests/src/test/projects/maven-conf-ignore-ext-def/pom.xml: ## @@ -0,0 +1,27 @@
Re: [PR] Default value must be manually handled [maven-mvnd]
gnodet commented on code in PR #953:
URL: https://github.com/apache/maven-mvnd/pull/953#discussion_r1575782068
##
client/src/main/java/org/mvndaemon/mvnd/client/DaemonParameters.java:
##
@@ -491,16 +491,22 @@ private static List
readCoreExtensionsDescriptor(Path multiModule
private static List
filterCoreExtensions(List coreExtensions) {
Set exclusions = new HashSet<>();
-String exclusionsString =
-
systemProperty(Environment.MVND_CORE_EXTENSIONS_EXCLUDE).asString();
+String exclusionsString =
systemProperty(Environment.MVND_CORE_EXTENSIONS_EXCLUDE)
+.asOptional()
Review Comment:
I think this should be:
```
String exclusionsString =
systemProperty(Environment.MVND_CORE_EXTENSIONS_EXCLUDE).orDefault().asString();
```
##
client/src/main/java/org/mvndaemon/mvnd/client/DaemonParameters.java:
##
@@ -491,16 +491,22 @@ private static List
readCoreExtensionsDescriptor(Path multiModule
private static List
filterCoreExtensions(List coreExtensions) {
Set exclusions = new HashSet<>();
-String exclusionsString =
-
systemProperty(Environment.MVND_CORE_EXTENSIONS_EXCLUDE).asString();
+String exclusionsString =
systemProperty(Environment.MVND_CORE_EXTENSIONS_EXCLUDE)
+.asOptional()
+.orElse(Environment.MVND_CORE_EXTENSIONS_EXCLUDE.getDefault());
if (exclusionsString != null) {
exclusions.addAll(Arrays.stream(exclusionsString.split(","))
.filter(e -> e != null && !e.trim().isEmpty())
.collect(Collectors.toList()));
}
-return coreExtensions.stream()
-.filter(e -> !exclusions.contains(e.getGroupId() + ":" +
e.getArtifactId()))
-.collect(Collectors.toList());
+if (!exclusions.isEmpty()) {
+LOG.info("Excluded extensions (GA): {}", exclusions);
Review Comment:
This should definitely not be part of the output, and not 4 times.
I'll revert this chunk, or log to debug at most.
##
integration-tests/src/test/projects/maven-conf-ignore-ext/pom.xml:
##
@@ -0,0 +1,27 @@
Re: [PR] Default value must be manually handled [maven-mvnd]
gnodet commented on PR #953: URL: https://github.com/apache/maven-mvnd/pull/953#issuecomment-2070825470 > @gnodet If you run this PR locally: > > * it will pass ok, but > * maven-conf-ignore-ext IT is **wrong**: maven.conf where ignore is, is not observed by mvnd. Also local repository will show that extension foo:bar is attempted to resolve, but naturally unsuccessfully > * this means that mvnd happily goes along instead to fail (as it failed to resolve the extension) > > Any idea? Yes, the problem is here: * https://github.com/apache/maven-mvnd/blob/dc4179fc3ba193e18d84dc090abfe336649d63ed/daemon-m40/src/main/java/org/apache/maven/cli/DaemonMavenCli.java#L699 * https://github.com/apache/maven-mvnd/blob/dc4179fc3ba193e18d84dc090abfe336649d63ed/daemon-m39/src/main/java/org/apache/maven/cli/DaemonMavenCli.java#L615 A failure to resolve the extension simply prints a warning and ignore That should be fixed. -- 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Default value must be manually handled [maven-mvnd]
cstamas commented on PR #953: URL: https://github.com/apache/maven-mvnd/pull/953#issuecomment-2070176626 @gnodet If you run this PR locally: * it will pass ok, but * maven-conf-ignore-ext IT is **wrong**: maven.conf where ignore is, is not observed by mvnd. Also local repository will show that extension foo:bar is attempted to resolve, but naturally unsuccessfully * this means that mvnd happily goes along instead to fail (as it failed to resolve the extension) Any idea? -- 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Default value must be manually handled [maven-mvnd]
cstamas commented on PR #953: URL: https://github.com/apache/maven-mvnd/pull/953#issuecomment-2070153385 Sure, this will be in it. But as i wrote, seems there is something else to be fixed as IT this PR adds shows... -- 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Default value must be manually handled [maven-mvnd]
wendigo commented on PR #953: URL: https://github.com/apache/maven-mvnd/pull/953#issuecomment-2070141407 Can we get this in for alpha14? -- 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Default value must be manually handled [maven-mvnd]
cstamas commented on PR #953: URL: https://github.com/apache/maven-mvnd/pull/953#issuecomment-2067741336 The code change is OK and fixes issue, but something is wrong. Created https://github.com/apache/maven-mvnd/issues/957 -- 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Default value must be manually handled [maven-mvnd]
cstamas commented on PR #953: URL: https://github.com/apache/maven-mvnd/pull/953#issuecomment-2067647961 Am adding IT (something I'd done initially would catch this), and will merge -- 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Default value must be manually handled [maven-mvnd]
wendigo commented on PR #953: URL: https://github.com/apache/maven-mvnd/pull/953#issuecomment-2067647580 That works :) Thanks! -- 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
