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

Initial preparation for Java 21 #2986

Merged
merged 26 commits into from May 14, 2024
Merged

Initial preparation for Java 21 #2986

merged 26 commits into from May 14, 2024

Conversation

LorenzoBettini
Copy link
Contributor

This is part of #2686

It adds another GitHub Actions workflow for building with JDK21 (only in Linux to avoid matrix explosion), making sure that everything builds and tests are green with Java 21 and Java 17.

Many tests had to be adapted:

  • either by adding stub classes to avoid tests being fragile w.r.t. new Java API, both concerning new added methods (e.g., getLast) and changes in supertype hierarchies (added SequencedCollection: List extends SequencedCollection in Java 21)
  • or by adapting tests according to the Java version (it's already been done in the past); e.g., the new private method in Object wait0.
  • A particular case: strictfp doesn't seem to be present in the byte code anymore when compiling with JDK 21 so the corresponding tests are skipped with Java 21. By the way, a few tests where strictfp was tested contained a copy-and-paste error and publicFinalMethod was never tested (this has been fixed)
  • lastOrNull (Added IterableExtensions.lastOrNull and deprecated last #2983) is used only when strictly needed in test inputs.
  • maven-plugin-plugin has been updated or it couldn't build plugin descriptor for Java 21

IMPORTANT: this does NOT make the JDK compile with target 21: the target is still 11; so, it doesn't test byte code with Java version 21; at least, that's my understanding. This means that Maven jdt.core has NOT been upgraded yet; similarly, the Java enum has not been updated. That part will be dealt with in a further PR.

JDK 21 is enforced with the toolchains: an automatic Maven profile is activated automatically. In that profile, the release option is 21 as well. Toolchains is not activated otherwise.

This is just the initial step towards Java 21, and I think it might be worthwhile to merge, if you think that's fine.

@cdietrich
Copy link
Member

cdietrich commented Apr 10, 2024

what was the background behind the SuppressWarnings change? (and other stubs)

in the past we did if (JavaRuntimeVersion.isJava17OrLater()) ....

@LorenzoBettini
Copy link
Contributor Author

what was the background behind the SuppressWarnings change? (and other stubs)

It's documented here 2791444#diff-331411999fe50a10ba8910972acaf1b716881e03a7f535f17573f49c8aabe528R18
In Java 21 the annotation lost this part @Target({ TYPE, FIELD, METHOD, PARAMETER, CONSTRUCTOR, LOCAL_VARIABLE, MODULE })

in the past we did if (JavaRuntimeVersion.isJava17OrLater()) ....

Concerning the stub classes in general it was suggested/discussed here #2686 (comment) by @szarnekow

When it was unavoidable or simply easier I used the JavaRuntimeVersion but for other cases I created stubbed classes to keep use safer for future Java API evolution and to have our tests less fragile w.r.t. Java API evolutions.

@LorenzoBettini
Copy link
Contributor Author

@cdietrich @szarnekow I rebased the PR on top of current main.

Could you please give it a look and in case approve it so that I can continue working on the Java 21 issue?

@cdietrich
Copy link
Member

i am ok with the changes. @szarnekow wdyt

@LorenzoBettini
Copy link
Contributor Author

I rebased on the current main.

I recently added this 8f8b905
I know it sounds very strange, but that's the only way I found to make Maven/Tycho combination happy in using the toolchains generated by setup-java. I prefer to use that one especially if we also build with macOS Arm (#3015) because manually managing the toolchains would become harder; it's not a problem in JIRO instead.

Finally, I'd like this one to be merged ASAP, please: my other fork's branch where I further experiment with Java 21 is build on top of this, and rebasing both this and the other branch is becoming a burden.

cc @cdietrich @szarnekow

@LorenzoBettini LorenzoBettini force-pushed the lb_java_21 branch 2 times, most recently from ef482c9 to ad4edb4 Compare April 30, 2024 10:00
otherwise the descriptor goals fails analyzing class files compiled with
Java 21
because in Java 21
getLast() has been introduced, throwing NoSuchElementException
instead of returning null
To avoid interefence with the new Java 21 method getLast
or the test will fail in Java 21
The test publicNativeMethod was indeed testing "publicStrictFpMethod":
copy and paste error?
@LorenzoBettini
Copy link
Contributor Author

@cdietrich @szarnekow I rebased and I'm waiting for the build to finished, then I'll merge this one and prepare the PR for the actual Java 21 support.

@LorenzoBettini LorenzoBettini merged commit aa5fb22 into main May 14, 2024
10 of 11 checks passed
@LorenzoBettini LorenzoBettini deleted the lb_java_21 branch May 14, 2024 16:50
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.

None yet

2 participants