Skip to content
This repository has been archived by the owner on May 3, 2022. It is now read-only.

Add cross-compilation for x86 and arm64 targets (including Apple Silicon) #7

Conversation

dennisameling
Copy link
Contributor

@dennisameling dennisameling commented Sep 12, 2020

Currently, zkgroup only supports x64. This PR adds cross-compilation support to zkgroup. It assumes that a x64 host is being used.

Also bumps the Rust toolchain to 1.49.0 as that's needed for Apple Silicon support.

Related to signalapp/Signal-Desktop#3745 and signalapp/Signal-Desktop#4461.

Supported architectures:

  • Windows:
    • x64
    • ia32
    • arm64
  • Linux:
    • x64
  • MacOS:
    • x64
    • arm64 (Apple Silicon)

@KishanBagaria
Copy link

This is great! Can you also add in support for Apple silicon Macs? I believe the Rust target is aarch64-apple-darwin

@dennisameling
Copy link
Contributor Author

Potentially - will have a look later today! 😊

@KishanBagaria
Copy link

I did a find and replace all of x86_64-apple-darwin with aarch64-apple-darwin, removed the hardcoding of 1.41.1 toolchain since it doesn't support aarch64-apple-darwin, and ran it on an M1 Mac to confirm it works: https://github.com/KishanBagaria/zkgroup/commit/11d19638a2bea140b84469ced6fe9290577d3322

@dennisameling
Copy link
Contributor Author

Gotta love Rust for their compilers! 🎉 Can you also confirm it works if you use it in combination with Signal Desktop? Having a native binary isn't always a guarantee that it works in combination with the consuming app 😊

@KishanBagaria
Copy link

Yep! That's what I confirmed it with :) (altho not the entire Signal-Desktop but a stripped down, forked version of v1.39.6)

@dennisameling
Copy link
Contributor Author

Awesome! Will incorporate your changes into this PR later today :)

@dennisameling
Copy link
Contributor Author

dennisameling commented Feb 9, 2021

I realized that, just like you, I only tested by replacing the build arch to arm64 back in September. But in a real-world scenario, this library would be supporting multiple architectures at the same time. I think 2f4bdfc would be a feasible option (it doesn't consider cross-compilation from e.g. x64 to arm64 yet though), but I haven't had time to test it yet. Will do so in the coming days. Given that this PR has been open for a while, I don't think we have to rush too much to squeeze in these changes.

@dennisameling dennisameling marked this pull request as draft February 9, 2021 20:53
@dennisameling
Copy link
Contributor Author

@KishanBagaria this PR now supports cross-compiling for Apple Silicon, but the downside is that I had to bump the Rust toolchain to 1.49.0 which introduces some negative side-effects: https://github.com/dennisameling/zkgroup/runs/1882504030?check_suite_focus=true

Could you try cloning this PR locally and then running:

cd ffi/node
make libzkgroup CARGO_TARGET=x86_64-apple-darwin NODE_ARCH=x64
make libzkgroup CARGO_TARGET=aarch64-apple-darwin NODE_ARCH=arm64

You should then be able to find libzkgroup-x64.dylib and libzkgroup-arm64.dylib in the ffi/node folder.

I also updated Native.ts to recognize the architecture that is being cross-compiled (if npm_config_arch is set), or the arch that NodeJS is currently running on (in that order):

const arch = process.env.npm_config_arch || process.arch;
let libraryPath = join(rootPath.replace('app.asar', 'app.asar.unpacked'), 'libzkgroup-' + arch);

I'll try to fix CI later this week 👍🏼

@KishanBagaria
Copy link

@dennisameling I'll DM you access to my rented arm64 Mac, not using it for anything atm :)

@dennisameling dennisameling changed the title Add cross-compilation for x86 and arm64 Windows Add cross-compilation for x86 and arm64 targets (including Apple Silicon) Feb 11, 2021
@dennisameling
Copy link
Contributor Author

Was able to let CI generate the binaries both for Windows and Mac arm64 (Apple Silicon), so that's good news! 🚀 https://github.com/dennisameling/zkgroup/releases/tag/v0.7.1-test6

@sebdanielsson
Copy link

Is arm64 for Linux on the roadmap as well?

@dennisameling
Copy link
Contributor Author

@sebdanielsson I tried cross-compiling for Linux arm64 as well, but on a x86_64 host that leads to linker errors:

          /home/linuxbrew/.linuxbrew/bin/ld: aarch64 architecture of input file `/home/dennis/.rustup/toolchains/1.49.0-x86_64-unknown-linux-gnu/lib/rustlib/aarch64-unknown-linux-gnu/lib/libcompiler_builtins-dedf96718b1b5f55.rlib(compiler_builtins-dedf96718b1b5f55.compiler_builtins.9btqwus4-cgu.3.rcgu.o)' is incompatible with i386:x86-64 output
          /home/linuxbrew/.linuxbrew/bin/ld: aarch64 architecture of input file `/home/dennis/.rustup/toolchains/1.49.0-x86_64-unknown-linux-gnu/lib/rustlib/aarch64-unknown-linux-gnu/lib/libcompiler_builtins-dedf96718b1b5f55.rlib(compiler_builtins-dedf96718b1b5f55.compiler_builtins.9btqwus4-cgu.80.rcgu.o)' is incompatible with i386:x86-64 output
          /home/linuxbrew/.linuxbrew/bin/ld: aarch64 architecture of input file `/home/dennis/.rustup/toolchains/1.49.0-x86_64-unknown-linux-gnu/lib/rustlib/aarch64-unknown-linux-gnu/lib/libcompiler_builtins-dedf96718b1b5f55.rlib(compiler_builtins-dedf96718b1b5f55.compiler_builtins.9btqwus4-cgu.1.rcgu.o)' is incompatible with i386:x86-64 output
          /home/linuxbrew/.linuxbrew/bin/ld: final link failed: file in wrong format
          collect2: error: ld returned 1 exit status

Building on an arm64 host (Surface Pro X using WSL2 Ubuntu 20.04) works though. Since GH Actions is x64-only, an option might be to add a Dockerfile which uses things like crossbuild-essential-arm64: https://github.com/atom/node-keytar/pull/352/files#diff-c6758de1a8743104c2c4937f73f633ca77ab7475961efaa7c44b86c5b86061cf

Do you have any other suggestions? If not, I'll go ahead and try with the Dockerfile. Should do the trick

@dennisameling
Copy link
Contributor Author

Since there were some updates in this repo currently which are conflicting with my changes, I decided to just create a new PR as that was the fastest: #14

Closing this PR in favor of the newer one

@dennisameling
Copy link
Contributor Author

Is arm64 for Linux on the roadmap as well?

@sebdanielsson I got it to work with cross-compilation on Linux to arm64 and ia32: #14

Pre-built binaries are now in dennisameling/signal-zkgroup-node@5f28c4b 🎉

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

Successfully merging this pull request may close these issues.

3 participants