Details

    • Number of attachments :
      5

      Description

      Currently plugin sorts artifacts on its own (alphabetic order???) because the order of dependencies that comes from maven is not reliable (random?). This breaks tests that use JBoss Embedded which works under maven surefire plugin because it still puts dependencies that came first at the beginning of the classpath). Apparently not all classpath elements are put in random order. At least I get the first ones listed in dependencies also first in the classpath (can be seen as $

      {surefire.test.class.path}

      and in target/surefire-reports/TEST-TestSuite.xml).

      While there is not much we can do for maven eclipse plugin a solution is on the way: MNG-1412. When maven 2.0.9 is released maven eclipse plugin can revert back to the default classpath order.

      Can we somehow schedule this?

      Another question: is there anyway to put certain dependencies in the first place except for renaming them so that alphabetic order does the job?

      1. ideal.classpath
        1 kB
        Jean-Noel Rouvignac
      2. MECLIPSE-388.patch
        2 kB
        David J. M. Karlsen
      3. MECLIPSE-388.patch
        2 kB
        David J. M. Karlsen
      4. MECLIPSE-388-it-test.patch
        8 kB
        David J. M. Karlsen

        Issue Links

          Activity

          Hide
          Arnaud Heritier added a comment -

          It could be possible but I wouldn't force to have maven 2.0.9 to use the next version of the plugin.
          I'll try to see, but I think we'll rewrite/refactor an important part after the 2.5 release (perhaps it will be a 3.0)

          Show
          Arnaud Heritier added a comment - It could be possible but I wouldn't force to have maven 2.0.9 to use the next version of the plugin. I'll try to see, but I think we'll rewrite/refactor an important part after the 2.5 release (perhaps it will be a 3.0)
          Hide
          Siarhei Dudzin added a comment -

          I agree, forcing to maven 2.0.9 would be a bit extreme... What about having an option or something like that (a disadvantage is that too many options isn't nice)? That about performing sorting based on maven version (also not very nice but no extra complexity in configuration for the end user)?

          Show
          Siarhei Dudzin added a comment - I agree, forcing to maven 2.0.9 would be a bit extreme... What about having an option or something like that (a disadvantage is that too many options isn't nice)? That about performing sorting based on maven version (also not very nice but no extra complexity in configuration for the end user)?
          Hide
          Jeroen Ruijgers added a comment -

          the ordering has changed in 2.5. i had a problem with a dependency of a subproject. in the project is a class with the same package/name as in de dependency, but on the eclipse classpath the dependency came before the subproject. therefore the wrong class was used in eclipse and i got a nice red cross.

          Show
          Jeroen Ruijgers added a comment - the ordering has changed in 2.5. i had a problem with a dependency of a subproject. in the project is a class with the same package/name as in de dependency, but on the eclipse classpath the dependency came before the subproject. therefore the wrong class was used in eclipse and i got a nice red cross.
          Hide
          Kamil Demecki added a comment -

          Maven issue with classpath orderring is corected http://jira.codehaus.org/browse/MNG-1412 (maven 2.0.9) but plugin now using a alphabetic order. Mayby it was ok when ordering with maven was unpredictable, but there are good reasons to implement this and corrent bug.

          Show
          Kamil Demecki added a comment - Maven issue with classpath orderring is corected http://jira.codehaus.org/browse/MNG-1412 (maven 2.0.9) but plugin now using a alphabetic order. Mayby it was ok when ordering with maven was unpredictable, but there are good reasons to implement this and corrent bug.
          Hide
          Siarhei Dudzin added a comment -

          It appears that MNG-1412 uses LinkedHashSet instead of HashSet. If we could determine which one is used we cold apply corresponding sorting algorithm (that is alphabetic or as-is). No forcing to a maven version is necessary then and yet the result would be reproducable for pre- and post-MNG-1412 versions of maven.

          What do you think?

          Show
          Siarhei Dudzin added a comment - It appears that MNG-1412 uses LinkedHashSet instead of HashSet. If we could determine which one is used we cold apply corresponding sorting algorithm (that is alphabetic or as-is). No forcing to a maven version is necessary then and yet the result would be reproducable for pre- and post- MNG-1412 versions of maven. What do you think?
          Hide
          Kamil Demecki added a comment -

          I woudl have to check it Yesterday I worked on this issue with succcess. I apply ordering from maven from invocation this function List org.apache.maven.project.MavenProject.getDependencies() in IdeDependency[] org.apache.maven.plugin.ide.AbstractIdeSupportMojo.doDependencyResolution() throws MojoExecutionException and it's working When I will have time I prepare patch for trunk.

          Show
          Kamil Demecki added a comment - I woudl have to check it Yesterday I worked on this issue with succcess. I apply ordering from maven from invocation this function List org.apache.maven.project.MavenProject.getDependencies() in IdeDependency[] org.apache.maven.plugin.ide.AbstractIdeSupportMojo.doDependencyResolution() throws MojoExecutionException and it's working When I will have time I prepare patch for trunk.
          Hide
          Kamil Demecki added a comment -

          Yes replace HashSet and HashMap in AbstractIdeSupportMojo with LinkedHashSet and LinkedHashMap works. It only requires removing sorting classpath and maven order is hold.

          Show
          Kamil Demecki added a comment - Yes replace HashSet and HashMap in AbstractIdeSupportMojo with LinkedHashSet and LinkedHashMap works. It only requires removing sorting classpath and maven order is hold.
          Hide
          Kamil Demecki added a comment -

          Is this place adequate to talk about developing this issue ? Maven has mailing list and dont have forum. Jira Comments is forum for developing this plugin ?

          Show
          Kamil Demecki added a comment - Is this place adequate to talk about developing this issue ? Maven has mailing list and dont have forum. Jira Comments is forum for developing this plugin ?
          Hide
          Siarhei Dudzin added a comment -

          maven-dev mailing list seem appropriate

          Show
          Siarhei Dudzin added a comment - maven-dev mailing list seem appropriate
          Hide
          Arnaud Heritier added a comment -

          We recommend that you discuss with us on the mailing list and you summarize our exchanges in the issue.

          Show
          Arnaud Heritier added a comment - We recommend that you discuss with us on the mailing list and you summarize our exchanges in the issue.
          Hide
          David J. M. Karlsen added a comment -

          Requiring 2.0.9 doesn't seem like a big issue for me (especially not when you compare that to having a random/invalid classpath).
          Also, it's important to preserve the order of test-resources, f.ex:

          <testResources>
          <testResource>
          <directory>src/test/resources</directory>
          </testResource>
          <testResource>
          <directory>src/main/config</directory>
          </testResource>
          </testResources>

          src/test/resources should be before src/main/config - this is not the case p.t.

          Show
          David J. M. Karlsen added a comment - Requiring 2.0.9 doesn't seem like a big issue for me (especially not when you compare that to having a random/invalid classpath). Also, it's important to preserve the order of test-resources, f.ex: <testResources> <testResource> <directory>src/test/resources</directory> </testResource> <testResource> <directory>src/main/config</directory> </testResource> </testResources> src/test/resources should be before src/main/config - this is not the case p.t.
          Hide
          David J. M. Karlsen added a comment -

          Here's a patch which use a LinkedHashSet and adds test resources before the normal resources

          Show
          David J. M. Karlsen added a comment - Here's a patch which use a LinkedHashSet and adds test resources before the normal resources
          Hide
          Benjamin Bentmann added a comment -

          Here's a patch which [...] adds test resources before the normal resources

          If the intention is to catch up with the test class path used by Maven/Surefire, we should also consider to add the main compile source roots after the test compile source roots.

          Show
          Benjamin Bentmann added a comment - Here's a patch which [...] adds test resources before the normal resources If the intention is to catch up with the test class path used by Maven/Surefire, we should also consider to add the main compile source roots after the test compile source roots.
          Hide
          Benjamin Bentmann added a comment -

          It appears that MNG-1412 uses LinkedHashSet instead of HashSet. If we could determine which one is used we cold apply corresponding sorting algorithm

          We could query the Maven version from class path: META-INF/maven/org.apache.maven/maven-core/pom.properties.

          We only might need to require Maven 2.0.9 for building the Eclipse Plugin itself to ensure the tests are stable.

          Show
          Benjamin Bentmann added a comment - It appears that MNG-1412 uses LinkedHashSet instead of HashSet. If we could determine which one is used we cold apply corresponding sorting algorithm We could query the Maven version from class path: META-INF/maven/org.apache.maven/maven-core/pom.properties. We only might need to require Maven 2.0.9 for building the Eclipse Plugin itself to ensure the tests are stable.
          Hide
          David J. M. Karlsen added a comment -

          Updated patch, order is now:

          test sources
          main sources
          test resources
          main resources

          I'll have a go at the integration-tests tomorrow and add them as well

          Show
          David J. M. Karlsen added a comment - Updated patch, order is now: test sources main sources test resources main resources I'll have a go at the integration-tests tomorrow and add them as well
          Hide
          David J. M. Karlsen added a comment -

          The attached patch fixes all of the integration tests related to this issue but two:
          testProject03(org.apache.maven.plugin.eclipse.it.EclipsePluginIT)
          testMyEclipseProject03(org.apache.maven.plugin.eclipse.it.MyEclipsePluginIT)

          These need some further investigation - probably EclipseSourceDir's equals method should take outputDir into consideration.

          This test fail (unrelated to the patch):
          testProject6(org.apache.maven.plugin.eclipse.it.RadPluginIT)

          Show
          David J. M. Karlsen added a comment - The attached patch fixes all of the integration tests related to this issue but two: testProject03(org.apache.maven.plugin.eclipse.it.EclipsePluginIT) testMyEclipseProject03(org.apache.maven.plugin.eclipse.it.MyEclipsePluginIT) These need some further investigation - probably EclipseSourceDir's equals method should take outputDir into consideration. This test fail (unrelated to the patch): testProject6(org.apache.maven.plugin.eclipse.it.RadPluginIT)
          Hide
          Arnaud Heritier added a comment -

          We only might need to require Maven 2.0.9 for building the Eclipse Plugin itself to ensure the tests are stable.

          We already need to build le plugin with 2.0.8 for encoding tests. It won't be a problem to use 2.0.9. I'll add an enforcer rule when this one will we released...

          Show
          Arnaud Heritier added a comment - We only might need to require Maven 2.0.9 for building the Eclipse Plugin itself to ensure the tests are stable. We already need to build le plugin with 2.0.8 for encoding tests. It won't be a problem to use 2.0.9. I'll add an enforcer rule when this one will we released...
          Hide
          Benjamin Bentmann added a comment -

          The ordering of test vs main classes has been fixed in MECLIPSE-318, remaining is the ordering of dependencies. Interesting are these source comments:

          // TODO if (..magic property equals orderDependencies..)
          // TODO get the right comparator depending on orderDependencies={name,nearness..};
          

          Not yet sure how to interpret these.

          Show
          Benjamin Bentmann added a comment - The ordering of test vs main classes has been fixed in MECLIPSE-318 , remaining is the ordering of dependencies. Interesting are these source comments: // TODO if (..magic property equals orderDependencies..) // TODO get the right comparator depending on orderDependencies={name,nearness..}; Not yet sure how to interpret these.
          Hide
          Arnaud Heritier added a comment -

          dependencies order is a problem, but having the good list of dependencies is more important I think : MECLIPSE-472
          I think we have to review all the mechanism of dependencies to sanitize it.

          Show
          Arnaud Heritier added a comment - dependencies order is a problem, but having the good list of dependencies is more important I think : MECLIPSE-472 I think we have to review all the mechanism of dependencies to sanitize it.
          Hide
          Florian Probst added a comment -

          The classpath order is the reason why we're still using version 2.4 of the eclipse plugin. It's important that the pom order of the dependencies is reflected in Eclipse. Imagine you are coding in eclipse, finished and the code does not compile when you'd like to deploy the artifact. This isn't very common, but without changing the build order the problem maybe can't be solved.

          Now we would like to use the additionalConfig/location feature added in 2.5 and have to use a newer version. The issue here was last discussed two years ago and there is still no fix version given. I found a third party version of the plugin called 2.8-cpfix on the internet:

          http://stackoverflow.com/questions/793054/maven-classpath-order-issues

          It would be great having a 2.9 fixing this issue instead of using 2.8-cpfix!

          Show
          Florian Probst added a comment - The classpath order is the reason why we're still using version 2.4 of the eclipse plugin. It's important that the pom order of the dependencies is reflected in Eclipse. Imagine you are coding in eclipse, finished and the code does not compile when you'd like to deploy the artifact. This isn't very common, but without changing the build order the problem maybe can't be solved. Now we would like to use the additionalConfig/location feature added in 2.5 and have to use a newer version. The issue here was last discussed two years ago and there is still no fix version given. I found a third party version of the plugin called 2.8-cpfix on the internet: http://stackoverflow.com/questions/793054/maven-classpath-order-issues It would be great having a 2.9 fixing this issue instead of using 2.8-cpfix!
          Hide
          Jean-Noel Rouvignac added a comment -

          Even though the sumarry does not reflect it, I think MECLIPSE-665
          is a duplicate of the current bug.

          By the way, do you have any news on this bug? I am facing it as well where I want a hibernate.cfg.xml file from one project to be picked up based on the ordering of the depencencies in the classpath. I have a test which work fine in Surefire, but since Eclipse is using an alphabetical ordering, it is picking up the hibernate.cfg.xml file from another project making the test fail. I had to do some classloader magic (ClassLoader.getResources()) to make it work all the time at the expense of longer running tests. Everything that adds up time to run a test is definitely not nice.

          Thanks.

          Show
          Jean-Noel Rouvignac added a comment - Even though the sumarry does not reflect it, I think MECLIPSE-665 is a duplicate of the current bug. By the way, do you have any news on this bug? I am facing it as well where I want a hibernate.cfg.xml file from one project to be picked up based on the ordering of the depencencies in the classpath. I have a test which work fine in Surefire, but since Eclipse is using an alphabetical ordering, it is picking up the hibernate.cfg.xml file from another project making the test fail. I had to do some classloader magic (ClassLoader.getResources()) to make it work all the time at the expense of longer running tests. Everything that adds up time to run a test is definitely not nice. Thanks.
          Hide
          Barrie Treloar added a comment -

          I've had an attempt to apply the patch, and the code base has shifted since it was created, so it doesn't apply cleanly.
          But it looks like the changes are already there.

          • EclipsePlugin.buildDirectoryList() uses LinkedHashSet instead of TreeSet.
          • Classpath will either have test classpath before main, or main before test based on the configuration element "testSourcesLast".

          The it-test.patch doesn't have anything in the classpath that actually has dependencies.
          So there is no way to tell if the classpath ordering is done correctly.
          There should be at least two projects that have the same dependencies (but ordered differently in the pom) with correspondingly different .classpath's being generated.

          I'll try to create such a test and see whether the latest code works as expected.

          Show
          Barrie Treloar added a comment - I've had an attempt to apply the patch, and the code base has shifted since it was created, so it doesn't apply cleanly. But it looks like the changes are already there. EclipsePlugin.buildDirectoryList() uses LinkedHashSet instead of TreeSet. Classpath will either have test classpath before main, or main before test based on the configuration element "testSourcesLast". The it-test.patch doesn't have anything in the classpath that actually has dependencies. So there is no way to tell if the classpath ordering is done correctly. There should be at least two projects that have the same dependencies (but ordered differently in the pom) with correspondingly different .classpath's being generated. I'll try to create such a test and see whether the latest code works as expected.
          Hide
          Barrie Treloar added a comment -

          The supplied patch doesn't do what is needed.

          Its taken a couple of days to hack into this to find where all the bits are sorted and how it all hangs together and make the changes.

          Things that are now ordered correctly:

          • AbstractEclipseManifestWriter.java: META-INF/MANIFEST.MF
          • EclipseClasspathWriter.java: .classpath
          • EclipseOSGiManifestWriter.java: (deprecated anyway)
          • EclipseProjectWriter.java: .project
          • RadApplicationXMLWriter.java: META-INF/.modulemaps
          • RadLibCopier.java: (library files aren't ordered on disk)
          • RadWebSettingsWriter.java: .websettings
          • AbstractWtpResourceWriter.java: .settings/org.eclipse.wst.common.component
          • EclipseWtpApplicationXMLWriter.java: target/eclipseEar/META-INF/application.xml, target/eclipseEar/META-INF/.modulemaps

          Can someone watching this bug checkout the source and build the plugin and run against their own projects to confirm it is working?

          Show
          Barrie Treloar added a comment - The supplied patch doesn't do what is needed. Its taken a couple of days to hack into this to find where all the bits are sorted and how it all hangs together and make the changes. Things that are now ordered correctly: AbstractEclipseManifestWriter.java: META-INF/MANIFEST.MF EclipseClasspathWriter.java: .classpath EclipseOSGiManifestWriter.java: (deprecated anyway) EclipseProjectWriter.java: .project RadApplicationXMLWriter.java: META-INF/.modulemaps RadLibCopier.java: (library files aren't ordered on disk) RadWebSettingsWriter.java: .websettings AbstractWtpResourceWriter.java: .settings/org.eclipse.wst.common.component EclipseWtpApplicationXMLWriter.java: target/eclipseEar/META-INF/application.xml, target/eclipseEar/META-INF/.modulemaps Can someone watching this bug checkout the source and build the plugin and run against their own projects to confirm it is working?
          Hide
          Luc De Graef added a comment -

          Hello,
          I am willing to test this (as it is really an annoying issue). But as a newbie, where can I check out the sources. They are not available at http://svn.apache.org/viewvc/maven/plugins/tags/

          Show
          Luc De Graef added a comment - Hello, I am willing to test this (as it is really an annoying issue). But as a newbie, where can I check out the sources. They are not available at http://svn.apache.org/viewvc/maven/plugins/tags/
          Show
          Barrie Treloar added a comment - https://svn.apache.org/repos/asf/maven/plugins/trunk/maven-eclipse-plugin/
          Hide
          Luc De Graef added a comment -

          Initially, we had many problems during unit tests towards an embedded GF (when running from within Eclipse).
          We used the 2.9-SNAPSHOT build, and all our 'classpath'-issues vanished like snow for the sun.
          Thanks for this solution, it will save us many headaches.

          Show
          Luc De Graef added a comment - Initially, we had many problems during unit tests towards an embedded GF (when running from within Eclipse). We used the 2.9-SNAPSHOT build, and all our 'classpath'-issues vanished like snow for the sun. Thanks for this solution, it will save us many headaches.
          Hide
          Jean-Noel Rouvignac added a comment -

          Yes it seems to better respect the depndency tree. I attached a file with a sample example and the result of running mvn eclipse:eclipse with m-e-p 2.8 vs. 2.9-SNAPSHOT.

          Thank you very much Barrie!

          Show
          Jean-Noel Rouvignac added a comment - Yes it seems to better respect the depndency tree. I attached a file with a sample example and the result of running mvn eclipse:eclipse with m-e-p 2.8 vs. 2.9-SNAPSHOT. Thank you very much Barrie!
          Hide
          Barrie Treloar added a comment -

          Jean-Noel, thanks for the feedback but its reads as a mixed message. I'm not sure if this is fixed or there is more work to be done.
          I suspect more work.

          When you say it better respects the dependency tree, are you saying that the output is not correct?

          Looking at the 2.9 output, I'm suspicious about why the JRE_CONTAINER is third on the classpath (i.e. splattered in amongst everything else).

          Also why is M2_REPO/javax/persistence/persistence-api/1.0/persistence-api-1.0.jar second?
          Its a transitive dependency of org.hibernate:hibernate-annotations:jar:3.2.1.ga:compile.
          The ordering doesn't look like the order I expect.

          Would you be able to re-arrange the 2.9.classpath into the order you expect?

          I've pulled out the dependency mojo which uses a shared component for building the tree.
          I vaguely recall something about that not being 100% correct but I think I'll look at switching the m-e-p implementation from a home-grown to shared components.
          At least that will mean it can get fixed once.

          Work is busy at the moment so progress may stall for a bit.

          Show
          Barrie Treloar added a comment - Jean-Noel, thanks for the feedback but its reads as a mixed message. I'm not sure if this is fixed or there is more work to be done. I suspect more work. When you say it better respects the dependency tree, are you saying that the output is not correct? Looking at the 2.9 output, I'm suspicious about why the JRE_CONTAINER is third on the classpath (i.e. splattered in amongst everything else). Also why is M2_REPO/javax/persistence/persistence-api/1.0/persistence-api-1.0.jar second? Its a transitive dependency of org.hibernate:hibernate-annotations:jar:3.2.1.ga:compile. The ordering doesn't look like the order I expect. Would you be able to re-arrange the 2.9.classpath into the order you expect? I've pulled out the dependency mojo which uses a shared component for building the tree. I vaguely recall something about that not being 100% correct but I think I'll look at switching the m-e-p implementation from a home-grown to shared components. At least that will mean it can get fixed once. Work is busy at the moment so progress may stall for a bit.
          Hide
          Jean-Noel Rouvignac added a comment - - edited

          Hi Barrie,

          Many thanks for your help on this issue.
          I had a think about what would make sense as a .classpath file to me on this example. I based myself on the dependency tree as it is the only way users can change how maven will order the jars.
          I reached the conclusion that a breadth-first search, ordered by the position in the dependencies is what I want. First of all you want to see your top level dependencies being included in the classpath in the order they are declared, then come the second level dependencies also following the order of the top level dependencies, etc.

          So if you have the folowing dependencies:

           myproject
             +- a
                +- a1
                +- a2
                +- a3
             +- b
                +- b1
                +- b2
                +- b3
             +- c
                +- c1
                +- c2
                +- c3
          

          Then I would expect to see the following classpath being generated:

          myproject classes;a;b;c;a1;a2;a3;b1;b2;b3;c1;c2;c3

          I attached the corresponding .classpath for the little test I previously attached.

          Thanks,
          Jean-NoŽl

          Show
          Jean-Noel Rouvignac added a comment - - edited Hi Barrie, Many thanks for your help on this issue. I had a think about what would make sense as a .classpath file to me on this example. I based myself on the dependency tree as it is the only way users can change how maven will order the jars. I reached the conclusion that a breadth-first search, ordered by the position in the dependencies is what I want. First of all you want to see your top level dependencies being included in the classpath in the order they are declared, then come the second level dependencies also following the order of the top level dependencies, etc. So if you have the folowing dependencies: myproject +- a +- a1 +- a2 +- a3 +- b +- b1 +- b2 +- b3 +- c +- c1 +- c2 +- c3 Then I would expect to see the following classpath being generated: myproject classes;a;b;c;a1;a2;a3;b1;b2;b3;c1;c2;c3 I attached the corresponding .classpath for the little test I previously attached. Thanks, Jean-NoŽl
          Hide
          Robert Scholte added a comment -

          Jean-NoŽl,

          this is not the order Maven will use to compile. So to stay as close as possible to Maven it should be:
          a;a1;a2;a3;b;b1;b2;b3;c;c1;c2;c3;

          Show
          Robert Scholte added a comment - Jean-NoŽl, this is not the order Maven will use to compile. So to stay as close as possible to Maven it should be: a;a1;a2;a3;b;b1;b2;b3;c;c1;c2;c3;
          Hide
          Jean-Noel Rouvignac added a comment -

          Hi Robert, this order looks good enough to me. As long as Maven and the maven-eclipse-plugin do not yield different results when compiling without good reason (like the fact the project and the tests of this project are two distinct projects in Maven but only one in Eclipse).

          Show
          Jean-Noel Rouvignac added a comment - Hi Robert, this order looks good enough to me. As long as Maven and the maven-eclipse-plugin do not yield different results when compiling without good reason (like the fact the project and the tests of this project are two distinct projects in Maven but only one in Eclipse).
          Hide
          Barrie Treloar added a comment -

          From feedback for this "iteration" of the solution is more palatable to users.

          The next step is to look at aether and how its doing the dependency graph so that this can be shared appropriately.

          Show
          Barrie Treloar added a comment - From feedback for this "iteration" of the solution is more palatable to users. The next step is to look at aether and how its doing the dependency graph so that this can be shared appropriately.

            People

            • Assignee:
              Barrie Treloar
              Reporter:
              Siarhei Dudzin
            • Votes:
              11 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: