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.

Sign up for GitHub

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

[Gradle JDK Automanagement] Install only used JDKs #479

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from
Open

[Gradle JDK Automanagement] Install only used JDKs #479

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
75a946e
java versions
crogoz Nov 22, 2024
7a71212
default ot false
crogoz Nov 22, 2024
48ec497
.
crogoz Nov 22, 2024
7a57614
Add generated changelog entries
svc-changelog Nov 22, 2024
dbc81da
address comments
crogoz Nov 25, 2024
33ab223
readme
crogoz Nov 25, 2024
25ce299
fix
crogoz Nov 25, 2024
2d592ff
wip
crogoz Nov 25, 2024
374e740
option
crogoz Nov 25, 2024
3eabf8c
old gradle-jdks
crogoz Nov 25, 2024
60577fe
libraryTarget first
crogoz Nov 25, 2024
d2402de
.
crogoz Nov 26, 2024
72999fe
.
crogoz Nov 26, 2024
a79d9e3
Merge branch 'develop' into cr/filter-gradle-jdks
crogoz Dec 9, 2024
2d2bf0e
multiple targets
crogoz Dec 9, 2024
92baa20
tests
crogoz Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions changelog/@unreleased/pr-472.v2.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type: fix
fix:
description: Add `daemonJdkOnly ` property for gradle jdk directory generation
links:
- https://github.com/palantir/gradle-jdks/pull/472
11 changes: 11 additions & 0 deletions gradle-jdks/src/main/java/com/palantir/gradle/jdks/GradleJdksConfigs.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,11 @@
import org.gradle.api.file.Directory;
import org.gradle.api.provider.MapProperty;
import org.gradle.api.provider.Property;
import org.gradle.api.provider.SetProperty;
import org.gradle.api.tasks.Input;
import org.gradle.api.tasks.Nested;
import org.gradle.api.tasks.TaskAction;
import org.gradle.api.tasks.options.Option;
import org.gradle.jvm.toolchain.JavaLanguageVersion;

/**
Expand All @@ -43,6 +45,15 @@ public abstract class GradleJdksConfigs extends DefaultTask {
public static final String GRADLE_JDKS_SETUP_SCRIPT = "gradle-jdks-setup.sh";
public static final String GRADLE_JDKS_FUNCTIONS_SCRIPT = "gradle-jdks-functions.sh";

/**
* 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();
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@crogoz crogoz Dec 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also probably need a test for this behaviour, whatever it ends up being.


@Nested
public abstract MapProperty<JavaLanguageVersion, List<JdkDistributionConfig>> getJavaVersionToJdkDistros();

Expand Down
10 changes: 8 additions & 2 deletions gradle-jdks/src/main/java/com/palantir/gradle/jdks/JdkDistributionConfigurator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.gradle.api.Project;
import org.gradle.api.logging.Logger;
import org.gradle.api.logging.Logging;
import org.gradle.api.provider.SetProperty;
import org.gradle.jvm.toolchain.JavaLanguageVersion;

public final class JdkDistributionConfigurator {
Expand All @@ -36,8 +37,13 @@ public final class JdkDistributionConfigurator {
private static final JavaLanguageVersion MINIMUM_SUPPORTED_JAVA_VERSION = JavaLanguageVersion.of(11);

public static Map<JavaLanguageVersion, List<JdkDistributionConfig>> getJavaVersionToJdkDistros(
Project project, JdkDistributions jdkDistributions, JdksExtension jdksExtension) {
Set<JavaLanguageVersion> javaVersions = jdksExtension.getJdkMajorVersionsToUse().get().stream()
Project project,
JdkDistributions jdkDistributions,
JdksExtension jdksExtension,
SetProperty<String> extraLanguageVersions) {
Set<JavaLanguageVersion> javaVersions = Stream.concat(
jdksExtension.getJdkMajorVersionsToUse().get().stream(),
extraLanguageVersions.get().stream().map(JavaLanguageVersion::of))
.filter(javaLanguageVersion -> javaLanguageVersion.canCompileOrRun(MINIMUM_SUPPORTED_JAVA_VERSION))
.collect(Collectors.toSet());
return javaVersions.stream()
Expand Down
37 changes: 31 additions & 6 deletions gradle-jdks/src/main/java/com/palantir/gradle/jdks/ToolchainsPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,19 @@

package com.palantir.gradle.jdks;

import com.palantir.baseline.plugins.javaversions.BaselineJavaVersionExtension;
import com.palantir.baseline.plugins.javaversions.BaselineJavaVersionsExtension;
import com.palantir.baseline.plugins.javaversions.ChosenJavaVersion;
import com.palantir.gradle.jdks.GradleWrapperPatcher.GradleWrapperPatcherTask;
import com.palantir.gradle.jdks.enablement.GradleJdksEnablement;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.gradle.api.Plugin;
import org.gradle.api.Project;
import org.gradle.api.Task;
import org.gradle.api.logging.Logger;
import org.gradle.api.logging.Logging;
import org.gradle.api.provider.Provider;
import org.gradle.api.tasks.TaskProvider;
import org.gradle.api.tasks.wrapper.Wrapper;
import org.gradle.language.base.plugins.LifecycleBasePlugin;
Expand Down Expand Up @@ -58,12 +63,32 @@ public void apply(Project rootProject) {
JdksExtension jdksExtension = JdksPlugin.extension(rootProject, jdkDistributions);

rootProject.getPluginManager().withPlugin("com.palantir.baseline-java-versions", unused -> {
rootProject
.getExtensions()
.getByType(BaselineJavaVersionsExtension.class)
.getSetupJdkToolchains()
.set(false);
BaselineJavaVersionsExtension baselineJavaVersionsExtension =
rootProject.getExtensions().getByType(BaselineJavaVersionsExtension.class);
baselineJavaVersionsExtension.getSetupJdkToolchains().set(false);

jdksExtension.getJdkMajorVersionsToUse().set(rootProject.provider(() -> Stream.of(
jdksExtension.getDaemonTarget(),
baselineJavaVersionsExtension.libraryTarget(),
baselineJavaVersionsExtension.runtime().map(ChosenJavaVersion::javaLanguageVersion),
baselineJavaVersionsExtension
.distributionTarget()
.map(ChosenJavaVersion::javaLanguageVersion))
.map(Provider::get)
.collect(Collectors.toSet())));
});

rootProject.subprojects(
proj -> proj.getPluginManager().withPlugin("com.palantir.baseline-java-version", unused -> {
BaselineJavaVersionExtension projectVersions =
proj.getExtensions().getByType(BaselineJavaVersionExtension.class);

jdksExtension.getJdkMajorVersionsToUse().addAll(rootProject.provider(() -> Stream.of(
projectVersions.target(), projectVersions.runtime())
.map(Provider::get)
.map(ChosenJavaVersion::javaLanguageVersion)
.collect(Collectors.toSet())));
}));
TaskProvider<Wrapper> wrapperTask = rootProject.getTasks().named("wrapper", Wrapper.class);

TaskProvider<GenerateGradleJdksConfigsTask> generateGradleJdkConfigs = rootProject
Expand All @@ -89,7 +114,7 @@ public void apply(Project rootProject) {
task.getDaemonJavaVersion().set(jdksExtension.getDaemonTarget());
task.getJavaVersionToJdkDistros()
.putAll(rootProject.provider(() -> JdkDistributionConfigurator.getJavaVersionToJdkDistros(
rootProject, jdkDistributions, jdksExtension)));
rootProject, jdkDistributions, jdksExtension, task.getIncludedJavaMajorVersion())));
task.getCaCerts().putAll(jdksExtension.getCaCerts());
});

Expand Down
38 changes: 37 additions & 1 deletion ...e-jdks/src/test/groovy/com/palantir/gradle/jdks/GradleJdkToolchainsIntegrationTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,42 @@ class GradleJdkToolchainsIntegrationTest extends GradleJdkIntegrationSpec {
gradleVersionNumber << GRADLE_TEST_VERSIONS
}

def '#gradleVersionNumber: only required java versions are configured'() {
gradleVersion = gradleVersionNumber
setupJdksHardcodedVersions('17')
applyBaselineJavaVersions()
applyApplicationPlugin()

file('gradle.properties') << 'palantir.jdk.setup.enabled=true'
file('src/main/java/Main.java') << java17PreviewCode

// language=Groovy
buildFile << """
javaVersions {
libraryTarget = '17'
}
""".stripIndent(true)

//language=groovy
def subprojectLib21 = addSubproject 'subproject-lib-21', '''
apply plugin: 'java-library'
javaVersion {
target = 17
runtime = 21
}
'''.stripIndent(true)
writeJavaSourceFile(getMainJavaCode(), subprojectLib21)

when:
runTasksSuccessfully('wrapper')

then: 'generates directories for all jdk versions'
Files.list(projectDir.toPath().resolve("gradle/jdks")).map { it -> it.getFileName().toString() }.collect(Collectors.toList()).containsAll(List.of("17", "21"))

where:
gradleVersionNumber << GRADLE_TEST_VERSIONS
}

def '#gradleVersionNumber: fails if the jdk version is not configured'() {
setupJdksHardcodedVersions()
applyBaselineJavaVersions()
Expand All @@ -219,7 +255,7 @@ class GradleJdkToolchainsIntegrationTest extends GradleJdkIntegrationSpec {

buildFile << '''
javaVersions {
distributionTarget = '15'
libraryTarget = 15
}
'''.stripIndent(true)
writeHelloWorld(projectDir)
Expand Down