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

Support Windows ARM64 (aarch64-pc-windows-msvc) #1664

Merged
merged 1 commit into from
Nov 21, 2022

Conversation

p0deje
Copy link
Contributor

@p0deje p0deje commented Nov 19, 2022

This is my naive attempt to add support for aarch64-pc-windows-msvc architecture. I am not doing Rust development, yet I need it for the Rust bits in https://github.com/SeleniumHQ/selenium/tree/trunk/rust monorepo to work on Windows 10 VM running on Apple M1 (this uses ARM64 version of Windows).

I am not 100% sure how to properly test that it works correctly, I mainly searched for x86_64-pc-windows-msvc in the project, added aarch64-pc-windows-msvc where it made sense, and re-generated some of the files by running the following command:

bazel run //util:fetch_shas //proto/3rdparty:crates_vendor //crate_universe/3rdparty:crates_vendor //bindgen/3rdparty:crates_vendor 

It was also necessary to explicitly add aarch64-pc-windows-msvc to the rustup targets on GitHub runner. Otherwise, it was compiling for x86_64. WIth this change, I managed to make cargo-bazel compile (see build).

Please let me know if there is anything missing. Maybe it's possible to make a pre-release so I could try using this on Windows ARM virtual machine?

@google-cla
Copy link

google-cla bot commented Nov 19, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Collaborator

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! it looks like you have one test to update and some examples.

---- lockfile::test::digest_with_config stdout ----
thread 'lockfile::test::digest_with_config' panicked at 'assertion failed: `(left == right)`
  left: `Digest("33dbf61e3b2aabacadaf7ff0c9862af25703cb851436efcbdf8552735be844ba")`,
 right: `Digest("756a613410573552bb8a85d6fcafd24a9df3000b8d943bf74c38bda9c306ef0e")`', crate_universe/src/lockfile.rs:254:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    lockfile::test::digest_with_config

This just needs to have the sha256 updated since it's expected to have changed. Next you'll need to run REPIN=workspace bazel query //... in examples/crate_universe, examples/ios_build. As far as actually testing on aarch64, you'll probably want to open a ticket at https://github.com/bazelbuild/continuous-integration asking about that.

@p0deje
Copy link
Contributor Author

p0deje commented Nov 19, 2022

Thanks! it looks like you have one test to update and some examples.
This just needs to have the sha256 updated since it's expected to have changed. Next you'll need to run REPIN=workspace bazel query //... in examples/crate_universe, examples/ios_build.

Thank you, this should now be fixed, I've also rebased and squashed the commits.

As far as actually testing on aarch64, you'll probably want to open a ticket at https://github.com/bazelbuild/continuous-integration asking about that.

I meant to test how good it works and if it's enough to be used. I'm just not sure I've done everything I had to. 😅

Copy link
Collaborator

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you run the release github action to confirm a aarch64 windows binary is successfully produced? #1195 shows how to perform releases which should work for your fork.

@p0deje
Copy link
Contributor Author

p0deje commented Nov 20, 2022

@UebelAndre Thank you for telling me to do so, I've then managed to use the rules on my VM and found a couple of problems I needed to fix to make it work.

I had to comment out the validation job because it checks that release can only be run on the main branch. After that, it succeeded in https://github.com/p0deje/rules_rust/actions/runs/3509347602 and created the release at https://github.com/p0deje/rules_rust/releases/tag/0.13.1.

Now I can finally load the Selenium codebase which uses rules_rust. I haven't tried compiling yet, but it's already the progress.

Copy link
Collaborator

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much an awesome work! Looks good to me 😄

@UebelAndre UebelAndre merged commit 8bb25b8 into bazelbuild:main Nov 21, 2022
@p0deje p0deje deleted the aarch64-pc-windows-msvc branch November 21, 2022 02:14
@p0deje
Copy link
Contributor Author

p0deje commented Nov 21, 2022

Thank you, looking forward to using the new release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants