-
-
Notifications
You must be signed in to change notification settings - Fork 251
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
Use VS 2022 by default for all JDK versions + skip freetype patching #3907
Conversation
…ype patching Signed-off-by: Stewart X Addison <sxa@redhat.com>
I believe Semeru has switched over to VS2022 for all jdk versions for some time now. We should do the same for the OpenJ9 builds at Adoptium. |
Signed-off-by: Stewart X Addison <sxa@redhat.com>
Thanks - PR updated. That makes it simpler :-) Note for reviewers: We can probably optimise this file a bit more sine we have fewer conditions which are different across versions. |
PR has failed the JDK8 check but it's attempting to build with compiler version 19.40.33807 instead of the 19.37.32822 which we use which I believe is the cause of the check failure and would impact running on our machines. Noting that I'm not sure where the VS2022 comes from the github checks. https://github.com/adoptium/temurin-build/blob/master/.github/workflows/build.yml has an explicit reference to the URLs for earlier compiler versions but not 2022. |
8b2b849
to
9c00951
Compare
Signed-off-by: Stewart X Addison <sxa@redhat.com>
Removing jdk8u/windows test for now (Ideally I'd make it conditionally set |
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 (although as discussed in Slack, I'd like to switch over to using the docker image as a base once we've got one published)
jdk8u test removed for now as the default win2022 image in GitHub Actions has a pre-installed VS2022 compiler which is newer than the one we want and fails to compiler jdk8u. |
Let us know when you have a Windows build docker image. I'd be interested in using it too. |
@AdamBrousseau There is a functional dockerfile for this in https://github.com/adoptium/infrastructure/pull/3702/files :-) |
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.
Looks good!
@sxa safe to merge? |
Yep I think so now - was leaving it a bit to ensure that anyone who wanted to comment could do so. |
Updating to use VS2022 for all Temurin builds. This change has been approved by the PMC today, but if it causes any unexpected issues it can be reverted.
freetype patching can (and should be, otherwise it tries to reference things that don't exist) be skipped when we're not building JDK8 with VS2017 otherwise it tries to reference a file that isn't where it's expecting.
I'm currently testing with this branch on a machine which only has the VS2022 build tools installed and will report back once it's complete, but it's got to the middle of the build with jdk8u so I'm confident no other issues will show up, and standalone testing before creating this PR looked good.
Alternate solutions:
TOOLCHAIN_VERSION
overrides instead of deleting them for now so that we knew where they all were.This should leave OpenJ9 unaffected, but I'll tag in @AdamBrousseau too (LMK if you want this to apply to OpenJ9 as well)
Closes #2922