Allow specifying different versions of JDKs per operating system and architecture #286

CRogers · 2024-02-16T18:21:31Z

Before this PR

Currently, gradle-jdks only allows you to specify one version of a JDK per java major version. This means all operating systems and architectures must use the same JDK version.

However, this falls apart in the presence of operating system only releases by JDK vendors. For example, Amazon Corretto recently released a "linux" only release for JDK 17 (17.0.10.8.1) and 21 (21.0.2.14.1).

We attempt to use same JDKs in both our prod environments and CI (which are linux based) and on developer's local machines (almost always macos). We had an unfortunate situation where automated upgrades of JDKs upgraded people's repos to these versions that did not have macos distributions, causing them to be unable to compile or test code locally.

After this PR

==COMMIT_MSG==
Allow specifying different versions of JDKs per operating system and architecture
==COMMIT_MSG==

We still want to get the latest JDKs in prod, even if they're on a linux only release. Our compliance constraints mean we must fix CVEs on a short timeframe, and these CVE fixes may be in a linux only release. Or there could be some critically important bugfix we need (but currently wouldn't be able to get without breaking people's dev setups). Even the current bug is scary and we should ensure gets onto prod ("AArch64: crypto pmull based CRC32/CRC32C intrinsics clobber V8-V15 registers").

There's changes internally to rest of the stack of Gradle plugins that deal with JDKs. Most notably, they all use the same json representation to list the latest JDKs, and we have an excavator that can produce this json file automatically.

This file format is in this repo because:

I want to make it easy to load into our Gradle configuration
When we build the feature to manage the Gradle daemon JDK, I want to produce a native-image binary that will "pre-install" the latest JDKs on CI images etc so the JDKs are already downloaded, which would necessitate the file format code living in this repo.

Example latest jdk json format

{
  "jdksPerJavaVersion": {
    "8": {
      "distribution": "amazon-corretto",
      "os": {
        "macos": {
          "arch": {
            "x86-64": {
              "version": "8.402.08.1"
            },
            "aarch64": {
              "version": "8.402.08.1"
            }
          }
        },
        "linux-glibc": {
          "arch": {
            "x86-64": {
              "version": "8.402.08.1"
            },
            "aarch64": {
              "version": "8.402.08.1"
            }
          }
        },
        "linux-musl": {
          "arch": {
            "x86-64": {
              "version": "8.402.08.1"
            },
            "aarch64": {
              "version": "8.402.08.1"
            }
          }
        },
        "windows": {
          "arch": {
            "x86": {
              "version": "8.402.08.1"
            },
            "x86-64": {
              "version": "8.402.08.1"
            }
          }
        }
      }
    },
    "11": {
      "distribution": "amazon-corretto",
      "os": {
        "macos": {
          "arch": {
            "x86-64": {
              "version": "11.0.22.7.1"
            },
            "aarch64": {
              "version": "11.0.22.7.1"
            }
          }
        },
        "linux-glibc": {
          "arch": {
            "x86": {
              "version": "11.0.22.7.1"
            },
            "x86-64": {
              "version": "11.0.22.7.1"
            },
            "aarch64": {
              "version": "11.0.22.7.1"
            }
          }
        },
        "linux-musl": {
          "arch": {
            "x86-64": {
              "version": "11.0.22.7.1"
            },
            "aarch64": {
              "version": "11.0.22.7.1"
            }
          }
        },
        "windows": {
          "arch": {
            "x86": {
              "version": "11.0.22.7.1"
            },
            "x86-64": {
              "version": "11.0.22.7.1"
            }
          }
        }
      }
    },
    "17": {
      "distribution": "amazon-corretto",
      "os": {
        "macos": {
          "arch": {
            "x86-64": {
              "version": "17.0.10.7.1"
            },
            "aarch64": {
              "version": "17.0.10.7.1"
            }
          }
        },
        "linux-glibc": {
          "arch": {
            "x86-64": {
              "version": "17.0.10.8.1"
            },
            "aarch64": {
              "version": "17.0.10.8.1"
            }
          }
        },
        "linux-musl": {
          "arch": {
            "x86-64": {
              "version": "17.0.10.8.1"
            },
            "aarch64": {
              "version": "17.0.10.8.1"
            }
          }
        },
        "windows": {
          "arch": {
            "x86-64": {
              "version": "17.0.10.7.1"
            }
          }
        }
      }
    },
    "21": {
      "distribution": "amazon-corretto",
      "os": {
        "macos": {
          "arch": {
            "x86-64": {
              "version": "21.0.2.13.1"
            },
            "aarch64": {
              "version": "21.0.2.13.1"
            }
          }
        },
        "linux-glibc": {
          "arch": {
            "x86-64": {
              "version": "21.0.2.14.1"
            },
            "aarch64": {
              "version": "21.0.2.14.1"
            }
          }
        },
        "linux-musl": {
          "arch": {
            "x86-64": {
              "version": "21.0.2.14.1"
            },
            "aarch64": {
              "version": "21.0.2.14.1"
            }
          }
        },
        "windows": {
          "arch": {
            "x86-64": {
              "version": "21.0.2.13.1"
            }
          }
        }
      }
    }
  }
}

Possible downsides?

changelog-app · 2024-02-16T18:21:35Z

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

Allow specifying different versions of JDKs per operating system and architecture

Check the box to generate changelog(s)

Generate changelog entry

…r/different-versions

felixdesouza

mostly curious about mixing of non-provider/property types inside the extensions, but not a huge blocker

felixdesouza · 2024-02-19T19:28:20Z

gradle-jdks/src/main/java/com/palantir/gradle/jdks/JdkExtension.java

@@ -24,11 +30,46 @@ public abstract class JdkExtension {

    public abstract Property<JdkDistributionName> getDistributionName();

+    private final Map<Os, JdkOsExtension> jdkOsExtensions = new HashMap<>();


why not use a NamedDomainObjectContainer ?

With NamedDomain.... need you to give a string for the name (or implement Named, which bring in a needless gradle dep). I don't really think it buys much? Unless there's some magic you can do with it I'm not aware of.

Given how it's used, I think it doesn't buy us as much, since we need it to statically exist. I believe it creates most of the methods for you, but that doesn't really matter if you're calling it from java

felixdesouza · 2024-02-20T15:15:37Z

gradle-jdks/src/main/java/com/palantir/gradle/jdks/CurrentArch.java

+import java.util.Set;
+
+final class CurrentArch {
+    static Arch current() {


nit: since Current* is already in the name, how about CurrentArch.get()?

felixdesouza · 2024-02-20T15:16:13Z

gradle-jdks/src/main/java/com/palantir/gradle/jdks/CurrentOs.java

-    WINDOWS;
-
-    @Value.Default
+final class CurrentOs {
    public static Os current() {


same thing re CurrentOs.get()

felixdesouza · 2024-02-20T15:21:19Z

gradle-jdks/src/main/java/com/palantir/gradle/jdks/JdksPlugin.java

+        String version = jdkExtension
+                .jdkFor(currentOs)
+                .jdkFor(currentArch)
+                .getJdkVersion()
+                .get();


curious, why force property retrieval here? do you not get into races with configuration?

This method, javaInstallationForLanguageVersion is called from a lazy location, but you're right this could probably be even more lazy to avoid problems.

felixdesouza · 2024-02-20T15:22:31Z

gradle-jdks/src/test/groovy/com/palantir/gradle/jdks/JdksExtensionProjectSpec.groovy

+        then:
+        stdout.contains('jdkVersion macos aarch64: 11.1')
+        stdout.contains('jdkVersion linux-glibc aarch64: 11.2')
+        stdout.contains('jdkVersion linux-glibc x64: 11.3')


felixdesouza · 2024-02-20T16:44:25Z

👍 pending Current*.get()

felixdesouza · 2024-02-21T13:16:05Z

👍

svc-autorelease · 2024-02-21T13:16:18Z

Released 0.35.0

CRogers added 7 commits February 13, 2024 13:53

wrote some code

91c4149

is it done?

693c6aa

Better test name

865c30e

Json in gradle-jdks

687cfd8

silly fixes

b9b151e

Add Optionals

d8e2b9d

json fixes

9c08097

svc-changelog and others added 6 commits February 16, 2024 18:21

Add generated changelog entries

7dc5787

Merge branch 'develop' of github.com:palantir/gradle-jdks into callum…

156a70f

…r/different-versions

Update readme

8147d38

checkstyle

c729223

Merge branch 'develop' of github.com:palantir/gradle-jdks into callum…

d5e6560

…r/different-versions

Remove higher level versions

8806fa1

CRogers mentioned this pull request Feb 19, 2024

Support different JDK versions per operating system and architecture palantir/gradle-jdks-latest#180

Merged

CRogers marked this pull request as ready for review February 19, 2024 19:10

CRogers requested a review from felixdesouza February 19, 2024 19:10

felixdesouza reviewed Feb 20, 2024

View reviewed changes

CRogers added 3 commits February 21, 2024 12:55

current -> get

cce4c46

more lazy

e9ca38e

finalizeValueOnRead

171e5c8

CRogers added autorelease merge when ready labels Feb 21, 2024

bulldozer-bot bot merged commit b59b494 into develop Feb 21, 2024
6 checks passed

bulldozer-bot bot deleted the callumr/different-versions branch February 21, 2024 13:16

CRogers mentioned this pull request Feb 23, 2024

Try to avoid This property is in an unexpected state errors #294

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow specifying different versions of JDKs per operating system and architecture #286

Allow specifying different versions of JDKs per operating system and architecture #286

CRogers commented Feb 16, 2024 •

edited

Loading

changelog-app bot commented Feb 16, 2024 •

edited by CRogers

Loading

felixdesouza left a comment

felixdesouza Feb 19, 2024

CRogers Feb 20, 2024

felixdesouza Feb 20, 2024

felixdesouza Feb 20, 2024

felixdesouza Feb 20, 2024

felixdesouza Feb 20, 2024

CRogers Feb 20, 2024

felixdesouza Feb 20, 2024

felixdesouza Feb 20, 2024

felixdesouza commented Feb 20, 2024

felixdesouza commented Feb 21, 2024

svc-autorelease commented Feb 21, 2024

		@@ -24,11 +30,46 @@ public abstract class JdkExtension {

		public abstract Property<JdkDistributionName> getDistributionName();

		private final Map<Os, JdkOsExtension> jdkOsExtensions = new HashMap<>();

Allow specifying different versions of JDKs per operating system and architecture #286

Allow specifying different versions of JDKs per operating system and architecture #286

Conversation

CRogers commented Feb 16, 2024 • edited Loading

Before this PR

After this PR

Possible downsides?

changelog-app bot commented Feb 16, 2024 • edited by CRogers Loading

Generate changelog in changelog/@unreleased

felixdesouza left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

felixdesouza commented Feb 20, 2024

felixdesouza commented Feb 21, 2024

svc-autorelease commented Feb 21, 2024

CRogers commented Feb 16, 2024 •

edited

Loading

changelog-app bot commented Feb 16, 2024 •

edited by CRogers

Loading

Generate changelog in `changelog/@unreleased`