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

added aarch64 build binary for windows #10306

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

kalradivyanshu
Copy link

Summary

Adds an aarch64 windows build to build for windows on arm. Addresses: #1141

Test Plan

I build uv for windows aarch64 locally by running cargo build --target aarch64-pc-windows-msvc, it worked.

@zanieb
Copy link
Member

zanieb commented Jan 5, 2025

For some reason the "Approve workflow" button is missing — some GitHub bug. I'll push an empty commit to trigger the run.

@zanieb
Copy link
Member

zanieb commented Jan 5, 2025

Screenshot 2025-01-05 at 13 59 03

@kalradivyanshu
Copy link
Author

@zanieb sorry about that, idk how that got there, fixed it.

@charliermarsh charliermarsh added windows Specific to the Windows platform releases Related to building and distributing release artifacts of uv labels Jan 5, 2025
@kalradivyanshu
Copy link
Author

@zanieb
image
I will remove the dev drive, and try again.

@kalradivyanshu
Copy link
Author

https://github.com/astral-sh/uv/actions/runs/12622606852/job/35170731355

image

Swatinem/rust-cache@v2 failed. I will try to find one compatible with windows aarch64 in the morning, if you have any pointers, let me know, thanks 👍

@kalradivyanshu
Copy link
Author

I don't think the runner has rust/cargo installed, since rustc -vV should not have failed. Is this an issue in the runner setup @zanieb?

@zanieb
Copy link
Member

zanieb commented Jan 5, 2025

Interesting. It looks like the aarch64 runner image is missing a bunch of the tools present on the x86_64 runners. Not much to do there but install what we need, I guess. I'd look for other people using the runner online and see if they have some standard setup we can copy.

(I did not have alternative image options when setting up the runner)

@kalradivyanshu
Copy link
Author

I tried https://github.com/dtolnay/rust-toolchain but it failed with bash not found.

@kalradivyanshu
Copy link
Author

I am trying to get clang working, its almost there, it fails while building ring, with
cargo:warning=Compiler family detection failed due to error: ToolNotFound: Failed to find tool. Is `clang` installed? (see https://docs.rs/cc/latest/cc/#compile-time-requirements for help)

I had the same issue with my local, I had to add VS component for clang, I tried adding that no luck, will try again in a while.

Doc: https://learn.microsoft.com/en-us/visualstudio/install/workload-component-id-vs-build-tools?view=vs-2022

@zanieb
Copy link
Member

zanieb commented Jan 8, 2025

I'm impressed by your perseverance :)

@kalradivyanshu
Copy link
Author

Not sure if perseverance or sunk-cost fallacy, but thanks :)

@kalradivyanshu
Copy link
Author

kalradivyanshu commented Jan 8, 2025

image

@zanieb 19th try is the charm 🎉 Here is the run: https://github.com/astral-sh/uv/actions/runs/12664851516/job/35293615729, I downloaded the exe from:

Artifact download URL: https://github.com/astral-sh/uv/actions/runs/12664851516/artifacts/2399688891

Its working fine on my snapdragon x-elite windows aarch 64:
image

2 questions:

  1. Do we need - uses: Swatinem/rust-cache@v2 I am assuming it had something to do with the dev directory that didn't attach with aarch64 worker, so maybe its useless? Where will the cache persist if there is no disk?
  2. Why are we not doing cargo build --release?

Let me know if there are any tests you want me to run/add to the CI! Thanks!

@zanieb
Copy link
Member

zanieb commented Jan 8, 2025

Do we need - uses: Swatinem/rust-cache@v2 I am assuming it had something to do with the dev directory that didn't attach with aarch64 worker, so maybe its useless? Where will the cache persist if there is no disk?

Let me look at the logs.

Why are we not doing cargo build --release?

This is our "regular" CI that runs on every pull request for testing. There's a separate workflow that build release binaries for distribution.

@kalradivyanshu
Copy link
Author

@zanieb do you want me to add the tests that are there for x86 to aarch64?

@zanieb
Copy link
Member

zanieb commented Jan 8, 2025

@kalradivyanshu that'd be great!

I'm looking into making things faster over in #10402

@kalradivyanshu
Copy link
Author

Yes, it currently takes 25 mins, which is insane tbh. Also I will rename the windows step to windows-x86 to match macOS naming.

@zanieb
Copy link
Member

zanieb commented Jan 9, 2025

It looks like we'll have to hold off on those failing system checks and integration tests until this runner is better supported.

@zanieb
Copy link
Member

zanieb commented Jan 9, 2025

Wdyt of #10402 (comment)

@kalradivyanshu
Copy link
Author

@zanieb I will try to get clang to work w/o using Native desktop.

I think it might be worth it to only use the native ARM64 runner in a release CI, Rust can use --target aarch64... just fine on x86_64 hosts and this whole ">10 min CI just for getting a the bare minimum setup to compile rust" is kind of just burning money for no upside.

But also, I agree with this? I feel like uv is mostly depend on rust, and rust toolchain is giving aarch64 support on x86 means they have tested it, so that might cut our build time by a lot? Why do you think it might lead to issues down the road?

Also, i ran tests yesterday, many failed, will fix those in coming days.

@zanieb
Copy link
Member

zanieb commented Jan 9, 2025

But also, I agree with this? I feel like uv is mostly depend on rust, and rust toolchain is giving aarch64 support on x86 means they have tested it, so that might cut our build time by a lot? Why do you think it might lead to issues down the road?

It might make sense to put up a separate pull request that just cross-compiles the binary then? We won't be able to test it, but at least we can say "it builds" in regular CI then follow by adding it tot he release process.

My concern with

I think it might be worth it to only use the native ARM64 runner in a release CI

was that if we're not testing the build in CI the same way we do for releases, we can regress accidentally.

@kalradivyanshu
Copy link
Author

It might make sense to put up a separate pull request that just cross-compiles the binary then? We won't be able to test it, but at least we can say "it builds" in regular CI then follow by adding it tot he release process.

I was thinking in this PR only, we add the aarch64 build via x86, and then run all the tests on aarch64, that way, we should be good, we can say it builds and passes all the tests in aarch64?

was that if we're not testing the build in CI the same way we do for releases, we can regress accidentally.

This makes sense, but if all the tests pass, we can build it for release in x86 only, it will be cheaper and faster, I think microsoft is pushing for windows on ARM so a proper aarch64 runner shouldn't be far away, then we can just have build on aarch64 too.

If that works for you I can start testing it.

@zanieb
Copy link
Member

zanieb commented Jan 9, 2025

Yeah I think we're aligned here. I'm also happy to just have the cross-build working then tackle testing in follow-up pull requests.

@zanieb
Copy link
Member

zanieb commented Jan 9, 2025

(I suggested a separate PR for the cross-build approach so we can retain all the history here and start fresh on the conversation for a different approach)

@kalradivyanshu
Copy link
Author

Oh ok, makes sense, I will open the new PR, will let you know. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
releases Related to building and distributing release artifacts of uv windows Specific to the Windows platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants