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

Allow building with QA on Java 17+ #1208

Merged
merged 3 commits into from
Jan 22, 2024

Conversation

groldan
Copy link
Member

@groldan groldan commented Jan 5, 2024

Running mvn ... -Dqa with Java 17/21 breaks errorprone, as in:

[INFO] --- compiler:3.8.0:compile (default-compile) @ gwc-core ---
[INFO] Compiling 201 source files to /home/groldan/git/geowebcache/geowebcache/core/target/classes
An exception has occurred in the compiler (17.0.8). Please file a bug against the Java compiler via the Java bug reporting page (https://bugreport.java.com) after checking the Bug Database (https://bugs.java.com) for duplicates. Include your program, the following diagnostic, and the parameters passed to the Java compiler in your report. Thank you.
java.lang.IllegalAccessError: class com.google.errorprone.BaseErrorProneJavaCompiler (in unnamed module @0x3d3e9163) cannot access class com.sun.tools.javac.api.BasicJavacTask (in module jdk.compiler) because module jdk.compiler does not export com.sun.tools.javac.api to unnamed module @0x3d3e9163
        at com.google.errorprone.BaseErrorProneJavaCompiler.addTaskListener(BaseErrorProneJavaCompiler.java:95)
...

Fixed with:


Fixes to build with Java 21:

  • Thread.getId() is deprecated. Since it's only used for reporting errors, use Thread.getName() instead
  • new URL(String) is deprecated. Use new URI(String).toURL() instead, throwing MalformedURLException to preserve the old behavior in methods that throw IOException. This is centralized in the new utility class/method org.geowebcache.util.URLs.of(String url)
  • ExecutorService implements AutoCloseable in Java 21, add @SuppressWarnings("PMD.CloseResource") in the test classes that use it
  • Add @SuppressFBWarnings for new rules related to POJO fields unset/null, when they're assigned using reflection by XStream

@groldan groldan marked this pull request as draft January 5, 2024 06:46
Running `mvn ... -Dqa` with Java 17/21 breaks errorprone.

Fixed with:

* Update errorProne.version:2.18.0 -> 2.24.1
* Add `geowebcache/.mvn/jvm.config` with the required `--add-exports`
for the `maven-compiler-plugin` as explained at https://errorprone.info/docs/installation#jdk-16.
Fixes spotbugs when run with Java 21, otherwise getting

```
Execution spotbugs of goal com.github.spotbugs:spotbugs-maven-plugin:4.7.3.4:spotbugs failed:
java.lang.UnsupportedOperationException: The Security Manager is deprecated and will be
removed in a future release
```
@groldan groldan force-pushed the allow_qa_build_with_java_17plus branch from 9ad063d to 5fb1ad3 Compare January 5, 2024 07:39
@groldan groldan marked this pull request as ready for review January 5, 2024 07:44
Comment on lines 269 to 271
proxyUrl = new URL(getGwcConfig().getProxyUrl());
proxyUrl = new URI(getGwcConfig().getProxyUrl()).toURL();
log.fine("Using proxy " + proxyUrl.getHost() + ":" + proxyUrl.getPort());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I guess correctly, this change is related to the deprecation or "new URL(string)" in Java 21.

Rather than repeating this pattern over and over, along with the exception rethrowing, could we have a dedicated support method, similar (but not same) to what we have URLs.fileToURL() in GeoTools?

Copy link
Member Author

@groldan groldan Jan 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You got it. See org.geowebcache.util.URLs

@groldan groldan force-pushed the allow_qa_build_with_java_17plus branch from 5fb1ad3 to 94d2d95 Compare January 12, 2024 20:13
@groldan groldan requested a review from aaime January 12, 2024 20:15
@groldan groldan force-pushed the allow_qa_build_with_java_17plus branch from 94d2d95 to 001bba7 Compare January 12, 2024 20:21
* `Thread.getId()` is deprecated. Since it's only used for reporting
  errors, use `Thread.getName()` instead
* `new URL(String)` is deprecated. Use `new URI(String).toURL()`
  instead, throwing `MalformedURLException` to preserve the old
  behavior in methods that throw `IOException`. This is centralized in
  the new utility class/method `org.geowebcache.util.URLs.of(String
url)`
* `ExecutorService` implements `AutoCloseable` in Java 21, add
`@SuppressWarnings("PMD.CloseResource")` in the test classes that use it
* Add `@SuppressFBWarnings` for new rules
@groldan groldan force-pushed the allow_qa_build_with_java_17plus branch from 001bba7 to b5ea3b7 Compare January 12, 2024 20:22
@aaime
Copy link
Member

aaime commented Jan 22, 2024

Looks good to me, merging.

@aaime aaime merged commit 9b55d9a into GeoWebCache:main Jan 22, 2024
7 checks passed
@groldan groldan deleted the allow_qa_build_with_java_17plus branch August 26, 2024 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants