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

Set the java launcher for Checkstyle tasks, too #2351

Merged
merged 4 commits into from
Aug 8, 2022

Conversation

j-baker
Copy link
Contributor

@j-baker j-baker commented Aug 8, 2022

The gradle-jdks plugin aims to download JDKs from a source and use them
at runtime. In theory, this means that in order to develop using such a
library, one only needs a JDK installed that can run Gradle (there are
some limitations but this is the theory).

Inside the Checkstyle task is a Java launcher property. When this is
resolved, it will query the Java version service, which will query
the configured JDKs. This means that the baseline-java-versions
configured JDK is not used, and if the gradle-jdks approach is used, the
Checkstyle task will fail.

I think that this change is enough to correctly override the Java
launcher used.

Fixes palantir/gradle-jdks#93

The gradle-jdks plugin aims to download JDKs from a source and use them
at runtime. In theory, this means that in order to develop using such a
library, one only needs a JDK installed that can run Gradle (there are
some limitations but this is the theory).

Inside the Checkstyle task is a Java launcher property. When this is
resolved, it will query the Java version service, which will query
the configured JDKs. This means that the baseline-java-versions
configured JDK is not used, and if the gradle-jdks approach is used, the
Checkstyle task will fail.

I _think_ that this change is enough to correctly override the Java
launcher used.

Fixes palantir/gradle-jdks#93
@changelog-app
Copy link

changelog-app bot commented Aug 8, 2022

Generate changelog in changelog/@unreleased

Type
See change types. Select one:

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Set the java launcher for Checkstyle tasks, too

Check the box to generate changelog(s)

  • Generate changelog entry

@j-baker
Copy link
Contributor Author

j-baker commented Aug 8, 2022

I verified that this fixed my problem locally. I wasn't sure how to write a test that isn't overengineered. Concretely my problem is that it's failing on CI, where my Java version is not installed by default.

public void execute(Checkstyle checkstyle) {
checkstyle.getJavaLauncher().set(javaToolchain.flatMap(BaselineJavaToolchain::javaLauncher));
}
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Mind adding a test that this resolves?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not entirely sure what this means. What sort of test would you like me to write? I'm not really sure how to write the Checkstyle equivalent of the Java one.

@bulldozer-bot bulldozer-bot bot merged commit 4405aec into develop Aug 8, 2022
@bulldozer-bot bulldozer-bot bot deleted the jbaker/override_for_checkstyle branch August 8, 2022 12:35
@svc-autorelease
Copy link
Collaborator

Released 4.153.0

This was referenced Aug 8, 2022
@pkoenig10
Copy link
Member

Just want to flag that this bumps the minimum Gradle version to 7.5.

https://github.com/gradle/gradle/blob/af40a01f65c321846765e40500a5271a868ffd15/subprojects/code-quality/src/main/groovy/org/gradle/api/plugins/quality/Checkstyle.java#L180-L189

We likely have quite a few internal repos that are not yet on Gradle 7.5.

@carterkozak
Copy link
Contributor

Good flag, let's add a check rather than increasing the minimum supported gradle release: #2360

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

Successfully merging this pull request may close these issues.

Gradle provisioning is applied if the user does not have the relevant JDK already installed
5 participants