T thanks. I’ll need to dig into this. It will be at least a couple of weeks before I can get to this. One quick thought. Is it possible something needs to be added to runtime scope? That’s hard to detect but it might explain what you see.
On Sun, May 1, 2022 at 10:20 PM Henning Schmiedehausen < henn...@schmiedehausen.org> wrote: > Hi Elliotte, > > Thank you for the comments. I have read your comments on MDEP-791. I very > much believe in "show, not tell", so here is a (condensed down) real-world > example of the problems that I (and others) are facing: > > Please clone https://github.com/hgschmie/dep804 > > This project contains two service builds. The basic structure are four > modules: > > service --> dependency --> lib1 --> lib2 > > There is also a build that shades the libraries into the dependency module > (dependency-shaded) and then uses that shaded dependency to build a shaded > service: > > service-shaded -> dependency-shaded (which contains dependency, lib1 and > lib2) > > To establish a baseline with 3.1.2, run "mvn -Pold-dep clean install" (Use > Java 11 for this). > > The code should build successfully, all unit tests pass. No errors are > thrown. > > Test the services: > > java -jar service-shaded/target/service-shaded-0.1-SNAPSHOT-shaded.jar > Main code start! > DependencyCode is at test.DependencyCode@2eee9593 > DependencyCode code start! > lib1 is at test.lib1.Lib1@7907ec20 > lib 1 start! > lib 2 is at test.lib2.Lib2@546a03af > lib 2 start! > > java -jar service/target/service-0.1-SNAPSHOT-shaded.jar > Main code start! > DependencyCode is at test.DependencyCode@2eee9593 > DependencyCode code start! > lib1 is at test.lib1.Lib1@7907ec20 > lib 1 start! > lib 2 is at test.lib2.Lib2@546a03af > lib 2 start! > > Both services start. All is good. Now, rebuild with the new (3.3.0) plugin: > > mvn -fae clean install > [...] > [INFO] --- maven-dependency-plugin:3.3.0:analyze-only (basepom.default) @ > service --- > [WARNING] Non-test scoped test only dependencies found: > [WARNING] test:lib1:jar:0.1-SNAPSHOT:compil > [...] > INFO] --- maven-dependency-plugin:3.3.0:analyze-only (basepom.default) @ > dependency-shaded --- > [WARNING] Non-test scoped test only dependencies found: > [WARNING] test:lib2:jar:0.1-SNAPSHOT:compile > [...] > INFO] service ............................................ FAILURE [ 0.798 > s] > [INFO] dependency-shaded .................................. FAILURE [ > 0.662 s] > > ... not good. Two of the modules fail with the dreaded "Non-test scoped > test only dependencies found:" > > Let's fix that. Put "lib1" into test scope in the "service" module and > "lib2" in test scope in the "dependency-shaded" module to fix that problem. > > mvn -fae clean install > [...] > [INFO] T E S T S > [INFO] ------------------------------------------------------- > [INFO] Running test.ServiceTest > [ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: > 0.076 s <<< FAILURE! - in test.ServiceTest > [ERROR] test.ServiceTest.testLib1 Time elapsed: 0.008 s <<< ERROR! > java.lang.NoClassDefFoundError: test/lib2/Lib2 > at test.lib1.Lib1.<init>(Lib1.java:7) > at test.ServiceTest.<init>(ServiceTest.java:10) > [...] > ... 29 more > > [INFO] > [INFO] Results: > [INFO] > [ERROR] Errors: > [ERROR] ServiceTest.<init>:10 » NoClassDefFound test/lib2/Lib2 > [INFO] > [ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0 > [...] > [INFO] service-shaded ..................................... FAILURE [ > 0.541 s] > > The unit test in service-shaded fails. We haven't even touched the > service-shaded module! How could that happen? > > Let's ignore that for now. Just skip the tests: > > mvn -fae -DskipTests clean install > ... build successfully... > > java -jar service/target/service-0.1-SNAPSHOT-shaded.jar > Exception in thread "main" java.lang.NoClassDefFoundError: test/lib1/Lib1 > at test.DependencyCode.<init>(DependencyCode.java:6) > at test.Main.<init>(Main.java:4) > at test.Main.main(Main.java:8) > Caused by: java.lang.ClassNotFoundException: test.lib1.Lib1 > at > > java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:581) > at > > java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178) > at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522) > ... 3 more > > java -jar service-shaded/target/service-shaded-0.1-SNAPSHOT-shaded.jar > Exception in thread "main" java.lang.NoClassDefFoundError: test/lib2/Lib2 > at test.lib1.Lib1.<init>(Lib1.java:7) > at test.DependencyCode.<init>(DependencyCode.java:6) > at test.Main.<init>(Main.java:4) > at test.Main.main(Main.java:8) > Caused by: java.lang.ClassNotFoundException: test.lib2.Lib2 > at > > java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:581) > at > > java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178) > at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522) > ... 4 more > > None of the services work anymore! There are dependencies missing. Because > changing the scope of dependencies from "compile" to "test" to satisfy a > plugin warning affected something in the build process. > > That was the state with 3.2.0 which you adamantly insisted that it was > perfect and did everything well. > > With 3.3.0, there is now a workaround with exclusions. Let's try that. Put > the deps back in compile scope and add exclusions. Now the projects build > again, tests pass and the services start again. > > What this did is basically *remove all that this new check does*! Because > the new check is incorrect (as shown above, forcing dependencies into > "test" scope breaks stuff left and right) So event for this trivial build, > one ends up with > > diff --git a/dependency-shaded/pom.xml b/dependency-shaded/pom.xml > index ca439ae..2dce740 100644 > --- a/dependency-shaded/pom.xml > +++ b/dependency-shaded/pom.xml > @@ -42,6 +42,19 @@ > </dependencies> > > <build> > + <pluginManagement> > + <plugins> > + <plugin> > + <groupId>org.apache.maven.plugins</groupId> > + <artifactId>maven-dependency-plugin</artifactId> > + <configuration> > + <ignoredNonTestScopedDependencies> > + > <ignoredNonTestScopedDependency>test:lib2</ignoredNonTestScopedDependency> > + </ignoredNonTestScopedDependencies> > + </configuration> > + </plugin> > + </plugins> > + </pluginManagement> > <plugins> > <plugin> > <groupId>org.apache.maven.plugins</groupId> > diff --git a/service/pom.xml b/service/pom.xml > index 0f8fe5f..fed011e 100644 > --- a/service/pom.xml > +++ b/service/pom.xml > @@ -45,4 +45,20 @@ > <scope>test</scope> > </dependency> > </dependencies> > + > + <build> > + <pluginManagement> > + <plugins> > + <plugin> > + <groupId>org.apache.maven.plugins</groupId> > + <artifactId>maven-dependency-plugin</artifactId> > + <configuration> > + <ignoredNonTestScopedDependencies> > + > <ignoredNonTestScopedDependency>test:lib1</ignoredNonTestScopedDependency> > + </ignoredNonTestScopedDependencies> > + </configuration> > + </plugin> > + </plugins> > + </pluginManagement> > + </build> > </project> > > Or I can just add > > diff --git a/pom.xml b/pom.xml > index c65c6c7..c0e276f 100644 > --- a/pom.xml > +++ b/pom.xml > @@ -41,6 +41,22 @@ > </dependencies> > </dependencyManagement> > > + <build> > + <pluginManagement> > + <plugins> > + <plugin> > + <groupId>org.apache.maven.plugins</groupId> > + <artifactId>maven-dependency-plugin</artifactId> > + <configuration> > + <ignoredNonTestScopedDependencies> > + > <ignoredNonTestScopedDependency>*</ignoredNonTestScopedDependency> > + </ignoredNonTestScopedDependencies> > + </configuration> > + </plugin> > + </plugins> > + </pluginManagement> > + </build> > + > <profiles> > <profile> > <id>old-dep</id> > > to the root pom, thus rendering *the whole thing useless*. Now consider a > project with hundreds of modules, millions of lines of code, literally a > thousand dependencies in multiple languages. Do you *seriously* believe > that anyone will do that leg work to satisfy a check that was wrong to > begin with? > > Adding the blanket exclusion to the root closes any path to incremental > improvement. Because now it is *impossible* to turn off the blanket > exclusion for a sub project. Here is how that would work with a flag: > > diff --git a/pom.xml b/pom.xml > index c65c6c7..a729cec 100644 > --- a/pom.xml > +++ b/pom.xml > @@ -15,6 +15,8 @@ > <properties> > <basepom.check.skip-all>true</basepom.check.skip-all> > > <basepom.check.skip-dependency>false</basepom.check.skip-dependency> > + > > > <dep.plugin.dependency.version>3.3.1-MDEP804.1-SNAPSHOT</dep.plugin.dependency.version> > + <ignore-all-non-test-scoped>false</ignore-all-non-test-scoped> > </properties> > > <modules> > @@ -41,6 +43,20 @@ > </dependencies> > </dependencyManagement> > > + <build> > + <pluginManagement> > + <plugins> > + <plugin> > + <groupId>org.apache.maven.plugins</groupId> > + <artifactId>maven-dependency-plugin</artifactId> > + <configuration> > + > > > <ignoreAllNonTestScoped>${ignore-all-non-test-scoped}</ignoreAllNonTestScoped> > + </configuration> > + </plugin> > + </plugins> > + </pluginManagement> > + </build> > + > <profiles> > <profile> > <id>old-dep</id> > diff --git a/dependency-shaded/pom.xml b/dependency-shaded/pom.xml > index ca439ae..7c9f58e 100644 > --- a/dependency-shaded/pom.xml > +++ b/dependency-shaded/pom.xml > @@ -10,6 +10,10 @@ > <artifactId>dependency-shaded</artifactId> > <packaging>jar</packaging> > > + <properties> > + <ignore-all-non-test-scoped>true</ignore-all-non-test-scoped> > + </properties> > + > <dependencyManagement> > <dependencies> > <dependency> > diff --git a/service/pom.xml b/service/pom.xml > index 0f8fe5f..bca7d16 100644 > --- a/service/pom.xml > +++ b/service/pom.xml > @@ -13,8 +13,10 @@ > > <properties> > <basepom.shaded.main-class>test.Main</basepom.shaded.main-class> > + <ignore-all-non-test-scoped>true</ignore-all-non-test-scoped> > </properties> > > + > <dependencyManagement> > <dependencies> > <dependency> > > Now all my modules that build are protected against any of the changes > (they will report an error if a new dep that is test-only is added in > compile scope) and for the two problematic modules, I turn off the check by > setting the respective property to "true". I can now migrate these modules > one at a time. > > This is simply impossible with the exclusion list. Unless you want to > plaster wildcard excludes all across the code base. > > I feel that at this point, your objection is based on your personal > opinions, not facts. I am not sure what more facts I can present. This has > been presented to you again and again in MDEP-791, MDEP-753, MDEP-757 but > you keep ignoring them. > > Adding ignoredNonTestScopedDependency was IMHO already some grudging > admittance that the changes from 3.1.2 -> 3.2.0 were seriously flawed and > caused a lot of pain with users that tried to upgrade based on the desire > to move past JDK 11. > > I would like to move back to the 3.1.2 compatible behavior or at the very > least add a flag that allows switching back to that behavior. The current > situation ("add an exclusion list with a wildcard") is IMHO not an > acceptable state and it hurts anyone that maintains non-trivial builds with > maven. > > -h > > > > > > > > > > > > > > On Sun, May 1, 2022 at 3:07 PM Elliotte Rusty Harold <elh...@ibiblio.org> > wrote: > > > The last time I looked into this my personal conclusion was that > > everything was now working as intended, and we don't want or need more > > flags. It feels like epicycles on epicycles. My current opinion is: > > > > 1. The latest version of the plugin correctly identifies needed and > > unneeded dependencies, including dependencies that could have overly > > broad scope. > > > > 2. All these checks are completely optional. If a project doesn't find > > these helpful, it doesn't have to use the dependencies plugin. > > > > Unless I am factually wrong about one of these claims (certainly > > possible, if so please describe exactly how with a reproducible > > example) I don't see any reason to revisit this now, and I think > > adding further flags is a bad idea. It's not only more complex. It's > > actively wrong. > > > > What developers who choose to turn on this check want is simple: warn > > about any unnecessary dependency, including dependencies whose scope > > is too broad. That's what the plugin should do, and I *think* that's > > what it currently does. > > > > On Sun, May 1, 2022 at 8:59 PM Henning Schmiedehausen > > <henn...@schmiedehausen.org> wrote: > > > > > > Hi, > > > > > > [I discussed this on slack with @sjaranowski, he suggested to bring > this > > > conversation here ] > > > > > > The latest releases of the maven dependency plugin (3.2.0, 3.3.0) > > > introduced some changes in the way test dependencies are handled and > > > potentially flagged. Especially the "Non-test scoped test only > > dependencies > > > found" warning/error has turned into quite a problem for larger > projects > > > that worked fine with the 3.1.x versions of the dependency plugin. > > > > > > 3.3.0 introduced the "ignoredNonTestScopedDependencies" exclusion list > in > > > the configuration which helps to work around those problems by allowing > > > flagged dependencies to be ignored. > > > > > > The challenge with this is that retrofitting on larger projects with a > > lot > > > of dependencies now flagged either leads to a lot of very brittle > > > per-module configuration work or adding a blanket exclusion to a base > pom > > > with no recourse within modules. > > > > > > I proposed therefore adding another flag to the plugin in MDEP-804. > Flags > > > are useful because they can be driven out of properties which in turn > can > > > be overridden on a per-module base. This e.g. allows me to set a > blanket > > > policy of ignoring these problems by default and then turn that blanket > > > policy off on a per-module base and add finer grained exclusions. This > > > solves both by problems and also the issues described in MDEP-791. > > > > > > @sjaranowski pushed back a bit in the PR, questioning whether we want > > more > > > flags. What is important to me is that flags are much more useful than > > > exclusion lists, because I can drive them out of properties. > > > > > > It would be good to get some more opinions. The PR ( > > > https://github.com/apache/maven-dependency-plugin/pull/211) is ready > to > > go > > > in (it has integration tests). Technically, I still have my commit bit > > (and > > > I assume Maven is still CTR) but I haven't comitted to that code base > > for a > > > long time and I don't want to step on any toes. > > > > > > -h > > > > > > (Wow, this may be my first post to dev@maven in six+ years. Feels > good. > > :-) > > > ) > > > > > > > > -- > > Elliotte Rusty Harold > > elh...@ibiblio.org > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org > > For additional commands, e-mail: dev-h...@maven.apache.org > > > > > -- Elliotte Rusty Harold elh...@ibiblio.org
