Skip to content
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

Fails for Windows ARM64 GitHub runners #25

Open
jxy-s opened this issue Jun 4, 2024 · 24 comments
Open

Fails for Windows ARM64 GitHub runners #25

jxy-s opened this issue Jun 4, 2024 · 24 comments
Labels
wontfix This will not be worked on

Comments

@jxy-s
Copy link

jxy-s commented Jun 4, 2024

According to the following job it appears that setup-bazel is not functioning when using Windows ARM64 GitHub runners:
https://github.com/archonitelabs/radiant-cpp/actions/runs/9359175843/job/25800924517

See also:
https://github.blog/2024-06-03-arm64-on-github-actions-powering-faster-more-efficient-build-systems/

@jxy-s
Copy link
Author

jxy-s commented Jun 4, 2024

The workflow is using an older version, bazel-contrib/setup-bazel@0.8.2 - forgive me if this is already fixed. I can retest with the latest version in a bit.

@jxy-s
Copy link
Author

jxy-s commented Jun 4, 2024

Confirmed running with latest version shows the same issue: https://github.com/archonitelabs/radiant-cpp/actions/runs/9371722658/job/25801604485?pr=18

For posterity, the extra slash here is necessary: bazel test --test_output=all ///... - the non-ARM64 one works fine, and this is necessary to work around an idiosyncrasy with PowerShell on windows:

https://github.com/archonitelabs/radiant-cpp/blob/a9fa01fcd620944bb2052448797594c7697bb5bd/.github/workflows/unit-tests.yaml#L54-L83

@jxy-s
Copy link
Author

jxy-s commented Jun 4, 2024

@p0deje
Copy link
Member

p0deje commented Jun 4, 2024

@jxy-s Your test fails with Error: bash: command not found - it doesn't seem to be related to setup-bazel.

@jxy-s
Copy link
Author

jxy-s commented Jun 4, 2024

🤔 I see, I guess I easily overlooked that since the other non-ARM windows machine worked with it. Let me try an alternative.

@jxy-s
Copy link
Author

jxy-s commented Jun 4, 2024

Okay so using cmd instead of bash shows: https://github.com/archonitelabs/radiant-cpp/actions/runs/9372646381/job/25804580277?pr=18

Run bazel test --test_output=all //...
'bazel' is not recognized as an internal or external command,
operable program or batch file.
Error: Process completed with exit code 1.

@p0deje
Copy link
Member

p0deje commented Jun 4, 2024

That's because there is no Bazel installed on GitHub runner, even though it's pre-installed on Windows X64. If you want for the action to install one for you, you need to pass bazelisk-version: 1x input.

@jxy-s
Copy link
Author

jxy-s commented Jun 4, 2024

https://github.com/archonitelabs/radiant-cpp/actions/runs/9372804523/job/25805096299?pr=18

  Error: Error: Unable to find Bazelisk version 1.x for platform windows/arm64
      at downloadBazelisk (C:\a\_actions\bazel-contrib\setup-bazel\0.8.5\dist\index.js:90:1)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)
      at setupBazelisk (C:\a\_actions\bazel-contrib\setup-bazel\0.8.5\dist\index.js:43:1)
      at setupBazel (C:\a\_actions\bazel-contrib\setup-bazel\0.8.5\dist\index.js:26:1)
      at run (C:\a\_actions\bazel-contrib\setup-bazel\0.8.5\dist\index.js:12:1)

I tried also with 1.20.0 instead of 1.x.
https://github.com/archonitelabs/radiant-cpp/actions/runs/9372780009/job/25805012776

  Error: Error: Unable to find Bazelisk version 1.20.0 for platform windows/arm64
      at downloadBazelisk (C:\a\_actions\bazel-contrib\setup-bazel\0.8.5\dist\index.js:90:1)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)
      at setupBazelisk (C:\a\_actions\bazel-contrib\setup-bazel\0.8.5\dist\index.js:43:1)
      at setupBazel (C:\a\_actions\bazel-contrib\setup-bazel\0.8.5\dist\index.js:26:1)
      at run (C:\a\_actions\bazel-contrib\setup-bazel\0.8.5\dist\index.js:12:1)

@p0deje could this be related to?: bazelbuild/bazelisk#572

@p0deje
Copy link
Member

p0deje commented Jun 4, 2024

Yes, it's related to this issue. If Bazelisk ships a native ARM64 binary, it would work out-of-the-box with setup-bazel.
We could work it around for now by forcing x64 architecture on Windows.

@jxy-s
Copy link
Author

jxy-s commented Jun 4, 2024

The drawback of forcing x64 bazelisk on Windows is that bazel won't use the right toolchain and will compile non-native x64 on ARM64, which is not desired. I think the best course of action is to resolve it in the bazelisk repo and produce an ARM64 binary for Windows.

@p0deje
Copy link
Member

p0deje commented Jun 10, 2024

The drawback of forcing x64 bazelisk on Windows is that bazel won't use the right toolchain and will compile non-native x64 on ARM64, which is not desired. I think the best course of action is to resolve it in the bazelisk repo and produce an ARM64 binary for Windows.

Actually Bazel would still download proper toolchains and compilation should work correctly. I agree that it should be fixed upstream, but in the mean time - can you test p0deje/fix-windows-arm64 branch and let me know if it helps?

@jxy-s
Copy link
Author

jxy-s commented Jun 10, 2024

I am testing, but I'm confident that this is not going to do what is desired. Windows ARM64 using non-native bazel will build an x64 binary. In the case of a testing workflow that is very much undesired. You want to build and run tests on a native platform. I'll collect data and report back.

I suggest not merging #28 until we've come to a consensus here.

For reference: bazelbuild/bazel#22164 (comment) and bazelbuild/bazelisk#572

@jxy-s
Copy link
Author

jxy-s commented Jun 10, 2024

It seems like it's still broken for another reason? Looks like bazelisk-version: 1.x here is succeeding but the command can't locate the newly installed bazelisk? I tried specifying both bazel and bazelisk without luck.

https://github.com/archonitelabs/radiant-cpp/actions/runs/9451455948/job/26032443902#step:4:5

https://github.com/archonitelabs/radiant-cpp/actions/runs/9451414740/job/26032294653?pr=19#step:4:5

@p0deje
Copy link
Member

p0deje commented Jun 10, 2024

Yes, I think there is an issue with how file is added to the tool cache, let me try to prepare another version.

@p0deje
Copy link
Member

p0deje commented Jun 10, 2024

@jxy-s Can you please re-run with debug logs enabled and share here?

@jxy-s
Copy link
Author

jxy-s commented Jun 10, 2024

@p0deje
Copy link
Member

p0deje commented Jun 10, 2024

Maybe that's because .exe is missing from filename, can you try a latest commit?

@jxy-s
Copy link
Author

jxy-s commented Jun 10, 2024

https://github.com/archonitelabs/radiant-cpp/actions/runs/9454749231/job/26043007626?pr=21

Looks like that fixes the missing bazel (aka bazelisk) - however there is already a problem emerging with bazelisk 😄:

Run bazel test --test_output=all -s --toolchain_resolution_debug=.* //...
  bazel test --test_output=all -s --toolchain_resolution_debug=.* //...
  shell: C:\Windows\system3[2](https://github.com/archonitelabs/radiant-cpp/actions/runs/9454749231/job/26043007626?pr=21#step:4:2)\cmd.EXE /D /E:ON /V:OFF /S /C "CALL "{0}""
  
2024/06/10 20:01:20 Downloading https://releases.bazel.build/7.1.1/release/bazel-7.1.1-windows-x86_64.exe...
Error: Process completed with exit code -1073741515.
0xC0000135    STATUS_DLL_NOT_FOUND
{Unable To Locate Component} This application has failed to start because %hs was not found. Reinstalling the application might fix this problem.

Looking past that for a moment, bazelisk is not downloading the correct bazel. It is installing bazel-7.1.1-windows-x86_64.exe. That is going to do the wrong thing for an ARM64 Windows machine, explained here: bazelbuild/bazel#22164 (comment)

Anyway, all that aside. I think there are two things here.

  1. bazelisk needs to provide a native ARM64 binary for Windows
  2. setup-bazel might need to make a request to update tools for Windows ARM64 support: https://github.com/actions/runner-images

@p0deje
Copy link
Member

p0deje commented Jun 10, 2024

  1. Ok, agreed on that, let's not attempt to work it around in setup-bazel and fix it upstream in Bazelisk. It seems like a straightforward fix so hopefully the team can do it.
  2. I actually don't follow how you used Windows ARM64 runner - I don't think GHA provides such a runner and you use something called archonite-windows-arm64 instead.

@jxy-s
Copy link
Author

jxy-s commented Jun 10, 2024

I actually don't follow how you used Windows ARM64 runner - I don't think GHA provides such a runner and you use something called archonite-windows-arm64 instead.

It is still a GitHub-hosted runner, it's just set up through the Archonite organization. The "Standard GitHub-hosted runners" does not yet include the ARM64 Windows or Linux machines in their set, but you can add them manually.

Link for information on this (and how to set them up) is in the original message in this thread: https://github.blog/2024-06-03-arm64-on-github-actions-powering-faster-more-efficient-build-systems/

image

image

@p0deje
Copy link
Member

p0deje commented Jun 10, 2024

It is still a GitHub-hosted runner, it's just set up through the Archonite organization. The "Standard GitHub-hosted runners" does not yet include the ARM64 Windows or Linux machines in their set, but you can add them manually.

I see. This doesn't seem to be available to open-source projects yet, so you'd need to file the issue yourself.

Screenshot 2024-06-10 at 13 45 44

@jxy-s
Copy link
Author

jxy-s commented Jun 10, 2024

You need to be looking at the organization level, I think you're looking at the project/repo level.

Configured here: https://github.com/bazel-contrib
Not here: https://github.com/bazel-contrib/setup-bazel

Anyway, it is available to open-source projects. Radiant is open source under the Archonite organization:
https://github.com/archonitelabs
https://github.com/archonitelabs/radiant-cpp

@p0deje
Copy link
Member

p0deje commented Jun 10, 2024

The organization itself must be on Team / Enterprise plan, then it's available to its repositories:

Screenshot 2024-06-10 at 14 28 24

@jxy-s
Copy link
Author

jxy-s commented Jun 10, 2024

Yeah, that tracks with the documentation. Soon enough they'll be available in the standard set for everyone.

Hopefully we can get a bit ahead of the curve here before more people run into issues.

@p0deje p0deje added the wontfix This will not be worked on label Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants