-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
[MNG-8012] Warn if in-reactor BOM import happens #1434
Conversation
This should be in fact prevented. --- https://issues.apache.org/jira/browse/MNG-8012
@@ -1830,6 +1845,15 @@ private org.apache.maven.api.model.DependencyManagement doLoadDependencyManageme | |||
return null; | |||
} | |||
|
|||
if (importSource instanceof FileModelSource && request.getRootDirectory() != null) { | |||
URI sourceUri = ((FileModelSource) importSource).getLocationURI(); | |||
if (request.getRootDirectory().toUri().relativize(sourceUri) != sourceUri) { |
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.
Fwiw, this test is unintuitive... a comment would be welcomed. I had to check the javadoc...
Or maybe using FileModelSource#getPath()
instead or URIs ?
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.
Fixed
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.
I don't want to be pushy, but what about the following ?
Path sourcePath = ((FileModelSource) importSource).getPath();
if (sourcePath.startsWith(request.getRootDirectory()) {
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.
Pushed, so who's pushy now?
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.
LGTM
I believe this implementation is incomplete. It does not discern between a BOM (a module which manages the versions of the modules in the reactor) and a dependency management module (a module which manages the versions of external libraries used in the reactor).
|
This should be in fact prevented.
Also, model builder missed File uses are corrected.
https://issues.apache.org/jira/browse/MNG-8012