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

Migrate to Java 21 #5421

Merged

Conversation

leonardehrenfried
Copy link
Member

Summary

This migrates the build and container image to use Java 21, which is the latest LTS version with a lot of nice features.

Heads up, @miles-grant-ibigroup .

@codecov
Copy link

codecov bot commented Oct 12, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a9b0151) 66.80% compared to head (ef8e807) 66.80%.
Report is 3 commits behind head on dev-2.x.

Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #5421      +/-   ##
=============================================
- Coverage      66.80%   66.80%   -0.01%     
+ Complexity     15557    15556       -1     
=============================================
  Files           1806     1806              
  Lines          69821    69821              
  Branches        7357     7357              
=============================================
- Hits           46646    46642       -4     
- Misses         20718    20721       +3     
- Partials        2457     2458       +1     

see 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jtorin
jtorin previously approved these changes Oct 20, 2023
@leonardehrenfried leonardehrenfried marked this pull request as draft October 24, 2023 08:19
@leonardehrenfried leonardehrenfried marked this pull request as ready for review November 2, 2023 18:32
@vpaturet
Copy link
Contributor

vpaturet commented Nov 6, 2023

There is a transitive dependency on Netty, which has a compatibility issue with JDK 21 (https://netty.io/news/2023/09/21/4-1-99-Final.html)
The dependency comes from:

  • azure service bus --> patch available
  • asynchttpclient --> no patch available as of today, netty should be updated explicitly in the OTP pom file.

@leonardehrenfried we should probably review the dependencies that are currently ignored in the Renovate configuration, some might need to be patched for JDK 21.

@leonardehrenfried
Copy link
Member Author

It seems that it's a problem on Mac OS only. Did you hit that bug on your machine?

@leonardehrenfried
Copy link
Member Author

Also, it seems that this is no longer needed as netex-java-model uses a newer version:

OpenTripPlanner/pom.xml

Lines 559 to 569 in 322a457

<!--
The jaxb-api provides annotations like XmlElement indicating how classes should be (de)serialized.
The jaxb-runtime pulls in the jaxb-api of the same version transitively. But we specify both api and
runtime dependencies anyway here at the top level to prevent transitive dependencies from pulling in
mismatched versions. This is overriding an older version of jaxb-api pulled in by the netex-java-model.
-->
<dependency>
<groupId>jakarta.xml.bind</groupId>
<artifactId>jakarta.xml.bind-api</artifactId>
<version>3.0.1</version>
</dependency>

@leonardehrenfried
Copy link
Member Author

I have updated all Azure dependencies so that the netty dependencies look like this now:

❯ mvn dependency:list|grep netty
[INFO]    io.netty:netty-tcnative-boringssl-static:jar:2.0.62.Final:compile
[INFO]    io.netty:netty-resolver-dns-native-macos:jar:osx-x86_64:4.1.100.Final:compile
[INFO]    io.netty:netty-tcnative-boringssl-static:jar:linux-aarch_64:2.0.62.Final:compile
[INFO]    io.netty:netty-codec-http2:jar:4.1.100.Final:compile
[INFO]    io.netty:netty-codec-socks:jar:4.1.60.Final:compile
[INFO]    io.netty:netty-transport:jar:4.1.60.Final:compile
[INFO]    io.netty:netty-handler-proxy:jar:4.1.60.Final:compile
[INFO]    io.netty:netty-transport-native-unix-common:jar:4.1.60.Final:compile
[INFO]    io.projectreactor.netty:reactor-netty-http:jar:1.0.38:compile
[INFO]    io.netty:netty-tcnative-boringssl-static:jar:osx-x86_64:2.0.62.Final:compile
[INFO]    com.typesafe.netty:netty-reactive-streams:jar:2.0.4:compile
[INFO]    io.netty:netty-handler:jar:4.1.60.Final:compile
[INFO]    io.netty:netty-codec:jar:4.1.60.Final:compile
[INFO]    com.azure:azure-core-http-netty:jar:1.13.9:compile
[INFO]    io.netty:netty-tcnative-classes:jar:2.0.62.Final:compile
[INFO]    io.netty:netty-codec-dns:jar:4.1.100.Final:compile
[INFO]    io.netty:netty-codec-http:jar:4.1.60.Final:compile
[INFO]    io.netty:netty-resolver-dns-classes-macos:jar:4.1.100.Final:compile
[INFO]    io.netty:netty-tcnative-boringssl-static:jar:windows-x86_64:2.0.62.Final:compile
[INFO]    io.netty:netty-common:jar:4.1.60.Final:compile
[INFO]    io.netty:netty-resolver-dns:jar:4.1.100.Final:compile
[INFO]    io.netty:netty-tcnative-boringssl-static:jar:osx-aarch_64:2.0.62.Final:compile
[INFO]    io.netty:netty-tcnative-boringssl-static:jar:linux-x86_64:2.0.62.Final:compile
[INFO]    io.netty:netty-buffer:jar:4.1.60.Final:compile
[INFO]    io.netty:netty-transport-native-epoll:jar:linux-x86_64:4.1.60.Final:compile
[INFO]    io.projectreactor.netty:reactor-netty-core:jar:1.0.38:compile
[INFO]    io.grpc:grpc-netty-shaded:jar:1.58.0:compile
[INFO]    org.asynchttpclient:async-http-client-netty-utils:jar:2.12.3:compile
[INFO]    io.netty:netty-transport-native-kqueue:jar:osx-x86_64:4.1.60.Final:compile
[INFO]    io.netty:netty-resolver:jar:4.1.60.Final:compile

It's a mixture of 4.1.60 and 4.1.100.

AHC hasn't had a proper release in a while but there are beta versions of AHC 3 available. Do you know what version 3 is about?

@leonardehrenfried
Copy link
Member Author

AHC 3.0.0.Beta3 seems compatible with version 2 and uses the newest version of Netty: I build graph and ran some updaters and everything worked fine in Java 21.

@vpaturet
Copy link
Contributor

vpaturet commented Nov 6, 2023

Did you try to pin the Netty version to 4.1.100? This may be cleaner than using a beta version.

@vpaturet
Copy link
Contributor

vpaturet commented Nov 6, 2023

It seems that it's a problem on Mac OS only. Did you hit that bug on your machine?

Nope, this works fine on my Linux machine and in our cloud deployment.

@leonardehrenfried
Copy link
Member Author

Did you try to pin the Netty version to 4.1.100? This may be cleaner than using a beta version.

I tried the Netty BOM which didn't have the desired effect. Is this what you suggested?

@vpaturet
Copy link
Contributor

vpaturet commented Nov 6, 2023

Adding:

        <dependency>
            <groupId>io.netty</groupId>
            <artifactId>netty-bom</artifactId>
            <version>4.1.100.Final</version>
            <type>pom</type>
            <scope>import</scope>
        </dependency>

under <dependencyManagement> will pin the dependency.
Do you mean the updater failed with this config?

@vpaturet
Copy link
Contributor

vpaturet commented Nov 6, 2023

Also, it seems that this is no longer needed as netex-java-model uses a newer version:

OpenTripPlanner/pom.xml

Lines 559 to 569 in 322a457

<!--
The jaxb-api provides annotations like XmlElement indicating how classes should be (de)serialized.
The jaxb-runtime pulls in the jaxb-api of the same version transitively. But we specify both api and
runtime dependencies anyway here at the top level to prevent transitive dependencies from pulling in
mismatched versions. This is overriding an older version of jaxb-api pulled in by the netex-java-model.
-->
<dependency>
<groupId>jakarta.xml.bind</groupId>
<artifactId>jakarta.xml.bind-api</artifactId>
<version>3.0.1</version>
</dependency>

It is recommended to explicitly set all direct dependencies (i.e. the one that are visible in java imports) in the pom.
See https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html

Although transitive dependencies can implicitly include desired dependencies, it is a good practice to explicitly specify the dependencies your source code uses directly. This best practice proves its value especially when the dependencies of your project change their dependencies.

However in this particular case this is an API, not an implementation, so incompatibilities would be detected at compile time.
This can be removed when we update NeTEx Java model, I'll update the corresponding PR.

@leonardehrenfried
Copy link
Member Author

Adding:

        <dependency>
            <groupId>io.netty</groupId>
            <artifactId>netty-bom</artifactId>
            <version>4.1.100.Final</version>
            <type>pom</type>
            <scope>import</scope>
        </dependency>

under <dependencyManagement> will pin the dependency. Do you mean the updater failed with this config?

Thanks for that. I forgot about the scope definition and now it does what we expect it to do:

❯ mvn dependency:list|grep netty
[INFO]    io.netty:netty-resolver-dns-native-macos:jar:osx-x86_64:4.1.100.Final:compile
[INFO]    io.netty:netty-tcnative-boringssl-static:jar:windows-x86_64:2.0.61.Final:compile
[INFO]    io.netty:netty-tcnative-boringssl-static:jar:osx-aarch_64:2.0.61.Final:compile
[INFO]    io.netty:netty-transport-classes-epoll:jar:4.1.100.Final:compile
[INFO]    io.netty:netty-codec-http2:jar:4.1.100.Final:compile
[INFO]    io.projectreactor.netty:reactor-netty-http:jar:1.0.38:compile
[INFO]    com.typesafe.netty:netty-reactive-streams:jar:2.0.4:compile
[INFO]    io.netty:netty-transport:jar:4.1.100.Final:compile
[INFO]    io.netty:netty-tcnative-boringssl-static:jar:osx-x86_64:2.0.61.Final:compile
[INFO]    io.netty:netty-tcnative-boringssl-static:jar:linux-aarch_64:2.0.61.Final:compile
[INFO]    com.azure:azure-core-http-netty:jar:1.13.9:compile
[INFO]    io.netty:netty-codec:jar:4.1.100.Final:compile
[INFO]    io.netty:netty-codec-dns:jar:4.1.100.Final:compile
[INFO]    io.netty:netty-transport-native-kqueue:jar:osx-x86_64:4.1.100.Final:compile
[INFO]    io.netty:netty-resolver:jar:4.1.100.Final:compile
[INFO]    io.netty:netty-transport-classes-kqueue:jar:4.1.100.Final:compile
[INFO]    io.netty:netty-buffer:jar:4.1.100.Final:compile
[INFO]    io.netty:netty-resolver-dns-classes-macos:jar:4.1.100.Final:compile
[INFO]    io.netty:netty-common:jar:4.1.100.Final:compile
[INFO]    io.netty:netty-tcnative-boringssl-static:jar:2.0.61.Final:compile
[INFO]    io.netty:netty-transport-native-epoll:jar:linux-x86_64:4.1.100.Final:compile
[INFO]    io.netty:netty-codec-socks:jar:4.1.100.Final:compile
[INFO]    io.netty:netty-handler-proxy:jar:4.1.100.Final:compile
[INFO]    io.netty:netty-resolver-dns:jar:4.1.100.Final:compile
[INFO]    io.netty:netty-transport-native-unix-common:jar:4.1.100.Final:compile
[INFO]    io.netty:netty-codec-http:jar:4.1.100.Final:compile
[INFO]    io.projectreactor.netty:reactor-netty-core:jar:1.0.38:compile
[INFO]    io.netty:netty-tcnative-classes:jar:2.0.61.Final:compile
[INFO]    io.netty:netty-handler:jar:4.1.100.Final:compile
[INFO]    io.grpc:grpc-netty-shaded:jar:1.58.0:compile
[INFO]    org.asynchttpclient:async-http-client-netty-utils:jar:2.12.3:compile
[INFO]    io.netty:netty-tcnative-boringssl-static:jar:linux-x86_64:2.0.61.Final:compile

@vpaturet
Copy link
Contributor

vpaturet commented Nov 6, 2023

I simplified Jakarta dependency management in 650193c

@leonardehrenfried leonardehrenfried merged commit 36fa295 into opentripplanner:dev-2.x Nov 9, 2023
5 checks passed
t2gran pushed a commit that referenced this pull request Nov 9, 2023
@leonardehrenfried leonardehrenfried deleted the java-21 branch November 9, 2023 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants