Skip to content

[MNG-7629] Improve resolution of modules within a multi-module build#912

Closed
gnodet wants to merge 1 commit intoapache:masterfrom
gnodet:MNG-7629
Closed

[MNG-7629] Improve resolution of modules within a multi-module build#912
gnodet wants to merge 1 commit intoapache:masterfrom
gnodet:MNG-7629

Conversation

@gnodet
Copy link
Copy Markdown
Contributor

@gnodet gnodet commented Dec 14, 2022

JIRA issue: https://issues.apache.org/jira/browse/MNG-7629

  • Partial revert of MNG-6118
  • [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed

PR with IT at apache/maven-integration-testing#219

================

As discussed in the JIRA, the fix for MNG-6118 has several problems:

  • it requires reading the whole reactor when building a subtree
  • it does not work with attached artifacts
    This PR partially reverts MNG-6118 so that maven does not read the full reactor anymore and fixes the ReactorReader so that it keeps a copy of each artifact to be installed/deployed in the ${multiModuleProjectDirectory}/target/project-local-repo directory and uses them to resolve artifacts during build.

@gnodet gnodet requested review from cstamas and mthmulders December 14, 2022 21:28
@gnodet gnodet marked this pull request as draft December 14, 2022 21:29
@gnodet
Copy link
Copy Markdown
Contributor Author

gnodet commented Dec 14, 2022

Some ITs failures are caused by the changes in the new reactor reader. The current behaviour is to allow inter-module dependencies at the compile phase. I wonder how this is actually valid ? For example some plugins do modify the content of the artifacts at package phase, such as the maven-shade-plugin which can be used to rewrite classes. So the output of the compile phase is not always something that can be consumed. While I do agree that mvn package should work, those tests kinda imply the mvn compile should always work, which I think is wrong.
Another example is when building a plugin inside the reactor as the plugin descriptor is only generated during the process-classes phase. I suppose I can find a number of such use cases.
Generally speaking, I think the package phase is the one that builds artifacts that can be consumed, so expecting things to work before that phase is not a good idea imho.

@mthmulders
Copy link
Copy Markdown
Contributor

It's going to take me a bit of time to understand, but I definitely want to review this.

@gnodet gnodet requested a review from michael-o December 17, 2022 01:24
@michael-o
Copy link
Copy Markdown
Member

It's going to take me a bit of time to understand, but I definitely want to review this.

I'll try to understand by end of next week.

Copy link
Copy Markdown
Contributor

@mthmulders mthmulders left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a multimodule project, when I run mvn package on the root level, it correctly copies the JAR to target/project-local-repo. When I then run mvn package on a submodule, it fails with

Could not resolve dependencies for project <groupId>:<artifactId>:jar:0.1-SNAPSHOT: Could not find artifact <groupId>:<artifactId>:jar:0.1-SNAPSHOT

From the ticket and the conversation we had, I would expect mvn compile not to work anymore - which I still consider a big loss! But mvn package was supposed to work in this scenario, right?

Comment thread maven-core/src/main/java/org/apache/maven/ReactorReader.java Outdated
Comment thread maven-core/src/main/java/org/apache/maven/ReactorReader.java Outdated
Comment thread maven-core/src/main/java/org/apache/maven/ReactorReader.java
@gnodet
Copy link
Copy Markdown
Contributor Author

gnodet commented Dec 19, 2022

In a multimodule project, when I run mvn package on the root level, it correctly copies the JAR to target/project-local-repo. When I then run mvn package on a submodule, it fails with

Could not resolve dependencies for project :🫙0.1-SNAPSHOT: Could not find artifact :🫙0.1-SNAPSHOT

From the ticket and the conversation we had, I would expect mvn compile not to work anymore - which I still consider a big loss! But mvn package was supposed to work in this scenario, right?

I've tried to improve the PR to fully fix the mvn verify -o -f submodule use case. It was failing in various scenarii because the workspace reader was not used during in the early reactor phase.
This happen in maven master if you run an initial mvn package but then try to mvn validate -f maven-settings -o:

➜  maven git:(MNG-7629) /opt/homebrew/bin/mvn verify -DskipTests -o -f maven-settings  
[INFO] Scanning for projects...
[ERROR] [ERROR] Some problems were encountered while processing the POMs:
[ERROR] Non-resolvable import POM: Cannot access apache.snapshots (https://repository.apache.org/snapshots) in offline mode and the artifact org.apache.maven:maven-bom:pom:4.0.0-alpha-4-SNAPSHOT has not been downloaded from it before. @ org.apache.maven:maven:4.0.0-alpha-4-SNAPSHOT, /Users/gnodet/work/git/maven/pom.xml, line 186, column 19
 @ 
[ERROR] The build could not read 1 project -> [Help 1]
[ERROR]   
[ERROR]   The project org.apache.maven:maven-settings:4.0.0-alpha-4-SNAPSHOT (/Users/gnodet/work/git/maven/maven-settings/pom.xml) has 1 error
[ERROR]     Non-resolvable import POM: Cannot access apache.snapshots (https://repository.apache.org/snapshots) in offline mode and the artifact org.apache.maven:maven-bom:pom:4.0.0-alpha-4-SNAPSHOT has not been downloaded from it before. @ org.apache.maven:maven:4.0.0-alpha-4-SNAPSHOT, /Users/gnodet/work/git/maven/pom.xml, line 186, column 19 -> [Help 2]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/ProjectBuildingException
[ERROR] [Help 2] http://cwiki.apache.org/confluence/display/MAVEN/UnresolvableModelException

while with this PR, all previously built artifacts can be reused in a subsequent build.

@gnodet gnodet changed the title [MNG-7629] Fix resolution of reactor dependencies [MNG-7629] Improve resolution of modules within a multi-module build Dec 21, 2022
@gnodet
Copy link
Copy Markdown
Contributor Author

gnodet commented Dec 21, 2022

I'm going to split the two issues as they can be handled separately:

  • MNG-7629: cache all built artifacts in a per-multi-project repository so that they can be reused in later builds, do not point to {{target/classes}} to ensure correct build
  • MNG-7646: do not parse all poms in the reactor when building a subtree

@gnodet gnodet force-pushed the MNG-7629 branch 4 times, most recently from e5a965b to f208668 Compare January 11, 2023 22:55
@gnodet
Copy link
Copy Markdown
Contributor Author

gnodet commented Jan 14, 2023

Closing this PR in favour of #954

@gnodet gnodet closed this Jan 14, 2023
@gnodet gnodet deleted the MNG-7629 branch January 25, 2023 13:20
@jira-importer
Copy link
Copy Markdown

Resolve #8824

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants