Issue Details (XML | Word | Printable)

Key: MNG-3284
Type: Bug Bug
Status: Reopened Reopened
Priority: Critical Critical
Assignee: John Casey
Reporter: Nigel Magnay
Votes: 8
Watchers: 7
Operations

If you were logged in you would be able to see more operations.
Maven 2

Cached plugins are used, even when the specifically declared

Created: 14/Nov/07 07:15 AM   Updated: 22/Oct/08 09:47 AM
Component/s: Dependencies, Plugins and Lifecycle
Affects Version/s: 2.0.7
Fix Version/s: 2.0.11

Time Tracking:
Not Specified

File Attachments: 1. Text File 0001-Initial-fix-to-see-if-we-can-have-1-version-of-a-pl.patch (14 kB)
2. File 0001-Initial-fix-to-see-if-we-can-have-1-version-of-a-pl.patch.svn (14 kB)
3. File maven-bug-2.tar (40 kB)
4. Text File MNG-3284.patch (14 kB)
5. File mng3284-usingCachedPlugins.tar (50 kB)
6. File plugin.diff (14 kB)
7. File pluginbug.tar (10 kB)

Issue Links:
Related
 


 Description  « Hide
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).


 All   Comments   Work Log   Change History      Sort Order: Ascending order - Click to sort in descending order
Nigel Magnay added a comment - 23/Jan/08 06:04 AM
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.


Nigel Magnay added a comment - 23/Jan/08 06:10 AM
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...


Brett Porter added a comment - 10/Mar/08 05:19 AM
this looks good to me - if you have a chance could you turn the reproduction into the integration test format?

Brett Porter added a comment - 10/Mar/08 05:24 AM
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.


Nigel Magnay added a comment - 10/Mar/08 07:13 AM
Convert JUnit test to maven integration test

Brian Fox added a comment - 10/Mar/08 08:55 PM
Identified as blocker to upgrading. Lets try to include this (even has an IT)

Brian Fox added a comment - 10/Mar/08 09:05 PM
any chance you can make that patch against svn?

Nigel Magnay added a comment - 11/Mar/08 11:35 AM
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..

Brian Fox added a comment - 11/Mar/08 11:52 AM
perfect timing...i just finished merging in the IT.

Brian Fox added a comment - 11/Mar/08 12:30 PM
patch doesn't work, it still has git junk in there. I'll try to do it manually

Brian Fox added a comment - 11/Mar/08 01:47 PM - 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


Nigel Magnay added a comment - 12/Mar/08 09:51 AM
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.


John Casey added a comment - 12/Mar/08 02:22 PM
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.


John Casey added a comment - 12/Mar/08 02:27 PM
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.


John Casey added a comment - 12/Mar/08 02:28 PM
Applied the latest patch, and everything seems good.

Brian Fox added a comment - 13/Mar/08 07:41 PM
This is still causing problems. See the IT output here: https://ci.sonatype.org/job/Maven-2.0.x-freestyle/18/console

Nigel Magnay added a comment - 14/Mar/08 04:41 AM
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.


Nigel Magnay added a comment - 14/Mar/08 05:14 AM
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..

?


John Casey added a comment - 14/Mar/08 11:03 AM
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).

Brian Fox added a comment - 14/Mar/08 07:57 PM
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.

Brian Fox added a comment - 17/Mar/08 02:51 PM
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


John Casey added a comment - 17/Mar/08 03:03 PM
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.


John Casey added a comment - 17/Mar/08 03:09 PM
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).

Nigel Magnay added a comment - 17/Mar/08 03:28 PM
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?


John Casey added a comment - 17/Mar/08 03:42 PM
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.


Nigel Magnay added a comment - 17/Mar/08 04:54 PM
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


John Casey added a comment - 03/Jul/08 07:06 PM
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).