eclipse-pde / eclipse.pde

Eclipse Public License 2.0
25 stars 62 forks source link

multiple versions of gson bundle in target platform causing issues #482

Open martinlippert opened 1 year ago

martinlippert commented 1 year ago

I have a target platform that has two versions of the gson bundle:

com.google.gson_2.10.1
com.google.gson_2.9.1.v20220915-1632

I have a plugin project in my workspace that has

Import-Package: com.google.gson;version="[2.9.1,2.10.0)"

PDE flags an error saying that it can't find a bundle for com.google.gson. When I remove the version constraint from the dependency, the error goes away and the dependency is resolved against gson 2.10.1.

laeubi commented 1 year ago

Your import forbids gson 2.10.1 (if we assume that bundleversion == package version what is not a requirement), so probably you have some other requirement (transitively?) that requires a >= 2.10.1 gson?

So one would need a bit more information, at best:

  1. a target file that include both gsons
  2. a bundle that shows your mentioned error
martinlippert commented 1 year ago

There are definitely bundles in the target environment that require a gson >= 2.10.1, but since the import package statement that PDE complains about gets resolved in my target environment (it is basically a full IDE installation), there should be no conflict that prevents resolution here. At least that is what I am thinking here at the moment... ;-)

I will try to create a small sample to reproduce this and update this issue here once I have that.

martinlippert commented 1 year ago

Here are steps how to reproduce this issue:

The target environment will automatically be the running Eclipse instance, which contains both versions of the com.google.gson bundle. Nevertheless, the Import-Package statement is flagged with an error.

martinlippert commented 1 year ago

@laeubi Hope this helps you to reproduce the issue. Felt like the easiest way to reproduce this without anything specific from my side, like a specific target environment, a specific project, or anything in this area. If you need anything else, let me know. Happy to help as good as I can here.

merks commented 1 year ago

This reproduces the problem in an easy to debug way:

pde-issue-482.zip

You can import this in a debug-launched IDE (the Oomphed Platform SDK IDE), activate the gson.target as the active target platform and you'll see the problem.

The underlying problem is that this helper method assumes that each package is exported once:

image

I added printing logic to show what's not being added to the map (or being replaced in the map) and the variables view shows the contents of the map that's used later to verify package imports...

merks commented 1 year ago

Looking more closely, I'm not sure this is the only problem or the problem. I think it's also a (more fundamental) problem that these gson bundles end up being marked as unresolved as a whole. I'm guessing that their package import Import-Package: sun.misc; version="0.0.0" is treated as unsatisfied, but that's just a guess at this point.

merks commented 1 year ago

@martinlippert Which exact error message do you see?

martinlippert commented 1 year ago

No available bundle exports package 'com.google.gson'

merks commented 1 year ago

@martinlippert

That's very different from what I see. Are you seeing this is a recent 4.27 2023-03 build?

Debugging deeper, it's this constraint that is not satisfied by the 2.10.1 version:

[Require-Capability: osgi.ee; filter="(&(osgi.ee=JavaSE)(version=1.7))", Require-Capability: osgi.ee; filter="(|(&(osgi.ee=JavaSE)(version=1.7))(&(osgi.ee=JavaSE)(version=1.8)))"]

And this for the 2.9.1.x version

Require-Capability: osgi.ee; filter="(&(osgi.ee=JavaSE)(version=1.7))"

But that doesn't really make much sense...

This is the error I see:

image

merks commented 1 year ago

For what it's worth, here's the debug tracing from org.eclipse.osgi.internal.module.ResolverImpl

BEGIN RESOLUTION RESOLVING [com.google.gson_2.9.1.v20220915-1632] Trying to resolve: [com.google.gson_2.9.1.v20220915-1632], Require-Capability: osgi.ee; filter="(&(osgi.ee=JavaSE)(version=1.7))" GENERICS JavaSE[com.google.gson_2.9.1.v20220915-1632] failed to resolve Trying to resolve: [com.google.gson_2.9.1.v20220915-1632], Require-Capability: osgi.ee; filter="(&(osgi.ee=JavaSE)(version=1.7))" Trying to resolve: [com.google.gson_2.9.1.v20220915-1632], com.google.gson CHECKING: com.google.gson_2.10.1, com.google.gson Trying to resolve: [com.google.gson_2.10.1], Require-Capability: osgi.ee; filter="(&(osgi.ee=JavaSE)(version=1.7))" GENERICS JavaSE[com.google.gson_2.10.1] failed to resolve Trying to resolve: [com.google.gson_2.10.1], Require-Capability: osgi.ee; filter="(|(&(osgi.ee=JavaSE)(version=1.7))(&(osgi.ee=JavaSE)(version=1.8)))" Trying to resolve: [com.google.gson_2.10.1], com.google.gson.annotations CHECKING: com.google.gson_2.10.1, com.google.gson.annotations Found match: [com.google.gson_2.10.1]. Wiring [com.google.gson_2.10.1]:com.google.gson.annotations CHECKING: com.google.gson_2.9.1.v20220915-1632, com.google.gson.annotations Trying to resolve: [com.google.gson_2.10.1], sun.misc [com.google.gson_2.10.1] NOT RESOLVED Found match: [com.google.gson_2.10.1]. Wiring [com.google.gson_2.9.1.v20220915-1632]:com.google.gson CHECKING: com.google.gson_2.9.1.v20220915-1632, com.google.gson Trying to resolve: [com.google.gson_2.9.1.v20220915-1632], com.google.gson.annotations CHECKING: com.google.gson_2.10.1, com.google.gson.annotations

  • [com.google.gson_2.10.1] is unresolvable Found match: [com.google.gson_2.10.1]. Wiring [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.annotations CHECKING: com.google.gson_2.9.1.v20220915-1632, com.google.gson.annotations Trying to resolve: [com.google.gson_2.9.1.v20220915-1632], com.google.gson.internal CHECKING: com.google.gson_2.9.1.v20220915-1632, com.google.gson.internal Found match: [com.google.gson_2.9.1.v20220915-1632]. Wiring [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal Trying to resolve: [com.google.gson_2.9.1.v20220915-1632], com.google.gson.internal.bind CHECKING: com.google.gson_2.9.1.v20220915-1632, com.google.gson.internal.bind Found match: [com.google.gson_2.9.1.v20220915-1632]. Wiring [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal.bind Trying to resolve: [com.google.gson_2.9.1.v20220915-1632], com.google.gson.internal.bind.util CHECKING: com.google.gson_2.9.1.v20220915-1632, com.google.gson.internal.bind.util Found match: [com.google.gson_2.9.1.v20220915-1632]. Wiring [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal.bind.util Trying to resolve: [com.google.gson_2.9.1.v20220915-1632], com.google.gson.internal.reflect CHECKING: com.google.gson_2.9.1.v20220915-1632, com.google.gson.internal.reflect Found match: [com.google.gson_2.9.1.v20220915-1632]. Wiring [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal.reflect Trying to resolve: [com.google.gson_2.9.1.v20220915-1632], com.google.gson.internal.sql CHECKING: com.google.gson_2.9.1.v20220915-1632, com.google.gson.internal.sql Found match: [com.google.gson_2.9.1.v20220915-1632]. Wiring [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal.sql Trying to resolve: [com.google.gson_2.9.1.v20220915-1632], com.google.gson.reflect CHECKING: com.google.gson_2.10.1, com.google.gson.reflect
  • [com.google.gson_2.10.1] is unresolvable Found match: [com.google.gson_2.10.1]. Wiring [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.reflect CHECKING: com.google.gson_2.9.1.v20220915-1632, com.google.gson.reflect Trying to resolve: [com.google.gson_2.9.1.v20220915-1632], com.google.gson.stream CHECKING: com.google.gson_2.10.1, com.google.gson.stream
  • [com.google.gson_2.10.1] is unresolvable Found match: [com.google.gson_2.10.1]. Wiring [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.stream CHECKING: com.google.gson_2.9.1.v20220915-1632, com.google.gson.stream Trying to resolve: [com.google.gson_2.9.1.v20220915-1632], sun.misc [com.google.gson_2.9.1.v20220915-1632] NOT RESOLVED RESOLVING [com.google.gson_2.10.1]
  • [com.google.gson_2.10.1] is unresolvable RESOLVING [com.google.gson.source_2.9.1.v20220915-1632] [com.google.gson.source_2.9.1.v20220915-1632] RESOLVED RESOLVING [com.google.gson.source_2.10.1] [com.google.gson.source_2.10.1] RESOLVED RESOLVING [org.example.gson_1.0.0.qualifier] Trying to resolve: [org.example.gson_1.0.0.qualifier], Require-Capability: osgi.ee; filter="(&(osgi.ee=JavaSE)(version=11))" Trying to resolve: [org.example.gson_1.0.0.qualifier], com.google.gson IMPORT com.google.gson[org.example.gson_1.0.0.qualifier] failed to resolve [org.example.gson_1.0.0.qualifier] NOT RESOLVED **** Result Wirings **
    • WIRING for [com.google.gson.source_2.10.1] (r) no requires (w) no imports
    • WIRING for [com.google.gson.source_2.9.1.v20220915-1632] (r) no requires (w) no imports
    • WIRING for [org.example.gson_1.0.0.qualifier] (r) no requires (w) [org.example.gson_1.0.0.qualifier]:com.google.gson -> NULL!!!
    • WIRING for [com.google.gson_2.10.1] (r) no requires (w) [com.google.gson_2.10.1]:com.google.gson.annotations -> [com.google.gson_2.10.1]:com.google.gson.annotations (w) [com.google.gson_2.10.1]:sun.misc -> OPTIONAL (could not be wired)
    • WIRING for [com.google.gson_2.9.1.v20220915-1632] (r) no requires (w) [com.google.gson_2.9.1.v20220915-1632]:com.google.gson -> [com.google.gson_2.10.1]:com.google.gson (w) [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.annotations -> [com.google.gson_2.10.1]:com.google.gson.annotations (w) [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal -> [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal (w) [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal.bind -> [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal.bind (w) [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal.bind.util -> [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal.bind.util (w) [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal.reflect -> [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal.reflect (w) [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal.sql -> [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal.sql (w) [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.reflect -> [com.google.gson_2.10.1]:com.google.gson.reflect (w) [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.stream -> [com.google.gson_2.10.1]:com.google.gson.stream (w) [com.google.gson_2.9.1.v20220915-1632]:sun.misc -> OPTIONAL (could not be wired) END RESOLUTION
merks commented 1 year ago

@martinlippert I see you did say a 2023-03 JEE package so it should be quite a recent build...

merks commented 1 year ago

I think the failures I saw were due to OSGi being missing from the target platform.

This slightly larger target platform:

pde-issue-482.zip

reproduces the error bad message:

Description Resource Path Location Type No available bundle exports package 'com.google.gson' MANIFEST.MF /org.example.gson/META-INF line 6 Plug-in Problem

merks commented 1 year ago

At this point, the resolver decides the import is not satisfied by the export because the substitution is not null, but the substitution also does not satisfy the import while the ignored export does:

image

I think this is because several of the packages from com.google.gson_2.9.1.v20220915-1632 are wired to packages exported by com.google.gson_2.10.1:

** Result Wirings **

  • WIRING for [org.eclipse.osgi_3.18.300.v20230105-2227] (r) no requires (w) no imports
  • WIRING for [org.eclipse.osgi.source_3.18.300.v20230105-2227] (r) no requires (w) no imports
  • WIRING for [com.google.gson.source_2.10.1] (r) no requires (w) no imports
  • WIRING for [com.google.gson.source_2.9.1.v20220915-1632] (r) no requires (w) no imports
  • WIRING for [org.example.gson_1.0.0.qualifier] (r) no requires (w) [org.example.gson_1.0.0.qualifier]:com.google.gson -> NULL!!! (w) [org.example.gson_1.0.0.qualifier]:org.eclipse.osgi.container -> [org.eclipse.osgi_3.18.300.v20230105-2227]:org.eclipse.osgi.container
  • WIRING for [com.google.gson_2.10.1] (r) no requires (w) [com.google.gson_2.10.1]:com.google.gson.annotations -> [com.google.gson_2.10.1]:com.google.gson.annotations (w) [com.google.gson_2.10.1]:sun.misc -> OPTIONAL (could not be wired)
  • WIRING for [com.google.gson_2.9.1.v20220915-1632] (r) no requires (w) [com.google.gson_2.9.1.v20220915-1632]:com.google.gson -> [com.google.gson_2.10.1]:com.google.gson (w) [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.annotations -> [com.google.gson_2.10.1]:com.google.gson.annotations (w) [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal -> [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal (w) [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal.bind -> [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal.bind (w) [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal.bind.util -> [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal.bind.util (w) [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal.reflect -> [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal.reflect (w) [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal.sql -> [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.internal.sql (w) [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.reflect -> [com.google.gson_2.10.1]:com.google.gson.reflect (w) [com.google.gson_2.9.1.v20220915-1632]:com.google.gson.stream -> [com.google.gson_2.10.1]:com.google.gson.stream (w) [com.google.gson_2.9.1.v20220915-1632]:sun.misc -> OPTIONAL (could not be wired) END RESOLUTION

And that wiring happens because the Orbit versions imports its own exported packages but with a very wide version range.

Import-Package: com.google.gson;version="[2.9,3)",com.google.gson.annota tions;version="[2.9,3)",com.google.gson.internal;version="[2.9,3)",com. google.gson.internal.bind;version="[2.9,3)",com.google.gson.internal.bi nd.util;version="[2.9,3)",com.google.gson.internal.reflect;version="[2. 9,3)",com.google.gson.internal.sql;version="[2.9,3)",com.google.gson.re flect;version="[2.9,3)",com.google.gson.stream;version="[2.9,3)",sun.mi sc;resolution:=optional

I'm not sure there is bug in the resolver or if the wide version ranges on the bundle are sort of wrong. One question though is how does this work and runtime and shouldn't the resolver reflect that same behavior?

merks commented 1 year ago

@tjwatson

I think this is a bug in the resolver logic, but I'd have no clue how to fix it. There are other possible alternative wirings where there would not be a problem... Also, given that the bundle exports package com.google.gson;version="2.9.1" how is it actually valid to "substitute" that package with a package with a different version?

laeubi commented 1 year ago

@merks PDE uses the old no longer supported resolver so more than possibly a "hack" to prevent this special case might not happen if even that.

I wanted to migrate this to the new OSGi Resource Model (and Resolver) but that's something that will need a lot of effort. In the end it might fix such issues but the Orbit version looks a bit strange, not sure if this is "handcrafted" or generated, so probably it is much more better to move away from the old gson version instead of trying to fix things.

merks commented 1 year ago

@laeubi

All the newer Orbit things like this use BND recipes, e.g.,

https://git.eclipse.org/c/orbit/orbit-recipes.git/tree/google/com.google.gson_2.10.1/osgi.bnd

Indeed something that is equivalent to what really happens at runtime would be better.

So far my attempts to hack something haven't worked well. One thing I haven't tried is to eliminate this hooking up of substitutions. I just seems question to substitute a versioned package with a package with a different version because clearly the substitute might fail to satisfy some import package constraints... Perhaps something very narrow like don't substitute a versioned package with a package with a different version...

I'll play some more, time permitting...

laeubi commented 1 year ago

The official gson uses the following:

Bundle-Version: 2.9.1
Export-Package: 
  com.google.gson;uses:="com.google.gson.reflect,com.google.gson.stream";version="2.9.1",
  com.google.gson.annotations;version="2.9.1",
  com.google.gson.reflect;version="2.9.1",
  com.google.gson.stream;version="2.9.1"
Import-Package: 
  sun.misc;resolution:=optional,
  com.google.gson.annotations

and that's pretty much what one would expects, so why do orbit need to bundle this at all with different headers? Maybe you can just replace the orbit variant with the official maven artifact and see if it behaves different?

by the way that's exactly the reason why I'm not really a fan of using substitution packages "just in case" if they are actually nothing one would ever like to substitute....

merks commented 1 year ago

@jonahgraham FYI, I've found this "import your own exports" thing kind of strange too...

Here's a hack that fixes the problem:

image

The candidates are normally listed with the highest version first. This special case is just to make the first candidate be your own exported package if such a one is in the list...

I would appreciate @tjwatson expert opinion on such a hack. Perhaps guarded by dev mode?

laeubi commented 1 year ago

Here's a hack that fixes the problem

probably you can submit this as an PR so we see if maybe any test fails?

tjwatson commented 1 year ago

I'm not sure about your fix. It reorders the candidates which seems problematic. What concerns me is that we have a substitute version 2.10.1 that does not match the import range. That should not have been possible. Can you debug where org.eclipse.osgi.internal.module.VersionSupplier.setSubstitute(VersionSupplier) is getting called to set that incorrectly?

jonahgraham commented 1 year ago

@jonahgraham FYI, I've found this "import your own exports" thing kind of strange too...

Thanks for the FYI. Interesting to note @laeubi's https://github.com/eclipse-pde/eclipse.pde/issues/482#issuecomment-1438610627 shows that even the official one imports its own exports (at least one of them).

laeubi commented 1 year ago

@jonahgraham FYI, I've found this "import your own exports" thing kind of strange too...

Thanks for the FYI. Interesting to note @laeubi's #482 (comment) shows that even the official one imports its own exports (at least one of them).

Package com.google.gson uses com.google.gson.reflect and com.google.gson.stream so the com.google.gson.annotations is the only one that could be substituted, but in the end these substitutions are not really useful in this case ...

merks commented 1 year ago

@tjwatson

The self-import of com.google.gson;version="[2.9,3)" by the com google.gson 2.9.1.v20220915-1632 bundle resolves to the com google.gson 2.10.1 package. That's valid and is within the version range for that import. That's what causes the substitution to be set and that's what we seeing that in the wiring:

  • WIRING for [com.google.gson_2.9.1.v20220915-1632] (r) no requires (w) [com.google.gson_2.9.1.v20220915-1632]:com.google.gson -> [com.google.gson_2.10.1]:com.google.gson

That's what the hack fixes so the wiring is like this:

* WIRING for [com.google.gson_2.9.1.v20220915-1632]
    (r) no requires
    (w) [com.google.gson_2.9.1.v20220915-1632]:com.google.gson -> [com.google.gson_2.9.1.v20220915-1632]:com.google.gson

That's what makes the 2.9.1.x package available to be imported by the example bundle.

I know it's a hack and questionable at that. But the overall behavior of not finding a solution when one definitely exists and is also bug. I don't know how to solve that...

tjwatson commented 1 year ago

Then I do not understand what you are showing in https://github.com/eclipse-pde/eclipse.pde/issues/482#issuecomment-1438438832

Where it looks like the resolverImport and resolverExport are from the same bundle yet the export is substituted with what I thought was an incorrect version according to the range of the importer.

merks commented 1 year ago

@tjwatson

In https://github.com/eclipse-pde/eclipse.pde/issues/482#issuecomment-1438438832 it shows the import from the example:

image

being resolved/tested against the exported package from com.google.gson 2.9.1.v20220915-1632 (the only valid resolution of the import) but being ignored (treated as not satisfying the constraint) because it's substituted by the substitution package exported from com.google.gson 2.10.1 which fits correctly into the import range of com.google.gson;version="[2.9,3)" as specified in the com.google.gson 2.9.1.v20220915-1632 bundle, but this (valid) substitution but does not satisfy the import constraint of the example. So the result is that import of the example is not resolvable.

So to summarize, the substitution being set is not incorrect, but that results in the overall resolution no longer being possible because the package with the necessary version has been substituted and hence excluded from the packages available for resolution elsewhere.

merks commented 1 year ago

@tjwatson

To see a simple analog of this problem, paste the following into org.eclipse.osgi.tests.services.resolver.SubstitutableExportsTest:

    private State getSubstituteQuestionableState() throws BundleException {
        // Basic substitutable export test with A, B, C all exporting and importing x,y
        // packages
        // D, E, F all requiring A, B, C respectively to access x, y packages
        // all should get packages x and y from A
        State state = buildEmptyState();
        Hashtable manifest = new Hashtable();
        long bundleID = 0;

        manifest.clear();
        manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2"); //$NON-NLS-1$
        manifest.put(Constants.BUNDLE_SYMBOLICNAME, "A"); //$NON-NLS-1$
        manifest.put(Constants.BUNDLE_VERSION, "1.0.0"); //$NON-NLS-1$
        manifest.put(Constants.EXPORT_PACKAGE, "x; version=1.0"); //$NON-NLS-1$
        manifest.put(Constants.IMPORT_PACKAGE, "x; version=1.0"); //$NON-NLS-1$
        BundleDescription a = state.getFactory().createBundleDescription(state, manifest,
                (String) manifest.get(Constants.BUNDLE_SYMBOLICNAME) + (String) manifest.get(Constants.BUNDLE_VERSION),
                bundleID++);

        manifest.clear();
        manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2"); //$NON-NLS-1$
        manifest.put(Constants.BUNDLE_SYMBOLICNAME, "B"); //$NON-NLS-1$
        manifest.put(Constants.BUNDLE_VERSION, "1.0.0"); //$NON-NLS-1$
        manifest.put(Constants.EXPORT_PACKAGE, "x; version=2.0"); //$NON-NLS-1$
        manifest.put(Constants.IMPORT_PACKAGE, "x; version=2.0"); //$NON-NLS-1$
        BundleDescription b = state.getFactory().createBundleDescription(state, manifest,
                (String) manifest.get(Constants.BUNDLE_SYMBOLICNAME) + (String) manifest.get(Constants.BUNDLE_VERSION),
                bundleID++);

        manifest.clear();
        manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2"); //$NON-NLS-1$
        manifest.put(Constants.BUNDLE_SYMBOLICNAME, "C"); //$NON-NLS-1$
        manifest.put(Constants.BUNDLE_VERSION, "1.0.0"); //$NON-NLS-1$
        manifest.put(Constants.IMPORT_PACKAGE, "x; version=\"[1.0,2.0)\""); //$NON-NLS-1$
        BundleDescription c = state.getFactory().createBundleDescription(state, manifest,
                (String) manifest.get(Constants.BUNDLE_SYMBOLICNAME) + (String) manifest.get(Constants.BUNDLE_VERSION),
                bundleID++);

        state.addBundle(a);
        state.addBundle(b);
        state.addBundle(c);
        return state;
    }

    public void testSubstitutableExports026() throws BundleException {
        State state = getSubstituteQuestionableState();
        state.resolve();
        BundleDescription a = state.getBundle(0);
        BundleDescription b = state.getBundle(1);
        BundleDescription c = state.getBundle(2);
        assertTrue("c.isResolved is incorrect", c.isResolved()); //$NON-NLS-1$
    }

A simpler "hack" in org.eclipse.osgi.internal.module.ResolverImpl.resolveImport(ResolverImport, List) is to change the following:

image

I.e., substitute the export only if it's the same version. This will cause other tests to fail.

I can't say whether the current behavior is correct or not. What is clear though is that this is not an issue that can be fixed by PDE.

tjwatson commented 1 year ago

I'll try to take a look at this today:

I can't say whether the current behavior is correct or not. What is clear though is that this is not an issue that can be fixed by PDE.

I'm not too worried about making the changes here needed by PDE, although I would like to make sure it is a correct solution. PDE is one of the last (the last?) users of the old resolver. We could almost consider it to simply be part of PDE at this point.

merks commented 1 year ago

Note that on epp-dev, they (DSL package M3) are having runtime problems related to these plugins as well:

+1 for DSL (mac osx aarch64)

Minor issue lsp4e/lsp4j can not be started (gson versions installed are: 2.9.1, 2.10.1):

org.osgi.framework.BundleException: Could not resolve module: org.eclipse.lsp4e [308]
  Unresolved requirement: Require-Bundle: org.eclipse.lsp4j; bundle-version="[0.19.0,0.20.0)"
    -> Bundle-SymbolicName: org.eclipse.lsp4j; bundle-version="0.19.0.v20221118-0359"
       org.eclipse.lsp4j [310]
         Unresolved requirement: Import-Package: com.google.gson; version="[2.9.1,2.10.0)"
  Unresolved requirement: Require-Bundle: org.eclipse.lsp4j.jsonrpc; bundle-version="[0.19.0,0.20.0)"
    -> Bundle-SymbolicName: org.eclipse.lsp4j.jsonrpc; bundle-version="0.19.0.v20221118-0359"
       org.eclipse.lsp4j.jsonrpc [314]
         Unresolved requirement: Import-Package: com.google.gson; version="[2.9.1,2.10.0)" 
tjwatson commented 1 year ago

I'm convinced we should do anything here. The fact is the bundle says its export can be substituted and they allow it to be substituted with another minor version increase of the package seems wrong. That makes contracts that use require-bundle against such a bundle very unpredictable because they can all of a sudden get access to a minor version increase without having changed their requirements at all.

The suggested hack will result in almost nothing getting substituted unless it results in a uses constraint violation because most all bundles will import using a range that can be satisfied by their own export.

jonahgraham commented 1 year ago

Why does bnd generate these self imports then? Is that a bug in bnd?

tjwatson commented 1 year ago

Why does bnd generate these self imports then? Is that a bug in bnd?

I cannot answer for bnd exactly. I suspect it is analyzing the usage of the exported package by the rest of the bundle. If it determines that the usage of the exported package can safely consume a minor version increase of the package (using semantic version rules) then it allows an import like "[1.0, 2.0)". On the other hand if the other parts of the bundle implement consumer types from the exported bundle then it must use a range that is more strict like "[1.0, 1.1)".

The resolver could be made more smart I suppose, but it will only add yet another variable to the np-hard problem because it will now have to back out of substitute decisions.

If this is going to be a PDE/dev only solution you could probably to with the hack but gate it with dev mode.

laeubi commented 1 year ago

First of all you can disable that behavior with noimports:=true see https://bnd.bndtools.org/heads/export_package.html And I'm honestly almost always disable that because I think its only useful for choose cases, but there are others who think it should actually always be enabled and there are some explanation (spoiler: its not a bug) https://bnd.discourse.group/t/using-noimport-with-export-annotation/253

In short (as far as I have understood), this is done whenever you have a package exported and in the same using that in your bundle in some code that is internal. If you only export it (pure API) it is not chosen for substitution.

This is mainly useful for the case where there is an API (lets say osgi.cm) and I implement the API and for convenience reason I pack that API with my bundle and export the packages. Lets say there is another implementation, doing the same, with a slightly different API version then a consumer of both can only ever use one of them. But with the substitution one export is now replaced with one of them so it works again.

So here the both API+Implementor is the gson and the consumer is the importing bundle, so as mentioned above already, if that bundle has a (transitive) requirement in any form (uses clause, require bundle that requires it, importing a package of a provider that require it, ...) then this must be replaced to get a consistent class space. If one can partition the bundles into a set that requires gson 2.9 and one set that only requires 2.10 it should resolve (@tjwatson please correct me if I'm wrong) because it is save as they never will share any classes, but given the not so well seperated eclipse eco-system I really doubt that.

But in the example provided by @merks I'm a bit confused as if it really only contains the one bundle it should be resolvable, I think gson 2.10 should not be chosen at all for the initial set of candidates here and then there is no need for substitution and therefore no problem at all..

So I think the problem is not that substitution is happen, but why does PDE pass the resolver a bundle at all that do not satisfy any requirement of the bundle? In this case I would assume that the resolver is getting passed exactly one bundle and then even the hardest np-problem becomes a trivial case...

merks commented 1 year ago

Personally tend to agree with Tom.

Furthermore, I see a fundamental problem here. If I have a versioned package in a bundle and I have an import of that same package in that bundle that allows another version of that package, then no one can safely specify an import restricted to my versioned package's range because that package can and will be substituted and doing so will make the problem unsolvable. That just seems wrong; if a substitute package has a different version than the package being substituted it can and sometimes will make resolution impossible. And that @laeubi is the explanation to your confusion: yes the 2.9.x bundle is the only one that makes the problem solvable, but that's not how resolution actually works, the 2.9.x bundle's exported package is masked/hidden/invisible by the 2.10.x bundles exported package.

I'll also remind folks again that this problem happens at runtime with the new resolver:

https://www.eclipse.org/lists/epp-dev/msg06706.html

That to me is another good argument not to change the resolver to handle this case differently.

Note that from a PDE point of view, this problem can be addressed by the user disabling the version they don't want:

image

Also note @laeubi that the PDE is expected to provide exactly the set of bundles made available in the target platform. PDE can't know that the gson 2.10.x being present is problematic. So if the user don't want a bundle to be considered during resolution, they should remove it themselves.

So again, I think importing your own exports with a wide version range is just bogus, the resolver making such versioned substitutions is bogus, and the result is bogus. Both the old and new resolver do this bogus thing... Perhaps that bogus behavior is exactly as specified by OSGi, in which case only the bundle doing this bogus thing should stop doing it...

laeubi commented 1 year ago

@merks I'm not confused, PDE is just using the resolver wrong in my opinion, I would assume that this bundle can perfectly resolve in a real OSGi runtime.

Also note @laeubi that the PDE is expected to provide exactly the set of bundles made available in the target platform.

Why? there is no reason for this if you really only have this single bundle. This is exactly how P2 works, it first slices the units by any possible provider for any possible requirement, then this set is used to compute a satisfiable solution.

If I have a versioned package in a bundle and I have an import of that same package in that bundle that allows another version of that package, then no one can safely specify an import restricted to my versioned package's range because that package can and will be substituted and doing so will make the problem unsolvable.

This is not true, the problem is solvable for this single bundle, and if it is not solvable then only because the provided version range won't work anyways. As you can see P2 can resolve the problem and simply includes both items. So there is a partition in the problem that solves this problem for the input set. As your example bundle has no further requirements, it should fit in any of both partitions.

laeubi commented 1 year ago

Here I have added a Tycho build to @merks example with a product:

gson.with.tycho.zip

the build succeeds and produces the desired output for the product

org.example.gson/product/target/products/bundle.product/linux/gtk/x86_64$ ls -l plugins/
  com.google.gson_2.9.1.v20220915-1632.jar
  org.eclipse.equinox.launcher_1.6.400.v20210924-0641.jar
  org.eclipse.equinox.launcher.gtk.linux.x86_64_1.2.700.v20221108-1024
  org.eclipse.osgi_3.18.300.v20230211-2021.jar
  org.example.gson_1.0.0.202302250818.jar

so this shows the problem is actually solvable given the provided example input (target file + bundle).

laeubi commented 1 year ago

I also have now created a product that contains both gson bundles and this is the result:

g! lb
START LEVEL 6
   ID|State      |Level|Name
    0|Active     |    0|OSGi System Bundle (3.18.300.v20230211-2021)|3.18.300.v20230211-2021
    1|Resolved   |    4|Gson: Google Json Library for Java (2.9.1.v20220915-1632)|2.9.1.v20220915-1632
    2|Resolved   |    4|Gson (2.10.1)|2.10.1
    3|Active     |    4|Apache Felix Gogo Command (1.1.2)|1.1.2
    4|Active     |    4|Apache Felix Gogo Runtime (1.1.6)|1.1.6
    5|Active     |    4|Apache Felix Gogo Shell (1.1.4)|1.1.4
    6|Active     |    4|Console plug-in (1.4.500.v20211021-1418)|1.4.500.v20211021-1418
    7|Resolved   |    4|Equinox Launcher (1.6.400.v20210924-0641)|1.6.400.v20210924-0641
    8|Resolved   |    4|Equinox Launcher Linux X86_64 Fragment (1.2.700.v20221108-1024)|1.2.700.v20221108-1024
    9|Active     |    4|Gson Example (1.0.0.202302250902)|1.0.0.202302250902

as one can see all bundles are resolved, and I have also activated the Gson Example, the wiring report also shows that the example was wired to the 2.9.1 version of Gson:

g! b 9
org.example.gson_1.0.0.202302250902 [9]
  Id=9, Status=ACTIVE
  "No registered services."
  No services in use.
  No exported packages
  Imported packages
    com.google.gson; version="2.9.1" <com.google.gson_2.9.1.v20220915-1632 [1]>
    org.eclipse.osgi.container; version="1.6.0" <org.eclipse.osgi_3.18.300.v20230211-2021 [0]>
  No fragment bundles
  No required bundles

So the problem is compile time satisfiable and runtime satisfiable!

I also submitted this now as a test-case for Tycho:

HannesWell commented 1 year ago

Thank you @merks and @laeubi for your detailed investigations. This is gson issue is also quite annoying in M2E.

as one can see all bundles are resolved, and I have also activated the Gson Example, the wiring report also shows that the example was wired to the 2.9.1 version of Gson:

Without in depth knowledge about the rational how the old resolver is implemented I agree that, if the substitution does not satisfy the version range, the substitute should simply be ignored and instead the original package should be wired. Classspace consistency would then only be ensured if corresponding bundles don't share objects.

At this point, the resolver decides the import is not satisfied by the export because the substitution is not null, but the substitution also does not satisfy the import while the ignored export does:

image

I think this is because several of the packages from com.google.gson_2.9.1.v20220915-1632 are wired to packages exported by com.google.gson_2.10.1:

With the above in mind I wonder if the substitute should only be considered if it satisfies the constraint.

return vs.getSubstitute() == null && constraint.isSatisfiedBy(vs.getBaseDescription());

Could become

VersionSupplier substitute = vs.getSubstitute();
if (substitute != null && constraint.isSatisfiedBy(substitute.getBaseDescription())) {
    return false;
}
return constraint.isSatisfiedBy(vs.getBaseDescription());
laeubi commented 1 year ago

As said either the resolver or PDE should simply remove all items that are never satisfy any requirement (transitively) of the root (here: the bundle to resolve), this will reduce the NP problem and would prevent this situation entirely (for the given example).

merks commented 1 year ago

@HannesWell

I tried exactly that solution first, but downstream logic still does not produce the desired result because the logic is affected by the fact that the substitution has been set.

@laeubi

I think there are no simple solutions to np-hard problems. It would be really great from someone to prove that statement wrong.

With this drastically simplified test case sample we have a problem effectively involving 3 bundles where indeed we could simply eliminate one of the versions of gson. But if we add a 4th bundle with Import-Package: com.google.gson;version="[2.10.0,2.11.0)" then both versions of com.google.gson are needed to satisfy each of the two the bundles' constraints (and as long as there is no visible relation between those to bundles, a solution involving both versions of gson would be correct, as you've noted already). So in that slightly larger case we can't simply eliminate anything and we are back the problem to how to deal with the imported export rather than at looking toward the problem of how to simply reduce the problem space. Although I guess you are no longer proposing that?

But more importantly, note that both the JEE and DSL 2022-03 products contain these two versions of gson. And while the JEE product has no wiring problems (like your example, so yes a solution is possible and even works at runtime) the DSL product does have wiring problems (unlike your example, so correct wiring remains a problem in general) because of exactly this restricted package import range. So to repeat, these two exceptions are logged:

org.osgi.framework.BundleException: Could not resolve module: org.eclipse.lsp4e [312] Unresolved requirement: Require-Bundle: org.eclipse.lsp4j; bundle-version="[0.19.0,0.20.0)" -> Bundle-SymbolicName: org.eclipse.lsp4j; bundle-version="0.19.0.v20221118-0359" org.eclipse.lsp4j [314] Unresolved requirement: Import-Package: com.google.gson; version="[2.9.1,2.10.0)" Unresolved requirement: Require-Bundle: org.eclipse.lsp4j.jsonrpc; bundle-version="[0.19.0,0.20.0)" -> Bundle-SymbolicName: org.eclipse.lsp4j.jsonrpc; bundle-version="0.19.0.v20221118-0359" org.eclipse.lsp4j.jsonrpc [318] Unresolved requirement: Import-Package: com.google.gson; version="[2.9.1,2.10.0)" org.osgi.framework.BundleException: Could not resolve module: org.eclipse.wildwebdeveloper.xml [482] Unresolved requirement: Require-Bundle: org.eclipse.lsp4j; bundle-version="0.9.0" -> Bundle-SymbolicName: org.eclipse.lsp4j; bundle-version="0.19.0.v20221118-0359" org.eclipse.lsp4j [314] Unresolved requirement: Import-Package: com.google.gson; version="[2.9.1,2.10.0)" Unresolved requirement: Require-Bundle: org.eclipse.lsp4e; bundle-version="0.13.4" -> Bundle-SymbolicName: org.eclipse.lsp4e; bundle-version="0.15.0.202211292024"; singleton:="true" org.eclipse.lsp4e [312] Unresolved requirement: Require-Bundle: org.eclipse.lsp4j; bundle-version="[0.19.0,0.20.0)" -> Bundle-SymbolicName: org.eclipse.lsp4j; bundle-version="0.19.0.v20221118-0359" Unresolved requirement: Require-Bundle: org.eclipse.lsp4j.jsonrpc; bundle-version="[0.19.0,0.20.0)" -> Bundle-SymbolicName: org.eclipse.lsp4j.jsonrpc; bundle-version="0.19.0.v20221118-0359" org.eclipse.lsp4j.jsonrpc [318] Unresolved requirement: Import-Package: com.google.gson; version="[2.9.1,2.10.0)" Unresolved requirement: Require-Bundle: org.eclipse.lsp4j.jsonrpc -> Bundle-SymbolicName: org.eclipse.lsp4j.jsonrpc; bundle-version="0.19.0.v20221118-0359"

Note too that there is another suitable thing involved here in that p2 (as used by Tycho) doesn't not try to solve the wiring problem. This is why Tycho/p2 happily produces the JEE, DSL, and your sample product, and this is also why wiring problems are not noticed until they show up in the DSL product at runtime...

So not only do we need a solution that works for PDE, we need one that works at runtime in general, all boiling down to this issue of how to handle/wire imported exports.

In any case, if someone has a general solution, they're more than welcome to demonstrate concretely that it works well for PDE and at runtime. Mostly urgently, the DSL product is broken...

laeubi commented 1 year ago

This works at runtime, the wiring problem you see is a completely different case, And this has nothing to do with np-problem at all.

So there is the PDE case, what simply doing wrong things (for whatever reason), where one needs to resolve one bundle, and there is no need to include the offending bundle as there is no way that a bundle require both gsons at the same time! This is a pure compile-time problem and it can be solved like Tycho/p2 does it because the actual runtime wiring is simply irrelevant.

And then there is a problem of combining stuff at runtime that do not work together because inter-dependecies, but just because there are bundles with the same name involved do not mean this is the same problem, as @tjwatson was already pointed out the PDE reolver is not the one that is actually used by OSGi (anymore).

I think I also mentioned it elsewhere that lsp4e/j simply use to strict version ranges that effectively require that lsp4e/j and wwd must closely match each other regarding these dependencies, so if it works for one product but not the other then there is some discrepancy in this somewhere, but this log-extract is just not enough to decide whats wrong here.

merks commented 1 year ago

You mention 'there is some discrepancy somewhere". I definitely I agree with that.

The log for the DSL product says Unresolved requirement: Import-Package: com.google.gson; version="[2.9.1,2.10.0) even though the installation details show that the bundle exporting 2.9.1 is installed:

image

Moreover, the OSGi console log shows both versions of gson are not just installed but resolved:

7|Resolved   |    4|Gson (2.10.1.v20230109-0753)|2.10.1.v20230109-0753
8|Resolved   |    4|Gson: Google Json Library for Java (2.9.1.v20220915-1632)|2.9.1.v20220915-1632

Looking that the wiring of bundle 8, I see the bundle is wired like this:

g! b 8 com.google.gson_2.9.1.v20220915-1632 [8] Id=8, Status=RESOLVED Data Root=D:\Users\test17\dsl-latest\eclipse\configuration\org.eclipse.osgi\8\data "No registered services." No services in use. No exported packages Imported packages com.google.gson; version="2.10.1" <com.google.gson_2.10.1.v20230109-0753 [7]> com.google.gson.annotations; version="2.10.1" <com.google.gson_2.10.1.v20230109-0753 [7]> com.google.gson.internal; version="2.10.1" <com.google.gson_2.10.1.v20230109-0753 [7]> com.google.gson.internal.bind; version="2.10.1" <com.google.gson_2.10.1.v20230109-0753 [7]> com.google.gson.internal.bind.util; version="2.10.1" <com.google.gson_2.10.1.v20230109-0753 [7]> com.google.gson.internal.reflect; version="2.10.1" <com.google.gson_2.10.1.v20230109-0753 [7]> com.google.gson.internal.sql; version="2.10.1" <com.google.gson_2.10.1.v20230109-0753 [7]> com.google.gson.reflect; version="2.10.1" <com.google.gson_2.10.1.v20230109-0753 [7]> com.google.gson.stream; version="2.10.1" <com.google.gson_2.10.1.v20230109-0753 [7]> sun.misc; version="0.0.0" <org.eclipse.osgi_3.18.300.v20230211-2021 [0]> No fragment bundles No required bundles

This does looks (to me) exactly like the problem we are discussing here. I.e., it looks like every exported package has been replaced/substituted by an imported package from 2.10.1. The actual exports in the MANIFEST.MF of that bundle are like this:

Export-Package: com.google.gson.internal;x-internal:=true;version="2.9.1 ";uses:="com.google.gson,com.google.gson.reflect,com.google.gson.stream ",com.google.gson.internal.bind;x-internal:=true;version="2.9.1";uses:= "com.google.gson,com.google.gson.internal,com.google.gson.reflect,com.g oogle.gson.stream",com.google.gson.internal.bind.util;x-internal:=true; version="2.9.1",com.google.gson.internal.reflect;x-internal:=true;versi on="2.9.1";uses:="com.google.gson",com.google.gson.internal.sql;x-inter nal:=true;version="2.9.1";uses:="com.google.gson,com.google.gson.intern al.bind",com.google.gson;version="2.9.1";uses:="com.google.gson.interna l,com.google.gson.reflect,com.google.gson.stream",com.google.gson.annot ations;version="2.9.1",com.google.gson.reflect;version="2.9.1",com.goog le.gson.stream;version="2.9.1"

It's nevertheless possible that I have somehow misread the factual details. What discrepancy might I have overlooked? I.e., why does the wiring show no exported packages from 2.9.x, or why is the import of a 2.9.x package resolved?

laeubi commented 1 year ago

@merks one important part to know about such resolution failures is that they report all unresolved items at that time, so it is likely that there is another problem that results in such problem underneath if everything "looks fine" for that requirement.

So if you look closer, it does not only complain about gson, it complains also about required bundles... so most likely one of those bundles producing an impossible state (e.g. require the lower gson version while one of its dependent requiring the higher gson) or has a use-contraint violation.

So the problem for PDE is that actually one needs to resolve it for one bundle, and for your given example there is obviously a solution (I think we agree here) while it is unclear about the original report (as we do not know what else is required). So even if you add another bundle requiring a newer gson it will work, because is not forced to substitute the package and should work.

The problem in the runtime on the other hand seems that there are overlapping classpaces and then the resolver must substitute the package! That's quite hard to track, but I would try to uninstall one of the gsons, then refresh the packages and see what bundles fail, then do it the other way round, and then I would assume there is one or more bundles that overlap.

Another option is to add -Dequinox.resolver.revision.batch.size=1 what forces equinox to resolve only one bundle at a time and might give some more hints as well.

merks commented 1 year ago

For the one where it works (JEE), the packages aren't substituted so I expect you are right about the class space comment:

g! b 10 com.google.gson_2.9.1.v20220915-1632 [10] Id=10, Status=RESOLVED Data Root=D:\Users\test17\jee-latest\eclipse\configuration\org.eclipse.osgi\10\data "No registered services." No services in use. Exported packages com.google.gson.internal; version="2.9.1"[exported] com.google.gson.internal.bind; version="2.9.1"[exported] com.google.gson.internal.bind.util; version="2.9.1"[exported] com.google.gson.internal.reflect; version="2.9.1"[exported] com.google.gson.internal.sql; version="2.9.1"[exported] com.google.gson; version="2.9.1"[exported] com.google.gson.annotations; version="2.9.1"[exported] com.google.gson.reflect; version="2.9.1"[exported] com.google.gson.stream; version="2.9.1"[exported] Imported packages sun.misc; version="0.0.0" <org.eclipse.osgi_3.18.300.v20230211-2021 [0]>

Running ./eclipse.exe -clean -vmargs -Dequinox.resolver.revision.batch.size=1 produced the same logged errors:

org.osgi.framework.BundleException: Could not resolve module: org.eclipse.lsp4e [312] Unresolved requirement: Require-Bundle: org.eclipse.lsp4j.jsonrpc; bundle-version="[0.19.0,0.20.0)" -> Bundle-SymbolicName: org.eclipse.lsp4j.jsonrpc; bundle-version="0.19.0.v20221118-0359" org.eclipse.lsp4j.jsonrpc [318] Unresolved requirement: Import-Package: com.google.gson; version="[2.9.1,2.10.0)" Unresolved requirement: Require-Bundle: org.eclipse.lsp4j; bundle-version="[0.19.0,0.20.0)" -> Bundle-SymbolicName: org.eclipse.lsp4j; bundle-version="0.19.0.v20221118-0359" org.eclipse.lsp4j [314] Unresolved requirement: Import-Package: com.google.gson; version="[2.9.1,2.10.0)"

No fragment bundles No required bundles

The JEE and DSL examples are available here for your study:

https://www.eclipse.org/downloads/packages/release/2023-03/m3

One thing interesting here is note is that while the resolver used by PDE always substitutes packages, the runtime resolver does not always do that...

laeubi commented 1 year ago

One thing interesting here is note is that while the resolver used by PDE always substitutes packages, the runtime resolver does not always do that...

I think @tjwatson can explain better, but substitution is to give the resolver more freedom, so it can substitute the package but it not must do this. Of course this makes the problem even bigger...

The JEE and DSL examples are available here for your study:

If i do

g! lb org.eclipse.lsp4j
START LEVEL 6
   ID|State      |Level|Name
  308|Resolved   |    4|org.eclipse.lsp4j (0.20.0.v20230216-1815)|0.20.0.v20230216-1815
  309|Installed  |    4|org.eclipse.lsp4j (0.19.0.v20221118-0359)|0.19.0.v20221118-0359
  310|Resolved   |    4|org.eclipse.lsp4j.debug (0.20.0.v20230216-1815)|0.20.0.v20230216-1815
  311|Resolved   |    4|org.eclipse.lsp4j.generator (0.20.0.v20230216-1815)|0.20.0.v20230216-1815
  312|Resolved   |    4|org.eclipse.lsp4j.jsonrpc (0.20.0.v20230216-1815)|0.20.0.v20230216-1815
  313|Installed  |    4|org.eclipse.lsp4j.jsonrpc (0.19.0.v20221118-0359)|0.19.0.v20221118-0359
  314|Resolved   |    4|org.eclipse.lsp4j.jsonrpc.debug (0.20.0.v20230216-1815)|0.20.0.v20230216-1815
  315|Resolved   |    4|org.eclipse.lsp4j.websocket (0.20.0.v20230216-1815)|0.20.0.v20230216-1815
  316|Resolved   |    4|org.eclipse.lsp4j.websocket.jakarta (0.20.0.v20230216-1815)|0.20.0.v20230216-1815

the first thing I notice is that it seems we have two version of lsp4j while this might work it look very unusual to me.

Also it seems ls4j 0.20.0.v20230216-1815 can be resolved but 0.19.0.v20221118-0359 can not, so probably the resolver is making the best choice here to resolve as much as possible...

jonahgraham commented 1 year ago

LSP4J is supposed to be a non singleton and allowed to be installed multiple times (as much as gson is). Of course that is just in theory, and maybe this is exposing issues with that.

laeubi commented 1 year ago

@jonahgraham thats why I wrote it might work ;-)

I just assume no one really is testing such setup and given the narrow version ranges seems unlikely to work at the moment, especially as only some of the lsp4j items are in different versions ... so my guess is that having different versions is more accidental than by purpose.

mickaelistria commented 1 year ago

Can we rephrase the issue to "Orbit variant of gson make target-platform resolution fail" ? I don't see an issue in PDE here, but more that the Orbit variant of Orbit has incorrect metadata to guarantee proper resolution.

laeubi commented 1 year ago

The meta-data is not incorrect, it might not what one desire though. Still PDE is handling this case wrong as shown by @merks example actually can be resolved by Tycho (and Tycho also uses OSGi resolver!) and by an OSGi framework, so the message that no one export that package is wrong and substitution (for the simple example) should not happen.

merks commented 1 year ago

I think the Orbit metadata is questionable at best, though not strictly incorrect. If you export a package x.y.z and then in the same bundle import that package with version range [x.y.z,x+1.0.0), no one can safely import your package with range [x.y.z, x.y.z+1) because you've allow that to be replaced/substituted. We see that sometimes it works with multiple versions, i.e., JEE product and sometimes it doesn't, i.e., DSL product. So such an import just seems like a bad idea to me, asking for problems to happen. The PDE error message is also unhelpful and misleading, though perhaps not strictly wrong because it's exactly the problem/error one sees in the DSL product.

Anyway, I'm trying to make this problem go away by making the Orbit gson 2.9.1 go away...

laeubi commented 1 year ago

@merks I agree that the metadata is questionable, but your conclusion is wrong. The import can safely be done, see my example! It is just be problematic in the case here where old+new lsp4x and they do not declare requirements correctly (but that's not to blame on the gson completely) or we even have an "impossible" state where also the maven-gson will just show another error I fear, but we will see how it works out, maybe this change is the required difference to guide the resolver "on the right track" ...