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

Cached plugins are used, even when the specifically declared

    Details

    • Number of attachments :
      7

      Description

      In the attached project, you can build module A, then build module B, but the top level aggregator project will fail at B.

      The reason this happens is that maven seems to cache plugins. When B is built in isolation, all things are fine - but when built in aggregation, one of the plugins that it uses has already been instantiated, and so it uses that one. This is incorrect, since the declared version is different in B, and is relying on functionality not present in the version declared in A.

      I have seen similar behaviour when a plugin relies on other plugins to get work done - all of a sudden a build mysteriously stops working, because of a completely unrelated plugin.

      This is pretty painful because

      • it's possible to get into a 'no solution', where one project relies on one behaviour so can't upgrade, and one project relies on new behaviour, so can't downgrade.
      • you get builds that work OK in isolation, but not in their project. This is bad. Also builds tied together in bigger aggregator projects can fail in mysterious ways (mysterious because the user /has/ specified the plugin version, and maven has ignored them, or it's a plugin dependency that got there first)
      • subtle build ordering changes can cause new failures (the example has B depend on A - but the bug might only manifest itself in certain build orders that change even when B and A don't).
      1. 0001-Initial-fix-to-see-if-we-can-have-1-version-of-a-pl.patch
        14 kB
        Nigel Magnay
      2. 0001-Initial-fix-to-see-if-we-can-have-1-version-of-a-pl.patch.svn
        14 kB
        Nigel Magnay
      3. maven-bug-2.tar
        40 kB
        Nigel Magnay
      4. MNG-3284.patch
        14 kB
        Brian Fox
      5. mng3284-usingCachedPlugins.tar
        50 kB
        Nigel Magnay
      6. plugin.diff
        14 kB
        Nigel Magnay
      7. pluginbug.tar
        10 kB
        Nigel Magnay

        Issue Links

          Activity

          Hide
          Nigel Magnay added a comment -

          This is an easy-to-reproduce test :

          Build the module mojo
          Build the module mojo2

          then from top level do
          mvn install

          it should use 2 different versions, you can observe that it only uses one.

          Show
          Nigel Magnay added a comment - This is an easy-to-reproduce test : Build the module mojo Build the module mojo2 then from top level do mvn install it should use 2 different versions, you can observe that it only uses one.
          Hide
          Nigel Magnay added a comment -

          This is my initial attempt at a fix, which seems to work for me.

          The plugin coordinator and manager change to always use versioned identifiers (rather than 'key' which doesn't include the version).

          There's some mention of version not being included - but this was never an issue (fixed already?). If there is a counterexample of where this fix won't work, I'd be happy to work a bit further on it...

          Show
          Nigel Magnay added a comment - This is my initial attempt at a fix, which seems to work for me. The plugin coordinator and manager change to always use versioned identifiers (rather than 'key' which doesn't include the version). There's some mention of version not being included - but this was never an issue (fixed already?). If there is a counterexample of where this fix won't work, I'd be happy to work a bit further on it...
          Hide
          Brett Porter added a comment -

          this looks good to me - if you have a chance could you turn the reproduction into the integration test format?

          Show
          Brett Porter added a comment - this looks good to me - if you have a chance could you turn the reproduction into the integration test format?
          Hide
          Brett Porter added a comment -

          I'm not sure MNG-3217, MNG-3013 is related to this, but the other certainly is. I think this fix is going to be safe in the cases where it works and no worse in the cases where it doesn't. From what I remember, the plugin manager will still cache the containers based on a less comprehensive key so you might not get a complete solution with this.

          I do think this would require some more comprehensive testing but is worth reviewing.

          Show
          Brett Porter added a comment - I'm not sure MNG-3217 , MNG-3013 is related to this, but the other certainly is. I think this fix is going to be safe in the cases where it works and no worse in the cases where it doesn't. From what I remember, the plugin manager will still cache the containers based on a less comprehensive key so you might not get a complete solution with this. I do think this would require some more comprehensive testing but is worth reviewing.
          Hide
          Nigel Magnay added a comment -

          Convert JUnit test to maven integration test

          Show
          Nigel Magnay added a comment - Convert JUnit test to maven integration test
          Hide
          Brian Fox added a comment -

          Identified as blocker to upgrading. Lets try to include this (even has an IT)

          Show
          Brian Fox added a comment - Identified as blocker to upgrading. Lets try to include this (even has an IT)
          Hide
          Brian Fox added a comment -

          any chance you can make that patch against svn?

          Show
          Brian Fox added a comment - any chance you can make that patch against svn?
          Hide
          Nigel Magnay added a comment -

          I think this should apply with SVN (if that's what you meant?) - it's the same patch but with --no-prefix; I'm told tortoiseSVN is a bit picky about patch formats..

          Show
          Nigel Magnay added a comment - I think this should apply with SVN (if that's what you meant?) - it's the same patch but with --no-prefix; I'm told tortoiseSVN is a bit picky about patch formats..
          Hide
          Brian Fox added a comment -

          perfect timing...i just finished merging in the IT.

          Show
          Brian Fox added a comment - perfect timing...i just finished merging in the IT.
          Hide
          Brian Fox added a comment -

          patch doesn't work, it still has git junk in there. I'll try to do it manually

          Show
          Brian Fox added a comment - patch doesn't work, it still has git junk in there. I'll try to do it manually
          Hide
          Brian Fox added a comment - - edited

          Attaching an svn patch (MNG-3284.patch) by hand applying these changes to 2.0.9. The IT is already committed to the core-its but with this patch there is a crash:

          INFO] ------------------------------------------------------------------------
          [ERROR] FATAL ERROR
          [INFO] ------------------------------------------------------------------------
          [INFO] The PluginDescriptor for the plugin Plugin [org.apache.maven.plugins:maven-plugin-plugin] was not found.
          [INFO] ------------------------------------------------------------------------
          [INFO] Trace
          java.lang.IllegalStateException: The PluginDescriptor for the plugin Plugin [org.apache.maven.plugins:maven-plugin-plugin] was not found.
          at org.apache.maven.plugin.DefaultPluginManager.addPlugin(DefaultPluginManager.java:329)
          at org.apache.maven.plugin.DefaultPluginManager.verifyVersionedPlugin(DefaultPluginManager.java:215)

          If you can fix the patch and reattach, then we can include for 2.0.9

          Show
          Brian Fox added a comment - - edited Attaching an svn patch ( MNG-3284 .patch) by hand applying these changes to 2.0.9. The IT is already committed to the core-its but with this patch there is a crash: INFO] ------------------------------------------------------------------------ [ERROR] FATAL ERROR [INFO] ------------------------------------------------------------------------ [INFO] The PluginDescriptor for the plugin Plugin [org.apache.maven.plugins:maven-plugin-plugin] was not found. [INFO] ------------------------------------------------------------------------ [INFO] Trace java.lang.IllegalStateException: The PluginDescriptor for the plugin Plugin [org.apache.maven.plugins:maven-plugin-plugin] was not found. at org.apache.maven.plugin.DefaultPluginManager.addPlugin(DefaultPluginManager.java:329) at org.apache.maven.plugin.DefaultPluginManager.verifyVersionedPlugin(DefaultPluginManager.java:215) If you can fix the patch and reattach, then we can include for 2.0.9
          Hide
          Nigel Magnay added a comment -

          Ok - I've re-rolled the patch, and tried to execute it as an SVN diff.

          I've run it against the current set of 2.0.9 ITs and it seems to work.

          Show
          Nigel Magnay added a comment - Ok - I've re-rolled the patch, and tried to execute it as an SVN diff. I've run it against the current set of 2.0.9 ITs and it seems to work.
          Hide
          John Casey added a comment -

          I've applied the latest patch, and everything (including the IT that Brian committed) seems happy.

          I'd like to provide one caveat to all of this, though:

          plugin-level dependencies still cannot vary for a single plugin version in the same build. This is a particular problem when using the ant-run plugin. I'll see if I can dig up the issue (I'm sure it's been logged), and link it here. I've also got a simple test project that I can attach to the issue, which is what I've used to verify that this is indeed fixed in the Maven 2.1 code.

          Show
          John Casey added a comment - I've applied the latest patch, and everything (including the IT that Brian committed) seems happy. I'd like to provide one caveat to all of this, though: plugin-level dependencies still cannot vary for a single plugin version in the same build. This is a particular problem when using the ant-run plugin. I'll see if I can dig up the issue (I'm sure it's been logged), and link it here. I've also got a simple test project that I can attach to the issue, which is what I've used to verify that this is indeed fixed in the Maven 2.1 code.
          Hide
          John Casey added a comment -

          These two issues deal with subtly different aspects of plugin variance within a multimodule project, with MNG-3284 specifying variance of plugin versions, and MNG-1323 specifying variance of plugin-level dependencies.

          I have a test case that I'll attach to MNG-1323 that displays the issue.

          Show
          John Casey added a comment - These two issues deal with subtly different aspects of plugin variance within a multimodule project, with MNG-3284 specifying variance of plugin versions, and MNG-1323 specifying variance of plugin-level dependencies. I have a test case that I'll attach to MNG-1323 that displays the issue.
          Hide
          John Casey added a comment -

          Applied the latest patch, and everything seems good.

          Show
          John Casey added a comment - Applied the latest patch, and everything seems good.
          Hide
          Brian Fox added a comment -

          This is still causing problems. See the IT output here: https://ci.sonatype.org/job/Maven-2.0.x-freestyle/18/console

          Show
          Brian Fox added a comment - This is still causing problems. See the IT output here: https://ci.sonatype.org/job/Maven-2.0.x-freestyle/18/console
          Hide
          Nigel Magnay added a comment -

          Hmm - that's confusing me.

          On my machine, both trunk and trunk-1 (without and with the patch respectively) both only have 1 IT failure, in testitMNG2861 - I've updated the core-it tests and rebuilt, so I don't see why I get different results.

          So I must be doing something wrong; I'm currently re-running on my machine with the same commandline that I can see from the hudson build to see what happens.

          Show
          Nigel Magnay added a comment - Hmm - that's confusing me. On my machine, both trunk and trunk-1 (without and with the patch respectively) both only have 1 IT failure, in testitMNG2861 - I've updated the core-it tests and rebuilt, so I don't see why I get different results. So I must be doing something wrong; I'm currently re-running on my machine with the same commandline that I can see from the hudson build to see what happens.
          Hide
          Nigel Magnay added a comment -

          I've just executed the following:
          svn co -r 636941 https://svn.apache.org/repos/asf/maven/components/branches/maven-2.0.x /tmp/maven-2.0.x
          cd /tmp/maven-2.0.x
          mvn clean install -B -U -e -Dmaven.repo.local=/tmp/maven-2.0.x.repo -Pstrict,run-its -Dsurefire.useFile=false

          And still things seem to be happy bar 2861..

          ?

          Show
          Nigel Magnay added a comment - I've just executed the following: svn co -r 636941 https://svn.apache.org/repos/asf/maven/components/branches/maven-2.0.x /tmp/maven-2.0.x cd /tmp/maven-2.0.x mvn clean install -B -U -e -Dmaven.repo.local=/tmp/maven-2.0.x.repo -Pstrict,run-its -Dsurefire.useFile=false And still things seem to be happy bar 2861.. ?
          Hide
          John Casey added a comment -

          I verified this also. I get the same result, except for me 2123 and 2861 are failing, but Brian tells me these are failing on his copy (with 3284 rolled back).

          Show
          John Casey added a comment - I verified this also. I get the same result, except for me 2123 and 2861 are failing, but Brian tells me these are failing on his copy (with 3284 rolled back).
          Hide
          Brian Fox added a comment -

          we will retry the patch once everything is stabilized. I know that as soon as I rolled it back, everything was ok, both on my machine and the hudson one and the error was the same as I saw when I applied it originally.

          Show
          Brian Fox added a comment - we will retry the patch once everything is stabilized. I know that as soon as I rolled it back, everything was ok, both on my machine and the hudson one and the error was the same as I saw when I applied it originally.
          Hide
          Brian Fox added a comment -

          Tried again. My test results locally are nearly identical to hudson:

          Mine:
          Tests in error:
          testitMNG2277(org.apache.maven.integrationtests.MavenITmng2277AggregatorAndResolutionPluginsTest)
          testit0104(org.apache.maven.integrationtests.MavenIT0104Test)
          testit0075(org.apache.maven.integrationtests.MavenIT0075Test)
          testit0074(org.apache.maven.integrationtests.MavenIT0074Test)
          testit0073(org.apache.maven.integrationtests.MavenIT0073Test)
          testit0071(org.apache.maven.integrationtests.MavenIT0071Test)
          testit0068(org.apache.maven.integrationtests.MavenIT0068Test)
          testit0064(org.apache.maven.integrationtests.MavenIT0064Test)
          testit0049(org.apache.maven.integrationtests.MavenIT0049Test)
          testit0046(org.apache.maven.integrationtests.MavenIT0046Test)
          testit0045(org.apache.maven.integrationtests.MavenIT0045Test)
          testit0041(org.apache.maven.integrationtests.MavenIT0041Test)
          testit0040(org.apache.maven.integrationtests.MavenIT0040Test)
          testit0027(org.apache.maven.integrationtests.MavenIT0027Test)
          testit0022(org.apache.maven.integrationtests.MavenIT0022Test)
          testit0018(org.apache.maven.integrationtests.MavenIT0018Test)
          testit0012(org.apache.maven.integrationtests.MavenIT0012Test)

          Hudson:
          https://ci.sonatype.org/job/Maven-2.0.x-ITs/30/console

          Show
          Brian Fox added a comment - Tried again. My test results locally are nearly identical to hudson: Mine: Tests in error: testitMNG2277(org.apache.maven.integrationtests.MavenITmng2277AggregatorAndResolutionPluginsTest) testit0104(org.apache.maven.integrationtests.MavenIT0104Test) testit0075(org.apache.maven.integrationtests.MavenIT0075Test) testit0074(org.apache.maven.integrationtests.MavenIT0074Test) testit0073(org.apache.maven.integrationtests.MavenIT0073Test) testit0071(org.apache.maven.integrationtests.MavenIT0071Test) testit0068(org.apache.maven.integrationtests.MavenIT0068Test) testit0064(org.apache.maven.integrationtests.MavenIT0064Test) testit0049(org.apache.maven.integrationtests.MavenIT0049Test) testit0046(org.apache.maven.integrationtests.MavenIT0046Test) testit0045(org.apache.maven.integrationtests.MavenIT0045Test) testit0041(org.apache.maven.integrationtests.MavenIT0041Test) testit0040(org.apache.maven.integrationtests.MavenIT0040Test) testit0027(org.apache.maven.integrationtests.MavenIT0027Test) testit0022(org.apache.maven.integrationtests.MavenIT0022Test) testit0018(org.apache.maven.integrationtests.MavenIT0018Test) testit0012(org.apache.maven.integrationtests.MavenIT0012Test) Hudson: https://ci.sonatype.org/job/Maven-2.0.x-ITs/30/console
          Hide
          John Casey added a comment -

          When I built this locally, then installed it and tried to build again with the version I just built, I got the following:

          [INFO] Internal error in the plugin manager getting plugin 'org.apache.maven.plugins:maven-shade-plugin': Failed to create plugin container for plugin 'Plugin [org.apache.maven.plugins:maven-shade-plugin]': Cannot create child container, because child named 'maven-shade-plugin:org.apache.maven.plugins:1.0-beta-2-SNAPSHOT' already exists in parent 'app0'.

          I haven't reproduced Brian's problem above, but this in itself signals that the patch is not careful enough to avoid duplication of plugin containers. We need to take some more time and build up a better test suite for this issue, to guard against all the different problems that can occur when you start changing the plugin classloading scheme.

          Show
          John Casey added a comment - When I built this locally, then installed it and tried to build again with the version I just built, I got the following: [INFO] Internal error in the plugin manager getting plugin 'org.apache.maven.plugins:maven-shade-plugin': Failed to create plugin container for plugin 'Plugin [org.apache.maven.plugins:maven-shade-plugin] ': Cannot create child container, because child named 'maven-shade-plugin:org.apache.maven.plugins:1.0-beta-2-SNAPSHOT' already exists in parent 'app0'. I haven't reproduced Brian's problem above, but this in itself signals that the patch is not careful enough to avoid duplication of plugin containers. We need to take some more time and build up a better test suite for this issue, to guard against all the different problems that can occur when you start changing the plugin classloading scheme.
          Hide
          John Casey added a comment -

          We simply cannot be certain of this patch without more tests to check that the plugin classloaders are being constructed correctly. We're seeing too many different problems that are hard to reproduce consistently. It also will probably be useful to look at the documentation for the plugin and extension classloading refactor that I did for 2.1, once I get those docs done (next on my TODO list once 2.0.9 is staged).

          Show
          John Casey added a comment - We simply cannot be certain of this patch without more tests to check that the plugin classloaders are being constructed correctly. We're seeing too many different problems that are hard to reproduce consistently. It also will probably be useful to look at the documentation for the plugin and extension classloading refactor that I did for 2.1, once I get those docs done (next on my TODO list once 2.0.9 is staged).
          Hide
          Nigel Magnay added a comment -

          That's ok - I'll keep plugging away at getting the tests to pass..

          Unfortunately - my tests seem to work. I must be doing something wrong.

          I'm doing
          mvn clean install
          from maven-2.0.x

          then mvn clean test (or mvn clean test -e -U -B -Prun-its)
          from maven-2.0.x/maven-core-it-runner

          I see the same tests running as in hudson, I see
          mng2744checksumVerification
          failing
          but the testit* working.

          Is there some external dependency I need to recompile? How come testitMNG2277 fails for Brian Fox but not for hudson ? Could there be some change /difference in the IT framework that I'm not picking up?

          Show
          Nigel Magnay added a comment - That's ok - I'll keep plugging away at getting the tests to pass.. Unfortunately - my tests seem to work. I must be doing something wrong. I'm doing mvn clean install from maven-2.0.x then mvn clean test (or mvn clean test -e -U -B -Prun-its) from maven-2.0.x/maven-core-it-runner I see the same tests running as in hudson, I see mng2744checksumVerification failing but the testit* working. Is there some external dependency I need to recompile? How come testitMNG2277 fails for Brian Fox but not for hudson ? Could there be some change /difference in the IT framework that I'm not picking up?
          Hide
          John Casey added a comment -

          I'm not sure why this is failing in so many different ways, but I can say that the ITs were all passing in the three environments we tested: mine, hudson, and Brian's. When we applied the patch again (today, merged from Brian's rollback on Friday/Saturday/whenever it was), we got three different failure profiles on three different machines. Yours makes four where something is failing.

          On my side, it seemed like all the ITs ran when I did the following:

          1. svn up core-integration-testing
          2. cd core-integration-testing && mvn clean install
          3. cd maven-2.0.x && mvn -P run-its,strict clean install

          Then, when I installed the newly-built copy of maven into my apps directory (just a local directory structure I have where things like Maven live), and tried to build the maven-2.0.x project once again (#3 above) using that newly-installed version, it failed with the error I gave above (two of my entries back, about the child container already existing when calling the shade plugin). To me, this is an indication that we don't have sufficient test coverage, if that error could slip by, regardless of whether all the ITs succeed or not.

          Show
          John Casey added a comment - I'm not sure why this is failing in so many different ways, but I can say that the ITs were all passing in the three environments we tested: mine, hudson, and Brian's. When we applied the patch again (today, merged from Brian's rollback on Friday/Saturday/whenever it was), we got three different failure profiles on three different machines. Yours makes four where something is failing. On my side, it seemed like all the ITs ran when I did the following: 1. svn up core-integration-testing 2. cd core-integration-testing && mvn clean install 3. cd maven-2.0.x && mvn -P run-its,strict clean install Then, when I installed the newly-built copy of maven into my apps directory (just a local directory structure I have where things like Maven live), and tried to build the maven-2.0.x project once again (#3 above) using that newly-installed version, it failed with the error I gave above (two of my entries back, about the child container already existing when calling the shade plugin). To me, this is an indication that we don't have sufficient test coverage, if that error could slip by, regardless of whether all the ITs succeed or not.
          Hide
          Nigel Magnay added a comment -

          Out of interest, what platform are you / hudson running ?

          I've tried my build on linux and OS X (though both with maven 2.0.8 and 1.5.0_13) and get the same result - it seems there's something odd. I'll try some JVM variants and windows tomorrow and try and find why I get different test resutls

          Show
          Nigel Magnay added a comment - Out of interest, what platform are you / hudson running ? I've tried my build on linux and OS X (though both with maven 2.0.8 and 1.5.0_13) and get the same result - it seems there's something odd. I'll try some JVM variants and windows tomorrow and try and find why I get different test resutls
          Hide
          John Casey added a comment -

          Pushing to 2.0.11 so we can have a smaller set of high-value issues to target for the next release (2.0.10).

          Show
          John Casey added a comment - Pushing to 2.0.11 so we can have a smaller set of high-value issues to target for the next release (2.0.10).
          Hide
          Benjamin Bentmann added a comment -

          Fixed in r591391 and r744497, respectively.

          Show
          Benjamin Bentmann added a comment - Fixed in r591391 and r744497 , respectively.

            People

            • Assignee:
              Benjamin Bentmann
              Reporter:
              Nigel Magnay
            • Votes:
              8 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: