eclipse-lsp4j / lsp4j

A Java implementation of the language server protocol intended to be consumed by tools and language servers implemented in Java.
https://eclipse.org/lsp4j
Other
599 stars 143 forks source link

weird dependency resolution problems after upgrade #702

Closed martinlippert closed 1 year ago

martinlippert commented 1 year ago

After running an update, I found myself in a situation that various lsp4j bundles in version 019.0 as well as 0.21.1 are installed. While that should not be a problem on its own, the package wiring that I ended up with is somewhat strange.

g! packages org.eclipse.lsp4j.jsonrpc
osgi.wiring.package; bundle-symbolic-name="org.eclipse.lsp4j.jsonrpc"; bundle-version:Version="0.20.1.v20230228-1531"; version:Version="0.20.1"; osgi.wiring.package="org.eclipse.lsp4j.jsonrpc"; uses:="com.google.gson,org.eclipse.lsp4j.jsonrpc.json,org.eclipse.lsp4j.jsonrpc.messages"<org.eclipse.lsp4j.jsonrpc_0.20.1.v20230228-1531 [732]>
  org.eclipse.lsp4j.jsonrpc_0.19.0.v20221118-0359 [321] imports
  org.eclipse.lsp4e.debug_0.15.1.202301162058 [317] imports
  org.eclipse.lsp4j.debug_0.19.0.v20221118-0359 [320] imports
  org.eclipse.lsp4j.jsonrpc.debug_0.19.0.v20221118-0359 [322] imports
g! packages org.eclipse.lsp4j.jsonrpc.json
osgi.wiring.package; bundle-symbolic-name="org.eclipse.lsp4j.jsonrpc"; bundle-version:Version="0.19.0.v20221118-0359"; version:Version="0.19.0"; osgi.wiring.package="org.eclipse.lsp4j.jsonrpc.json"; uses:="com.google.gson,org.eclipse.lsp4j.jsonrpc,org.eclipse.lsp4j.jsonrpc.messages"<org.eclipse.lsp4j.jsonrpc_0.19.0.v20221118-0359 [321]>
  org.eclipse.lsp4j.jsonrpc_0.20.1.v20230228-1531 [732] imports
  org.eclipse.lsp4e.debug_0.15.1.202301162058 [317] imports
  org.eclipse.lsp4j.jsonrpc.debug_0.19.0.v20221118-0359 [322] imports

This looks like the package org.eclipse.lsp4j.jsonrpc is imported in version 0.19.0 from the 0.20.1 bundle and vice versa.

This somehow causes the bundle org.eclipse.lsp4j in version 0.20.1 to not resolve with this problem diagnosis:

g! diag 731
org.eclipse.lsp4j [731]
  Unresolved requirement: Import-Package: org.eclipse.lsp4j.jsonrpc.json; version="[0.20.0,1.0.0)"

It seems to me like the re-import of the exported packages is causing some confusion.

cdietrich commented 1 year ago

hmm where in the jars can you see that?

Manifest-Version: 1.0
Bnd-LastModified: 1677598326445
Bundle-ManifestVersion: 2
Bundle-Name: org.eclipse.lsp4j
Bundle-RequiredExecutionEnvironment: JavaSE-1.8
Bundle-SymbolicName: org.eclipse.lsp4j
Bundle-Vendor: Eclipse LSP4J
Bundle-Version: 0.20.1.v20230228-1531
Created-By: 1.8.0_202 (Oracle Corporation)
Export-Package: org.eclipse.lsp4j;uses:="com.google.gson.annotations,org
 .eclipse.lsp4j.adapters,org.eclipse.lsp4j.jsonrpc.messages,org.eclipse.
 lsp4j.jsonrpc.validation,org.eclipse.xtext.xbase.lib";version="0.20.1",
 org.eclipse.lsp4j.adapters;uses:="com.google.gson,com.google.gson.refle
 ct,com.google.gson.stream,org.eclipse.lsp4j,org.eclipse.lsp4j.jsonrpc.m
 essages";version="0.20.1",org.eclipse.lsp4j.launch;uses:="org.eclipse.l
 sp4j.jsonrpc,org.eclipse.lsp4j.services";version="0.20.1",org.eclipse.l
 sp4j.services;uses:="org.eclipse.lsp4j,org.eclipse.lsp4j.adapters,org.e
 clipse.lsp4j.jsonrpc.json,org.eclipse.lsp4j.jsonrpc.messages,org.eclips
 e.lsp4j.jsonrpc.services";version="0.20.1",org.eclipse.lsp4j.util;uses:
 ="org.eclipse.lsp4j";version="0.20.1"
Import-Package: com.google.common.base;version="[30.1,31)",com.google.gs
 on;version="[2.9.1,2.11)",com.google.gson.annotations;version="[2.9.1,2
 .11)",com.google.gson.reflect;version="[2.9.1,2.11)",com.google.gson.st
 ream;version="[2.9.1,2.11)",org.eclipse.lsp4j,org.eclipse.lsp4j.adapter
 s,org.eclipse.lsp4j.jsonrpc;version="[0.20,1)",org.eclipse.lsp4j.jsonrp
 c.json;version="[0.20,1)",org.eclipse.lsp4j.jsonrpc.json.adapters;versi
 on="[0.20,1)",org.eclipse.lsp4j.jsonrpc.messages;version="[0.20,1)",org
 .eclipse.lsp4j.jsonrpc.services;version="[0.20,1)",org.eclipse.lsp4j.js
 onrpc.validation;version="[0.20,1)",org.eclipse.lsp4j.services,org.ecli
 pse.lsp4j.util,org.eclipse.xtext.xbase.lib;version="[2.28,3)",org.eclip
 se.xtext.xbase.lib.util;version="[2.28,3)"
Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.8))"
Tool: Bnd-6.3.1.202206071316
org.eclipse.lsp4j.jsonrpc;version="[0.20,1)"

i assume the 0.19 will have

org.eclipse.lsp4j.jsonrpc;version="[0.19,1)"

and thus also match 0.20.x

in wonder why 0.19.0 is in the resolving game at all

cdietrich commented 1 year ago

am also not sure if the 0.21.1 in your messages is a typo?

martinlippert commented 1 year ago

am also not sure if the 0.21.1 in your messages is a typo?

Yes, my bad, typo, and fixed now (updated to 0.20.1 of course). Thanks for pointing that out.

martinlippert commented 1 year ago

hmm where in the jars can you see that?

In the headers of the org.eclipse.lsp4j.jsonrpc bundle, not the org.eclipse.lsp4j one. The jsonrpc ones seem to resolve against each others packages.

martinlippert commented 1 year ago

Actually, after running the update, org.eclipse.lsp4j is around in version 0.20.1 only. But org.eclipse.lsp4j.debug, org.eclipse.lsp4j.jsonrpc and org.eclipse.jsonrpc.debug are around in version 0.19.0 (for some reason, which I will try to find).

martinlippert commented 1 year ago

Looks like lsp4e got updated only partially (lsp4e.debug didn't made it through in the latest version here in my tests, causing the older version to stay around and dragging the older org.eclipse.lsp4j.jsonrpc bundles in). This seems like something that I could fix on my end (make sure the latest lsp4e updates that I apply include the debug bundles).

But the circular resolution still looks weird to me and the question why the jsonrpc bundles re-import packages without a version constraint looks to me like we should probably add those version constraints to the import-package, if that re-import is necessary. Thoughts?

cdietrich commented 1 year ago

hmmm i see this one

Import-Package: com.google.gson;version="[2.10.1,2.11)",com.google.gson. annotations;version="[2.10.1,2.11)",com.google.gson.reflect;version="[2 .10.1,2.11)",com.google.gson.stream;version="[2.10.1,2.11)",org.eclipse .lsp4j.jsonrpc,org.eclipse.lsp4j.jsonrpc.json,org.eclipse.lsp4j.jsonrpc .json.adapters,org.eclipse.lsp4j.jsonrpc.messages,org.eclipse.lsp4j.jso nrpc.services,org.eclipse.lsp4j.jsonrpc.validation

all of these stem from this bundle. this looks wired to me

cdietrich commented 1 year ago

we might might a ,!self.*

jar.bnd (
    'Import-Package': "com.google.gson.*;version=\"$versions.gson\",*"
)
cdietrich commented 1 year ago

something to experiment with https://ci.eclipse.org/lsp4j/job/lsp4j-multi-build/job/cd_issue702/lastSuccessfulBuild/artifact/build/p2-repository/

martinlippert commented 1 year ago

Thanks @cdietrich for setting this up, I tried to run a few experiments with it, basically around upgrading an old installation to a new version that had this lsp4j version included. While building and upgrading works fine, I run into this when starting up the IDE:

Internal error: java.lang.ClassCastException: class com.google.gson.internal.LinkedTreeMap cannot be cast to class com.google.gson.JsonObject (com.google.gson.internal.LinkedTreeMap and com.google.gson.JsonObject are in unnamed module of loader org.eclipse.osgi.internal.loader.EquinoxClassLoader @4cae7d5)
java.util.concurrent.CompletionException: java.lang.ClassCastException: class com.google.gson.internal.LinkedTreeMap cannot be cast to class com.google.gson.JsonObject (com.google.gson.internal.LinkedTreeMap and com.google.gson.JsonObject are in unnamed module of loader org.eclipse.osgi.internal.loader.EquinoxClassLoader @4cae7d5)
    at java.base/java.util.concurrent.CompletableFuture.encodeThrowable(CompletableFuture.java:315)
    at java.base/java.util.concurrent.CompletableFuture.completeThrowable(CompletableFuture.java:320)
    at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1807)
    at java.base/java.util.concurrent.CompletableFuture$AsyncRun.exec(CompletableFuture.java:1796)
    at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:373)
    at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1182)
    at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1655)
    at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1622)
    at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:165)
Caused by: java.lang.ClassCastException: class com.google.gson.internal.LinkedTreeMap cannot be cast to class com.google.gson.JsonObject (com.google.gson.internal.LinkedTreeMap and com.google.gson.JsonObject are in unnamed module of loader org.eclipse.osgi.internal.loader.EquinoxClassLoader @4cae7d5)
    at org.eclipse.lsp4e.LanguageServerWrapper.lambda$21(LanguageServerWrapper.java:868)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
    at org.eclipse.lsp4e.LanguageServerWrapper.registerCapability(LanguageServerWrapper.java:851)
    at org.eclipse.lsp4e.LanguageClientImpl.lambda$2(LanguageClientImpl.java:136)
    at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1804)
    ... 6 more

But it really seems to be a different and probably unrelated problem. Just for the reference here.

guw commented 1 year ago

@martinlippert looking at your exception it seems that the runtime classpath contains duplicate GSON versions and you happen to find a cased where com.google.gson.internal.LinkedTreeMap is taken from one version and com.google.gson.JsonObject from another version. This could be a wiring problem.

FWIW, I am finding myself in an unresolved target platform with a similar problem.

I don't know how but I end up with multiple LSP4J bundles in the target platform:

image

Now this doesn't seem to be a problem unless I am trying to run something.

image

I think PDE's Target Platform State has the answer:

image

The old GSON bundle is importing packages from the newer GSON bundle. I think is causing trouble. This basically means that there will be only one version of GSON at runtime. That's why LSP4J fails to resolve. It cannot find the old version anymore.

guw commented 1 year ago

@cdietrich I think the GSON bundle also needs a change (fix).

The problem is that there is a mismatch on version ranges between LSP4J and GSON. If GSON continues to import its own packages (self) it can do so. However, LSP4J needs to use the same version range for its imports of GSON as GSON does for its self imports. Otherwise there will always be a resolution problem.

So if LSP4J really needs that tight import range [==,=+) then GSON must use the same tight import range [==,=+) for its self imports. Right now it's set to [==,+), which conflicts with LSP4J (link).

guw commented 1 year ago

So, I see that the import range has been relaxed as part of #690: https://github.com/eclipse-lsp4j/lsp4j/commit/ff58da7b317b201fc1bca5fa36bb0d32b99a5965

Is it possible to backport this to 0.19.x? Otherwise it's going to remain challenging for older LSP4J versions (which we do seem to have in 2023-03).

I still think the Orbit GSON bundle should be changed to use a tighter self import (cc @jonahgraham)

cdietrich commented 1 year ago

we currently use

Import-Package: \
 com.google.gson.*;version="${range;[==,+);${package-version}}", \
 *;resolution:=optional

in orbit,

so this would have to become

Import-Package: \
 com.google.gson.*;version="${range;[==,=+);${package-version}}", \
 *;resolution:=optional

not to include next major as upper, but the next minor instead

Manifest-Version: 1.0
Created-By: Eclipse Bundle Recipe Maven Plug-in
Build-Jdk-Spec: 11
Automatic-Module-Name: com.google.gson
Bundle-ContactAddress: https://github.com/google/gson
Bundle-Description: Gson JSON library
Bundle-Developers: google;organization=Google;organizationUrl="https://w
 ww.google.com"
Bundle-DocURL: https://github.com/google/gson/gson
Bundle-License: "Apache-2.0";link="https://www.apache.org/licenses/LICEN
 SE-2.0.txt"
Bundle-ManifestVersion: 2
Bundle-Name: %bundleName
Bundle-RequiredExecutionEnvironment: JavaSE-1.7, JavaSE-1.8
Bundle-SCM: url="https://github.com/google/gson/gson/",connection="scm:g
 it:https://github.com/google/gson.git/gson",developer-connection="scm:g
 it:git@github.com:google/gson.git/gson",tag="gson-parent-2.10.1"
Bundle-SymbolicName: com.google.gson
Bundle-Vendor: %bundleVendor
Bundle-Version: 2.10.1.v20230323-1201
Export-Package: com.google.gson.internal;x-internal:=true;version="2.10.
 1";uses:="com.google.gson,com.google.gson.reflect,com.google.gson.strea
 m",com.google.gson.internal.bind;x-internal:=true;version="2.10.1";uses
 :="com.google.gson,com.google.gson.internal,com.google.gson.reflect,com
 .google.gson.stream",com.google.gson.internal.bind.util;x-internal:=tru
 e;version="2.10.1",com.google.gson.internal.reflect;x-internal:=true;ve
 rsion="2.10.1";uses:="com.google.gson",com.google.gson.internal.sql;x-i
 nternal:=true;version="2.10.1";uses:="com.google.gson,com.google.gson.i
 nternal.bind",com.google.gson;version="2.10.1";uses:="com.google.gson.i
 nternal,com.google.gson.reflect,com.google.gson.stream",com.google.gson
 .annotations;version="2.10.1",com.google.gson.reflect;version="2.10.1",
 com.google.gson.stream;version="2.10.1"
Import-Package: com.google.gson;version="[2.10,2.11)",com.google.gson.an
 notations;version="[2.10,2.11)",com.google.gson.internal;version="[2.10
 ,2.11)",com.google.gson.internal.bind;version="[2.10,2.11)",com.google.
 gson.internal.bind.util;version="[2.10,2.11)",com.google.gson.internal.
 reflect;version="[2.10,2.11)",com.google.gson.internal.sql;version="[2.
 10,2.11)",com.google.gson.reflect;version="[2.10,2.11)",com.google.gson
 .stream;version="[2.10,2.11)",sun.misc;resolution:=optional
Multi-Release: true
Originally-Created-By: 11.0.16.1 (Azul Systems, Inc.)
Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.7))"

or should we have no package import there as well, as mixing wont work anyway

Import-Package: sun.misc;resolution:=optional

Import-Package: \
 !com.google.gson.*, \
 *;resolution:=optional
jonahgraham commented 1 year ago

Ed Merks tried this by using the upstream version of gson (which was already a valid OSGi bundle) and the same/similar problems happen because even the originals have self-imports. It is very common for bnd created bundles to have self imports and I don't understand why in the lsp4j+gson case this is turning into a problem.

FTR I don't mind if someone want to change gson in Orbit, but my preference would be to remove the re-bundled gson in Orbit. I am working with Ed (meeting next week) to stop having bundles in Orbit that are already OSGi-ified using https://download.eclipse.org/oomph/simrel-maven/nightly/latest

guw commented 1 year ago

It is very common for bnd created bundles to have self imports and I don't understand why in the lsp4j+gson case this is turning into a problem.

@jonahgraham It's LSP4J narrow import range. This clashes with GSON. See my comment/analysis here. The GSON self imports with the wider version range ensure that all minor releases of GSON are collapsed at runtime to avoid multiple minor versions of the same package. Thus there will always be only one major version available.

LSP4J on the flip side requires multiple minor versions to be available. That's a conflict. I don't have background on this decision. Might be worth reconsidering.

jonahgraham commented 1 year ago

The background is we were shy about wider ranges because gson does not do semantic versioning (or at a minimum in the past they have had instances of API changes on maintenance releases). LSP4J 0.20.1 widdened the range to cover tested versions in https://github.com/eclipse-lsp4j/lsp4j/pull/690 and I am certainly open to widening it again.

The upstream (maven central) gson does not use version ranges at all on its self-imports, their import package is:

Import-Package: sun.misc;resolution:=optional,com.google.gson.annotations

and I don't know what that implies, does that mean any version of com.google.gson.annotations is acceptable?

Anyway, I am open to changes/widening of ranges/etc in LSP4J. Please provide a PR that resolves your issue. I am more than happy to release a 0.20.2 version (or whatever you need to unblock) just let me know.

jonahgraham commented 1 year ago

Is it possible to backport this to 0.19.x? Otherwise it's going to remain challenging for older LSP4J versions (which we do seem to have in 2023-03).

I am still catching up with the older messages. Yes I can do a 0.19.x with that change.

jonahgraham commented 1 year ago

sorry for the influx of messages - I seem to be doing a bit of stream of consciousness

A 0.19.1 with #690 backported will be the same as 0.20.1 is. The only differences between 0.19.0 and 0.20.1 is the gson version range + the version of LSP4J itself:

https://github.com/eclipse-lsp4j/lsp4j/compare/v0.19.0...v0.20.1

Therefore can you switch to 0.20.1? Or are you constrained elsewhere to 0.19.x in a way that is hard to resolve?

guw commented 1 year ago

@jonahgraham It seems WildWebtools has a dependency on 0.19.x in 2023-03.

jonahgraham commented 1 year ago

I can do a 0.19.1 release next week. It will basically be the same as 0.20.1 release - except for the version number of LSP4J itself.

cdietrich commented 1 year ago

FTR I don't mind if someone want to change gson in Orbit, but my preference would be to remove the re-bundled gson in Orbit. I am working with Ed (meeting next week) to stop having bundles in Orbit that are already OSGi-ified using https://download.eclipse.org/oomph/simrel-maven/nightly/latest

@merks @jonahgraham is there an issue where this is tracked?

merks commented 1 year ago

@jonahgraham and I have a meeting week to discuss Orbit/Maven. I opened this issue describing what I've done so far:

https://github.com/merks/simrel-maven/issues/3

jonahgraham commented 1 year ago

Fixed in #723