|
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... this looks good to me - if you have a chance could you turn the reproduction into the integration test format?
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. Convert JUnit test to maven integration test
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..
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] ------------------------------------------------------------------------ If you can fix the patch and reattach, then we can include for 2.0.9 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. 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. Applied the latest patch, and everything seems good.
This is still causing problems. See the IT output here: https://ci.sonatype.org/job/Maven-2.0.x-freestyle/18/console
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. I've just executed the following:
svn co -r 636941 https://svn.apache.org/repos/asf/maven/components/branches/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.. ? 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).
Tried again. My test results locally are nearly identical to hudson:
Mine: Hudson: 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. 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).
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 then mvn clean test (or mvn clean test -e -U -B -Prun-its) I see the same tests running as in hudson, I see 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? 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 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. 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 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).
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.