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

Configurable java toolchains #2193

Merged
merged 31 commits into from
Apr 28, 2022
Merged

Configurable java toolchains #2193

merged 31 commits into from
Apr 28, 2022

Conversation

CRogers
Copy link
Contributor

@CRogers CRogers commented Apr 12, 2022

Before this PR

We use Gradle's in built java toolchain support . However, it has many downsides:

  1. Autoprovisioning of JDKs downloads via AdoptOpenJDK/Adoptium, which is not our preferred JDK vendor (Azul is).
    • We use Azul, not Adoptium, in prod.
    • Adoptium does not provide aarch64 macos builds for Java 11 and 15.
  2. There is no way to specify the exact version of the JDK that is downloaded
    • JDKs are downloaded once per major version then never again.
    • There is no way to ensure 100% reproducible builds by ensure the same JDKs are used each time.
    • We can't line up the JDKs used by our test tasks with the JDKs we actually deploy in our prod docker images - this means for JDK bugs or behaviour changes across minor versions unit tests do not accurately test the same stuff as the deployed asset.
  3. There is no ability or extension point to add custom CA certs in sensible way, for interacting with a corporate firewall that does TLS inception for example.
  4. The Adoptium servers are used for downloading rather than being able to specify a custom internal mirror.
    • Firewalls can slow down large downloads from outside corporate networks.
    • Downloads over public internet is slow compared to using an internal mirror.

After this PR

Add a configuration point so plugins can supply their own JDKs per java major version:

MapProperty<JavaLanguageVersion, JavaInstallationMetadata> getJdks()

Another plugin I am yet to write will download JDKs based on some configuration from various different sources eg Azul Zulu, Amazon Corretto etc

Possible downsides?

@changelog-app
Copy link

changelog-app bot commented Apr 12, 2022

Generate changelog in changelog/@unreleased

Type

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

Description

Add a configuration point for providing your own JDKs to all java requiring tasks.

Check the box to generate changelog(s)

  • Generate changelog entry


javaVersions = project.getObjects().mapProperty(JavaLanguageVersion.class, String.class);
javaVersions.putAll(project.provider(() -> parseOutVersions(project, JAVA_VERSION_PATTERN)));
javaVersions.finalizeValueOnRead();
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think we could define these at most once per repo instead of per-module? (possibly BaselineJavaVersionsExtension.java, which only exists on the root project)
I don't think we would want the values to differ across modules.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Heh actually in the process of doing this right now. The BaselineJavaVersion vs BaselineJavaVersions difference was lost on me for a little bit.

Copy link
Contributor

Choose a reason for hiding this comment

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

I could have named things much better 😞 Sorry!


private Path findJavaHome(Path temporaryJdkPath) {
try (Stream<Path> files = Files.walk(temporaryJdkPath)) {
return files.filter(file -> Files.isDirectory(file) && file.endsWith(Paths.get("Contents/Home")))
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 only the mac jdks use this directory structure. We may want to find bin/java and walk back a directory

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mmmm, I actually only had to do this because the commons-compress or jarchivelib don't support symlinks, maybe with an artifact transform I wouldn't need to do this

}

try {
Files.move(findJavaHome(temporaryJdkPath), jdkPath, StandardCopyOption.ATOMIC_MOVE);
Copy link
Contributor

Choose a reason for hiding this comment

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

atomic move is great, however in some cases it's not possible (depending on the filesystem, or the src and destination are on different physical disks). Unsure if that will be an issue here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filesystems are scary

Comment on lines 37 to 47
Path jdkArchive = azulJdkDownloader.downloadJdkFor(jdkSpec);

Archiver archiver = ArchiverFactory.createArchiver(jdkArchive.toFile());
Path temporaryJdkPath = Paths.get(
jdkPath + ".in-progress-" + UUID.randomUUID().toString().substring(0, 8));
try {
try {
archiver.extract(jdkArchive.toFile(), temporaryJdkPath.toFile());
} catch (IOException e) {
throw new RuntimeException("Failed to extract jdk to directory", e);
}
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 it would be possible/beneficial to leverage gradle cacheable artifact transforms?
That way we could reuse the same unpacked jdk across projects/checkouts instead of extracting in each repo.

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 was actually just going to extract to ~/.gradle/gradle-baseline/jdks to reuse jdks across repos and depend on the using a hash to keep them the same - but this is worth investigating, particularly as I wouldn't have to write all this caching code myself!

bulldozer-bot bot pushed a commit to palantir/witchcraft-api that referenced this pull request Apr 28, 2022
###### _excavator_ is a bot for automating changes across repositories.

Changes produced by the roomba/latest-baseline-oss check.

# Release Notes
## 4.118.0
| Type | Description | Link |
| ---- | ----------- | ---- |
| Fix | Results of boolean logic are considered safe | palantir/gradle-baseline#2232 |


## 4.119.0
| Type | Description | Link |
| ---- | ----------- | ---- |
| Feature | Add a configuration point for providing your own JDKs to all java requiring tasks. | palantir/gradle-baseline#2193 |


## 4.120.0
| Type | Description | Link |
| ---- | ----------- | ---- |
| Fix | Fix intellij gradle integration copyright configuration | palantir/gradle-baseline#2234 |



To enable or disable this check, please contact the maintainers of Excavator.
This was referenced Apr 28, 2022
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.

4 participants