Maven 2 & 3
  1. Maven 2 & 3
  2. MNG-3394

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

            • Assignee:
              John Casey
              Reporter:
              Benjamin Bentmann
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: