-
-
Notifications
You must be signed in to change notification settings - Fork 12.4k
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
openjdk@17 17.0.10 #160094
Closed
Closed
openjdk@17 17.0.10 #160094
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,8 @@ | ||
class OpenjdkAT17 < Formula | ||
desc "Development kit for the Java programming language" | ||
homepage "https://openjdk.java.net/" | ||
url "https://github.com/openjdk/jdk17u/archive/refs/tags/jdk-17.0.9-ga.tar.gz" | ||
sha256 "365c6b7d506f25e2249cac7658ada8b72b8652ceb15bbc8316de3e6fe8ea0976" | ||
url "https://github.com/openjdk/jdk17u/archive/refs/tags/jdk-17.0.10-ga.tar.gz" | ||
sha256 "fac2539384ba8d86cdcf3553e69aaf4001a3cec1134bbf6f5f04f64f0acbc055" | ||
license "GPL-2.0-only" => { with: "Classpath-exception-2.0" } | ||
|
||
livecheck do | ||
|
@@ -50,12 +50,13 @@ class OpenjdkAT17 < Formula | |
|
||
fails_with gcc: "5" | ||
|
||
# From https://jdk.java.net/archive/ | ||
# ARM64: https://www.azul.com/downloads/?version=java-17-lts&package=jdk | ||
# Intel: https://jdk.java.net/archive/ | ||
resource "boot-jdk" do | ||
on_macos do | ||
on_arm do | ||
url "https://download.java.net/java/GA/jdk17.0.2/dfd4a8d0985749f896bed50d7138ee7f/8/GPL/openjdk-17.0.2_macos-aarch64_bin.tar.gz" | ||
sha256 "602d7de72526368bb3f80d95c4427696ea639d2e0cc40455f53ff0bbb18c27c8" | ||
url "https://cdn.azul.com/zulu/bin/zulu17.46.19-ca-jdk17.0.9-macosx_aarch64.tar.gz" | ||
sha256 "d6837676e55b97772b6512e253fdaf8ab282bb216c0f8366b6c5905cd02b5056" | ||
end | ||
on_intel do | ||
url "https://download.java.net/java/GA/jdk17.0.2/dfd4a8d0985749f896bed50d7138ee7f/8/GPL/openjdk-17.0.2_macos-x64_bin.tar.gz" | ||
|
@@ -64,8 +65,8 @@ class OpenjdkAT17 < Formula | |
end | ||
on_linux do | ||
on_arm do | ||
url "https://download.java.net/java/GA/jdk17.0.2/dfd4a8d0985749f896bed50d7138ee7f/8/GPL/openjdk-17.0.2_linux-aarch64_bin.tar.gz" | ||
sha256 "13bfd976acf8803f862e82c7113fb0e9311ca5458b1decaef8a09ffd91119fa4" | ||
url "https://cdn.azul.com/zulu/bin/zulu17.46.19-ca-jdk17.0.9-linux_aarch64.tar.gz" | ||
sha256 "90062201e7911696a449431a61dc0a55cd10cda516a9f2db54c410633a79302a" | ||
end | ||
on_intel do | ||
url "https://download.java.net/java/GA/jdk17.0.2/dfd4a8d0985749f896bed50d7138ee7f/8/GPL/openjdk-17.0.2_linux-x64_bin.tar.gz" | ||
|
@@ -77,7 +78,7 @@ class OpenjdkAT17 < Formula | |
def install | ||
boot_jdk = buildpath/"boot-jdk" | ||
resource("boot-jdk").stage boot_jdk | ||
boot_jdk /= "Contents/Home" if OS.mac? | ||
boot_jdk /= "Contents/Home" if OS.mac? && !Hardware::CPU.arm? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @chenrui333 I don't know what was the purpose of this change, but another PR was merged without, in case this was important to get. |
||
java_options = ENV.delete("_JAVA_OPTIONS") | ||
|
||
args = %W[ | ||
|
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was there a specific issue that required updating JDK on ARM?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I sometimes update these to use
bump version minus one
to build as it seems generally more appropriate.Here it's @chenrui333 's doing to move to these to Zulu and I don't know why.
Same for when on another bump resource was updated to build from binary release of the exact same version: generally you want to build with version
n-1
, so I didn't knew why use the same.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same major version may be worth it here as there is no OpenJDK 16 ARM release. It would allow avoiding a 3rd party JDK. For older OpenJDK, it may depend on if there are any required fixes, e.g. OpenJDK 10 is no longer supported so may not get all backports (Mainly issue for new platforms like Apple ARM).
Unless there is a required fix, I think we can stick to OpenJDK's binaries for bootstrap on newer OpenJDK formulae (
openjdk@17
andopenjdk
).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant for patch versions, like using 17.0.1 to build 17.0.2 (or latest 17.x.y for 18.0.0).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For OpenJDK,
n-1
refers to previous major version and is the standard choice of boot JDK.Changing patch release usually has minor/no impact and OpenJDK doesn't provide them after a certain point. They point to Temurin releases instead https://wiki.openjdk.org/display/JDKUpdates/JDK+17u#JDK17u-Downloads
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a bootstrap jdk, it is fine to use whatever latest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main thing is we only resort to Zulu as a last resort (e.g. OpenJDK7 doesn't exist and OpenJDK11 doesn't provide ARM release).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anyway, I think this should be reverted unless there is a specific issue. And if there is a specific issue, it should be included in the comment.
For example, the reason we use Zulu on
openjdk@8
:homebrew-core/Formula/o/[email protected]
Lines 50 to 52 in bceb694
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We definitely should not use a random non-open boot JDK here.