Include JDKs in Dists #1510
Merged
Include JDKs in Dists #1510
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Generate changelog in
|
...ckaging/src/main/java/com/palantir/gradle/dist/service/JavaServiceDistributionExtension.java
Show resolved
Hide resolved
...ckaging/src/main/java/com/palantir/gradle/dist/service/JavaServiceDistributionExtension.java
Outdated
Show resolved
Hide resolved
...-packaging/src/main/java/com/palantir/gradle/dist/service/JavaServiceDistributionPlugin.java
Outdated
Show resolved
Hide resolved
`Provider#zip` is introduced in 6.6. However, it seems that gradle introduced plugin bytecode rewriting sometime between 6.1 and 6.6. This means it now falls over on the jackson 2.15.2 dep, so we need to go all the way to 7.6
CRogers
commented
Aug 24, 2023
| @@ -179,7 +179,6 @@ class JavaServiceDistributionPluginTests extends GradleIntegrationSpec { | |||
| } | |||
|
|
|||
| repositories { | |||
| jcenter() | |||
There was a problem hiding this comment.
Deprecation warning with new higher version of gradle
CRogers
commented
Aug 24, 2023
| @@ -31,7 +31,7 @@ public class SlsBaseDistPlugin implements Plugin<Project> { | |||
|
|
|||
| public static final String SLS_DIST_USAGE = "sls-dist"; | |||
|
|
|||
| public static final GradleVersion MINIMUM_GRADLE = GradleVersion.version("6.1"); | |||
| public static final GradleVersion MINIMUM_GRADLE = GradleVersion.version("7.6"); | |||
There was a problem hiding this comment.
Provider#zip is introduced in 6.6. However, it seems that gradle introduced plugin bytecode rewriting sometime between 6.1 and 6.6. This means 6.6 now falls over on the jackson 2.15.2 dep from baseline, so we need to go all the way to 7.6
There was a problem hiding this comment.
7.6 for Java 19: https://docs.gradle.org/current/userguide/compatibility.html
There was a problem hiding this comment.
There was a problem hiding this comment.
happy to merge if 7.6 doesn't break and have a lower min version
There was a problem hiding this comment.
Huh, I wonder why it works...
|
👍 |
|
Released 7.37.0 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before this PR
We want to include JDKs in dists (reasons in this doc). There is currently no way to do this.
I had tried to do this all from an internal plugin, but it's impossible to set
javaHomecorrectly and maintain laziness.After this PR
We add a way to include a number of JDKs to a dist, up to one per version, and then set
javaHomein go-java-launcher to the correct one.Multiple JDKs are required as some internal services run user code, which select from multiple JDK versions, and these should continue to work. I'm assuming we won't need multiple variants of the same JDK version, as that would be hugely wasteful to deploy and we should avoid this as much as we can.
==COMMIT_MSG==
Add
jdksproperty todistributionextension to help include JDKs into dists.==COMMIT_MSG==
Possible downsides?