[jira] [Updated] (SUREFIRE-2190) optional dependencies and JPMS modules confuse surefire
[
https://issues.apache.org/jira/browse/SUREFIRE-2190?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Slawomir Jaranowski updated SUREFIRE-2190:
--
Fix Version/s: 3.2.0
> optional dependencies and JPMS modules confuse surefire
> ---
>
> Key: SUREFIRE-2190
> URL: https://issues.apache.org/jira/browse/SUREFIRE-2190
> Project: Maven Surefire
> Issue Type: Bug
> Components: JUnit 5.x support, Maven Surefire Plugin
>Affects Versions: 3.1.2
>Reporter: Henning Schmiedehausen
>Priority: Major
> Fix For: 3.2.0
>
>
> The surefire plugin, when executing tests for JPMS code, patches the test
> code "into" the module under test (using {{{}--patch-module{}}}). This work
> for compile+runtime dependencies (`requires`) but not for compile
> required/runtime optional dependencies ({{{}requires static{}}}).
> The plugin only adds the module under test using {{--add-modules
> module.under.test.id}} to the JVM that is executing the test classes. As
> {{requires static}} dependencies are not loaded transitively, any dependency
> that is optional for the main artifact but required for test code is not
> found.
> clone and build the test repo: [https://github.com/hgschmie/msurefire2190]
> This repo contains three artifacts with identical code:
> thing1 builds a main artifact without JPMS
> thing2 builds a main artifact with a strong ({{{}requires{}}}) dependency on
> jakarta.annotation.
> thing3 builds a main artifact with a compile-only ({{requires static}})
> dependency on jakarta.annotation.
> The code and its test classes are otherwise identical.
> Running {{mvn -DskipTests}} clean install builds all three modules and the
> test code.
> Running {{mvn surefire:test}} passes the tests in the first two modules but
> fails in the third.
> Explanation:
> The surefire plugin, when it executes tests using JPMS adds all referenced
> modules to the module path (in this case the module under test itself and the
> jakarta.annotations-api jar). It then adds the main module using
> {{--add-modules}} and patches this module with the test classes (using
> {{{}-patch-module{}}}, so that the test classes execute as part of the module.
> In case of a compile+runtime ({{requires}}) relationship, the JVM will find
> the required JPMS module on the module path and add it as accessible. This is
> why the "thing2" tests pass.
> In case of a compile only/runtime optional ({{requires static}})
> relationship, the JVM will not add the module transitively as it is
> considered a compilation-only dependency. For the code under test to be able
> to access the classes from jakarta.annotation, they must be declared as a
> module. However, the test code only adds the module under test with
> {{--add-modules}}. So the test classes do not find any classes from the
> jakarta.annotation module and the test fails.
> The fix is simple: Instead of just adding the module under test using
> {{--add-modules}}, the surefire plugin should use {{-add-modules
> ALL-MODULE-PATH}}.
> Which is correct, because the code is not looking for compile time only
> dependencies but actual runtime dependencies where the code under execution
> may also need to access optional runtime dependencies (see
> [https://nipafx.dev/java-modules-optional-dependencies/] for a slightly
> longer explanation).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Assigned] (SUREFIRE-2190) optional dependencies and JPMS modules confuse surefire
[
https://issues.apache.org/jira/browse/SUREFIRE-2190?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Slawomir Jaranowski reassigned SUREFIRE-2190:
-
Assignee: Henning Schmiedehausen
> optional dependencies and JPMS modules confuse surefire
> ---
>
> Key: SUREFIRE-2190
> URL: https://issues.apache.org/jira/browse/SUREFIRE-2190
> Project: Maven Surefire
> Issue Type: Bug
> Components: JUnit 5.x support, Maven Surefire Plugin
>Affects Versions: 3.1.2
>Reporter: Henning Schmiedehausen
>Assignee: Henning Schmiedehausen
>Priority: Major
> Fix For: 3.2.0
>
>
> The surefire plugin, when executing tests for JPMS code, patches the test
> code "into" the module under test (using {{{}--patch-module{}}}). This work
> for compile+runtime dependencies (`requires`) but not for compile
> required/runtime optional dependencies ({{{}requires static{}}}).
> The plugin only adds the module under test using {{--add-modules
> module.under.test.id}} to the JVM that is executing the test classes. As
> {{requires static}} dependencies are not loaded transitively, any dependency
> that is optional for the main artifact but required for test code is not
> found.
> clone and build the test repo: [https://github.com/hgschmie/msurefire2190]
> This repo contains three artifacts with identical code:
> thing1 builds a main artifact without JPMS
> thing2 builds a main artifact with a strong ({{{}requires{}}}) dependency on
> jakarta.annotation.
> thing3 builds a main artifact with a compile-only ({{requires static}})
> dependency on jakarta.annotation.
> The code and its test classes are otherwise identical.
> Running {{mvn -DskipTests}} clean install builds all three modules and the
> test code.
> Running {{mvn surefire:test}} passes the tests in the first two modules but
> fails in the third.
> Explanation:
> The surefire plugin, when it executes tests using JPMS adds all referenced
> modules to the module path (in this case the module under test itself and the
> jakarta.annotations-api jar). It then adds the main module using
> {{--add-modules}} and patches this module with the test classes (using
> {{{}-patch-module{}}}, so that the test classes execute as part of the module.
> In case of a compile+runtime ({{requires}}) relationship, the JVM will find
> the required JPMS module on the module path and add it as accessible. This is
> why the "thing2" tests pass.
> In case of a compile only/runtime optional ({{requires static}})
> relationship, the JVM will not add the module transitively as it is
> considered a compilation-only dependency. For the code under test to be able
> to access the classes from jakarta.annotation, they must be declared as a
> module. However, the test code only adds the module under test with
> {{--add-modules}}. So the test classes do not find any classes from the
> jakarta.annotation module and the test fails.
> The fix is simple: Instead of just adding the module under test using
> {{--add-modules}}, the surefire plugin should use {{-add-modules
> ALL-MODULE-PATH}}.
> Which is correct, because the code is not looking for compile time only
> dependencies but actual runtime dependencies where the code under execution
> may also need to access optional runtime dependencies (see
> [https://nipafx.dev/java-modules-optional-dependencies/] for a slightly
> longer explanation).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[GitHub] [maven] elharo commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1304137698
##
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+private void copyFile(File source, File destination) throws IOException {
Review Comment:
I wouldn't assume the existing method is correct. Please use the JDK
Files.copy method instead
--
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
[jira] [Commented] (MNG-7820) Remove dependency on plexus-utils
[
https://issues.apache.org/jira/browse/MNG-7820?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17758515#comment-17758515
]
ASF GitHub Bot commented on MNG-7820:
-
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1304137698
##
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+private void copyFile(File source, File destination) throws IOException {
Review Comment:
I wouldn't assume the existing method is correct. Please use the JDK
Files.copy method instead
> Remove dependency on plexus-utils
> -
>
> Key: MNG-7820
> URL: https://issues.apache.org/jira/browse/MNG-7820
> Project: Maven
> Issue Type: Task
>Reporter: Guillaume Nodet
>Priority: Major
> Fix For: 4.0.0-alpha-8
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[GitHub] [maven-assembly-plugin] timtebeek commented on pull request #153: [MNG-6847] Use diamond operator
timtebeek commented on PR #153: URL: https://github.com/apache/maven-assembly-plugin/pull/153#issuecomment-1691447385 Force pushed the spotless fix. Is there any way for my PRs to skip the manual CI approval? I think that's usually the case when you've previously contributed, but I've not yet seen that with Apache Maven. And is there a better place (such as a shared Slack) where I can discuss such matters? Just looking to help out in the way that causes the least friction; OK if it's not an option to grant me any special privileges! :) -- 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
[jira] [Commented] (MNG-6847) Explicit type can be replaced by the diamond operator
[ https://issues.apache.org/jira/browse/MNG-6847?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17758517#comment-17758517 ] ASF GitHub Bot commented on MNG-6847: - timtebeek commented on PR #153: URL: https://github.com/apache/maven-assembly-plugin/pull/153#issuecomment-1691447385 Force pushed the spotless fix. Is there any way for my PRs to skip the manual CI approval? I think that's usually the case when you've previously contributed, but I've not yet seen that with Apache Maven. And is there a better place (such as a shared Slack) where I can discuss such matters? Just looking to help out in the way that causes the least friction; OK if it's not an option to grant me any special privileges! :) > Explicit type can be replaced by the diamond operator > - > > Key: MNG-6847 > URL: https://issues.apache.org/jira/browse/MNG-6847 > Project: Maven > Issue Type: Improvement > Components: Core >Affects Versions: 3.6.3 >Reporter: Krosheninnikov Artem >Assignee: Michael Osipov >Priority: Trivial > Fix For: 4.0.0-alpha-2, 4.0.0 > > Time Spent: 20m > Remaining Estimate: 0h > > As the title says. It's a small change to reduce unneeded verbosity in the > code. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MENFORCER-490) Properly declare dependencies
[ https://issues.apache.org/jira/browse/MENFORCER-490?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17758522#comment-17758522 ] ASF GitHub Bot commented on MENFORCER-490: -- elharo opened a new pull request, #283: URL: https://github.com/apache/maven-enforcer/pull/283 (no comment) > Properly declare dependencies > - > > Key: MENFORCER-490 > URL: https://issues.apache.org/jira/browse/MENFORCER-490 > Project: Maven Enforcer Plugin > Issue Type: Bug >Reporter: Elliotte Rusty Harold >Assignee: Elliotte Rusty Harold >Priority: Minor > > mvn dependency:analyze reveals a number of undeclared dependencies in various > sub modules. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Created] (MSHADE-456) test-jar dependencies remain in POM after being shaded
Nick Tindall created MSHADE-456: --- Summary: test-jar dependencies remain in POM after being shaded Key: MSHADE-456 URL: https://issues.apache.org/jira/browse/MSHADE-456 Project: Maven Shade Plugin Issue Type: Bug Affects Versions: 3.5.0 Reporter: Nick Tindall When a shaded POM includes a *test-jar* dependency, it remains in the dependency-reduced POM despite being included in the uber-jar Minimal reproduction is available [here|https://github.com/nicktindall/shade-plugin-test-jar-issue] -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MDEP-832) Remove commons-collections-4
[
https://issues.apache.org/jira/browse/MDEP-832?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17758525#comment-17758525
]
Eric Lilja commented on MDEP-832:
-
The commit removes a usage of commons-collections-4, but the pom edit removes
removes commons-lang3 rather than commons-collections-4
> Remove commons-collections-4
>
>
> Key: MDEP-832
> URL: https://issues.apache.org/jira/browse/MDEP-832
> Project: Maven Dependency Plugin
> Issue Type: Improvement
>Affects Versions: 3.3.0
>Reporter: Karl Heinz Marbaise
>Assignee: Karl Heinz Marbaise
>Priority: Minor
> Fix For: next-release
>
>
> Remove the dependency:
> {code:xml}
>
> org.apache.commons
> commons-collections4
> 4.2
>
> {code}
> which is used only for a single method.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1304179696
##
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+private void copyFile(File source, File destination) throws IOException {
Review Comment:
It already uses `Files.copy`, so I think that's fine. If needed, it can be
inlined, but I don't see the benefit.
--
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
[jira] [Commented] (MNG-7820) Remove dependency on plexus-utils
[
https://issues.apache.org/jira/browse/MNG-7820?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17758526#comment-17758526
]
ASF GitHub Bot commented on MNG-7820:
-
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1304179696
##
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+private void copyFile(File source, File destination) throws IOException {
Review Comment:
It already uses `Files.copy`, so I think that's fine. If needed, it can be
inlined, but I don't see the benefit.
> Remove dependency on plexus-utils
> -
>
> Key: MNG-7820
> URL: https://issues.apache.org/jira/browse/MNG-7820
> Project: Maven
> Issue Type: Task
>Reporter: Guillaume Nodet
>Priority: Major
> Fix For: 4.0.0-alpha-8
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[GitHub] [maven] elharo commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1274774380
##
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+private void copyFile(File source, File destination) throws IOException {
+String message;
Review Comment:
declare where used, and don't reuse for two different messages, or just
inline the two uses of this variable
--
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
[jira] [Commented] (MNG-7820) Remove dependency on plexus-utils
[
https://issues.apache.org/jira/browse/MNG-7820?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17758550#comment-17758550
]
ASF GitHub Bot commented on MNG-7820:
-
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1274774380
##
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+private void copyFile(File source, File destination) throws IOException {
+String message;
Review Comment:
declare where used, and don't reuse for two different messages, or just
inline the two uses of this variable
> Remove dependency on plexus-utils
> -
>
> Key: MNG-7820
> URL: https://issues.apache.org/jira/browse/MNG-7820
> Project: Maven
> Issue Type: Task
>Reporter: Guillaume Nodet
>Priority: Major
> Fix For: 4.0.0-alpha-8
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[GitHub] [maven] elharo commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1304233854
##
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+private void copyFile(File source, File destination) throws IOException {
+String message;
+if (!source.exists()) {
+message = "File " + source + " does not exist";
+throw new IOException(message);
+} else if
(!source.getCanonicalPath().equals(destination.getCanonicalPath())) {
+File parentFile = destination.getParentFile();
+if (parentFile != null && !parentFile.exists()) {
+parentFile.mkdirs();
+}
+Files.copy(
+source.toPath(),
+destination.toPath(),
+StandardCopyOption.REPLACE_EXISTING,
+StandardCopyOption.COPY_ATTRIBUTES);
+if (source.length() != destination.length()) {
Review Comment:
This if block isn't needed. If Files.copy doesn't copy everything, an
IOException will be thrown above.
##
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+private void copyFile(File source, File destination) throws IOException {
+String message;
+if (!source.exists()) {
+message = "File " + source + " does not exist";
+throw new IOException(message);
+} else if
(!source.getCanonicalPath().equals(destination.getCanonicalPath())) {
+File parentFile = destination.getParentFile();
+if (parentFile != null && !parentFile.exists()) {
+parentFile.mkdirs();
Review Comment:
The return value isn't checked here. Use Files.createDirectories instead and
drop the exists check.
##
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+private void copyFile(File source, File destination) throws IOException {
Review Comment:
I think this can be static
--
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
[jira] [Commented] (MNG-7820) Remove dependency on plexus-utils
[
https://issues.apache.org/jira/browse/MNG-7820?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17758553#comment-17758553
]
ASF GitHub Bot commented on MNG-7820:
-
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1304233854
##
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+private void copyFile(File source, File destination) throws IOException {
+String message;
+if (!source.exists()) {
+message = "File " + source + " does not exist";
+throw new IOException(message);
+} else if
(!source.getCanonicalPath().equals(destination.getCanonicalPath())) {
+File parentFile = destination.getParentFile();
+if (parentFile != null && !parentFile.exists()) {
+parentFile.mkdirs();
+}
+Files.copy(
+source.toPath(),
+destination.toPath(),
+StandardCopyOption.REPLACE_EXISTING,
+StandardCopyOption.COPY_ATTRIBUTES);
+if (source.length() != destination.length()) {
Review Comment:
This if block isn't needed. If Files.copy doesn't copy everything, an
IOException will be thrown above.
##
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+private void copyFile(File source, File destination) throws IOException {
+String message;
+if (!source.exists()) {
+message = "File " + source + " does not exist";
+throw new IOException(message);
+} else if
(!source.getCanonicalPath().equals(destination.getCanonicalPath())) {
+File parentFile = destination.getParentFile();
+if (parentFile != null && !parentFile.exists()) {
+parentFile.mkdirs();
Review Comment:
The return value isn't checked here. Use Files.createDirectories instead and
drop the exists check.
##
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+private void copyFile(File source, File destination) throws IOException {
Review Comment:
I think this can be static
> Remove dependency on plexus-utils
> -
>
> Key: MNG-7820
> URL: https://issues.apache.org/jira/browse/MNG-7820
> Project: Maven
> Issue Type: Task
>Reporter: Guillaume Nodet
>Priority: Major
> Fix For: 4.0.0-alpha-8
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (MENFORCER-490) Properly declare dependencies
[ https://issues.apache.org/jira/browse/MENFORCER-490?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17758605#comment-17758605 ] ASF GitHub Bot commented on MENFORCER-490: -- elharo opened a new pull request, #284: URL: https://github.com/apache/maven-enforcer/pull/284 (no comment) > Properly declare dependencies > - > > Key: MENFORCER-490 > URL: https://issues.apache.org/jira/browse/MENFORCER-490 > Project: Maven Enforcer Plugin > Issue Type: Bug >Reporter: Elliotte Rusty Harold >Assignee: Elliotte Rusty Harold >Priority: Minor > > mvn dependency:analyze reveals a number of undeclared dependencies in various > sub modules. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[GitHub] [maven-build-cache-extension] atsteffen opened a new pull request, #98: Avoid hash calculation when is true
atsteffen opened a new pull request, #98: URL: https://github.com/apache/maven-build-cache-extension/pull/98 I have a large multi-module project with a top level project who only responsibility is to wrap together the other modules along with some large resources into a deployable zip or docker image. The top level project pulls together enough stuff that it gives an array size limit error when trying to calculate the cache for all of its inputs. Since this top level project should always be rebuilt if any code changes, it is a good candidate for using true. However, I found that using this parameter does not actually prevent the calculation of the hash. It seems reasonable to consider this parameter as disabling all aspects of caching, including hash calculation and saving of the cached artifact. I realize this change is rather surgical, and a more involved refactor might be warranted especially with respect to delicate boolean logic, and nullability of inputInfo in CacheContext. However, before I start any refactoring I wanted to make sure this issue was filed and discussed. Following this checklist to help us incorporate your contribution quickly and easily: - [ ] Make sure there is a [MBUILDCACHE JIRA issue](https://issues.apache.org/jira/browse/MBUILDCACHE) filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes. - [ ] Each commit in the pull request should have a meaningful subject line and body. - [ ] Format the pull request title like `[MBUILDCACHE-XXX] - Fixes bug in ApproximateQuantiles`, where you replace `MBUILDCACHE-XXX` with the appropriate JIRA issue. Best practice is to use the JIRA issue title in the pull request title and in the first line of the commit message. - [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why. - [ ] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically. - [ ] You have run the [Core IT][core-its] successfully. If your pull request is about ~20 lines of code you don't need to sign an [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure please ask on the developers list. To make clear that you license your contribution under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0) you have to acknowledge this by using the following check-box. - [ ] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0) - [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf). [core-its]: https://maven.apache.org/core-its/core-it-suite/ -- 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
[GitHub] [maven] gnodet commented on pull request #1208: [MNG-7820] Remove dependency on plexus-utils
gnodet commented on PR #1208: URL: https://github.com/apache/maven/pull/1208#issuecomment-1691906581 @elharo I don't really see the need to spend much time on rewriting code copied from a utility class. The DefaultWagonManager is part of maven-compat, so deprecated and not really maintained. Any possible cleanup could introduce problems, so I'm more in favour of a blunt copy/paste, even if the code is not the cleanest, to avoid any possible behaviour difference with this legacy code. For information, I've successfully run the whole IT test suite after changing the DefaultWagonManager to throw exceptions instead of actually downloading/uploading anything using a Wagon (and disabling the DefaultWagonManagerTest UT). However, it might still be in use by old plugins, so I would not delete it at this point (we do have warnings when using maven-compat since a few months afaik). But I would not spend too much time on this code either... -- 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
[jira] [Commented] (MNG-7820) Remove dependency on plexus-utils
[ https://issues.apache.org/jira/browse/MNG-7820?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17758632#comment-17758632 ] ASF GitHub Bot commented on MNG-7820: - gnodet commented on PR #1208: URL: https://github.com/apache/maven/pull/1208#issuecomment-1691906581 @elharo I don't really see the need to spend much time on rewriting code copied from a utility class. The DefaultWagonManager is part of maven-compat, so deprecated and not really maintained. Any possible cleanup could introduce problems, so I'm more in favour of a blunt copy/paste, even if the code is not the cleanest, to avoid any possible behaviour difference with this legacy code. For information, I've successfully run the whole IT test suite after changing the DefaultWagonManager to throw exceptions instead of actually downloading/uploading anything using a Wagon (and disabling the DefaultWagonManagerTest UT). However, it might still be in use by old plugins, so I would not delete it at this point (we do have warnings when using maven-compat since a few months afaik). But I would not spend too much time on this code either... > Remove dependency on plexus-utils > - > > Key: MNG-7820 > URL: https://issues.apache.org/jira/browse/MNG-7820 > Project: Maven > Issue Type: Task >Reporter: Guillaume Nodet >Priority: Major > Fix For: 4.0.0-alpha-8 > > -- This message was sent by Atlassian Jira (v8.20.10#820010)
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1304513934
##
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+private void copyFile(File source, File destination) throws IOException {
+String message;
+if (!source.exists()) {
+message = "File " + source + " does not exist";
+throw new IOException(message);
+} else if
(!source.getCanonicalPath().equals(destination.getCanonicalPath())) {
+File parentFile = destination.getParentFile();
+if (parentFile != null && !parentFile.exists()) {
+parentFile.mkdirs();
+}
+Files.copy(
+source.toPath(),
+destination.toPath(),
+StandardCopyOption.REPLACE_EXISTING,
+StandardCopyOption.COPY_ATTRIBUTES);
+if (source.length() != destination.length()) {
Review Comment:
I have no idea what could happen in case of concurrent access to the
repository...
Copy is not necessarily atomic, so I'm not sure that an exception will
always be thrown before this check.
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1304514826
##
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+private void copyFile(File source, File destination) throws IOException {
Review Comment:
> I wouldn't assume the existing method is correct. Please use the JDK
Files.copy method instead
It already uses Files.copy, so I think that's fine. If needed, it can be
inlined, but I don't see the benefit.
--
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
[jira] [Commented] (MNG-7820) Remove dependency on plexus-utils
[
https://issues.apache.org/jira/browse/MNG-7820?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17758633#comment-17758633
]
ASF GitHub Bot commented on MNG-7820:
-
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1304513934
##
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+private void copyFile(File source, File destination) throws IOException {
+String message;
+if (!source.exists()) {
+message = "File " + source + " does not exist";
+throw new IOException(message);
+} else if
(!source.getCanonicalPath().equals(destination.getCanonicalPath())) {
+File parentFile = destination.getParentFile();
+if (parentFile != null && !parentFile.exists()) {
+parentFile.mkdirs();
+}
+Files.copy(
+source.toPath(),
+destination.toPath(),
+StandardCopyOption.REPLACE_EXISTING,
+StandardCopyOption.COPY_ATTRIBUTES);
+if (source.length() != destination.length()) {
Review Comment:
I have no idea what could happen in case of concurrent access to the
repository...
Copy is not necessarily atomic, so I'm not sure that an exception will
always be thrown before this check.
> Remove dependency on plexus-utils
> -
>
> Key: MNG-7820
> URL: https://issues.apache.org/jira/browse/MNG-7820
> Project: Maven
> Issue Type: Task
>Reporter: Guillaume Nodet
>Priority: Major
> Fix For: 4.0.0-alpha-8
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (MNG-7820) Remove dependency on plexus-utils
[
https://issues.apache.org/jira/browse/MNG-7820?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17758635#comment-17758635
]
ASF GitHub Bot commented on MNG-7820:
-
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1304514826
##
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+private void copyFile(File source, File destination) throws IOException {
Review Comment:
> I wouldn't assume the existing method is correct. Please use the JDK
Files.copy method instead
It already uses Files.copy, so I think that's fine. If needed, it can be
inlined, but I don't see the benefit.
> Remove dependency on plexus-utils
> -
>
> Key: MNG-7820
> URL: https://issues.apache.org/jira/browse/MNG-7820
> Project: Maven
> Issue Type: Task
>Reporter: Guillaume Nodet
>Priority: Major
> Fix For: 4.0.0-alpha-8
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[GitHub] [maven-enforcer] elharo merged pull request #284: [MENFORCER-490] Declare maven-enforcer-extension dependencies
elharo merged PR #284: URL: https://github.com/apache/maven-enforcer/pull/284 -- 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
[jira] [Commented] (MENFORCER-490) Properly declare dependencies
[ https://issues.apache.org/jira/browse/MENFORCER-490?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17758670#comment-17758670 ] ASF GitHub Bot commented on MENFORCER-490: -- elharo merged PR #284: URL: https://github.com/apache/maven-enforcer/pull/284 > Properly declare dependencies > - > > Key: MENFORCER-490 > URL: https://issues.apache.org/jira/browse/MENFORCER-490 > Project: Maven Enforcer Plugin > Issue Type: Bug >Reporter: Elliotte Rusty Harold >Assignee: Elliotte Rusty Harold >Priority: Minor > > mvn dependency:analyze reveals a number of undeclared dependencies in various > sub modules. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MENFORCER-490) Properly declare dependencies
[ https://issues.apache.org/jira/browse/MENFORCER-490?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17758671#comment-17758671 ] ASF GitHub Bot commented on MENFORCER-490: -- elharo merged PR #283: URL: https://github.com/apache/maven-enforcer/pull/283 > Properly declare dependencies > - > > Key: MENFORCER-490 > URL: https://issues.apache.org/jira/browse/MENFORCER-490 > Project: Maven Enforcer Plugin > Issue Type: Bug >Reporter: Elliotte Rusty Harold >Assignee: Elliotte Rusty Harold >Priority: Minor > > mvn dependency:analyze reveals a number of undeclared dependencies in various > sub modules. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MENFORCER-490) Properly declare dependencies
[ https://issues.apache.org/jira/browse/MENFORCER-490?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17758750#comment-17758750 ] ASF GitHub Bot commented on MENFORCER-490: -- elharo opened a new pull request, #285: URL: https://github.com/apache/maven-enforcer/pull/285 (no comment) > Properly declare dependencies > - > > Key: MENFORCER-490 > URL: https://issues.apache.org/jira/browse/MENFORCER-490 > Project: Maven Enforcer Plugin > Issue Type: Bug >Reporter: Elliotte Rusty Harold >Assignee: Elliotte Rusty Harold >Priority: Minor > > mvn dependency:analyze reveals a number of undeclared dependencies in various > sub modules. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[GitHub] [maven] CrazyHZM commented on pull request #1208: [MNG-7820] Remove dependency on plexus-utils
CrazyHZM commented on PR #1208: URL: https://github.com/apache/maven/pull/1208#issuecomment-1692669455 > @elharo I don't really see the need to spend much time on rewriting code copied from a utility class. The DefaultWagonManager is part of maven-compat, so deprecated and not really maintained. Any possible cleanup could introduce problems, so I'm more in favour of a blunt copy/paste, even if the code is not the cleanest, to avoid any possible behaviour difference with this legacy code. > > For information, I've successfully run the whole IT test suite after changing the DefaultWagonManager to throw exceptions instead of actually downloading/uploading anything using a Wagon (and disabling the DefaultWagonManagerTest UT). However, it might still be in use by old plugins, so I would not delete it at this point (we do have warnings when using maven-compat since a few months afaik). But I would not spend too much time on this code either... > > This is different for the DAG stuff which is used by maven-core... @gnodet So we should still do some optimization on the copied DAG, right? -- 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
[jira] [Commented] (MNG-7820) Remove dependency on plexus-utils
[ https://issues.apache.org/jira/browse/MNG-7820?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17758794#comment-17758794 ] ASF GitHub Bot commented on MNG-7820: - CrazyHZM commented on PR #1208: URL: https://github.com/apache/maven/pull/1208#issuecomment-1692669455 > @elharo I don't really see the need to spend much time on rewriting code copied from a utility class. The DefaultWagonManager is part of maven-compat, so deprecated and not really maintained. Any possible cleanup could introduce problems, so I'm more in favour of a blunt copy/paste, even if the code is not the cleanest, to avoid any possible behaviour difference with this legacy code. > > For information, I've successfully run the whole IT test suite after changing the DefaultWagonManager to throw exceptions instead of actually downloading/uploading anything using a Wagon (and disabling the DefaultWagonManagerTest UT). However, it might still be in use by old plugins, so I would not delete it at this point (we do have warnings when using maven-compat since a few months afaik). But I would not spend too much time on this code either... > > This is different for the DAG stuff which is used by maven-core... @gnodet So we should still do some optimization on the copied DAG, right? > Remove dependency on plexus-utils > - > > Key: MNG-7820 > URL: https://issues.apache.org/jira/browse/MNG-7820 > Project: Maven > Issue Type: Task >Reporter: Guillaume Nodet >Priority: Major > Fix For: 4.0.0-alpha-8 > > -- This message was sent by Atlassian Jira (v8.20.10#820010)
[GitHub] [maven] CrazyHZM commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
CrazyHZM commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1305096986
##
maven-core/src/main/java/org/apache/maven/utils/introspection/ReflectionValueExtractor.java:
##
@@ -0,0 +1,325 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.utils.introspection;
+
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+/**
+ *
+ * Using simple dotted expressions to extract the values from an Object
instance, For example we might want to extract a
+ * value like: project.build.sourceDirectory
+ *
+ *
+ * The implementation supports indexed, nested and mapped properties similar
to the JSP way.
+ *
+ *
+ * @author mailto:ja...@maven.org";>Jason van Zyl
+ * @author mailto:vincent.sive...@gmail.com";>Vincent Siveton
+ *
+ * @see http://struts.apache.org/1.x/struts-taglib/indexedprops.html";>http://struts.apache.org/1.x/struts-taglib/indexedprops.html
+ */
+public class ReflectionValueExtractor {
Review Comment:
It is used by `org.apache.maven.plugin.PluginParameterExpressionEvaluatorV4`
and cannot be made public.
--
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
[jira] [Commented] (MNG-7820) Remove dependency on plexus-utils
[
https://issues.apache.org/jira/browse/MNG-7820?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17758817#comment-17758817
]
ASF GitHub Bot commented on MNG-7820:
-
CrazyHZM commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1305096986
##
maven-core/src/main/java/org/apache/maven/utils/introspection/ReflectionValueExtractor.java:
##
@@ -0,0 +1,325 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.utils.introspection;
+
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+/**
+ *
+ * Using simple dotted expressions to extract the values from an Object
instance, For example we might want to extract a
+ * value like: project.build.sourceDirectory
+ *
+ *
+ * The implementation supports indexed, nested and mapped properties similar
to the JSP way.
+ *
+ *
+ * @author mailto:ja...@maven.org";>Jason van Zyl
+ * @author mailto:vincent.sive...@gmail.com";>Vincent Siveton
+ *
+ * @see http://struts.apache.org/1.x/struts-taglib/indexedprops.html";>http://struts.apache.org/1.x/struts-taglib/indexedprops.html
+ */
+public class ReflectionValueExtractor {
Review Comment:
It is used by `org.apache.maven.plugin.PluginParameterExpressionEvaluatorV4`
and cannot be made public.
> Remove dependency on plexus-utils
> -
>
> Key: MNG-7820
> URL: https://issues.apache.org/jira/browse/MNG-7820
> Project: Maven
> Issue Type: Task
>Reporter: Guillaume Nodet
>Priority: Major
> Fix For: 4.0.0-alpha-8
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[GitHub] [maven] CrazyHZM commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
CrazyHZM commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1305099150
##
maven-core/src/main/java/org/apache/maven/utils/dag/CycleDetector.java:
##
@@ -0,0 +1,136 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.utils.dag;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * @author Michal Maczka
+ *
+ */
+public class CycleDetector {
+
+private static final Integer NOT_VISITED = 0;
+
+private static final Integer VISITING = 1;
+
+private static final Integer VISITED = 2;
+
+public static List hasCycle(final DAG graph) {
+final List vertices = graph.getVertices();
+
+final Map vertexStateMap = new HashMap<>();
+
+List retValue = null;
+
+for (Vertex vertex : vertices) {
+if (isNotVisited(vertex, vertexStateMap)) {
+retValue = introducesCycle(vertex, vertexStateMap);
+
+if (retValue != null) {
+break;
+}
+}
+}
+
+return retValue;
+}
+
+/**
+ * This method will be called when an edge leading to given vertex was
added and we want to check if introduction of
+ * this edge has not resulted in apparition of cycle in the graph
+ *
+ * @param vertex the vertex
+ * @param vertexStateMap the vertex Map
+ * @return the found cycle
+ */
+public static List introducesCycle(final Vertex vertex, final
Map vertexStateMap) {
Review Comment:
change to `private`.
--
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
[jira] [Commented] (MNG-7820) Remove dependency on plexus-utils
[
https://issues.apache.org/jira/browse/MNG-7820?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17758818#comment-17758818
]
ASF GitHub Bot commented on MNG-7820:
-
CrazyHZM commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1305099150
##
maven-core/src/main/java/org/apache/maven/utils/dag/CycleDetector.java:
##
@@ -0,0 +1,136 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.utils.dag;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * @author Michal Maczka
+ *
+ */
+public class CycleDetector {
+
+private static final Integer NOT_VISITED = 0;
+
+private static final Integer VISITING = 1;
+
+private static final Integer VISITED = 2;
+
+public static List hasCycle(final DAG graph) {
+final List vertices = graph.getVertices();
+
+final Map vertexStateMap = new HashMap<>();
+
+List retValue = null;
+
+for (Vertex vertex : vertices) {
+if (isNotVisited(vertex, vertexStateMap)) {
+retValue = introducesCycle(vertex, vertexStateMap);
+
+if (retValue != null) {
+break;
+}
+}
+}
+
+return retValue;
+}
+
+/**
+ * This method will be called when an edge leading to given vertex was
added and we want to check if introduction of
+ * this edge has not resulted in apparition of cycle in the graph
+ *
+ * @param vertex the vertex
+ * @param vertexStateMap the vertex Map
+ * @return the found cycle
+ */
+public static List introducesCycle(final Vertex vertex, final
Map vertexStateMap) {
Review Comment:
change to `private`.
> Remove dependency on plexus-utils
> -
>
> Key: MNG-7820
> URL: https://issues.apache.org/jira/browse/MNG-7820
> Project: Maven
> Issue Type: Task
>Reporter: Guillaume Nodet
>Priority: Major
> Fix For: 4.0.0-alpha-8
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[GitHub] [maven] CrazyHZM commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
CrazyHZM commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1305099465
##
maven-core/src/main/java/org/apache/maven/utils/dag/CycleDetector.java:
##
@@ -0,0 +1,136 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.utils.dag;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * @author Michal Maczka
+ *
+ */
+public class CycleDetector {
+
+private static final Integer NOT_VISITED = 0;
+
+private static final Integer VISITING = 1;
+
+private static final Integer VISITED = 2;
+
+public static List hasCycle(final DAG graph) {
+final List vertices = graph.getVertices();
+
+final Map vertexStateMap = new HashMap<>();
+
+List retValue = null;
+
+for (Vertex vertex : vertices) {
+if (isNotVisited(vertex, vertexStateMap)) {
+retValue = introducesCycle(vertex, vertexStateMap);
+
+if (retValue != null) {
+break;
+}
+}
+}
+
+return retValue;
+}
+
+/**
+ * This method will be called when an edge leading to given vertex was
added and we want to check if introduction of
Review Comment:
Done.
--
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
[jira] [Commented] (MNG-7820) Remove dependency on plexus-utils
[
https://issues.apache.org/jira/browse/MNG-7820?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17758819#comment-17758819
]
ASF GitHub Bot commented on MNG-7820:
-
CrazyHZM commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1305099465
##
maven-core/src/main/java/org/apache/maven/utils/dag/CycleDetector.java:
##
@@ -0,0 +1,136 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.utils.dag;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * @author Michal Maczka
+ *
+ */
+public class CycleDetector {
+
+private static final Integer NOT_VISITED = 0;
+
+private static final Integer VISITING = 1;
+
+private static final Integer VISITED = 2;
+
+public static List hasCycle(final DAG graph) {
+final List vertices = graph.getVertices();
+
+final Map vertexStateMap = new HashMap<>();
+
+List retValue = null;
+
+for (Vertex vertex : vertices) {
+if (isNotVisited(vertex, vertexStateMap)) {
+retValue = introducesCycle(vertex, vertexStateMap);
+
+if (retValue != null) {
+break;
+}
+}
+}
+
+return retValue;
+}
+
+/**
+ * This method will be called when an edge leading to given vertex was
added and we want to check if introduction of
Review Comment:
Done.
> Remove dependency on plexus-utils
> -
>
> Key: MNG-7820
> URL: https://issues.apache.org/jira/browse/MNG-7820
> Project: Maven
> Issue Type: Task
>Reporter: Guillaume Nodet
>Priority: Major
> Fix For: 4.0.0-alpha-8
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (MNG-7822) Add support for TRACE logging level (with style "bold,magenta")
[
https://issues.apache.org/jira/browse/MNG-7822?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17758822#comment-17758822
]
ASF GitHub Bot commented on MNG-7822:
-
CrazyHZM commented on code in PR #1215:
URL: https://github.com/apache/maven/pull/1215#discussion_r1305103976
##
api/maven-api-core/src/main/java/org/apache/maven/api/services/MessageBuilder.java:
##
@@ -27,6 +27,16 @@
* @see MessageBuilderFactory
*/
public interface MessageBuilder {
+
+/**
+ * Append message content in trace style.
+ * By default, bold magenta
+ * @param message the message to append
Review Comment:
Done.
> Add support for TRACE logging level (with style "bold,magenta")
> ---
>
> Key: MNG-7822
> URL: https://issues.apache.org/jira/browse/MNG-7822
> Project: Maven
> Issue Type: New Feature
> Components: Logging
>Reporter: Richard O'Sullivan
>Priority: Trivial
>
> Maven uses 'Style' in 'org.apache.maven.shared.utils.logging' to enumerate
> the available logging levels. Currently, the 'TRACE' level is not enumerated.
> It would be helpful to add something like: 'TRACE("bold,magenta" ) '.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[GitHub] [maven-mvnd] Joeywzr closed issue #880: How can I get the mvnd with 3.6.3 version maven?
Joeywzr closed issue #880: How can I get the mvnd with 3.6.3 version maven? URL: https://github.com/apache/maven-mvnd/issues/880 -- 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
[GitHub] [maven] gnodet commented on pull request #1208: [MNG-7820] Remove dependency on plexus-utils
gnodet commented on PR #1208: URL: https://github.com/apache/maven/pull/1208#issuecomment-1692789865 > > @elharo I don't really see the need to spend much time on rewriting code copied from a utility class. The DefaultWagonManager is part of maven-compat, so deprecated and not really maintained. Any possible cleanup could introduce problems, so I'm more in favour of a blunt copy/paste, even if the code is not the cleanest, to avoid any possible behaviour difference with this legacy code. > > For information, I've successfully run the whole IT test suite after changing the DefaultWagonManager to throw exceptions instead of actually downloading/uploading anything using a Wagon (and disabling the DefaultWagonManagerTest UT). However, it might still be in use by old plugins, so I would not delete it at this point (we do have warnings when using maven-compat since a few months afaik). But I would not spend too much time on this code either... > > This is different for the DAG stuff which is used by maven-core... > > @gnodet So we should still do some optimization on the copied DAG, right? Yes -- 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
[jira] [Commented] (MNG-7820) Remove dependency on plexus-utils
[ https://issues.apache.org/jira/browse/MNG-7820?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17758837#comment-17758837 ] ASF GitHub Bot commented on MNG-7820: - gnodet commented on PR #1208: URL: https://github.com/apache/maven/pull/1208#issuecomment-1692789865 > > @elharo I don't really see the need to spend much time on rewriting code copied from a utility class. The DefaultWagonManager is part of maven-compat, so deprecated and not really maintained. Any possible cleanup could introduce problems, so I'm more in favour of a blunt copy/paste, even if the code is not the cleanest, to avoid any possible behaviour difference with this legacy code. > > For information, I've successfully run the whole IT test suite after changing the DefaultWagonManager to throw exceptions instead of actually downloading/uploading anything using a Wagon (and disabling the DefaultWagonManagerTest UT). However, it might still be in use by old plugins, so I would not delete it at this point (we do have warnings when using maven-compat since a few months afaik). But I would not spend too much time on this code either... > > This is different for the DAG stuff which is used by maven-core... > > @gnodet So we should still do some optimization on the copied DAG, right? Yes > Remove dependency on plexus-utils > - > > Key: MNG-7820 > URL: https://issues.apache.org/jira/browse/MNG-7820 > Project: Maven > Issue Type: Task >Reporter: Guillaume Nodet >Priority: Major > Fix For: 4.0.0-alpha-8 > > -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MNG-7820) Remove dependency on plexus-utils
[ https://issues.apache.org/jira/browse/MNG-7820?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17758839#comment-17758839 ] ASF GitHub Bot commented on MNG-7820: - CrazyHZM commented on PR #1208: URL: https://github.com/apache/maven/pull/1208#issuecomment-1692812315 @gnodet All issues should have been resolved except for the `DefaultWagonManager` issue dispute, which was controversial. > Remove dependency on plexus-utils > - > > Key: MNG-7820 > URL: https://issues.apache.org/jira/browse/MNG-7820 > Project: Maven > Issue Type: Task >Reporter: Guillaume Nodet >Priority: Major > Fix For: 4.0.0-alpha-8 > > -- This message was sent by Atlassian Jira (v8.20.10#820010)
[GitHub] [maven-build-cache-extension] gnodet commented on pull request #91: [MBUILDCACHE-64] Exclusion mechanism bugfix
gnodet commented on PR #91: URL: https://github.com/apache/maven-build-cache-extension/pull/91#issuecomment-1692814388 I'm starting to wonder if the use of the JDK `PathMatcher` is sufficient here. If we want to optimize the file system access and not traverse _all_ files / subdirectories, we'll have to use [`Files.walkFileTree`](https://docs.oracle.com/javase/8/docs/api/java/nio/file/Files.html#walkFileTree-java.nio.file.Path-java.nio.file.FileVisitor-) as it has built-in support for skipping subtrees. However, I'm not sure there is any good implementation of glob syntax that would skip subtree when known to not match. -- 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
[jira] [Commented] (MBUILDCACHE-64) Apply global exclusions to folder names
[
https://issues.apache.org/jira/browse/MBUILDCACHE-64?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17758841#comment-17758841
]
ASF GitHub Bot commented on MBUILDCACHE-64:
---
gnodet commented on PR #91:
URL:
https://github.com/apache/maven-build-cache-extension/pull/91#issuecomment-1692814388
I'm starting to wonder if the use of the JDK `PathMatcher` is sufficient
here. If we want to optimize the file system access and not traverse _all_
files / subdirectories, we'll have to use
[`Files.walkFileTree`](https://docs.oracle.com/javase/8/docs/api/java/nio/file/Files.html#walkFileTree-java.nio.file.Path-java.nio.file.FileVisitor-)
as it has built-in support for skipping subtrees. However, I'm not sure there
is any good implementation of glob syntax that would skip subtree when known to
not match.
> Apply global exclusions to folder names
> ---
>
> Key: MBUILDCACHE-64
> URL: https://issues.apache.org/jira/browse/MBUILDCACHE-64
> Project: Maven Build Cache Extension
> Issue Type: Bug
>Affects Versions: 1.0.1
>Reporter: Frank Wagner
>Assignee: Olivier Lamy
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.1.0
>
>
> It is currently not possible to exclude folders by their name, like
> {quote}
>
>
> node_modules
> dist
> build
>
>
> ...
> {quote}
> That's because isFilteredOutSubpath(),
> [https://github.com/apache/maven-build-cache-extension/blob/master/src/main/java/org/apache/maven/buildcache/checksum/MavenProjectInput.java#L638,]
> uses startWith on normalized absolute paths.
> That function could be enhanced with an additional criterion like in
> [https://github.com/apache/maven-build-cache-extension/blob/master/src/main/java/org/apache/maven/buildcache/checksum/MavenProjectInput.java#L510]
> {{filteredOutPaths.stream().anyMatch(it ->
> it.getFileName().equals(entry.getFileName()))}}
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
