-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[MNG-8165] top/root/multimodule mixup #1604
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
Outdated
Show resolved
Hide resolved
|
Does this require some form of test? |
|
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. |
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
Outdated
Show resolved
Hide resolved
| throw new ParseException("Unrecognized maven.config file entries: " + unrecognized); | ||
| if (cliRequest.rootDirectory != null) { | ||
| try { | ||
| File configFile = |
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
|
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. |
gnodet
left a comment
There was a problem hiding this 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.
|
Resolve #9687 |
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 inmvnscript and passed to JVM as Java System Property: contains the calculated location of.mvndirectory (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.mvnsub-directory, and isnullif none found.In this respect,
rootDirectoryandmultiModuleProjectDirectoryare usually similar (same), andtopDirectoryis similar tobasedir.For start,
mvnscript stops when it arrives to FS root (/), hencemultiModuleProjectDirectorywill NOT point at root, even if.mvndirectory present in it. The other two are fine in this respect.But config and extensions were still loaded up from
multiModuleProjectDirectorydespite whatrootDirectorywas set to.Removal of
multiModuleProjectDirectoryin Maven 3 is not possible, but is deprecated and it's use is highly discouraged.https://issues.apache.org/jira/browse/MNG-8165