The workspace has errors about missing indirect classpath dependencies

merks commented 4 months ago

Version

6.10.0

Operating System

Linux/Unix, MacOS, Windows

Eclipse version

2024-06

Bug description

Without these changes:

I see these errors:

Actual behavior

The Oomphed workspace has errors with the latest installed tools.

Expected behavior

An error free workspace.

Relevant log output

No response

Other information

No response

tomaswolf commented 4 months ago

Haven't seen this. Which MANIFEST.MF is this? egit.core? That should not need slf4j. And it should not be necessary to add transitive dependencies in package imports.

merks commented 4 months ago

It's this one:

/org.eclipse.egit.core/META-INF/MANIFEST.MF

Perhaps some JDT oddity. Do you really have all the latest 2024-06 stuff installed?

tomaswolf commented 4 months ago

No, I have not. I'll see if I can reproduce with a fresh install. But:

egit.core must not have a direct dependency to org.apache.sshd. JGit is supposed to encapsulate the not totally stable upstream interfaces completely.
if we had to add transitive dependencies here, something must be broken elsewhere.

But didn't you have such an issue a while ago already, and it disappeared then? I remember having made that comment about JGit encapsulationg the Apache MINA SSHD interfaces already... found it: https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/pull/1968#issuecomment-2046831071

merks commented 4 months ago

I just fix it in my workspace modifying the MANIFEST.MF and then I don't check that in. I know this is a hack, which is why I don't create a PR to check in the hack. Anyway, there is no rush...

tomaswolf commented 4 months ago

Fresh installs (Eclipse for Committers) on Windows:

Reproduces with 2024-06, jre 22: first complains about slf4j.Logger, when that is "fixed", about HostConfigEntryResolver
Reproduces with 2024-03, jre 17: as above.
Does not reproduce with 2023-12, jre 17: build works; workspace clean.

Changing the MANIFEST.MF as indicated above would be wrong. slf4j.Logger is referenced in a private static final field in the FileRepository superclass, which is subclassed in EGit. HostConfigEntryResolver is referenced in a private final field in org.eclipse.jgit.transport.sshd.SshdSessionFactory, which is also subclassed.

I don't know why this occurs. To me this looks like a bug in JDT.

merks commented 4 months ago

Yes, that's exactly my experience! And it's really annoying. Of course someone will want a test case, but that's not a trivial exercise. 😱

@stephan-herrmann

Any ideas how we might boil down a test case? Just using the Oomph setup reproduces it quickly...

tomaswolf commented 4 months ago

Don't know. The common denominator in these two cases seems to be:

A bundle A declaring some class X.
A bundle B importing A.X, and declaring its own class Y, which has a private final or private static final field referencing X.
A bundle C importing B.Y and declaring its own class Z extends B.Y, but not having any reference to bundle A.

The presence or absence of explicit constructors might also have an effect.

It is apparently a known problem: https://github.com/eclipse-jdt/eclipse.jdt.core/issues/543 .

merks commented 4 months ago

Wow, you're very awesome. Oh dear though. 😢

tomaswolf commented 4 months ago

The common denominator in these two cases seems to be:

Of course I'm unable to reproduce it with a small test with just these constructs. So I guess the reproducer for JDT would be to use the EGit Oomph setup.

tomaswolf commented 4 months ago

So we have:

A JDT bug that prevents EGit from being built in Eclipse 2024-03 and 2024-06.
The maven tycho build works.
The JDT bug has existed for a long time and is unlikely to be fixed soon.
The JDT bug can be worked around by putting slf4j.api and org.apache.sshd.osgi onto the classpath.
MANIFEST.MF is needed and used at runtime, and should not have these extra package imports.

I would suggest to work around this bug not by changing MANIFEST.MF but by adding to file build.properties

additional.bundles = slf4j.api,\
                     org.apache.sshd.osgi

Either manually, or via the PDE Manifest editor: Additional JARs

That would be a work-around that we could merge. build.properties is really only for building and is not included in the binary artifact. It leaves MANIFEST.MF unchanged, and thus does not affect the OSGi bundle classloader's classpath, and moreover the change does exactly what JDT needs to avoid this bug: it places these two bundles on the classpath used for compiling.

merks commented 4 months ago

I had the same thought but was hunting for extra classpath which involved jars. I forgot about this one, which I do use in EMF too. I think we can all agree this is a good idea. 🏅 There is a tiny potential it could mask some real problem, but we must move forward...

tomaswolf commented 4 months ago

I notice additionally that both slf4j.api and org.apache.sshd.osgi:

are included from maven in the target platform with includeSource=true.
their maven source artifacts are not valid OSGi bundles (no Bundle-SymbolicName or Eclipse-SourceBundle manifest headers).
the JGit p2 repo does include the source artifacts for both bundles somehow with a MANIFEST.MF that has these headers.
I don't see either slf4j-api or sshd-osgi being downloaded from there in the maven tycho build, though. In fact, I don't see them being downloaded at all, probably because I already have them in my ~/.m2/repository.
The maven tycho build complains about them (and about some others, too, all included from maven): [WARNING] [6d69866b-71ce-4fad-b276-d52c5c67aa6f][extension>org.eclipse.tycho:tycho-maven-plugin:4.0.6] No digest algorithm is available to verify download of osgi.bundle,org.apache.sshd.osgi.source,2.12.1.
- This might be a bogus message, I rather suspect that there is no OSGi source bundle for it because the maven version is used. The JGit p2 repo does have checksums for them.

So another condition for reproducing the JDT bug is probably:

Bundle A must be in the target platform only (not in the workspace, where the sources would be available), and there must not be any valid OSGi source bundle available for it.

Unclear, though, why this only occurs since 2024-03. Perhaps something in PDE and the way it sets up its compilation classpath changed? (Don't see anything, though. There was a refactoring in that area, but it occurred before 2023-12.)

Also unclear: why does compilation work in maven tycho? Does it manage to pick up the valid OSGi source bundle from the JGit repo all the same? (Mvn command line: mvn clean install -DskipTests -Djgit-site=https://repo.eclipse.org/content/unzip/snapshots.unzip/org/eclipse/jgit/org.eclipse.jgit.repository/6.10.0-SNAPSHOT/org.eclipse.jgit.repository-6.10.0-SNAPSHOT.zip-unzip/.)

merks commented 4 months ago

I'm a little skeptical that lack of or presence of source bundles makes a difference. I don't expect JDT to be looking at sources at all for things in the target platform for compilation purposes. I expect those are only useful as source attachments while debugging or for mining Javadoc. But who knows.

What you say about the source jars not being proper OSGi bundle is true and it would seem that m2e is wrapping those because I see this in the orbit p2 repository artifacts:

tomaswolf commented 4 months ago

Gerrit change 1195191 adds the two bundles to the build classpath.

tomaswolf commented 4 months ago

Gerrit change 1195191 is merged.

eclipse-egit / egit