[MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed by gnodet · Pull Request #954 · apache/maven

gnodet · 2023-01-12T07:07:15Z

Similar to #912 but without removing the best effort artifact resolution to keep mvn test working if possible.

This require a few ITs to be modified:
apache/maven-integration-testing#234

… them across builds if needed

mthmulders

Left a couple of questions.

Also, I wonder if this change allows us to remove the fallbacks that were previously on line 150 - 159. Since those lines aren't changed, it seems GitHub doesn't allow me to comment on the spot :-\.

gnodet · 2023-01-17T07:16:44Z

Left a couple of questions.

Also, I wonder if this change allows us to remove the fallbacks that were previously on line 150 - 159. Since those lines aren't changed, it seems GitHub doesn't allow me to comment on the spot :-.

Good question. I think part of it could be removed, but we need to keep the determineBuildOutputDirectoryForArtifact in order to be able to partially run mvn test.

Another possible strategy instead of copying the artifacts would be to only record their paths.

This reverts commit 7421d07. # Conflicts: # maven-core/src/main/java/org/apache/maven/ReactorReader.java

mthmulders · 2023-01-17T19:26:33Z

... in order to be able to partially run mvn test.

I'm not sure I understand. You mean "run mvn test on a multi-module project without requiring the user to have ran mvn package first"?

gnodet · 2023-01-17T21:59:51Z

... in order to be able to partially run mvn test.

I'm not sure I understand. You mean "run mvn test on a multi-module project without requiring the user to have ran mvn package first"?

Yes. I haven't removed this part in this PR. If the main artifacts have not been packaged, they will point to the target/classes.

MartinKanters

I like the solution in general. Added small comments.

Co-authored-by: Martin Kanters <mkanters93@gmail.com>

gnodet · 2023-01-18T14:23:27Z

... in order to be able to partially run mvn test.

I'm not sure I understand. You mean "run mvn test on a multi-module project without requiring the user to have ran mvn package first"?

Yes. I haven't removed this part in this PR. If the main artifacts have not been packaged, they will point to the target/classes.

I've been able to remove the check to load the packaged artifacts.

gnodet · 2023-01-18T14:56:32Z

In the internal project local repository structure, I've added a child directory with the version of a given artifact. This gives a structure which is closer to the maven local repository, and it allows deleting artifacts during the clean phase without having to rely on a heuristic to split between classifier / version / extension.

@mthmulders @MartinKanters I'm done on this PR, so unless you have further comments, an approval before the merge would be nice !

…l repository

MartinKanters

Looks like an elegant solution to me. Nice job.

mthmulders

I've been able to remove the check to load the packaged artifacts.

Awesome.

In the internal project local repository structure, I've added a child directory with the version of a given artifact. This gives a structure which is closer to the maven local repository, and it allows deleting artifacts during the clean phase without having to rely on a heuristic to split between classifier / version / extension.

Also awesome.

Happy to see this improvement. Thanks a lot @gnodet!

jira-importer · 2025-07-07T08:01:51Z

Resolve #8824

[MNG-7629] Change reactor reader to copy packaged artifacts and reuse…

7421d07

… them across builds if needed

gnodet force-pushed the MNG-7629-alternative branch from 178a46e to 7421d07 Compare January 12, 2023 09:24

gnodet added 2 commits January 12, 2023 20:56

Fix

3809e6f

Revert changes on findVersions

318864d

gnodet force-pushed the MNG-7629-alternative branch 2 times, most recently from d7931d9 to 489e194 Compare January 13, 2023 16:19

Add temporary debug statement

9a5ac83

gnodet force-pushed the MNG-7629-alternative branch from 489e194 to 9a5ac83 Compare January 13, 2023 16:20

gnodet added 3 commits January 13, 2023 19:42

Do not consider old files

43a3b8d

Fix

b0b7396

Different fix

211952c

gnodet marked this pull request as ready for review January 14, 2023 16:27

gnodet mentioned this pull request Jan 14, 2023

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

Closed

gnodet requested review from MartinKanters, cstamas, michael-o and mthmulders January 14, 2023 16:29

mthmulders reviewed Jan 16, 2023

View reviewed changes

Improvements

109c5f6

gnodet added 2 commits January 17, 2023 11:03

Copy artifacts only after the package phase

2e83fa0

Use the correct lifecycle

8642057

gnodet requested a review from mthmulders January 17, 2023 12:58

Revert the changes on DefaultMaven, those are only required for MNG-7646

225cd80

This reverts commit 7421d07. # Conflicts: # maven-core/src/main/java/org/apache/maven/ReactorReader.java

gnodet mentioned this pull request Jan 17, 2023

[MNG-7646] Do not parse all projects in the reactor when building a subtree #963

Merged

michael-o removed their request for review January 17, 2023 20:22

MartinKanters reviewed Jan 18, 2023

View reviewed changes

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 Outdated

Update maven-core/src/main/java/org/apache/maven/ReactorReader.java

d5b1c55

Co-authored-by: Martin Kanters <mkanters93@gmail.com>

gnodet added 5 commits January 18, 2023 14:11

Rename a few methods

c8fd96e

Fix cleaning the project local repository

90f80fc

Use streams to install artifacts in the project local repository

4beeeb1

Clean a bit

f74c398

Packaged artifacts are now loaded from the project local repository

197b743

cstamas approved these changes Jan 18, 2023

View reviewed changes

cstamas reviewed Jan 18, 2023

View reviewed changes

Comment thread maven-core/src/main/java/org/apache/maven/ReactorReader.java Outdated

cstamas reviewed Jan 18, 2023

View reviewed changes

Comment thread maven-core/src/main/java/org/apache/maven/ReactorReader.java Outdated

Use the topmost project's build directory to resolve the project loca…

f23aee0

…l repository

MartinKanters approved these changes Jan 19, 2023

View reviewed changes

cstamas approved these changes Jan 19, 2023

View reviewed changes

mthmulders approved these changes Jan 19, 2023

View reviewed changes

Move logic from the even spy to the ReactorReader

065ff3b

gnodet merged commit c1a9001 into apache:master Jan 19, 2023

gnodet added this to the 4.0.0-alpha-4 milestone Jan 19, 2023

gnodet deleted the MNG-7629-alternative branch January 25, 2023 13:20

Conversation

gnodet commented Jan 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mthmulders left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gnodet commented Jan 17, 2023

Uh oh!

mthmulders commented Jan 17, 2023

Uh oh!

gnodet commented Jan 17, 2023

Uh oh!

MartinKanters left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gnodet commented Jan 18, 2023

Uh oh!

gnodet commented Jan 18, 2023

Uh oh!

Uh oh!

Uh oh!

MartinKanters left a comment

Choose a reason for hiding this comment

Uh oh!

mthmulders left a comment

Choose a reason for hiding this comment

Uh oh!

jira-importer commented Jul 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gnodet commented Jan 12, 2023 •

edited

Loading