Skip to content

Conversation

@cstamas
Copy link
Member

@cstamas cstamas commented Jul 9, 2024

This is a bug how in Maven different paths are detected and handled.

Maven 4 is not affected.

What is happening?

Properties/paths in play:

  • cliRequest.multiModuleProjectDirectory, an ancient (and deprecated) path that is calculated in mvn script and passed to JVM as Java System Property: contains the calculated location of .mvn directory (but does not bubble up to root)
  • cliRequest,topDirectory, new path since 3.9.x, that is basically "cwd adjusted for -f"
  • cliRequest.rootDirectory, new path since 3.9.x, that is nullable (does not have to exists). that is recursively searched for, from topDirectory by bubbling up to parent. It is searching for a directory that contains .mvn sub-directory, and is null if none found.

In this respect, rootDirectory and multiModuleProjectDirectory are usually similar (same), and topDirectory is similar to basedir.

For start, mvn script stops when it arrives to FS root (/), hence multiModuleProjectDirectory will NOT point at root, even if .mvn directory present in it. The other two are fine in this respect.

But config and extensions were still loaded up from multiModuleProjectDirectory despite what rootDirectory was set to.

Removal of multiModuleProjectDirectory in Maven 3 is not possible, but is deprecated and it's use is highly discouraged.


https://issues.apache.org/jira/browse/MNG-8165

This is a bug how in Maven different paths are
detected and handled.

Maven 4 is not affected.

---

https://issues.apache.org/jira/browse/MNG-8165
@cstamas cstamas added the bug Something isn't working label Jul 9, 2024
@cstamas cstamas added this to the 3.9.9 milestone Jul 9, 2024
@cstamas cstamas self-assigned this Jul 9, 2024
@cstamas cstamas marked this pull request as ready for review July 9, 2024 19:48
@michael-o
Copy link
Member

Does this require some form of test?

@cstamas
Copy link
Member Author

cstamas commented Jul 9, 2024

Is tested, as PR mentions, by MavenCLITest but also a lot of ITs. Otherwise, the 3.9.x introduced top/root are NOT passed in via system properties (unlike multiModuleDirectory, that IS and UT makes use of it). They rely purely on cwd and cmd line args.

@michael-o michael-o self-requested a review July 9, 2024 20:30
throw new ParseException("Unrecognized maven.config file entries: " + unrecognized);
if (cliRequest.rootDirectory != null) {
try {
File configFile =
Copy link
Contributor

Choose a reason for hiding this comment

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

Better switch to nio file api ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Least changes is what I wanted in 3.9, but I can do that (but seems unrelated)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I only saw it was targeted to 3.9.x later.

@gnodet
Copy link
Contributor

gnodet commented Jul 9, 2024

The same PR should be applied to master.

@cstamas
Copy link
Member Author

cstamas commented Jul 9, 2024

The same PR should be applied to master.

I assumed Maven 4 is not affected, so I was wrong?

@gnodet
Copy link
Contributor

gnodet commented Jul 9, 2024

The same PR should be applied to master.

I assumed Maven 4 is not affected, so I was wrong?

The code is roughly the same afaik.

Copy link
Contributor

@gnodet gnodet left a comment

Choose a reason for hiding this comment

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

The assumption that multiModuleProjectDirectory is "cwd adjusted for -f" is WRONG, so the PR is too.
Both topDirectory and rootDirectory have clear definitions and should not rely on an external computation done in bash scripts. This makes embedding Maven a pain.

Polyglot is broken on Maven 4, but I've been able to use it successfully with 3.9.8 and 3.9.7, so I'd like a clear reproducer for the issue first.

@cstamas cstamas closed this Jul 10, 2024
@cstamas cstamas deleted the maven-3.9.x-MNG-8165 branch July 10, 2024 08:22
@jira-importer
Copy link

Resolve #9687

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants