search

eclipse-m2e / m2e-core

Eclipse Public License 2.0
110 stars 113 forks source link

m2e 2.4.x bug: m2e incorrectly uses older transitive dependency if intermediate project is imported into workspace #1501

Closed garretwilson closed 1 year ago

garretwilson commented 1 year ago

I'm using Eclipse EE 2023-06 with Maven 3.9.1 on Windows 10 with OpenJDK 17. I had to upgrade m2e to v2.4.0.20230802-0719 because of #1414. I don't know if that snapshot version of m2e is causing the problem, but this was working fine before, so it wouldn't surprise me if the new m2e has a bug. I've spend a frustrating, tiring time of trying to coax Eclipse to act right, to no avail.

The summary (I'll explain how to reproduce it below) is that in the https://github.com/globalmentor/globalmentor-base project, which is at com.globalmentor:globalmentor-core:0.7.4-SNAPSHOT (a subproject), I recently (see commit history) added a com.globalmentor.collections.iterables.Iterables.toStream() method, but another project in Eclipse which is using com.globalmentor:globalmentor-core:0.7.4-SNAPSHOT is not seeing that method. Instead it seems to be using v0.7.2, not v0.7.4-SNAPSHOT. That other project is https://github.com/globalmentor/guise-mummy .

Reproducing this isn't simple, because multiple projects are in play. Let me try to give you a scenario; if you can't reproduce it with this, I can try to put together something simpler, but that will take a lot of work and at the moment I'm already exhausted from trying to straighten this out.

Start by cloning and importing all these projects into Eclipse:

All these are at SNAPSHOT versions. You'll notice that globalmentor-base has a subproject globalmentor-core. There you should easily find the latest com.globalmentor.collections.iterables.Iterables class, and note that it has a toStream(@Nonnull final Iterable<T> iterable) method. Note also that the version is com.globalmentor:globalmentor-base:0.7.4-SNAPSHOT.

Then in the guise-mummy project you'll see that guise-mummy is actually a subproject, which has the io.guise.mummy.deploy.aws.S3 class. On line 166 you'll see that it uses the new toStream() method above:

final String userAgentJsonArrayValues = toStream(userAgents).map(userAgent -> {

However Eclipse shows an error message for this line:

The method toStream(Iterable<String>) is undefined for the type S3

And if you click on the import static com.globalmentor.collections.iterables.Iterables.* in the S3 class, and then use "View in Project Explorer", you'll see that it's using the Iterables class from com.globalmentor:globalmentor-core:0.7.2!

Open the root guise-mummy-bom project from the guise-mummy repository. In pom.xml you'll see that in <dependencyManagement> it pulls in v0.7.4-SNAPSHOT:

<dependency>
  <groupId>com.globalmentor</groupId>
  <artifactId>globalmentor-core</artifactId>
  <version>0.7.4-SNAPSHOT</version>
</dependency>

But you don't have to take my word for it. Just run mvn dependency:tree. You'll see that the only globalmentor-core that appears in the output is com.globalmentor:globalmentor-core:jar:0.7.4-SNAPSHOT. So Eclipse and m2e are wrong to be linking to v0.7.2.

Finally go into each of the projects on the command line and do a mvn clean install (in the order listed above). On my machine they all compile with no errors. But in Eclipse guise-mummy is pulling in the wrong version of globalmentor-core.

garretwilson commented 1 year ago

This is draining; I don't know what to do I'm trying to downgrade to Eclipse EE 2023-06 with the old m2e to see if that fixes the problem. I even upped globalmentor-root to Java 11 to try to get around #1414. Yet even setting the version to Java 11 I get the error from #1414:

eclipse.buildId=4.28.0.20230608-1200
java.version=17.0.7
java.vendor=Eclipse Adoptium
BootLoader constants: OS=win32, ARCH=x86_64, WS=win32, NL=en_US
Framework arguments:  -product org.eclipse.epp.package.jee.product
Command-line arguments:  -os win32 -ws win32 -arch x86_64 -product org.eclipse.epp.package.jee.product

org.eclipse.core.jobs
Error
Sat Aug 05 21:24:33 BRT 2023
An internal error occurred during: "Importing Maven projects".

java.lang.IllegalArgumentException: Version 8 of project facet java does not exist.
garretwilson commented 1 year ago

I finally got the downgrade to Eclipse EE 2023-06 to work the old m2e. Here again m2e seemed to be stuck with the #1414 error, even after changing the versions. I finally had to remove the project and delete all Eclipse project-related directories and files (e.g. .settings and such) and the re-import the project.

Since things were working again, I switched back to m2e 2.4.0.20230802-0719. Things still work.

But I found out what confuses it. Look at the changes on this branch: https://github.com/globalmentor/globalmentor-base/tree/issues/JAVA-313

I changed the structure of the POMs to add an intermediate POM, and this puts m2e 2.4.0.20230802-0719 into a huge confusion when I remove the project and re-import it with the new structure.

I'll need to see whether I can get around it by deleting the settings files again, and whether this happens in m2e 2.3.x. But I'll have to do that tomorrow.

garretwilson commented 1 year ago

OK, here's what I know. I start out with an aggregate POM like this:

The modules in pom.xml are listed like this:

<modules>
  <module>core</module>
  <module>management</module>
</modules>

Then I add an intermediate parent-pom.xml, like this, in the root directory of the project:

"Foo" and "Bar" use parent-pom.xml as the parent, which in turn uses pom.xml as the parent. However the main pom.xml aggregates them all, like this:

<modules>
  <module>parent-pom.xml</module>
  <module>core</module>
  <module>management</module>
</modules>

An explanation of the reasoning for this, along with complete examples, are in by blog post Improving the Maven Bill of Materials (BOM) Pattern.

The problem is that if I remove the project, make the change, and then import the project back to Eclipse without removing the original Eclipse project files, Eclipse gets very confused about the version. As explained above, projects that reference this project inside Eclipse wind up referencing a previous version (perhaps brought in transitively by some other dependency), even though that does not reflect that mvn dependency:tree shows.

The only way I've found to move forward after changing the POM hierarchy is to 1) remove the project, 2) delete all the Eclipse project files, and then 3) import the project back into Eclipse. It would appear that something set up in the project files from the first hierarchy aren't being updated when re-importing the project back into Eclipse.

I performed these steps in the plain Eclipse EE 2023-06 with m2e 2.3.x, and then switch to the same Eclipse that had v2.4.x installed, and it still worked, so I can't say whether this m2e confusion is related to any v2.4.x changes or not.

garretwilson commented 1 year ago

Oh, no—this problem surfaced again, and this time it's not an issue of corrupted Eclipse projects.

I just published the globalmentor-root and globalmentor-base libraries listed above, so they are at non-SNAPSHOT versions on Maven central. The globalmentor-base project uses the hierarchy mentioned above, with the parent POM in the same directory as the project POM. This seems to confuse m2e, and an project using it sees a different version of globalmentor-base and therefore can't find certain methods.

Here are steps to reproduce:

  1. Clone https://github.com/globalmentor/globalmentor-web.git and import it into Eclipse as Maven project. (This library is still at a -SNAPSHOT version; once the final version is published to Maven Central and referenced from the next project, this step will not be necessary.)
  2. Clone https://github.com/globalmentor/guise-mummy.git and import it into Eclipse as Maven project.

You will see the Eclipse EE 2023-06 with m2e 2.4.100.20230802-0719 shows an error in io.guise.mummy.deploy.aws.S3 line 166:

The method toStream(Iterable<String>) is undefined for the type S3

However com.globalmentor:globalmentor-base:0.7.4, which was just released to Maven Central and is not even imported into Maven, actually has that method; it was added in v0.7.4.

You'll also see that building from the command line using OpenJDK 17 mvn clean verify works fine! You'll also see that mvn:dependency-tree shows that com.globalmentor:globalmentor-base:0.7.4 is being used.

Please investigate this problem. I'll be happy to answer any more questions. Right now this is breaking the project, and it looks like to get around this I'll have to publish another version of com.globalmentor:globalmentor-base to Maven Central with a different hierarchy so that m2e won't get confused.

Note that SLF4J is getting ready to release a BOM using this type of POM hierarchy, as I discussed with the SLF4J author already, so we may need to make last-minute changes in the next SLF4J release if this is going to cause problems with m2e. See SLF4J-437.

laeubi commented 1 year ago

The only way I've found to move forward after changing the POM hierarchy is to 1) remove the project, 2) delete all the Eclipse project files, and then 3) import the project back into Eclipse. It would appear that something set up in the project files from the first hierarchy aren't being updated when re-importing the project back into Eclipse.

Have you tried Project > Update Maven Project and just updated all projects in the workspace?

Beside that it seems this is not really something the usual committer/contributors encounter and thus handled with very low priority, so probably you like to provide a PR to fix that together with a test-case to verify it works?

Beside that, sponsoring is a good way to get specific issues fixed, so if this is crucial to your business and you likes to speed up the development of m2e in general a sponsoring would allow me to assign more time-slots to bug fixing or contact me for an individual contract for example fixing a specific issue.

garretwilson commented 1 year ago

Have you tried Project > Update Maven Project and just updated all projects in the workspace?

Yes, so many times I've lost count.

I should have emphasized more in that last comment that I have removed globalmentor-base altogether from Eclipse, so any theoretical corrupted project shouldn't be a problem—Eclipse should be pulling globalmentor-base directly from Maven Central (my local repository cache).

But I have more information that makes this more mysterious: my steps to reproduce the problem do not work! If I clone the same projects and import them into a new Eclipse installation, the problem does not appear!

And even more mysterious: the m2e Maven Dependency Hierarchy for guise-mummy clearly shows that globalmentor-core:0.7.4 is being used, which has the Iterables.toStream() method in question. However it also shows that globalmentor:awt-awt:0.6.1 is being used, which uses an older globalmentor-core:0.7.2 without Iterables.toStream(). Fine, the version declared in guise-mummy should override that. But I still get the error, and when I click on Iterables and say "Show in Project Explorer", it opens up the globalmentor-core:0.7.2 for globalmentor-awt!!

And even stranger, removing the globalmentor-awt project from Eclipse makes the problem go away!! There should be no difference, because globalmentor-awt is a project that has long been published to Maven Central. Somehow having globalmentor-awt present as a project in Eclipse makes Eclipse think that guise-mummy should use globalmentor-core:0.7.2 from globalmentor-awt rather than globalmentor-core:0.7.4. (Yes, I've removed `globalmentor-awt, cleaned all Eclipse-related files, and added it back; the problem still occurs.)

But still I can't reproduce this in a brand new Eclipse installation.

I realize you can't fix a strange problem if you can't reproduce it, so I'll keep trying to reproduce it. If by reading these descriptions you have any ideas of what might be causing this, please let me know.

garretwilson commented 1 year ago

Here's something perhaps significant, and perhaps related. When I clone https://github.com/globalmentor/guise-mummy.git and import it into a brand new Eclipse project, in the Project Explorer m2e shows the mummy subfolder as just a normal folder instead of a Maven module—that is, the mummy folder has no "M" icon overlay, it has no "Maven Dependencies" subfolder, etc.

m2e-core-1501-mummy-folder

However in the old Eclipse installation (the one giving the error), this project shows the mummy subfolder as a subproject guise-mummy (in mummy) [guise-mummy main] as it should! And note that this is the submodule in which the problem occurs (although it is occurring in the Eclipse that has the correct import).

m2e-core-1501-mummy-module

(Also note that the old installation correctly shows guise-mummy-bom, which is the artifactId; while the new installation shows guise-mummy, which is the project folder.)

Could someone clone https://github.com/globalmentor/guise-mummy.git and import it into Eclipse, and just tell me if the mummy subfolder shows up as a normal folder or as a subproject, as it should? It shouldn't take more than three minutes of your time.

garretwilson commented 1 year ago

… the Project Explorer m2e shows the mummy subfolder as just a normal folder instead of a Maven module …

I think I found the reason for this, and it appears to be a bug in itself. The guise-mummy/mummy/pom.xml artifact ID is set to guise-mummy. In the new Eclipse installation, Eclipse is already using guise-mummy for the top-level project, so apparently it thinks it has conflicting project names, and thus doesn't import guise-mummy/mummy/pom.xml correctly. If I rename the guise-mummy/mummy/pom.xml artifact ID to guise-mummy-artifactId, the mummy subdirectory is imported correctly.

I'll have filed this separately as #1504.

And at least I found out why this difference between my old and new test installation: in the latter the imported projects were inside the workspace directory. See #1504 for more details.

garretwilson commented 1 year ago

Bingo! I have reproduced this problem!! 🎉It may in fact be related to #1504.

You must clone these repositories to some directory outside of your workspace directory.

  1. Install Eclipse EE 2023-06. (I'm using Windows 10 with OpenJDK 17.)
  2. Update to m2e 2.4.100.20230802-0719 using the https://download.eclipse.org/technology/m2e/snapshots/2.4.0/ p2 repository. This is an m2e 2.4.x bug.
  3. git clone https://github.com/globalmentor/guise-mummy.git
  4. Import the guise-mummy project into Eclipse. (Verify that the top level project name is guise-mummy-bom; see #1504.)
  5. git clone https://github.com/globalmentor/globalmentor-web.git
  6. Import the globalmentor-web project into Eclipse. (Verify that the top level project name is globalmentor-web-bom; see #1504.)
  7. git clone https://github.com/globalmentor/globalmentor-awt.git
  8. Import the globalmentor-awt project into Eclipse.

As soon as you import globalmentor-awt into Eclipse, guise-mummy breaks—specifically the guise-mummy submodule. Apparently Eclipse+m2e stops using globalmentor-core:0.7.4 specified in guise-mummy, but instead starts using globalmentor-core:0.7.2 from globalmentor-awt—even though globalmentor-awt has been published to Maven Central! Something about its mere presence in globalmentor-awt imported into Eclipse confuses m2e.

If you delete the globalmentor-awt project, the problem goes away and m2e starts using the correct globalmentor-core:0.7.4 dependency again.

garretwilson commented 1 year ago

I have verified that this bug was introduced with m2e 2.4.x. A fresh installation of Eclipse EE 2023-06 with m23e 2.3.0.20230523-2033 does not exhibit this bug. If you add the snapshot repository https://download.eclipse.org/technology/m2e/snapshots/2.4.0/ and update to v2.4.100.20230802-0719, restart Eclipse and then refresh the Maven project; you'll the the error.

Apparently m2e 2.4.x introduced some sort of dependency lookup bug for projects that are imported into the workspace.

HannesWell commented 1 year ago

Bingo! I have reproduced this problem!! 🎉It may in fact be related to #1504.

Nice work. It would be great if you would setup up a dev-Eclipse for m2e, launch the provided launch config for the m2e-product and reproduce the problem there. You could then probably also try to investigate the reason for this in the sources and then a fix is usually relatively close. :) If EGit is necessary in the launched IDE, you probably have to add the EGit feature to the provided launch config.

garretwilson commented 1 year ago

It would be great if you would setup up a dev-Eclipse for m2e …

I'm sorry, but I must get back to my main project. I've lost several days with this and #1414 and #1504 and filing JGit bugs and filing Eclipse compiler bugs and filing XML editor bugs …

It makes me happy that I can put in sufficient work so that your team can reproduce this, but sincerely at some point I have to do my own work. If you have any questions or doubts let me know and I'll be happy to provide additional information.

garretwilson commented 1 year ago

At this point I suspect (hope 🤞) this problem never had anything to do with a corrupt project or the POM structure, but it is simply a new bug introduced in m2e 2.4.x. As we've seen, the problem only surfaces after 1) updating to v2.4.x, and 2) updating a Maven project with m2e.

lrozenblyum commented 1 year ago

Maybe similar/related to this bug https://github.com/eclipse-m2e/m2e-core/issues/1474

mickaelistria commented 1 year ago

@garretwilson Do you think you can create an automated test case with minimal projects derived from your earlier comment? Having a test case that verifies this story will certainly help in fixing it and preventing further regressions.

garretwilson commented 1 year ago

@garretwilson Do you think you can create an automated test case with minimal projects derived from your earlier comment?

As I don't know what is causing the problem, I don't even know what "minimal projects" means. Maybe we could reproduce this with a single project. Maybe what I've given you is already minimal. It's hard to guess without knowing what is causing this.

Seeing that I've already spent days to come up with at least some set of projects that should reproduce this, I would appreciate it if someone could please at least take five minutes to follow the test case I provided and confirm that they have successfully reproduced the issue.

garretwilson commented 1 year ago

For example, I'm wanting to release a new version of globalmentor-web to Maven Central for my own projects, which would also simplify this test case, but I'm afraid to touch anything because I want to make sure we're on the same page and that somebody else can reproduce this first.

It took a long time to find something reproducible, and I'm currently putting some of my own projects on hold waiting for someone to tell me the can reproduce this before I make any changes.

HannesWell commented 1 year ago

Seeing that I've already spent days to come up with at least some set of projects that should reproduce this, I would appreciate it if someone could please at least take five minutes to follow the test case I provided and confirm that they have successfully reproduced the issue.

I'll try to reproduce it tomorrow.

HannesWell commented 1 year ago

Bingo! I have reproduced this problem!! 🎉It may in fact be related to #1504.

I have tried to reproduce your problem and cloned all three git repos you mentioned into a folder outside of my workspace and just imported all projects from it. The project names are not exactly as mentioned them, but if there is an error with the workspace resolution the project name should not matter since it has actually nothing to do with the GAV.

I did not notice exactly the errors you got, but for me the Maven Dependencies Classpath container failed to resolve the globalmentor-css-def project in the workspace for the globalmentor-css project, even after some rounds of refresh, Right-click -> Maven -> Update Project ..., clean-all + full build, restart, ...

grafik

I'm not sure if this exactly your problem, but maybe m2e simply does not continue to build after the first of such errors and in my case the order is different due to the different project names. If you think this isn't related, please be more spectic how you imported the projects from the repos outside of your workspace? I.e. did you use Import-> Git -> Projects from Git (with Smart Import) -> ... or Import-> Maven -> Existing Maven Projects -> ...?

If there is really a problem with the workspace resolution I assume it is due to your relatively complex parent+bom structure in the same workspace. But that should then be reproducible with a stripped down version of your projects.

HannesWell commented 1 year ago

The error remains if I close all projects except for globalmentor-css, globalmentor-css-def and globalmentor-web-parent. But even globalmentor-web-parent can be closed.

garretwilson commented 1 year ago

I have tried to reproduce your problem …

First of all @HannesWell , thank you so much for taking the time to try to reproduce this. However I'm a little confused about some of the errors you got. I'll try to see what I can do stripping down this set of projects, but in the meantime let me respond to a couple of your questions.

… please be more spectic how you imported the projects from the repos outside of your workspace? I.e. did you use Import-> Git -> Projects from Git (with Smart Import) -> ... or Import-> Maven -> Existing Maven Projects -> ...?

After cloning the projects separately, I use File > Import > Maven > Existing Maven Projects.

I also see that one of your errors is that it failed to read an artifact descriptor for com.globalmentor:globalmentor-core:0.7.4. But how can that be, seeing that it was released to Maven Central? Why isn't your m2e downloading that artifact as needed?

Plus I see a long list of errors from missing libraries, some of which aren't even mine, and some of which have been available on Maven Central for years!

No, those are not the errors I'm getting. I have no idea why you are getting those errors. That is basic m2e load-from-Maven functionality.

The project names are not exactly as mentioned them …

I see you're looking at the "Package Explorer", not the newer "Project Explorer", so that might be why that is happening.

It's one thing if you can't reproduce the problem, but I'm seriously at a loss to explain what you're seeing. Surely we might have missed a step somewhere.

(I just download the Java EE 2023-06 ZIP file and expand it in a directory and run it from that. I'm not using the OOMPH or whatever. It shouldn't make a difference, but I thought I'd mention it.)

… just imported all projects from it …

If you mean that you just imported the parent directory of the three projects, then no, I can't guarantee that will reproduce the problem. You must import each of the projects separately, as per the instructions. Did you import each one of them separately using File > Import > Maven > Existing Maven Projects? (When you import each of the projects mentioned, e.g. guise-mummy, of course in each import it will show the subprojects under it, and you must leave them checked during the import.)

garretwilson commented 1 year ago

I have created a tiny project that easily reproduces this bug. I am attaching the ZIP file with the project to this ticket as m2e-core-bug-1501-test-project.zip. Here are the reproduction instructions, which are also present in the readme file in the project.

  1. Install Eclipse EE 2023-06. (I'm using Windows 10 with OpenJDK 17. I recommended downloading eclipse-jee-2023-06-R-win32-x86_64.zip and running Eclipse directly after unzipping.)
  2. Update to m2e 2.4.100.20230802-0719 using the https://download.eclipse.org/technology/m2e/snapshots/2.4.0/ p2 repository. This is an m2e 2.4.x bug.
  3. Unzip m2e-core-bug-1501-test-project.zip and import it using File > Import > Maven > Existing Maven Projects. At this point you should be able to compile and run com.example.m2e.Bug1501 with no errors. If you have errors at this point, you are doing something wrong.
  4. From the command line use git clone https://github.com/globalmentor/globalmentor-awt.git to clone the globalmentor-awt project to some directory outside of your workspace directory. (As a precaution, you can change to the globalmentor-awt diretory and use git describe to ensure that HEAD is at tag v0.6.1. In the future, if this repository changes, you'll need perform the extra step of checking out tag v0.6.1. But for now, as of 2023-08-20, checking out tag v0.6.1 is not necessary, as the repository HEAD is at v0.6.1 already.)
  5. Import the globalmentor-awt project into Eclipse using File > Import > Maven > Existing Maven Projects.

At this point m2e will become confused, and use the wrong version of globalmentor-core in the test project project: it will use com.globalmentor:globalmentor-core-0.7.2, brought in transitively through globalmentor-awt, rather than the correct com.globalmentor:globalmentor-core-0.7.4 specified in the test project. Because of this, the test project will no longer compile, and in the Bug1501 class Eclipse will say it cannot find Iterables.toStream(…), because that method was not present in com.globalmentor:globalmentor-core-0.7.2.

  1. (optional) Close the globalmentor-awt project. You'll see that the test project builds again with no errors, and you can run com.example.m2e.Bug1501 just fine again.

If you're still skeptical, go to the command line and issue mvn dependency:tree. You'll see that Maven gets the correct version of com.globalmentor:globalmentor-core-0.7.4:

[INFO] com.example:m2e-core-1501:jar:1.0.0-SNAPSHOT
[INFO] +- com.globalmentor:globalmentor-awt:jar:0.6.1:compile
[INFO] +- com.globalmentor:globalmentor-core:jar:0.7.4:compile
[INFO] +- com.google.code.findbugs:jsr305:jar:3.0.2:compile
[INFO] +- org.junit.jupiter:junit-jupiter-engine:jar:5.9.1:test
[INFO] |  +- org.junit.platform:junit-platform-engine:jar:1.9.1:test
[INFO] |  |  +- org.opentest4j:opentest4j:jar:1.2.0:test
[INFO] |  |  \- org.junit.platform:junit-platform-commons:jar:1.9.1:test
[INFO] |  +- org.junit.jupiter:junit-jupiter-api:jar:5.9.1:test
[INFO] |  \- org.apiguardian:apiguardian-api:jar:1.1.2:test
[INFO] +- org.hamcrest:hamcrest:jar:2.2:test
[INFO] +- com.github.npathai:hamcrest-optional:jar:2.0.0:test
[INFO] |  \- org.hamcrest:hamcrest-core:jar:1.3:test
[INFO] \- org.slf4j:slf4j-simple:jar:2.0.4:test
[INFO]    \- org.slf4j:slf4j-api:jar:2.0.4:test

You can even compile the project using Java:

mvn clean verify

The part that confuses m2e is in the POM:

<dependency>
  <groupId>com.globalmentor</groupId>
  <artifactId>globalmentor-awt</artifactId>
  <version>0.6.1</version>
</dependency>

<dependency>
  <groupId>com.globalmentor</groupId>
  <artifactId>globalmentor-core</artifactId>
  <version>0.7.4</version>
</dependency>

You'll see that globalmentor-core should be set at v0.7.4. The fact that globalmentor-awt references a previous version shouldn't matter—the version of globalmentor-core in the POM should have precedence. And in fact m2e does use the v0.7.4 version of globalmentor-core as the POM requests—until you actually load globalmentor-awt as a project into Eclipse. Loading globalmentor-awt into Eclipse should have zero bearing on the version of globalmentor-core, which is set in the project POM.

(Note that if you switch from m2e 2.3.0.20230523-2033 to m2e 2.4.100.20230802-0719 or vice versa, you'll afterward need to tell m2e to refresh the Maven projects using Alt+F5 to see this bug appear and disappear based upon which m2e version you're using. The bug therefore has something to do with modified 2.4 code that recalculates dependency graphs when refreshing the Maven project.)

This shouldn't be hard to track down. It was introduced in 2.4.x. Just go look at the new code that was added relating to the dependency graph regarding loaded projects.

HannesWell commented 1 year ago

3. Unzip m2e-core-bug-1501-test-project.zip and import it using File > Import > Maven > Existing Maven Projects. At this point you should be able to compile and run com.example.m2e.Bug1501 with no errors. If you have errors at this point, you are doing something wrong.

I followed you steps exactly and I indeed already have errors after only importing this simple reproducer project and I have similar errors like before. I first assumed these were follow-up errors on the failure in the Maven Depenency container, but copying the their description revealed the following (Note to our-self, these stack-traces could be made more visible, easier accessible):

Failed to read artifact descriptor for com.globalmentor:globalmentor-core:jar:0.7.4

org.eclipse.aether.resolution.ArtifactDescriptorException: Failed to read artifact descriptor for com.globalmentor:globalmentor-core:jar:0.7.4
    at org.apache.maven.repository.internal.DefaultArtifactDescriptorReader.loadPom(DefaultArtifactDescriptorReader.java:286)
[...]
Caused by: org.eclipse.aether.resolution.ArtifactResolutionException: The following artifacts could not be resolved: com.globalmentor:globalmentor-base:pom:0.7.4 (absent): Could not find artifact com.globalmentor:globalmentor-base:pom:0.7.4 in central (https://repo.maven.apache.org/maven2)
    at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolve(DefaultArtifactResolver.java:456)
    at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolveArtifacts(DefaultArtifactResolver.java:261)
    at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolveArtifact(DefaultArtifactResolver.java:242)
    at org.apache.maven.repository.internal.DefaultModelResolver.resolveModel(DefaultModelResolver.java:158)
    ... 47 more
Caused by: org.eclipse.aether.transfer.ArtifactNotFoundException: Could not find artifact com.globalmentor:globalmentor-base:pom:0.7.4 in central (https://repo.maven.apache.org/maven2)
    at io.takari.aether.connector.AetherRepositoryConnector$2.wrap(AetherRepositoryConnector.java:887)
    at io.takari.aether.connector.AetherRepositoryConnector$2.wrap(AetherRepositoryConnector.java:1)
    at io.takari.aether.connector.AetherRepositoryConnector$GetTask.flush(AetherRepositoryConnector.java:659)
    at io.takari.aether.connector.AetherRepositoryConnector.get(AetherRepositoryConnector.java:337)
    at org.eclipse.aether.internal.impl.DefaultArtifactResolver.performDownloads(DefaultArtifactResolver.java:516)
    at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolve(DefaultArtifactResolver.java:433)
    ... 50 more

So the root cause here is that globalmentor-base:0.7.4 cannot be resolved.

However com.globalmentor:globalmentor-base:0.7.4, which was just released to Maven Central and is not even imported into Maven, actually has that method; it was added in v0.7.4.

Are you sure the release fully succeeded? Looking at Maven-Central I cannot find it there, I can only see version 0.7.3: https://repo1.maven.org/maven2/com/globalmentor/globalmentor-base/

grafik

MvnRepository also shows 0.7.3 as latest version: https://mvnrepository.com/artifact/com.globalmentor/globalmentor-base

So I wonder have you installed globalmentor-base:0.7.4 in your local repository or do you have another repository specified in your users .m2/settings.xml?

garretwilson commented 1 year ago

So the root cause here is that globalmentor-base:0.7.4 cannot be resolved. … Are you sure the release fully succeeded?

Hi, @HannesWell . Yes I just realized that this morning. It appears that there was some problem with Maven Central and that POM did not get published. I have filed ticket MVNCENTRAL-8292 to find out what is going on. (Geez, what a never-ending saga.) At least this explains why you were not able to not able to reproduce the previous instructions.

I believe this is a separate issue unrelated to the bug described in this ticket. I will completely understand if you want to wait until we find more answers in MVNCENTRAL-8292 and resolve that issue. I'll certainly keep this ticket updated so we can progress.

In the meantime, though, if you want to continue reproducing the issue despite the missing globalmentor-base, I believe you can do the following (none of which need involve Eclipse, I think):

  1. Clone https://github.com/globalmentor/globalmentor-base.git.
  2. Make sure (e.g. git describe) that you are at tag v0.7.4.
  3. Do a mvn install.

I don't want to confuse this ticket. If you want to do the workaround, awesome; but if you want to wait until MVNCENTRAL-8292 is resolved, that's reasonable, too.

HannesWell commented 1 year ago

I'll try to find some time tomorrow, for now its too late here.

HannesWell commented 1 year ago

In the meantime, though, if you want to continue reproducing the issue despite the missing globalmentor-base, I believe you can do the following (none of which need involve Eclipse, I think):

1. Clone `https://github.com/globalmentor/globalmentor-base.git`.

2. Make sure (e.g. `git describe`) that you are at tag `v0.7.4`.

3. Do a `mvn install`.

After I have done this, I can now reproduce the issue exactly as you have described in https://github.com/eclipse-m2e/m2e-core/issues/1501#issuecomment-1685302872 and can confirm everything you mentioned.

This even happens in a simple Eclipse SDK plus m2e respectivly in an IDE launched from a m2e-dev workspace (makes it simpler to debug 👍🏽).

But I don't think that m2e really get confused about the version of globalmentor-core, because looking at the Maven Dependencies container, gm-core 0.7.4 is still listed: grafik

Therefore I assume that this is more an issue JDT Classpath configuration, maybe the order of entries or what is exported by referenced projects.

HannesWell commented 1 year ago

Bingo, this is indeed a regression from https://github.com/eclipse-m2e/m2e-core/pull/1424. If I comment out the line https://github.com/eclipse-m2e/m2e-core/blob/837d904d703bf73dc207af2111fc7dd60ef06a3e/org.eclipse.m2e.jdt/src/org/eclipse/m2e/jdt/internal/DefaultClasspathManagerDelegate.java#L151

and call Maven -> Update Project ... the error vanishes.

@laeubi I think #1424 needs adjustments or we need to revert this. It should be relatively simple to create a test-case for this. I assume the following is enough: Two projects A and B, where A depends on an older version of a dependency on that B depends and B depends on A and uses something only available in the newer version of the library.

HannesWell commented 1 year ago

@garretwilson you can work-around this problem by moving the dependencies in the workspace to the end of the list. In your example project m2e-core-1501, just change the dependencies section to the following and it compiles:

    <dependencies>
        <dependency>
            <groupId>com.globalmentor</groupId>
            <artifactId>globalmentor-core</artifactId>
        </dependency>
        <dependency>
            <groupId>com.globalmentor</groupId>
            <artifactId>globalmentor-awt</artifactId>
        </dependency>
    </dependencies>

This is of course not a solution, but should unblock you for now.

HannesWell commented 1 year ago

Just created https://github.com/eclipse-m2e/m2e-core/pull/1517 with a reproducer. The important parts are that the project in the workspace is referenced first and that the class, that is referenced already exists in the older version. Otherwise JDT will just continue to search the class-path and will only find the class in the latest version.

laeubi commented 1 year ago

@HannesWell I think this is the probably a bug of JDT because it searches deep-first while bread-first is what we want and one can expect here.

HannesWell commented 1 year ago

@HannesWell I think this is the probably a bug of JDT because it searches deep-first while bread-first is what we want and one can expect here.

I assume think this is a bug but more by design. As far as I understood it and simplified, the container creates a flat list of jars/directories as classpath and in this case it starts with adding project-a and its transitive) dependencies and then continues with the other dependencies of project-b. And then the Compiler searches that flat class-path as usual, like the ClassLoader does. So yes effectively this is a depth-first search, but I don't expect JDT or even Java will change that. That's probably the reason why the Maven Dependency container resolves all (transitive) dependencies, breath-first in Maven style and then adds the corresponding jars/projects instead of relying on exports of dependencies.

laeubi commented 1 year ago

JDT must take precedence of directly added dependencies, this has nothing to do with the Classlaoder (that is created as part of this later on).

So it must first add al level-one (directly declared), then decent into that transitive ones and so on... otherwise such situation can always happen.

One can even argue that if we say it is not an error, the order must be defined in the pom.xml otherwise as you have shown it to work already.

HannesWell commented 1 year ago

JDT must take precedence of directly added dependencies, this has nothing to do with the Classlaoder (that is created as part of this later on).

Is that written anywhere? I think on the very basic level the JDT Classpath is not that smart and its the task of container creators like M2E to properly order the entries.

But even if it turns out to be a bug in JDT I'm relatively sure that it won't be fixed for 2023-09. Therefore I suggest we revert #1424 and accept this loss in extra convenience for remote-debuggers and then either fix JDT or think of another solution for that. Because obviously this is causing regressions.

HannesWell commented 1 year ago

It turns out that the IClasspathEntryDescriptor that are configured here are not from JDT, but m2e itself and AbstractJavaProjectConfigurator.addMavenClasspathContainer() is reading it. So maybe we have to do the sorting in m2e.

laeubi commented 1 year ago

Is that written anywhere? I think on the very basic level the JDT Classpath is not that smart and its the task of container creators like M2E to properly order the entries.

Its either a bug (then it should be changed) or a feature (the the container creator can't do much), the container itself correctly declares the order reflected from the pom and m2e should expect it to handle this appropriately. I'm not sure if it is even specified in full detail how maven handle the case, but changing the order by m2e seems not very useful as it might be undesired then as well.

For me this more looks like a limitation of the Workspace resolution, because just consider project A is in the workspace and uses an older dependency, project B references A and includes a newer one (that has removed a method), then this will fail at runtime (e.g. unit test executed) but will not show any compile error on the project itself.

So even though the project B shows a problem now it previously has just hidden the problem of project A needing an update of its dependency to work in this scenario. I actually prefer to see the problem in the IDE than somewhere in runtime ...

HannesWell commented 1 year ago

Is that written anywhere? I think on the very basic level the JDT Classpath is not that smart and its the task of container creators like M2E to properly order the entries.

Its either a bug (then it should be changed) or a feature (the the container creator can't do much), the container itself correctly declares the order reflected from the pom and m2e should expect it to handle this appropriately. I'm not sure if it is even specified in full detail how maven handle the case, but changing the order by m2e seems not very useful as it might be undesired then as well.

The dependency resolution mechanism is well documented for Maven: https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html It even mentions a similar case and simply says that it is breath-first and thus in the case the project should be compiled to the version defined in its pom.

For me this more looks like a limitation of the Workspace resolution, because just consider project A is in the workspace and uses an older dependency, project B references A and includes a newer one (that has removed a method), then this will fail at runtime (e.g. unit test executed) but will not show any compile error on the project itself.

So even though the project B shows a problem now it previously has just hidden the problem of project A needing an update of its dependency to work in this scenario. I actually prefer to see the problem in the IDE than somewhere in runtime ...

While this it is definitely a desirable goal to get to now about such runtime conflicts in the IDE and your example is valid, the classpath-container is the wrong place to solve it and not even a standalone Maven knows it and AFAIK Maven is not designed to solve that problem. It would not even be a reliable solution, because it depends on the order of dependencies respectively if dependencies are openend in the workspace. And at the same time false positives, like in this case are likely, and a tool that gives false positives regularly is not very helpful.

If one really want's to prevent such runtime errors one needs to fully obey Semantic Versioning in all own artifacts and dependencies and needs a build-tools that checks compatibility and SemVer correctness, but we both know from OSGi that even then things are not that simple to solve in the IDE. :)

In my opinion the Maven Dependency classpath container should behave just as the maven resolver and should resolve breath-first regardless of if a dependency is in the workspace. Otherwise users just get confused and more issues like this will be opened. In that sense the previous behavior was correct.

Can't you achieve your goal of #1424 with another solution that does not affect the classpath so that we can revert #1424?

laeubi commented 1 year ago

In my opinion the Maven Dependency classpath container should behave just as the maven resolver and should resolve breath-first regardless of if a dependency is in the workspace. Otherwise users just get confused and more issues like this will be opened.

As said that's nothing m2e can solve, it is not the classpath container that do this, it is JDT / user code that reads the entries of a classpath container, so one would need a flag for the container to influence how that behaves, especially m2e might want to ignore all.

Can't you achieve your goal of https://github.com/eclipse-m2e/m2e-core/pull/1424 with another solution that does not affect the classpath so that we can revert https://github.com/eclipse-m2e/m2e-core/pull/1424?

I won't fully revert it but set the entry.setExported(false); explicitly and add a comment why we don't want this, I would recommend to simply include that change into your one with the test-case so we can see it actually fix the test-case.

HannesWell commented 1 year ago

Can't you achieve your goal of #1424 with another solution that does not affect the classpath so that we can revert #1424?

I won't fully revert it but set the entry.setExported(false); explicitly and add a comment why we don't want this, I would recommend to simply include that change into your one with the test-case so we can see it actually fix the test-case.

Done.

HannesWell commented 1 year ago

@garretwilson thanks for this bugreport. Can you please verify with the latest snapshot that the issue ist really fixed?

garretwilson commented 1 year ago

Can you please verify with the latest snapshot that the issue ist really fixed?

Yes! 🎉

I can confirm that v2.4.100.20230823-0832 fixes this bug! What a relief.

I have tested this both with an Eclipse installation upgrading from v2.4.100.20230802-0719 to v2.4.100.20230823-0832, as well as a plain Eclipse EE 2023-06 installation upgrading from v2.3.0.20230523-2033. Interestingly after upgrading from the buggy version v2.4.100.20230802-0719, I needed to refresh not only the project using the dependencies, in this case globalmentor-mummy, but also the project the presence of which in the workspace made the bug appear, in this case globalmentor-awt. As expected, upgrading from v2.3.0.20230523-2033 directly to v2.4.100.20230802-0719, skipping the buggy version, was completely transparent.

@HannesWell thank you for taking this bug seriously and taking the time to reproduce it and then fix it. @laeubi thank you for working with @HannesWell and providing input to help get this resolved quickly once it was reproduced.

garretwilson commented 1 year ago

As a side note, it turns out (see MVNCENTRAL-8292) that there is an issue with the Nexus Staging Maven Plugin which makes it skip publishing some artifact(s) if two POMs are in the same directory, which is why com.globalmentor:globalmentor-base:0.7.4 didn't appear on Maven Central and initially impeded @HannesWell from reproducing this issue. While MVNCENTRAL-8292 is being addressed, I have published com.globalmentor:globalmentor-base:0.7.5 with identical content but with a different POM structure so that the latest versions should have no missing artifacts on Maven Central. None of this has any direct impact on this current bug; I just wanted to give an update on that separate issue which we ran into while reproducing this bug.

HannesWell commented 1 year ago

Can you please verify with the latest snapshot that the issue ist really fixed?

Yes! 🎉

I can confirm that v2.4.100.20230823-0832 fixes this bug! What a relief.

Great. Thanks for testing!

As a side note, it turns out (see MVNCENTRAL-8292) that there is an issue with the Nexus Staging Maven Plugin which makes it skip publishing some artifact(s) if two POMs are in the same directory, which is why com.globalmentor:globalmentor-base:0.7.4 didn't appear on Maven Central and initially impeded @HannesWell from reproducing this issue. While MVNCENTRAL-8292 is being addressed, I have published com.globalmentor:globalmentor-base:0.7.5 with identical content but with a different POM structure so that the latest versions should have no missing artifacts on Maven Central. None of this has any direct impact on this current bug; I just wanted to give an update on that separate issue which we ran into while reproducing this bug.

That's good to know. Thanks for letting us know. Do you know if this problem will be addressed in the Nexus Staging Plugin?

Out of curiosity, what structure are you using now? I havn't looked it up, but wouldn't it be simpler to to have the bom-project as a sibling module of the other artifacts, use it as parent of the root pom and import it as bom there as well and at the same time aggregate it with the other artifacts, i.e. list it in the root pom's module section? As you have mentioned in your blog-post aggregation is independent from inheritance.

garretwilson commented 1 year ago

Out of curiosity, what structure are you using now?

It's actually the same logical structure. The difference is that instead of having parent-pom.xml in the top-level directory, in the same directory as pom.xml (which apparently was causing the problem), I put it back into a separate subfolder parent/pom.xml. It's the second-to-last pattern in my blog post. It seems like extra cruft to devote an entire subfolder to a single project definition file, but it's a tiny problem in relation to others.

… use it as parent of the root pom …

I get a little confused reading your suggestion, as I have a separate "root POM" as a completely separate project, which serves as the "bootstrap" POM for all the projects. I'm calling the per-project "root" the project "top-level POM". Maybe that's what you mean here.

In that case, you're proposing that the BOM be 1) the parent of the top-level POM (wait, so wouldn't the BOM be the top-level POM then?), 2) the sibling of the other artifacts, 3) import the BOM into the top-level … wait I'm sorry, I'm totally confused. I think there's a terminology problem, too, regarding "root" and "top-level'.

Anyway do you think we're getting off-topic in this ticket? Or is it OK since it's been closed? (I sort of think it might be better elsewhere.) Maybe you'd like to send me a direct email to discuss the blog post? Or even better, send an email to the Maven User List. I already requested feedback there, and I would really love to know if there is a better BOM pattern.

garretwilson commented 1 year ago

Do you know if this problem will be addressed in the Nexus Staging Plugin?

No idea yet. I only discovered what what triggering the bug this morning, and they said they'll send it over to the developers. I may have to file a separate bug for the plugin (this bug was for Maven Central). I imagine there's a long, winding road before we get any answers. 😅 I'll be sure and update MVNCENTRAL-8292) with any news.