-
Notifications
You must be signed in to change notification settings - Fork 241
8358538: Update GHA Windows runner to 2025 #3052
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
Conversation
/issue add JDK-8360042 |
👋 Welcome back shade! A progress list of the required criteria for merging this PR into |
@shipilev This change now passes all automated pre-integration checks. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been no new commits pushed to the ➡️ To integrate this PR with the above commit message to the |
This backport pull request has now been updated with issue from the original commit. |
@shipilev |
Windows build fails with:
Some more work needs to happen it seems for 11u. |
Pre-submit tests - linux-x86 / build - Build / test is infra related. |
@jerboaa @shipilev Yep. It seems https://bugs.openjdk.org/browse/JDK-8357193 should be backported to 11 too. |
@vieiro Thanks. Please create a PR for the backport. It seems a small enough change and would get this unblocked. |
Right, thanks. I'll wait for JDK-8357193 to get in first. |
I see there is already the PR: #3034 |
D'oh! I'll close #3053 then |
a72f153
to
3fc3ede
Compare
Pulled in new master that contains #3034 now. Let's see if it fixes the rest of the build. |
@shipilev Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information. |
Tier1 compiler failures on Windows seem related. Failures look like:
|
This happens because the It seems JDK-8248987 doesn't work as intented. If diff --git a/src/jdk.aot/share/classes/jdk.tools.jaotc/src/jdk/tools/jaotc/Linker.java b/src/jdk.aot/share/classes/jdk.tools.jaotc/src/jdk/tools/jaotc/Linker.java
index 92c80aad397..6af0c17512d 100644
--- a/src/jdk.aot/share/classes/jdk.tools.jaotc/src/jdk/tools/jaotc/Linker.java
+++ b/src/jdk.aot/share/classes/jdk.tools.jaotc/src/jdk/tools/jaotc/Linker.java
@@ -163,7 +163,7 @@ final class Linker {
*/
private static String getWindowsLinkPath(Main main) throws Exception {
try {
- Path vc141NewerLinker = getVC141AndNewerLinker();
+ Path vc141NewerLinker = getVC141AndNewerLinker(main);
if (vc141NewerLinker != null) {
return vc141NewerLinker.toString();
}
@@ -204,10 +204,11 @@ final class Linker {
return null;
}
- private static Path getVC141AndNewerLinker() throws Exception {
+ private static Path getVC141AndNewerLinker(Main main) throws Exception {
String programFilesX86 = System.getenv("ProgramFiles(x86)");
if (programFilesX86 == null) {
- throw new IllegalStateException("Could not read the ProgramFiles(x86) environment variable");
+ programFilesX86 = "C:\\Program Files (x86)";
+ main.printer.printlnInfo("Could not read the ProgramFiles(x86) environment variable. Using default '" + programFilesX86 + "'");
}
String vswherePath = programFilesX86 + "\\Microsoft Visual Studio\\Installer\\vswhere.exe";
Path vswhere = Paths.get(vswherePath); Let me know if you want me to create a new issue with that patch on top of this PR. |
Or, who knows, maybe we need a new |
Yes, please, if you can figure this out in a separate issue? I think at this point, with runners going away soon, we are better off pushing this backport, even with |
I've created https://bugs.openjdk.org/browse/JDK-8360816. Feel free to enhance as you see fit. A PR within the hour. |
Well, let's see what happens. |
Yep, looks like the fix in #3056 brings back the passing AOT compiler tests... |
OK. Looks like @vieiro's test fix works. I say we integrate this PR, followed by @vieiro's fix. /approval 8358538 request Same reason as for 21u. Requires JDK-8360042 as the followups, and JDK-8360816 as another test fix. JDK-8360042 would follow in the same commit. JDK-8360816 would be done as the subsequent PR. |
/approve yes |
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. Test failures are going to be fixed with #3056
@jerboaa |
Let's go! /integrate |
Going to push as commit 0927ca7. |
Fixes GHA. Current Windows runners are already in brown-out stage, and will be decommissioned by the end of the month. So we need to get it into update repos soon.
Additional testing:
Progress
Issues
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk11u-dev.git pull/3052/head:pull/3052
$ git checkout pull/3052
Update a local copy of the PR:
$ git checkout pull/3052
$ git pull https://git.openjdk.org/jdk11u-dev.git pull/3052/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 3052
View PR using the GUI difftool:
$ git pr show -t 3052
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk11u-dev/pull/3052.diff
Using Webrev
Link to Webrev Comment