Maven 2 & 3

Plugin versions inherited via <pluginManagement> cannot be overriden by <build>.<plugins> section of sub modules

Details

  • Number of attachments :
    1

Description

See the comments in the module POM of the attached demo project for more explanation.

Issue Links

Activity

Hide
Brett Porter added a comment -

I think this is intended behaviour - what if you change the pluginManagement of child modules?

Show
Brett Porter added a comment - I think this is intended behaviour - what if you change the pluginManagement of child modules?
Hide
Benjamin Bentmann added a comment -

I think this is intended behaviour

Oh please not. What would be the rationale for that? If a user explicitly declares a <version> element under <build>.<plugins> isn't that a clear indication of this intention to use that version? Do you want to explain to users why that is ignored and they need to hassle with setting up a <pluginManagement> entry just for the fun of overriding a plugin version inherited from the parent? Do you want to clarify why the little sentence "Plugin Management contains plugin elements in much the same way, except that rather than configuring plugin information for this particular project build, it is intended to configure project builds that inherit from this one." that users learned from http://maven.apache.org/pom.html#Plugin_Management is not quite up with the reality?

Last but not least, the lack to override the version only manifest itself
a) for a lifecycle phase, not for a direct mojo invocation
b) only for the first lifecycle phase

what if you change the pluginManagement of child modules?

As I already commented in the attached demo POMs, this successfully allows the childs to change the version.

Show
Benjamin Bentmann added a comment -
I think this is intended behaviour
Oh please not. What would be the rationale for that? If a user explicitly declares a <version> element under <build>.<plugins> isn't that a clear indication of this intention to use that version? Do you want to explain to users why that is ignored and they need to hassle with setting up a <pluginManagement> entry just for the fun of overriding a plugin version inherited from the parent? Do you want to clarify why the little sentence "Plugin Management contains plugin elements in much the same way, except that rather than configuring plugin information for this particular project build, it is intended to configure project builds that inherit from this one." that users learned from http://maven.apache.org/pom.html#Plugin_Management is not quite up with the reality? Last but not least, the lack to override the version only manifest itself a) for a lifecycle phase, not for a direct mojo invocation b) only for the first lifecycle phase
what if you change the pluginManagement of child modules?
As I already commented in the attached demo POMs, this successfully allows the childs to change the version.
Hide
Brett Porter added a comment -

right - I agree it makes sense - my main concern is inconsistency with the deendencyManagement. But if anything is ever given it should either be honoured or an error if it's incompatible. So changing the behaviour makes sense in this case.

Show
Brett Porter added a comment - right - I agree it makes sense - my main concern is inconsistency with the deendencyManagement. But if anything is ever given it should either be honoured or an error if it's incompatible. So changing the behaviour makes sense in this case.
Hide
Benjamin Bentmann added a comment -

my main concern is inconsistency with the deendencyManagement

Is this to say a child module's <dependencies>.<dependency> cannot override an inherited dependency version from the parent? The child POM should always win over the parent POM since it is the most specific descriptor for a build.

Show
Benjamin Bentmann added a comment -
my main concern is inconsistency with the deendencyManagement
Is this to say a child module's <dependencies>.<dependency> cannot override an inherited dependency version from the parent? The child POM should always win over the parent POM since it is the most specific descriptor for a build.
Hide
Shane Isbell added a comment -

I've look at the code. There is a problem in the DefaultPluginManager.verifyVersionedPlugin. The method correctly determines the version of the plugin but then it makes a last invocation of pluginCollector.getPluginDescriptor( plugin ), which over rides the correct version of the plugin. There is a comment in the code of pluginCollector.getPluginDescriptor, which seems to touch on this problem:

// TODO: include version, but can't do this in the plugin manager as it is not resolved to the right version
// at that point. Instead, move the duplication check to the artifact container, or store it locally based on
// the unresolved version?

I'll keep digging to see how to fix it.

Show
Shane Isbell added a comment - I've look at the code. There is a problem in the DefaultPluginManager.verifyVersionedPlugin. The method correctly determines the version of the plugin but then it makes a last invocation of pluginCollector.getPluginDescriptor( plugin ), which over rides the correct version of the plugin. There is a comment in the code of pluginCollector.getPluginDescriptor, which seems to touch on this problem: // TODO: include version, but can't do this in the plugin manager as it is not resolved to the right version // at that point. Instead, move the duplication check to the artifact container, or store it locally based on // the unresolved version? I'll keep digging to see how to fix it.
Hide
John Casey added a comment -

the *Management sections of the POM were originally designed to provide default values, not overrides. This behavior was changed for dependencies in MNG-1577 IIRC, but I don't know when the change was made for plugins. The behavior should probably be controlled in the class org.apache.maven.project.injection.DefaultModelDefaultsInjector.java (in maven-project). I've looked into this class in both 2.0.x and trunk, and they're the same...not sure whether that means the change happened before the split (unlikely) or was ported across (more likely), but in both cases it looks like it only uses the pluginManagement version if the main version is missing and the pluginManagement version is not.

I'm not really sure what's going on here, but hopefully that can shed a little light on the background for this.

Show
John Casey added a comment - the *Management sections of the POM were originally designed to provide default values, not overrides. This behavior was changed for dependencies in MNG-1577 IIRC, but I don't know when the change was made for plugins. The behavior should probably be controlled in the class org.apache.maven.project.injection.DefaultModelDefaultsInjector.java (in maven-project). I've looked into this class in both 2.0.x and trunk, and they're the same...not sure whether that means the change happened before the split (unlikely) or was ported across (more likely), but in both cases it looks like it only uses the pluginManagement version if the main version is missing and the pluginManagement version is not. I'm not really sure what's going on here, but hopefully that can shed a little light on the background for this.
Hide
John Casey added a comment -

The code on that ModelDefaultsInjector impl seems nearly identical between 2.0.x/HEAD and 2.1/HEAD (slight formatting diffs), but the two behave dramatically different on this point. 2.1-SNAPSHOT allows local override of plugin version (in the build/plugins section), where 2.0.9-SNAPSHOT does not. I'm not sure where else pluginManagement comes into play (it probably shouldn't be in play at all once the project instance is built), but the difference must lie outside of the defaults injector...

Sorry for the red herring.

Show
John Casey added a comment - The code on that ModelDefaultsInjector impl seems nearly identical between 2.0.x/HEAD and 2.1/HEAD (slight formatting diffs), but the two behave dramatically different on this point. 2.1-SNAPSHOT allows local override of plugin version (in the build/plugins section), where 2.0.9-SNAPSHOT does not. I'm not sure where else pluginManagement comes into play (it probably shouldn't be in play at all once the project instance is built), but the difference must lie outside of the defaults injector... Sorry for the red herring.
Hide
John Casey added a comment -

The problem seems to be coming from the DefaultLifecycleExecutor.getMojoDescriptor(..) method. After parsing or otherwise acquiring a Plugin object for the task being invoked, it blindly injects pluginManagement values without first checking the project for a matching non-pluginManagement plugin entry...then, it passes the plugin off to the DefaultPluginManager.verifyPlugin(..) method, which from what I can tell would have done this in the correct order (including the project/build/plugins/plugin info over the pluginManagement info).

I'm currently trying to figure out whether we can remove the project.injectPluginManagementInfo(..) call from DefaultLifecycleExecutor.getMojoDescriptor(..), to simplify things and restore some semblance of sanity.

Show
John Casey added a comment - The problem seems to be coming from the DefaultLifecycleExecutor.getMojoDescriptor(..) method. After parsing or otherwise acquiring a Plugin object for the task being invoked, it blindly injects pluginManagement values without first checking the project for a matching non-pluginManagement plugin entry...then, it passes the plugin off to the DefaultPluginManager.verifyPlugin(..) method, which from what I can tell would have done this in the correct order (including the project/build/plugins/plugin info over the pluginManagement info). I'm currently trying to figure out whether we can remove the project.injectPluginManagementInfo(..) call from DefaultLifecycleExecutor.getMojoDescriptor(..), to simplify things and restore some semblance of sanity.
Hide
John Casey added a comment -

Fixed, and IT is in place to keep it from coming back.

Show
John Casey added a comment - Fixed, and IT is in place to keep it from coming back.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: