[Gradle JDK Automanagement] Install only used JDKs #479
Conversation
| /** | ||
| * Option to include a specific java major version when generating/checking the gradle jdk configuration files. | ||
| */ | ||
| @Input | ||
| @Option( | ||
| option = "includeJava", | ||
| description = "Generates the jdk configuration directories for the Java major version.") | ||
| public abstract SetProperty<String> getIncludedJavaMajorVersion(); |
There was a problem hiding this comment.
I think we could avoid this if we were to regenerate the jdk configs at the same time (ie same gradle invocation in excavator) as bumping the versions? The problem should only present itself when changing the versions used, then trying to run gradle again but the wrong JDKs are included. I think we might be able to do that.
If that doesn't work, I think I'd prefer an option to just use all the configured versions rather than including specific ones. That way we don't need to keep updating excavators with specific versions, they can just include everything they need.
There was a problem hiding this comment.
I think we could avoid this if we were to regenerate the jdk configs at the same time (ie same gradle invocation in excavator) as bumping the versions? The problem should only present itself when changing the versions used, then trying to run gradle again but the wrong JDKs are included. I think we might be able to do that.
I am not sure how that would work. If we want to bump a version of jdk used (eg. javaVersions libraryTarget = 21), when running any gradle task ./gradlew setupJdks we don't have the jdk files configured so we are not installing anything. Then if jdk 21 doesn't exist, gradle will complain during the configuration phase that no toolchain was found for java == 21 when it was configuring eg. javaCompiler.
There was a problem hiding this comment.
Ah, makes sense, of course we have to do it in two invocations.
I think I'd just prefer the option then to use all the JDKs rather than opting into specific ones.
There was a problem hiding this comment.
I wonder if this should just be something you can set via an environment variable instead, so we can just set export GRADLE_JDKS_INSTALL_ALL_JDKS=true in the excavator and not have to worry about it setting it on all the different commands.
There was a problem hiding this comment.
Also probably need a test for this behaviour, whatever it ends up being.
|
waiting on #493 first & on updating the excavators. |
| @@ -47,6 +53,9 @@ public void apply(Project rootProject) { | |||
| + "Gradle version in order to use the JDK setup.", | |||
| GradleJdksEnablement.MINIMUM_SUPPORTED_GRADLE_VERSION)); | |||
| } | |||
| if (areFlowActionsSupported()) { | |||
| rootProject.getPluginManager().apply(ToolchainFailureFlowActionsPlugin.class); | |||
There was a problem hiding this comment.
the error message will only be available for repos >= 8.6
| : String.format("the language versions=%s", missingToolchains); | ||
| log.error( | ||
| "\n" | ||
| + "\u001B[31m****************************************************************************************************\n" |
There was a problem hiding this comment.
Logging with the red formatter, so it easy easy to spot:
| parameters.getBuildResult().get().getFailure().ifPresent(failure -> { | ||
| List<Throwable> noToolchainsAvailable = Throwables.getCausalChain(failure).stream() | ||
| .filter(throwable -> | ||
| throwable instanceof org.gradle.jvm.toolchain.internal.NoToolchainAvailableException) |
There was a problem hiding this comment.
this is an internal class, the testing-plugin would be useful here to make sure this workflow still works in more recent versions of gradle.
I had to add the full qualified name of the class here, because importing this class will fail the checkstyle.
gradle-jdks-setup/README.md
Outdated
| 2. Secondly, once the new files are generated, the javaVersion can be configured in the gradle build configuration. | ||
|
|
||
| ### Palantir specific, if using `baseline-java-version` through excavators | ||
| 1. Run `./gradlew generateGradleJdkConfigs --includeAllJdks`. The command will generate the Gradle jdk configuration files for all jdks. |
There was a problem hiding this comment.
all jdks the plugin is aware of.
|
|
||
| ### Manual bumps of the java versions | ||
| 1. Run `./gradlew generateGradleJdkConfigs --includeVersion=<newVersion> [--includeVersion=<otherNewVersion>]`. The command will generate the Gradle jdk configuration files for the major versions passed as parameters. | ||
| 2. Update the javaVersion(s)/jdks extension values with the desired java major version. |
There was a problem hiding this comment.
We still need to run ./gradlew setupJdks even with this approach as the user may stop using the old version, and we need to remove it
There was a problem hiding this comment.
Addressed, I thought I had the logic in the check task to test if there are any extra configuration files in gradle/jdks. I added now that logic in: 1710e98#diff-53f3309ab573fe65d62f8b5bf40e9bb94478d82b4d6434cb6bcf9b25516813a4R134
Before this PR
Atm we are generating jdk configurations for all jdks that are configured by gradle-jdks-latest. This means extra work if the repository only uses a subset of those JDKs. This is especially relevant in the context of circle VMs - we end up installing jdks that might not be used during circle ci jobs.
After this PR
Continuation for: #472.
gradle-baselineplugins.gw generateGradleJdkConfigs --includeAllJdksthat can generate all the jdk configuration files. Required by the excavator runs internally,gw generateGradleJdkConfigs --includeVersion=<newJavaMajorVersion>that can generate the jdk configuration files for new major versions.==COMMIT_MSG==
Install only used JDKs
==COMMIT_MSG==
Possible downsides?