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

Bump JDK 17 and using toolchain #1392

Closed
wants to merge 2 commits into from

Conversation

Jaehwa-Noh
Copy link
Contributor

@Jaehwa-Noh Jaehwa-Noh commented Apr 23, 2024

What I have done and why
• Replace sourceCompatibility and targetCompatibility to toolchain
• Add toolchain resolver org.gradle.toolchains.foojay-resolver-convention
• Bump JDK11 to JDK17

JDK 17

In this issuetracker shows, no need to fix the version of JDK at 11 to use desugaring.
https://issuetracker.google.com/issues/343505126

toolchain

According this documentation

We recommend that you always specify the Java toolchain, and either ensure that the specified JDK is installed, or add a toolchain resolver to your build.

If not specified, this defaults to the Java toolchain or JDK used to run Gradle. We recommend that you always explicitly specify a toolchain (preferred) or sourceCompatibility.

resolver

documentation

If you use Gradle 8.0.2 or higher, you also need to add a toolchain resolver plugin. This type of plugin manages which repositories to download a toolchain from. As an example, add to your settings.gradle(.kts) the following plugin:

Fix #1483

How I'm testing it

Choose at least one:

  • Unit tests
  • UI tests
  • Screenshot tests
  • N/A (provide justification)

@Jaehwa-Noh Jaehwa-Noh marked this pull request as draft April 23, 2024 08:04
@Jaehwa-Noh Jaehwa-Noh marked this pull request as ready for review April 23, 2024 10:56
@Jaehwa-Noh Jaehwa-Noh marked this pull request as draft May 30, 2024 07:22
@Jaehwa-Noh Jaehwa-Noh changed the title Preferred toolchain use Bump JDK 17 and using toolchain May 30, 2024
@Jaehwa-Noh Jaehwa-Noh marked this pull request as ready for review May 30, 2024 11:38
@keyboardsurfer
Copy link
Member

Thanks for your PR. Currently there are conflicts that need to be resolved before we can move on to possibly accepting your contribution. Please resolve the conflicts.

@Jaehwa-Noh Jaehwa-Noh marked this pull request as draft December 20, 2024 07:41
Change-Id: I53d35fc503e4390bc6e2ac58504d359e7ddacffd
Change-Id: Ic59bea192e57190251592beb88fd31297f1671a4
@Jaehwa-Noh Jaehwa-Noh marked this pull request as ready for review March 4, 2025 00:34
@Jaehwa-Noh
Copy link
Contributor Author

@keyboardsurfer Ready to merge.
Now the Gradle will run on jdk 17 regardless on developers gradle environment.

@javadude
Copy link
Contributor

javadude commented Mar 4, 2025

Hi! We've recently changed our direction on using toolchain for our samples. We still recommend it for developer projects, but we want to limit 3P Gradle plugins (in this case, the foojay resolver) that we use in samples.

To that end, we want to keep sourceCompatibility, targetCompatibility, and jvmTarget, and not use jvmToolchain in NiA.

The convention plugin has been updated to JDK 17, so I'll close this PR.

@javadude javadude closed this Mar 4, 2025
@Jaehwa-Noh
Copy link
Contributor Author

@javadude I just have a question.
If you don't mind, would you tell me the reason to limit 3P Gradle plugins such as foojay resolver?
It is good chance to notice it to other contributors.

In the libs.versions.toml, some sort of 3P Gradle plugins exist: module-graph, roborazzi and spotless.
Is there any differs between foojay and others?

@javadude
Copy link
Contributor

javadude commented Mar 5, 2025

Sure. We're considering maintenance over the longer term. Unfortunately, we don't have control over 3P plugins, so they can disappear at any time, they may lose maintainers and break when Gradle APIs change, or worse, be taken over by a malicious party. We're especially sensitive to Gradle plugins here, as they run on the system with all the rights of whatever user runs the build.

If there's a reasonable alternative (in this case, sticking with the source/targetCompatibility and jvmTarget, we prefer to keep it as basic as possible)

@Jaehwa-Noh
Copy link
Contributor Author

@javadude I understand your considers.
Actually foojay plugin is managed by Gradle (https://github.com/gradle/foojay-toolchains) and adressed in the official Gradle documentation (https://docs.gradle.org/current/userguide/toolchains.html#sub:download_repositories).
However, according your words, the problem is that this plugin using FooJay API for searching and downloading SDK.

If I find any official plugins, I will soon create new PR.
Or if it is okay to remove just foojay plugin in this PR, kindly let me know any time. I'll reopen and edit this PR.
Thank you.

@javadude
Copy link
Contributor

javadude commented Mar 5, 2025

Without a resolver, tool chain becomes painful if the exact JDK version isn't installed.

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.

[Bug]: Desugaring doesn't need to fix at JDK 11 version.
3 participants