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

Add prebuilds for ARM64 Macs #2354

Open
wants to merge 11 commits into
base: prebuilds
Choose a base branch
from

Conversation

jamesbvaughan
Copy link

@jamesbvaughan jamesbvaughan commented Feb 23, 2024

This is an updated attempt at #2245, making use of GitHub's new Apple Silicon runners.

With many Mac-based developers transitioning to Apple Silicon, this addition will have a significant impact on their cumulative yarn/npm install time.

@jamesbvaughan jamesbvaughan force-pushed the james/mac-arm64-prebuild branch 4 times, most recently from c255f18 to ac3db9a Compare February 23, 2024 02:44
@jamesbvaughan
Copy link
Author

I've temporarily removed the Linux and Windows builds from the workflow in order to reduce the Actions usage on my personal account while testing.

The current status here is that I'm hitting an error about napi.h not being found and I'm not sure how to proceed. Here's where you can see the error: https://github.com/jamesbvaughan/node-canvas/actions/runs/8014121461/job/21892227263#step:4:279

@adamcin
Copy link

adamcin commented Feb 23, 2024

I've temporarily removed the Linux and Windows builds from the workflow in order to reduce the Actions usage on my personal account while testing.

The current status here is that I'm hitting an error about napi.h not being found and I'm not sure how to proceed. Here's where you can see the error: https://github.com/jamesbvaughan/node-canvas/actions/runs/8014121461/job/21892227263#step:4:279

Hi @jamesbvaughan , it looks like you are building canvas sources that have migrated from nan to napi by way of node-addon-api, with related changes to gyp scripts described in this commit: jamesbvaughan@ce29f69

EDIT: I submitted a PR for your branch that I think may help jamesbvaughan#1

@jamesbvaughan
Copy link
Author

Alright, thanks to @adamcin's help, I think this is ready for a real review!

Let me know if you'd like me to clean up the git history at all.

@jamesbvaughan jamesbvaughan marked this pull request as ready for review February 28, 2024 04:42
@zbjornson
Copy link
Collaborator

Thanks for submitting this! Two notes:

@jamesbvaughan
Copy link
Author

Thanks @zbjornson. I'll hold off on further work here for now.

@rafaelmaiolla
Copy link

Is there any update in having arm64 pre-build binaries?

@mn4367
Copy link

mn4367 commented Apr 26, 2024

@rafaelmaiolla, in my opinion still boils down to bundling the generated native module with the native dependencies. You may have a look to issue #2036 which has a bit more info on why providing pre-built binaries is difficult.

Edit:

I just saw that executing/loading my bundled version let to a Killed: 9. The same error occurs, if one tries to execute a self-compiled executable on an Apple silicon chip if it isn't signed. Maybe I'll find the time to do this again and see what happens, if I sign the generated binary.

Follow up:

With regard to #2036 this is indeed a signing problem. Signing all generated/patched binaries (canvas.node, *.dylib) in build after running bundle.sh everything worked fine! This at least proves, that a prebuild self contained version can be built for Apple Silicon. I've done the signing with these tools. The interesting part here is that the tools work cross platform, so it's even possible to sign Apple Silicon binaries on a Windows or Linux machine!

@net-tech
Copy link

net-tech commented Jul 9, 2024

With regard to #2036 Signing all generated/patched binaries (canvas.node, *.dylib) in build after running bundle.sh everything worked fine! This at least proves, that a prebuild self contained version can be built for Apple Silicon. I've done the signing with these tools. The interesting part here is that the tools work cross platform, so it's even possible to sign Apple Silicon binaries on a Windows or Linux machine!

If singing is working then would the next step be for OP to add signing to their PR? I'm sorry if I sound impatient, but a prebuilt binary could remove around 50s on our GitHub actions which is why I'm eager for there to be a prebuilt binary.

@jamesbvaughan jamesbvaughan changed the base branch from master to prebuilds July 10, 2024 01:18
@jamesbvaughan
Copy link
Author

I've rebased these changes on the prebuilds branch and removed some unnecessary changes. If the code signing truly is all that's left, then I think we're close!

With regard to #2036 this is indeed a signing problem. Signing all generated/patched binaries (canvas.node, *.dylib) in build after running bundle.sh everything worked fine! This at least proves, that a prebuild self contained version can be built for Apple Silicon. I've done the signing with these tools. The interesting part here is that the tools work cross platform, so it's even possible to sign Apple Silicon binaries on a Windows or Linux machine!

@mn4367 Thanks for trying that out! Do you have any public code demonstrating the use of that code signing tool in a context like this? I haven't dealt with Apple's code signing before and if you have an example, it could save me a lot of time.

@mn4367
Copy link

mn4367 commented Jul 11, 2024

@jamesbvaughan

I haven't dealt with Apple's code signing before ...

Same here :-(.

Everything I did was to run prebuild/MacOS/bundle.sh and then run rcodesign sign <.dylib> with every single .dylib file in build/Release e.g. rcodesign sign ./build/Release/libcairo-gobject.2.dylib, rcodesign sign ./build/Release/libcairo.2.dylib etc.

After that I'm able to run all tests successfully but I've no idea if sign creates files that can be loaded on all users machines not only on my machine. I've uploaded a tarball here, maybe you can check if they work on your machine? Please ping me if you've downloaded the tarball so that I can delete it.

If it doesn't work directly after unpacking you could try calling xattr -dr com.apple.quarantine with every .dylib and .node and see if that works.

@jamesbvaughan
Copy link
Author

@mn4367 Thanks for all that. I've downloaded and unpacked your tarball and npm test passes for me with those files.

To be honest, I'm wading in unfamiliar territory here and I'm curious for @zbjornson's input on code signing.

The tests pass in the Make prebuilds CI workflow for my branch as-is, which makes me wonder if this is already good to go: https://github.com/jamesbvaughan/node-canvas/actions/runs/9866841579/job/27246266340 (The overall workflow failed because I don't have the necessary secrets for uploading, which is expected.)

@mn4367
Copy link

mn4367 commented Jul 13, 2024

@jamesbvaughan, I did another quick test: I created a Node.js SEA, signed it with rcodesign (the same way like for canvas) and send it over via Dropbox to a friend to see, if it can be run on his machine. Upon first execution there the OS complained about it (unknown developer etc.) but after allowing it to execute (via the System Settings App) it runs without further problems. Then I analyzed this executable and the node executable used to create this application:

My application:

$ codesign --display --verbose=4 ./my-sample-app 
Executable=/Users/somepath/my-sample-app
Identifier=my-sample-app
Format=Mach-O thin (arm64)
CodeDirectory v=20400 size=672899 flags=0x2(adhoc) hashes=21023+2 location=embedded
VersionPlatform=1
VersionMin=720896
VersionSDK=786688
Hash type=sha256 size=32
CandidateCDHash sha256=de3c83d5c5ff6fd32cb899d01fc275fd0a9763b2
CandidateCDHashFull sha256=de3c83d5c5ff6fd32cb899d01fc275fd0a9763b21c6c1e68d924a5ac4f6678a6
Hash choices=sha256
CMSDigest=de3c83d5c5ff6fd32cb899d01fc275fd0a9763b21c6c1e68d924a5ac4f6678a6
CMSDigestType=2
Executable Segment base=0
Executable Segment limit=66027520
Executable Segment flags=0x1
Page size=4096
CDHash=de3c83d5c5ff6fd32cb899d01fc275fd0a9763b2
Signature=adhoc
Info.plist=not bound
TeamIdentifier=not set
Sealed Resources=none
Internal requirements count=0 size=12

Node.js executable:

$ codesign --display --verbose=4 ./node
Executable=/Users/somepath/node
Identifier=node
Format=Mach-O thin (arm64)
CodeDirectory v=20500 size=730000 flags=0x10000(runtime) hashes=22802+7 location=embedded
VersionPlatform=1
VersionMin=720896
VersionSDK=786688
Hash type=sha256 size=32
CandidateCDHash sha256=8f343dfa30ae01932ad6c41f57f8d83bd4a747ff
CandidateCDHashFull sha256=8f343dfa30ae01932ad6c41f57f8d83bd4a747ffb1382c42f56a85b548492900
Hash choices=sha256
CMSDigest=8f343dfa30ae01932ad6c41f57f8d83bd4a747ffb1382c42f56a85b548492900
CMSDigestType=2
Executable Segment base=0
Executable Segment limit=66027520
Executable Segment flags=0x1
Page size=4096
CDHash=8f343dfa30ae01932ad6c41f57f8d83bd4a747ff
Signature size=8987
Authority=Developer ID Application: Node.js Foundation (HX7739G8FX)
Authority=Developer ID Certification Authority
Authority=Apple Root CA
Timestamp=8. Jul 2024 at 15:26:11
Info.plist=not bound
TeamIdentifier=HX7739G8FX
Runtime Version=12.1.0
Sealed Resources=none
Internal requirements count=1 size=164

So the only difference is that with rcodesign sign <file> a simple adhoc signature is created which forces users to explicitly give permission to execute while 'proper' signing with a 'real' developer certificate doesn’t need any user permissions. But there is a caveat: the (Zip) file which was downloaded by my friend has been unpacked autoamtically by Safari (as usual) and the contained unpacked executable has been marked as 'Downloaded from the Internet' (or similar, he has a german OS).

So there are basically two questions:

  1. Will macOS also complain for files downloaded by other means, e.g. by an npm script and not through Safari?
  2. Is it even necessary to give permissions for dylibs (like in the case of Canvas)?

If the answer to both questions is no, then rcodesign sign <file> creating an adhoc signature should be just enough to create the prebuilds for darwin-arm64 and darwin-x64. Signing with a real certificate would of course be the best solution.

If I find the time, I'll create a test repository with a pre-built, standalone canvas module. Maybe some people here can then download it and test if it works out of the box.

Edit, PS:
Thanks for testing the tarball! 👍

@mn4367
Copy link

mn4367 commented Jul 13, 2024

I found something interesting in the rcodesign docs in the meantime here: the last paragraph in this section clearly states that the quarantine attribute (which makes macOS bark) is not added to files downloaded by non-browser tools like curl or wget. So using an adhoc signature should be sufficient to create prebuilds for macOS. In the end, it no longer seems to be a technical requirement, but rather a question of 'reputation' whether or not to use a real certificate for signing.

@jamesbvaughan
Copy link
Author

I had a bit of time to work on this today and found that the creator of rcodesign now provides a GitHub Action for performing code signing: https://github.com/marketplace/actions/apple-code-signing

I've added that action to the workflow in this PR and it seems to be signing successfully.

@zbjornson Let me know if there's anything else I'm missing here.

@kevinji
Copy link

kevinji commented Sep 11, 2024

@zbjornson Is there anything blocking the release of this PR?

@de-robat
Copy link

Would like to know too. Just to add to the urgency, appflow will deprecated the old Mac builders and will switch to Silicon runners only at 1st of October 24. With e.g. jsdom relying on this module (which is a direct dependency of jest, isomorphic-dompurify, ...)) lots of builds will fail without this binding. What needs to be done to get this across the fnishline?

Copy link
Collaborator

@zbjornson zbjornson left a comment

Choose a reason for hiding this comment

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

Sorry for the silence. This looks straightforward. I don't have a mac to test on, but I'm happy to merge this, make the prebuild for 3.0-rc2 and make sure it works for folks.

One tiny question...

@@ -169,14 +174,19 @@ jobs:
npm install --ignore-scripts
. prebuild/macOS/preinstall.sh
cp prebuild/macOS/binding.gyp binding.gyp
node-gyp rebuild -j 2
node-gyp rebuild -j 2 --arch=${{ matrix.os.arch }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

node-gyp defaults to process.arch. Is this change needed?

Copy link
Author

Choose a reason for hiding this comment

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

Good question - I'm not experienced with node-gyp and I think I copied this from someplace else without testing it without that option. I'll try it out without it today.

Copy link
Author

Choose a reason for hiding this comment

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

Alright I've pushed a commit that removes that flag, and as far as I can tell, this step of the build process is completing as intended: https://github.com/jamesbvaughan/node-canvas/actions/runs/11115619922/job/30884399250#step:4:178

I should point out though that I'm wading in unfamiliar territory for me when it comes to node-gyp and macos binary signing, so I'd really appreciate if someone with more experience with those could help verify that everything looks correct here.

Copy link
Author

Choose a reason for hiding this comment

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

Alright I've pushed a commit that removes that flag, and as far as I can tell, this step of the build process is completing as intended: https://github.com/jamesbvaughan/node-canvas/actions/runs/11115619922/job/30884399250#step:4:178

I should point out though that I'm wading in unfamiliar territory for me when it comes to node-gyp and macos binary signing, so I'd really appreciate if someone with more experience with those could help verify that everything looks correct here.

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.

8 participants