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

Enable Windows on ARM builds for 0.16.x #1554

Closed
wants to merge 2 commits into from

Conversation

svenpaulsen
Copy link

Here's a PR to enable building 0.16.20 for Windows on ARM as requested in #1551.

Prerequisites to build from source:

  1. Download and install LLVM
  2. Download and install Perl
  3. Download and install NASM

@svenpaulsen svenpaulsen changed the title V0.16.20 patched Enable Windows on ARM builds for 0.16.x Nov 9, 2022
@svenpaulsen svenpaulsen mentioned this pull request Nov 9, 2022
@awakecoding
Copy link
Contributor

@svenpaulsen @briansmith I switched to this branch for my own builds, and it works 👍

@Alovchin91
Copy link
Contributor

Hi @briansmith Do you think this could be merged soon? It currently blocks quite a few projects…

@@ -238,6 +269,12 @@ pub(crate) mod arm {

#[cfg(all(
any(target_os = "android", target_os = "fuchsia", target_os = "linux"),

Choose a reason for hiding this comment

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

Shouldn't this line get deleted? The new block any block right under contains all the same oses, + windows.

jlfaucher added a commit to jlfaucher/executor that referenced this pull request Feb 16, 2023
@briansmith
Copy link
Owner

This is a very big change and I could use some hand-holding to understand it. I suspect that this PR is mostly copying things from the existing main branch. It would be extremely helpful if you could go through chunk-by-chunk in the GitHub review interface and explain where you copied the change from and why that change was copied, and go into more depth about how you came up with this set of changes in the first place.

Especially regarding the PerlAsm changes, it would really help me if you could document why and from where you copied those changes.

@briansmith
Copy link
Owner

I left some comments in rust-lang/rustup#2612 (comment) that are very relevant to the development and review of this PR.

@complexspaces
Copy link
Contributor

Hey everyone,

In case it is helpful, I just wanted to point out that we (1Password) backported the necessary components for WoA support in the 0.16 branch as part of 1Password/ring#8. The build.rs changes ended up being smaller then what was done here as the changes were rewritten to fit the state of 0.16 instead of backporting all of 0.17's new design. We've tested it and AFAWCT, it functions as expected in cross-compilation and in an ARM64 VM.

@Alovchin91
Copy link
Contributor

Alovchin91 commented Jul 23, 2023

@briansmith Would it make it easier / less uncertain for you if someone (me?) created a pull request from the 1Password’s branch? I understand that you’re concerned about the extent of changes in this PR.

@Alovchin91
Copy link
Contributor

@complexspaces Is it okay if I (or maybe you, if you’d prefer that) create a new PR from the branch you’ve mentioned?

@Alovchin91
Copy link
Contributor

Alternatively, @svenpaulsen would you maybe kindly respond to @briansmith’s concerns here: #1554 (comment) 😊 I hope either should help.

@jeremyd2019
Copy link

Especially regarding the PerlAsm changes, it would really help me if you could document why and from where you copied those changes.

openssl/openssl@b863e1e which added Windows on ARM support to openssl

@briansmith
Copy link
Owner

Closing this as not planned. It has been years since we've done anything with 0.16.x. If any of these changes would be helpful for 0.17, PLMK. Thanks!

@briansmith briansmith closed this May 21, 2024
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.

7 participants