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

Support for Gradle Java Toolchains #2211

Closed
sihutch opened this issue Jul 29, 2024 · 5 comments
Closed

Support for Gradle Java Toolchains #2211

sihutch opened this issue Jul 29, 2024 · 5 comments

Comments

@sihutch
Copy link

sihutch commented Jul 29, 2024

Gradle version: 7.6.4
Spotless version: 6.21.0
Java version: Coretto 17.0.11
Operating system and version: Mac OS 14.5

The Spotless Gradle Plugin does not support Gradle Java Toolchains

If Gradle is run on a Java 17 JDK. But we compile/test Java 21 code using the following toolchain configuration

java {
    toolchain {
        languageVersion = JavaLanguageVersion.of(21)
    }
}

Gradle can compile the Java 21 code but the Spotless Gradle Plugin does not use the toolchain JDK and fails with an exception when executing

./gradlew clean spotlessCheck

I have created a basic project with steps to recreate the problem.

Do you have any plans to support Java Toolchains?

@Goooler

This comment was marked as resolved.

@Goooler Goooler closed this as not planned Won't fix, can't repro, duplicate, stale Jul 31, 2024
@Goooler Goooler reopened this Jul 31, 2024
@mr-serjey
Copy link

Hi @Goooler, this issue is relevant for my project as well and I feel that I could contribute to fix the issue. But before I start working on PR, I wanted to make sure that implementation idea is viable...

The idea is to:

  1. Make FormatExtension.lazyActions aware of Gradle Jvm toolchain configuration.

  2. Migrate SpotlessTaskImpl to execute operations via Worker API (again with process isolation and with respect of toolchain configuration)

@Goooler, please let me know what you think about it and if you have any suggestions.

@aciccarello
Copy link

Looks like this might be a duplicate of #724

@Goooler
Copy link
Member

Goooler commented Aug 6, 2024

@mr-serjey Thanks for your response! I would prefer the second. WDYT? @nedtwigg

@nedtwigg
Copy link
Member

Correct, this is a dupe of #724, even though this issue has the better discussion, I'm going to close this one so that watchers of the original issue will get notified if something gets merged.

I don't have a preference for 1 vs 2, so I defer to the implementor and I think @Goooler has good insight that option 2 is probably easier. But I think that migrating the defaultVersion() concept in the Jvm.Support class is non-optional, the user experience will be poor without it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants