Upgrade build for Java 17 as base Java version#1406
Conversation
0320368 to
5d83fc7
Compare
a072a7e to
1e4eaf8
Compare
1e4eaf8 to
d7d3805
Compare
|
Hey @groldan I am having some feedback when running I am running Java 17, so something is odd: |
401ed5d to
101c362
Compare
|
Hi @jodygarnett, good catch, didn't even thought about running |
|
Thanks confirmed the fix worked, note I updated some docs while waiting for builds and so on as part of this PR. Q: did you force push or anything? My doc changes and commit history seem to be troubled. |
|
hi @jodygarnett , sorry about that I'm not used to direct changes to the pr and I'm pretty sure I rebased on top of main hence the force push |
jodygarnett
left a comment
There was a problem hiding this comment.
Look exhausting, thanks for the work Gabe. I have tested locally and update the docs.
ca54fa8 to
deb996a
Compare
| <local name="message"/> | ||
| <property name="message" value="autobuild not available:${line.separator} | ||
| ${line.separator} | ||
| virtualenv venv${line.separator} |
There was a problem hiding this comment.
Wondering if this would work with Windows installations?
On a related note, I've just upgraded to the latest Linux Mint and it's the first distro where I'm basically forced to use virtualenvs, the previous release allowed me to use pip directly.
There was a problem hiding this comment.
I don't see why it wouldn't, it's just printing a message and using ${line.separator} for cross platform compatibility.
In any case I'm pretty sure that was Jody's contribution. @jodygarnett do you know if that'll work on windows?
|
|
||
| 1. Download an OpenJDK release for your platform: | ||
|
|
||
| * https://adoptium.net/temurin/releases/?version=11 Temurin 11 (LTS) (Recommended) |
There was a problem hiding this comment.
Version 11?
I think you want https://adoptium.net/temurin/releases/?version=17&os=windows&arch=any
There was a problem hiding this comment.
The current link is still pointing at java 11 mind
| defaultCount + 100, | ||
| getInfoNames(config).size()); | ||
| // get a thread pool | ||
|
|
There was a problem hiding this comment.
I don't see the need for these two white lines... the comment refers to the code line below, if anything adding a white line makes it less readable. Maybe you added an annotation and then removed it, leaving an empty line behind?
| throws Exception { | ||
| SqliteConnectionManager connectionManager = new SqliteConnectionManager(poolSize, 10); | ||
| connectionManagersToClean.add(connectionManager); | ||
|
|
There was a problem hiding this comment.
Another newline that I guess is not necessary
| } | ||
| FileUtils.copyFile(seedFile, databaseFile); | ||
| // submitting the select tasks | ||
|
|
|
I pushed changes for the Assert.equals calls having arguments swapped (there's a quick fix for them in IntelliJ) |
92239da to
3a8121f
Compare
|
Thanks, squash-merged and ready to merge. |
|
Looks like none of my feedback was taken into consideration? Maybe it's just an oversight |
* Upgrade build to use Java 17 as the baseline version. * Configure maven-enforcer-plugin to require Java 17. * Upgrade GitHub Actions workflows to use Java 17. * Update documentation for Java 17, including developer and user guides. * Upgrade Jetty from 9.x to 10.0.25, the last version to support `javax.servlet`. * Replace `com.google.common.base.Objects.equal()` with `java.util.Objects.equal()`. * Update test configurations to use `localhost:8080` instead of deprecated demo servers. * Fix REST integration tests and address other issues found during local testing on Java 17. * Increase heap size for the QA build profile. * Fix swapped Assert.equals arguments Plugin version upgrades: * maven-jar-plugin 2.4 -> 3.0.2 * maven-enforcer-plugin 3.0.0-M3 -> 3.5.0 * maven-compiler-plugin 3.10.1 -> 3.14.0 * maven-surefire-plugin 2.22.1 -> 3.5.3 * maven-failsafe-plugin 3.3.1 -> 3.5.3 * maven-war-plugin 3.3.2 -> 3.4.0 * jetty-maven-plugin 9.4.14.v20181114 -> 10.0.25 * maven-javadoc-plugin 3.4.1 -> 3.11.2 * maven-checkstyle-plugin 3.1.1 -> 3.6.0 Co-authored-by: Andrea Aime <andrea.aime@gmail.com> Co-authored-by: Jody Garnett <jody.garnett@gmail.com>
3a8121f to
de648b8
Compare
Hi Andrea, yes, definitely an oversight. It should be good now. |
Configure maven-enforcer-plugin to require Java 17
Configure maven-enforcer-plugin to require maven 3.8+ as done in GeoServer.
Upgrade github actions workflows to use Java 17
Remove cobertura-maven-plugin (modules/extension/xsd/**), as it is not compatible with Java 11
Update maven-javadoc-plugin configuration to use legacyMode=true and not fail due to module-info.java not present.
Plugin version upgrades:
Add .mvn/jvm.config with necessary --add-exports and --add-opens for internal jdk.compiler packages.
This ensures that build-time tools and compiler plugins interacting with javac internals can function correctly under Java 17's module system.
Centralize maven plugin versions in the root pom's pluginManagement section.
This is a coordinated effort across GeoTools, GeoServer, and GeoWebCache