Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade Gson to 2.10 #3321

Closed
jlaur opened this issue Jan 16, 2023 · 15 comments · Fixed by #3817
Closed

Upgrade Gson to 2.10 #3321

jlaur opened this issue Jan 16, 2023 · 15 comments · Fixed by #3817
Labels
dependencies Pull requests that update a dependency file

Comments

@jlaur
Copy link
Contributor

jlaur commented Jan 16, 2023

Now that we have Java 17 support, it would be nice to upgrade Gson to the latest version (currently 2.10.1). Version 2.10 supports Java records: https://github.com/google/gson/releases/tag/gson-parent-2.10. Records are particularly useful when used in combination with Gson for serializing/deserializing from/to DTO's.

I'm working on a new binding (openhab/openhab-addons#14222) where I have initially taken advantage of this:

@NonNullByDefault
public record ElspotpriceRecord(@SerializedName("HourUTC") Instant hour,
        @SerializedName("SpotPriceDKK") double spotPrice) {
}

However, including this Gson dependency within the binding adds more than 250 kB, which is probably not acceptable:

  <dependencies>
    <dependency>
      <groupId>com.google.code.gson</groupId>
      <artifactId>gson</artifactId>
      <version>2.10.1</version>
    </dependency>
  </dependencies>

I tried upgrading from 2.9.1 to 2.10.1 in core, but ended up with a dependency problem:

[ERROR] Failed to execute goal on project org.openhab.core.bom.compile-model: Could not resolve dependencies for project org.openhab.core.bom:org.openhab.core.bom.compile-model:pom:4.0.0-SNAPSHOT: Failed to collect dependencies at org.eclipse.xtext:org.eclipse.xtext.xbase.ide:jar:2.29.0 -> org.eclipse.xtext:org.eclipse.xtext.ide:jar:2.29.0 -> org.eclipse.lsp4j:org.eclipse.lsp4j:jar:0.19.0 -> org.eclipse.lsp4j:org.eclipse.lsp4j.generator:jar:0.19.0 -> org.eclipse.lsp4j:org.eclipse.lsp4j.jsonrpc:jar:0.19.0 -> com.google.code.gson:gson:jar:[2.9.1,2.10): No versions available for com.google.code.gson:gson:jar:[2.9.1,2.10) within specified range -> [Help 1]

I don't know how to fix this, but I'm wondering if it would be possible and acceptable to have both 2.9.1 (because of this dependency) and 2.10.1 in core? At least 2.10.1 would only be included once and all bindings could take advantage of it.

@J-N-K
Copy link
Member

J-N-K commented Jan 16, 2023

Xtext 2.30 is expected at end of February and that updates to GSON 2.10.

@wborn
Copy link
Member

wborn commented Jan 17, 2023

I don't know how to fix this, but I'm wondering if it would be possible and acceptable to have both 2.9.1 (because of this dependency) and 2.10.1 in core? At least 2.10.1 would only be included once and all bindings could take advantage of it.

That will cause issues if you develop using Eclipse, because bnd only allows for using one version of a bundle. So it will break either everything Xtext related (depending on the older Gson) or all code depending on the new Gson bundle version.

@jlaur
Copy link
Contributor Author

jlaur commented Jan 18, 2023

For reference: xtext/xtext-reference-projects#318

I don't know how to fix this, but I'm wondering if it would be possible and acceptable to have both 2.9.1 (because of this dependency) and 2.10.1 in core? At least 2.10.1 would only be included once and all bindings could take advantage of it.

That will cause issues if you develop using Eclipse, because bnd only allows for using one version of a bundle. So it will break either everything Xtext related (depending on the older Gson) or all code depending on the new Gson bundle version.

@wborn - hmm, that sounds like a real problem. Unfortunately I'm not very much into Maven and dependencies, but I know the pain from managing dependencies in .NET. This time it seems we might get lucky because xtext is being upgraded soon, but if this wouldn't have happened, our progress could be slowed down a lot when stuck with versions from lowest denominators.

I hope there is some kind of solution for that. I might take the opportunity to learn more about Maven, but would be interested in any tips. 🙂 Anyway, this is slightly off-topic as it can now be considered a more general problem, since we'll hopefully soon be able to upgrade to Gson 2.10 when xtext does, and we are ready to upgrade xtext.

@clinique
Copy link
Contributor

Could we also figure the same with XStream ? Version 1.5.0+ is needed for records.

@jlaur
Copy link
Contributor Author

jlaur commented Mar 4, 2023

xtext 2.30 was released in the beginning of this week: https://www.eclipse.org/Xtext/releasenotes.html#/releasenotes/2023/02/27/version-2-30-0

Previous upgrade for reference: f9a74ab

@J-N-K
Copy link
Member

J-N-K commented Mar 5, 2023

I just tried.. it's a very difficult upgrade. XText now requires reload4j/1.2.24 which is only available in pax-logging-api/2.2.2, our karaf assembly currently provides pax-logging-api/2.2.0. There is now also a require statement in the manifest of org.eclipse.equinox.common for org.eclipse.osgi that we can't easily fulfill. I guess this has to wait.

@wborn wborn added the dependencies Pull requests that update a dependency file label Mar 11, 2023
@J-N-K
Copy link
Member

J-N-K commented Mar 24, 2023

@wborn Do you have an idea how we can get around the equinox runtime requirement for xtext?

@wborn
Copy link
Member

wborn commented Apr 16, 2023

I had a look and so far could only get a working distro with Xtext 2.30.0 and Gson 2.10 by manually removing the requirement from the org.eclipse.equinox.common bundle manifest.

@J-N-K
Copy link
Member

J-N-K commented Apr 16, 2023

Do you think we should create our own bundle in openhab-osgiify? Can you push your changes to a branch, I can then check what needs to be done to the bundle.

@wborn
Copy link
Member

wborn commented Apr 16, 2023

Have a look at wborn@2a587e2.

I also had to patch the Xtext bundles to work with Pax Logging 2.2.0 but that is hopefully fixed in the next Karaf version.
What also seems to fix feature verification is to provide the bundle but use start="false"

@jlaur
Copy link
Contributor Author

jlaur commented Sep 27, 2023

I also had to patch the Xtext bundles to work with Pax Logging 2.2.0 but that is hopefully fixed in the next Karaf version.

It seems with #3814 Pax Logging is at 2.2.3. I'm not sure where that leaves us with Xtext and ultimately Gson?

@J-N-K
Copy link
Member

J-N-K commented Sep 30, 2023

IIRC the issue is that the newer Xtext versions depend on slf4j 2.x and that was not available in pax-logging 2.2.0. pax-logging > 2.2.0 has slf4j, so this should no longer be an issue.

What remains is #3321 (comment).

@wborn
Copy link
Member

wborn commented Sep 30, 2023

I'll have another look and see what happens when upgrading to Xtext 2.32.0 instead of 2.30.0 which I previously tried.

wborn added a commit to wborn/openhab-core that referenced this issue Oct 1, 2023
Upgrades Xtext and its dependencies to:

* Xtext/Xtend 2.32.0
* LSP4J 0.21.0
* GSON 2.10.1
* Guava 32.1.2
* Guice 7.0.0

For release notes, see:

https://eclipse.dev/Xtext/releasenotes.html#/releasenotes/2023/08/27/version-2-32-0

Fixes openhab#3321

Signed-off-by: Wouter Born <github@maindrain.net>
wborn added a commit to wborn/openhab-core that referenced this issue Oct 1, 2023
Upgrades Xtext and its dependencies to:

* Xtext/Xtend 2.32.0
* LSP4J 0.21.0
* GSON 2.10.1
* Guava 32.1.2
* Guice 7.0.0

For release notes, see:

https://eclipse.dev/Xtext/releasenotes.html#/releasenotes/2023/08/27/version-2-32-0

Fixes openhab#3321

Signed-off-by: Wouter Born <github@maindrain.net>
kaikreuzer pushed a commit that referenced this issue Oct 1, 2023
Upgrades Xtext and its dependencies to:

* Xtext/Xtend 2.32.0
* LSP4J 0.21.0
* GSON 2.10.1
* Guava 32.1.2
* Guice 7.0.0

For release notes, see:

https://eclipse.dev/Xtext/releasenotes.html#/releasenotes/2023/08/27/version-2-32-0

Fixes #3321

Signed-off-by: Wouter Born <github@maindrain.net>
@openhab-bot
Copy link
Collaborator

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/govee-dreamcoulour-led-strips/84435/34

@wborn
Copy link
Member

wborn commented Oct 8, 2023

Could we also figure the same with XStream ? Version 1.5.0+ is needed for records.

There doesn't seem to be a XStream 1.5.0 release yet @clinique.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
5 participants