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

Update Gradle Tooling API wrapper to 8.5 #7022

Closed
wants to merge 1 commit into from

Conversation

mbien
Copy link
Member

@mbien mbien commented Feb 1, 2024

  • bump from 8.4 to 8.5
  • gradle 8.5 now supports running on JDK 21 release notes

targets delivery

8.5 now supports running on JDK 21
@mbien mbien added Gradle [ci] enable "build tools" tests Upgrade Library Library (Dependency) Upgrade ci:all-tests [ci] enable all tests ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels Feb 1, 2024
@mbien mbien added this to the NB21 milestone Feb 1, 2024
Copy link
Contributor

@lkishalmi lkishalmi left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks!

@sdedic
Copy link
Member

sdedic commented Feb 1, 2024

Does not manifest.mf module version need bumping ?

@mbien
Copy link
Member Author

mbien commented Feb 1, 2024

Does not manifest.mf module version need bumping ?

@sdedic AFAIK no, since this is done once per release anyway via #6576, #6959

@mbien
Copy link
Member Author

mbien commented Feb 2, 2024

seems to be working. It didn't complain about JDK 21 when I tested it.

@neilcsmith-net
Copy link
Member

neilcsmith-net commented Feb 2, 2024

I won't be able to test until next week, but on principle I'm -0.5 on this one.

Why? What issues are fixed with this? The tooling API is meant to be forward compatible, and afaik does not suffer from the same max JDK concerns. So what problems with it or our use of it require an update just as we're about to finalise the release? And are there other ways to address those now or in future so that doesn't happen again?

EDIT : I also hadn't noticed this was skipped when #6926 was being discussed.

@lkishalmi
Copy link
Contributor

Well, there is one painful shortcoming in NB Gradle support. The Boot Classpath of a project is actually taken from the Runtime Classpath:

JavaPlatform platform = JavaRunUtils.getActivePlatform(project).second();

That would make the use of the new Java 21 API-s uncomfortable (would not be appear, marked red, but would compile after all)

@mbien
Copy link
Member Author

mbien commented Feb 2, 2024

I won't be able to test until next week, but on principle I'm -0.5 on this one.

Why? What issues are fixed with this? The tooling API is meant to be forward compatible,

I actually don't have any race horses in the race here or whatever the correct sentence is :)

NB 21rc2 on JDK 21:
image
image

With this PR applied I didn't get the "resolve problems dialog". I simply assumed that we forgot to update the lib wrapper to 8.5, so I quickly opened this PR. The gradle tests can also currently not run on JDK 21 which I was re minded of while doing #7019.

@mbien
Copy link
Member Author

mbien commented Feb 3, 2024

closing in favor of #6926 since 8.6 was just released. To make neil's decision easier to figure out what to merge ;)

@mbien mbien closed this Feb 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:all-tests [ci] enable all tests ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) Gradle [ci] enable "build tools" tests Upgrade Library Library (Dependency) Upgrade
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants