[MASSEMBLY-874] maven-assembly plugin always downloads dependencies from net

[turbanoff]

use main list of remote repositories when build dependency set. Fix for https://issues.apache.org/jira/browse/MASSEMBLY-874 Logic of fix is somewhat similar to https://github.com/apache/maven-assembly-plugin/commit/d9a06f5559fadc6275db4b4d3dc49b0d81f77385 : set actual remote repositories inside ProjectBuildingRequest

See comment https://issues.apache.org/jira/browse/MASSEMBLY-874?focusedCommentId=17016987&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17016987

What I found out;

1. AddDependencySetsTask builds project (what for?) for each dependency artifact. https://github.com/apache/maven-assembly-plugin/blob/master/src/main/java/org/apache/maven/plugins/assembly/archive/task/AddDependencySetsTask.java#L157
2. AddDependencySetsTask calls DefaultProjectBuilder.build method. This method need pom file to build project https://github.com/apache/maven/blob/master/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java#L307
3. DefaultProjectBuilder resolves pom file via DefaultRepositorySystem.resolveArtifact call. https://github.com/apache/maven/blob/master/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java#L318
4. Then it goes down to usage of EnhancedLocalRepositoryManager.find method here https://github.com/apache/maven-resolver/blob/master/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultArtifactResolver.java#L318
5. Implementation of this method do not just use artifact from local cache, but before usage is checks _remote.repositories file content and matches it with repositories. https://github.com/apache/maven-resolver/blob/master/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/EnhancedLocalRepositoryManager.java#L105
6. It uses file only if there is repository match. From my case, described in JIRA comment, artifact ipf-api was downloaded from shared remote repository, but by default only default repository is considered. Default repository by default it's central (see https://github.com/apache/maven/blob/master/maven-core/src/main/java/org/apache/maven/repository/RepositorySystem.java#L56), but we have mirror for it (called Nexus)

• [*] I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004

[eolivelli]

Can you please add an integration test like:

https://github.com/apache/maven-assembly-plugin/commit/d9a06f5559fadc6275db4b4d3dc49b0d81f77385

[turbanoff]

Strange. In integration test code sets all repositories

"main@1" prio=5 tid=0x1 nid=NA runnable
  java.lang.Thread.State: RUNNABLE
	  at org.apache.maven.project.DefaultProjectBuildingRequest.setRemoteRepositories(DefaultProjectBuildingRequest.java:132)
	  at org.apache.maven.project.DefaultProjectBuilder.initProject(DefaultProjectBuilder.java:642)
	  at org.apache.maven.project.DefaultProjectBuilder.build(DefaultProjectBuilder.java:583)
	  at org.apache.maven.project.DefaultProjectBuilder.build(DefaultProjectBuilder.java:372)
	  at org.apache.maven.graph.DefaultGraphBuilder.collectProjects(DefaultGraphBuilder.java:400)
	  at org.apache.maven.graph.DefaultGraphBuilder.getProjectsForMavenReactor(DefaultGraphBuilder.java:391)
	  at org.apache.maven.graph.DefaultGraphBuilder.build(DefaultGraphBuilder.java:78)
	  at org.apache.maven.DefaultMaven.buildGraph(DefaultMaven.java:511)
	  at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:221)
	  at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:194)
	  at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:107)
	  at org.apache.maven.cli.MavenCli.execute(MavenCli.java:955)
	  at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:290)
	  at org.apache.maven.cli.MavenCli.main(MavenCli.java:194)
	  at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(NativeMethodAccessorImpl.java:-1)
	  at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	  at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	  at java.lang.reflect.Method.invoke(Method.java:566)
	  at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:289)
	  at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:229)
	  at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:415)
	  at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:356)

Need more debugging.

[turbanoff]

I stuck with integration test. Looks like it's too hard or impossible to write proper regression test. Cause of this is that setRemoteRepositories is always called in integration test in this stack trace (i updated maven-code version to 3.6.3 to match my maven installation)

 at org.apache.maven.project.DefaultProjectBuildingRequest.setRemoteRepositories(DefaultProjectBuildingRequest.java:132)
	  at org.apache.maven.project.DefaultProjectBuilder.initProject(DefaultProjectBuilder.java:642)
	  at org.apache.maven.project.DefaultProjectBuilder.build(DefaultProjectBuilder.java:583)
	  at org.apache.maven.project.DefaultProjectBuilder.build(DefaultProjectBuilder.java:372)
	  at org.apache.maven.graph.DefaultGraphBuilder.collectProjects(DefaultGraphBuilder.java:400)
	  at org.apache.maven.graph.DefaultGraphBuilder.getProjectsForMavenReactor(DefaultGraphBuilder.java:391)
	  at org.apache.maven.graph.DefaultGraphBuilder.build(DefaultGraphBuilder.java:78)
	  at org.apache.maven.DefaultMaven.buildGraph(DefaultMaven.java:511)
	  at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:221)
	  at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:194)
	  at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:107)
	  at org.apache.maven.cli.MavenCli.execute(MavenCli.java:955)
	  at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:290)
	  at org.apache.maven.cli.MavenCli.main(MavenCli.java:194)
	  at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(NativeMethodAccessorImpl.java:-1)
	  at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	  at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	  at java.lang.reflect.Method.invoke(Method.java:566)
	  at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:289)
	  at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:229)
	  at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:415)
	  at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:356)

This method setRemoteRepositories is not called in my production case because parentModel == null here https://github.com/apache/maven/blob/master/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java#L912

Since integration tests in maven-assembly-plugins always has parent https://github.com/apache/maven-assembly-plugin/blob/master/src/it/it-project-parent/pom.xml in their case it's always != null and setRemoteRepositories is always called with full list of remote repositories.

@eolivelli do you know any ways to somehow prevent adding parent module to org.apache.maven.model.building.DefaultModelBuildingResult#modelIds to make this expression return null in integration tests run? I've tried to build only child module (by set invoker.goals=install -U -pl :child), but it doesn't help.

Current test prototype massembly-874.it.patch.zip

[turbanoff]

@eolivelli @struberg any chances that it could be merged without integration test? I don't know how to write reliable test-case for it.

[rfscholte]

I've replaced EasyMock with Mockito. However, this plugin should have an integration test instead of a unittest, so we can verify the logging for unexpected downloads.

[ricardozanini]

Hi! Are there any updates on this?

[zhaomoran]

@rfscholte @turbanoff Hi! Will this problem be followed up and repaired?

[Axinet]

It would be good to solve it finally, as version 2.6 (the last one without that regression) stops working with other, newer maven versions and plugins :(.

[slawekjaranowski]

Issue was closed, so I also close PR