Set `JAVA_HOME` in gradle-jdks-setup script #447

crogoz · 2024-10-24T14:06:09Z

Before this PR

In case there is no JAVA_HOME, then ./gradlew will look at java executable (https://github.com/palantir/gradle-jdks/blob/develop/gradlew#L142). If no java is installed, then the ./gradlew command will fail.

example: https://app.circleci.com/pipelines/github/palantir/gradle-jdks/2274/workflows/66d6abd4-88f9-4525-aed9-e91363f1921a/jobs/6532 when trying to drop the "Install Java" step from the circle-ci template (369952b#diff-78a8a19706dbd2a4425dd72bdab0502ed7a2cef16365ab7030a5a0588927bf47L93-L98)

After this PR

Gradle JDK Automanagement will also set JAVA_HOME used by ./gradlew to launch Gradle (https://github.com/palantir/gradle-jdks/blob/develop/gradlew#L128-L150).

==COMMIT_MSG==
gradle-jdks-setup.sh script sets JAVA_HOME to gradle JDK
==COMMIT_MSG==

Possible downsides?

changelog-app · 2024-10-24T14:06:12Z

Generate changelog in `changelog/@unreleased`

What do the change types mean?

feature: A new feature of the service.
improvement: An incremental improvement in the functionality or operation of the service.
fix: Remedies the incorrect behaviour of a component of the service in a backwards-compatible way.
break: Has the potential to break consumers of this service's API, inclusive of both Palantir services
and external consumers of the service's API (e.g. customer-written software or integrations).
deprecation: Advertises the intention to remove service functionality without any change to the
operation of the service itself.
manualTask: Requires the possibility of manual intervention (running a script, eyeballing configuration,
performing database surgery, ...) at the time of upgrade for it to succeed.
migration: A fully automatic upgrade migration task with no engineer input required.

Note: only one type should be chosen.

How are new versions calculated?

❗The break and manual task changelog types will result in a major release!
🐛 The fix changelog type will result in a minor release in most cases, and a patch release version for patch branches. This behaviour is configurable in autorelease.
✨ All others will result in a minor version release.

Type

Description

gradle-jdks-setup.sh script sets JAVA_HOME to gradle JDK

Check the box to generate changelog(s)

Generate changelog entry

…r/java-home

CRogers · 2024-10-24T15:50:59Z

...ationTest/java/com/palantir/gradle/jdks/setup/GradleJdkInstallationSetupIntegrationTest.java

-        caResources
-                .readPalantirRootCaFromSystemTruststore()
-                .map(AliasContentCert::getContent)
-                .ifPresent(content -> {
-                    try {
-                        Files.write(workingDir.resolve("palantir.crt"), content.getBytes(StandardCharsets.UTF_8));
-                    } catch (IOException e) {
-                        throw new RuntimeException("Failed to write cert", e);
-                    }
-                });


No longer need this as we just read certs from the system truststore right?

CRogers · 2024-10-24T15:52:02Z

gradle-jdks-setup/src/integrationTest/resources/testing-script.sh

+# >>> Gradle JDK setup >>>
+# !! Contents within this block are managed by 'palantir/gradle-jdks' !!
+if [ -f gradle/gradle-jdks-setup.sh ]; then
+    if ! . gradle/gradle-jdks-setup.sh; then
+        echo "Failed to set up JDK, running gradle/gradle-jdks-setup.sh failed with non-zero exit code"
+        exit 1
+    fi
+    # Setting JAVA_HOME to the gradle daemon to make sure gradlew uses this jdk for `JAVACMD`
+    JAVA_HOME="$GRADLE_DAEMON_JDK"
 fi
-echo "Example.com cert:" $(/root/.gradle/gradle-jdks/amazon-corretto-11.0.21.9.1/bin/keytool -keystore /root/.gradle/gradle-jdks/amazon-corretto-11.0.21.9.1/lib/security/cacerts -list -alias gradleJdks_example.com -storepass changeit)
+# <<< Gradle JDK setup <<<


Is this manually placed here or is it kept up to date with gradlew-patch.sh somehow?

crogoz · 2024-10-25T09:16:45Z

gradle-jdks-setup/build.gradle

@@ -42,7 +42,7 @@ tasks.integrationTest {
    dependsOn tasks.fatJar
 }

-tasks.build.dependsOn tasks.fatJar
+tasks.build.dependsOn tasks.fatJar, tasks.integrationTest


the integration tests were not running before :(

crogoz · 2024-10-25T09:18:32Z

gradle-jdks-setup/src/main/resources/gradle-jdks-setup.sh

@@ -74,6 +74,7 @@ gradle_daemon_jdk_distribution_local_path=$(read_value "$APP_GRADLE_DIR"/jdks/"$
 "$GRADLE_JDKS_HOME"/"$gradle_daemon_jdk_distribution_local_path"/bin/java -cp "$APP_GRADLE_DIR"/gradle-jdks-setup.jar com.palantir.gradle.jdks.setup.GradleJdkInstallationSetup daemonSetup "$APP_HOME" "$GRADLE_JDKS_HOME/$gradle_daemon_jdk_distribution_local_path"

 # [Used by ./gradlew only] Setting the Gradle Daemon Java Home to the JDK distribution
-set -- "-Dorg.gradle.java.home=$GRADLE_JDKS_HOME/$gradle_daemon_jdk_distribution_local_path" "$@"
+export GRADLE_DAEMON_JDK="$GRADLE_JDKS_HOME/$gradle_daemon_jdk_distribution_local_path"


I am not setting the JAVA_HOME directly here as this script is called by the palantir-gradle-jdks intellij plugin as well and I don't think we should modify the JAVA_HOME there (when gradle jdk automanagement is set, then we also set the Project SDK)

I am I right in thinking setting JAVA_HOME wouldn't really do anything bad though? We'd set the JAVA_HOME then the script would end to no real effect (when called from intellij in the before gradle step right?)

Not that I think it really matters, the export GRADLE_DAEMON_JDK approach here is also fine.

I am I right in thinking setting JAVA_HOME wouldn't really do anything bad though? We'd set the JAVA_HOME then the script would end to no real effect (when called from intellij in the before gradle step right?

I think you are right (I tested this initially inside Intellij) and it seemed fine. But I wouldn't want to add to any confusion if in the end something inside Intellij changes and the java home is somehow used.

autorelease3 · 2024-10-25T16:40:05Z

Released 0.52.0

crogoz added 2 commits October 24, 2024 16:00

set java home

d2fab83

fix no java_home set

bcbcb4b

crogoz changed the title ~~Cr/java home~~ Set JAVA_HOME in gradle-jdks-setup script Oct 24, 2024

svc-changelog and others added 10 commits October 24, 2024 14:06

Add generated changelog entries

b97f469

set

1cf1a67

Merge branch 'cr/java-home' of github.com:palantir/gradle-jdks into c…

4cdd6c7

…r/java-home

set

39acc92

checl

e269cf6

test hava_home

6794c75

add certs

fa57ae2

add certs

8c2a628

java home

0726559

test output

b6bb8d8

CRogers reviewed Oct 24, 2024

View reviewed changes

gradlew patch

6f1043b

crogoz commented Oct 25, 2024

View reviewed changes

fix output format

6f9a3e7

crogoz requested a review from CRogers October 25, 2024 09:59

CRogers approved these changes Oct 25, 2024

View reviewed changes

crogoz added autorelease merge when ready labels Oct 25, 2024

CRogers closed this Oct 25, 2024

CRogers reopened this Oct 25, 2024

bulldozer-bot bot merged commit 2447cc1 into develop Oct 25, 2024
6 checks passed

bulldozer-bot bot deleted the cr/java-home branch October 25, 2024 16:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set `JAVA_HOME` in gradle-jdks-setup script #447

Set `JAVA_HOME` in gradle-jdks-setup script #447

crogoz commented Oct 24, 2024 •

edited

Loading

changelog-app bot commented Oct 24, 2024 •

edited by crogoz

Loading

CRogers Oct 24, 2024

crogoz Oct 24, 2024

CRogers Oct 24, 2024

crogoz Oct 25, 2024

crogoz Oct 25, 2024

CRogers Oct 25, 2024

CRogers Oct 25, 2024

crogoz Oct 25, 2024

autorelease3 bot commented Oct 25, 2024

Set JAVA_HOME in gradle-jdks-setup script #447

Set JAVA_HOME in gradle-jdks-setup script #447

Conversation

crogoz commented Oct 24, 2024 • edited Loading

Before this PR

After this PR

Possible downsides?

changelog-app bot commented Oct 24, 2024 • edited by crogoz Loading

Generate changelog in changelog/@unreleased

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

autorelease3 bot commented Oct 25, 2024

Set `JAVA_HOME` in gradle-jdks-setup script #447

Set `JAVA_HOME` in gradle-jdks-setup script #447

crogoz commented Oct 24, 2024 •

edited

Loading

changelog-app bot commented Oct 24, 2024 •

edited by crogoz

Loading

Generate changelog in `changelog/@unreleased`