-
-
Notifications
You must be signed in to change notification settings - Fork 249
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
Remove hyphen from bootjdk symlinks #3992
Remove hyphen from bootjdk symlinks #3992
Conversation
Without the hyphens in the symlinks the upstream configure is able to find the bootjdks without a problem, see adoptium/infrastructure#2912 (comment) |
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.
LGTM. Noting that make-adopt-build-farm.sh passes JDK_BOOT_DIR
into makejdk-any-platform:
And #393 was added to cover a lot of the JDKXX_BOOT_DIR
variables but there is potential for stripping a lot of that code our in the ultimate goal of simplification later.
FYI @AdamBrousseau - this will probably go live soon along with the correspinding infrastructure PR. you were ok with this in adoptium/infrastructure#2912 (comment) but since that was a while ago now I'm letting you know in case you have any last minute objections :-)
@@ -49,7 +49,7 @@ export GNUPGHOME | |||
|
|||
BOOT_JDK_VARIABLE="JDK${JDK_BOOT_VERSION}_BOOT_DIR" | |||
if [ ! -d "$(eval echo "\$$BOOT_JDK_VARIABLE")" ]; then | |||
bootDir="$PWD/jdk-$JDK_BOOT_VERSION" |
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 noting that for these versions it doesn't matter whether they have the -
or not as it's a temporary directory used to download the build anyway, but no harm in it being consistent with the "default` top level path names on the machines that we will be creating.
@Haroon-Khel - I know there's another PR related to this, will let you merge this if that's safe to do so independantly |
To reiterate my comment here adoptium/infrastructure#3773 (comment) Im going to hold off on the windows changes. It looks like the upstream configure looks in C:\Program Files\Java (/cygdrive/c/Program Files/Java) which is where we already unpack the JDK on our machines and |
Ref adoptium/infrastructure#2912 and the infra pr adoptium/infrastructure#3773
The pr removes the hyphen from the bootjdk symlinks, ie /usr/lib/jvm/jdk-11 becomes /usr/lib/jvm/jdk11. The change has been made to the linux, mac and windows playbooks, so the build scripts need to be updated to be able to detect the new symlinks.
Leaving this as a draft so it doesnt get merged before or during the release