Add node multi-arch/cross-compilation support #195
Conversation
dennisameling
changed the title
dennisameling
changed the title
| print('ERROR: --node_arch is required') | ||
| return 1 | ||
| if node_arch.startswith('..\\'): | ||
| node_arch = node_arch[3:] |
There was a problem hiding this comment.
@jack-signal, do you remember what this ..\ check is for? It doesn't seem like any of these would come up in arguments.
There was a problem hiding this comment.
It's certainly not supposed to. But it was showing up when invoked via gyp on Windows. I think the way gyp (or possibly Python) are parsing command line arguments on Windows is wrong, but fixing that was more than I wanted to deal with.
There was a problem hiding this comment.
Yeah I also had it show up on Windows, very weird indeed... I think it's Python's parsing of command line arguments on Windows though, rather than Gyp, IIRC
dennisameling
force-pushed
the
add-cross-compilation-support
branch
from
6e2cfba to
e20408c
Compare
dennisameling
force-pushed
the
add-cross-compilation-support
branch
from
ed9cfa5 to
0042e0d
Compare
| - name: Get Node version from .nvmrc | ||
| id: get-nvm-version | ||
| shell: bash | ||
| run: echo "::set-output name=node-version::$(cat .nvmrc)" |
There was a problem hiding this comment.
This is needed because the Windows image doesn't have nvm installed
|
Alright, this should be good to go now @jrose-signal. Thanks for the feedback! 🚀 |
|
Thanks for the contribution, and for working through all the follow-up! |
Needed for Apple Silicon/Windows arm64/Windows ia32
Ref signalapp/Signal-Desktop#3745 (comment)
Ref signalapp/Signal-Desktop#1636 (comment)
Ref signalapp/Signal-Desktop#4461
How to test:
npm_config_archis only needed when you're cross-compiling from a x64 host, e.g. x64 to ia32.Branch with generated binaries: https://github.com/dennisameling/libsignal-client-node/tree/cross-compilation-compat/build
Tested on: