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

gh-actions: Add macOS builds #5036

Merged
merged 1 commit into from
Jan 17, 2022
Merged

gh-actions: Add macOS builds #5036

merged 1 commit into from
Jan 17, 2022

Conversation

wojtekmach
Copy link
Contributor

@wojtekmach wojtekmach commented Jul 8, 2021

Another stab at #4487.

We statically link OpenSSL:

% otool -L otp/otp/lib/crypto-5.0.2/priv/lib/crypto.so
otp/otp/lib/crypto-5.0.2/priv/lib/crypto.so:
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1281.100.1)

See https://rentzsch.tumblr.com/post/33696323211/wherein-i-write-apples-technote-about-openssl-on for rationale.

We also statically link wxWidgets:

% otool -L otp/otp/lib/wx-2.0.1/priv/wxe_driver.so
otp/otp/lib/wx-2.0.1/priv/wxe_driver.so:
        /System/Library/Frameworks/IOKit.framework/Versions/A/IOKit (compatibility version 1.0.0, current version 275.0.0)
        /System/Library/Frameworks/Carbon.framework/Versions/A/Carbon (compatibility version 2.0.0, current version 164.0.0)
        /System/Library/Frameworks/Cocoa.framework/Versions/A/Cocoa (compatibility version 1.0.0, current version 23.0.0)
        /System/Library/Frameworks/QuartzCore.framework/Versions/A/QuartzCore (compatibility version 1.2.0, current version 1.11.0)
        /System/Library/Frameworks/AudioToolbox.framework/Versions/A/AudioToolbox (compatibility version 1.0.0, current version 1000.0.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1292.100.5)
        /System/Library/Frameworks/OpenGL.framework/Versions/A/OpenGL (compatibility version 1.0.0, current version 1.0.0)
        /System/Library/Frameworks/AGL.framework/Versions/A/AGL (compatibility version 1.0.0, current version 1.0.0)
        /System/Library/Frameworks/WebKit.framework/Versions/A/WebKit (compatibility version 1.0.0, current version 611.1.21)
        /usr/lib/libexpat.1.dylib (compatibility version 7.0.0, current version 8.0.0)
        /usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.11)
        /System/Library/Frameworks/Security.framework/Versions/A/Security (compatibility version 1.0.0, current version 59754.100.106)
        /usr/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)
        /usr/lib/libcurl.4.dylib (compatibility version 7.0.0, current version 9.0.0)
        /usr/local/opt/xz/lib/liblzma.5.dylib (compatibility version 8.0.0, current version 8.5.0)
        /usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 905.6.0)

Worth remembering that when downloading the release from GitHub via Web browser, macOS will put the file under quarantine:

% ~/Downloads% xattr -l otp_macos_24.0.3.tar
com.apple.quarantine: 0082;60e71fca;Safari;

and after unpacking the executables will be put under quarantine as well, meaning they cannot be executed and you'll get a security warning. Here's how we can remove the tarball from quarantine:

% xattr -d otp_macos_24.0.3.tar

This will not be a problem when downloading the tarball with curl, etc.

@wojtekmach wojtekmach changed the title Add macOS builds to GitHub actions ci: Add macOS builds Jul 8, 2021
@wojtekmach
Copy link
Contributor Author

There was a little bit of churn getting the build right but I believe it is now ready for review.

@wojtekmach
Copy link
Contributor Author

For anyone wanting to test it, the artefacts can be found on the checks page:

image

@KennethL KennethL added the team:PO Assigned to OTP team PO label Jul 12, 2021
@dominicletz
Copy link
Contributor

Good stuff @wojtekmach! Works perfectly on my intel machine, but trying this on my M1 it creates a segfault instead of starting rosetta. I've added a pull request on your home repo/branch to add a rosetta hint to the plist file, that should enable running this prebuilt image even on M1's until M1 universal binaries are fully supported by erlang.

Cheers.

@wojtekmach
Copy link
Contributor Author

@dominicletz random: do you have a preference between compiling against wxwidgets 3.1.4 or the latest 3.1.5? I sticked to the older version because that's what we used for Windows, but happy to bump to the latest if that is desirable. Could be another PR if that's what the OTP team prefers to better keep track of. Also see #5043.

@dominicletz
Copy link
Contributor

For macOS the 3.1.5 version is definitely much better. The still relatively new osx/ port in wxWidgets has received tons of fixes between 3.1.4 and 3.1.5

@wojtekmach
Copy link
Contributor Author

Thank you! In that case I'm gonna go ahead and bump here to 3.1.5. And if there's interest to do the same for Windows, happy to do it here or in a separate PR.

.github/scripts/build-macos-wxwidgets.sh Outdated Show resolved Hide resolved
.github/workflows/main.yaml Outdated Show resolved Hide resolved
@wojtekmach
Copy link
Contributor Author

@dominicletz I've updated the PR. In my testing I found that it worked without changing Info.plist so I leaved that out. I also added less entitlements.

There's still problem with loading crypto.so under rosetta:

~/test% arch
arm64
~/test% ./bin/erl
Erlang/OTP 25 [DEVELOPMENT] [erts-12.0.3] [source-7f791b7ae7] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit:ns]

Eshell V12.0.3  (abort with ^G)
1> crypto:start().
=ERROR REPORT==== 7-Aug-2021::13:37:33.872765 ===
Unable to load crypto library. Failed with error:
"load_failed, Failed to load NIF library: 'dlopen(/Users/wojtek/test/lib/crypto-5.0.2/priv/lib/crypto.so, 2): no suitable image found.  Did find:
        /Users/wojtek/test/lib/crypto-5.0.2/priv/lib/crypto.so: code signature in (/Users/wojtek/test/lib/crypto-5.0.2/priv/lib/crypto.so) not valid for use in process using Library Validation: mapped file has no cdhash, completely unsigned? Code has to be at least ad-hoc signed.
        /Users/wojtek/test/lib/crypto-5.0.2/priv/lib/crypto.so: stat() failed with errno=35'"
OpenSSL might not be installed on this system.

I tried adding the com.apple.security.cs.disable-library-validation entitlement, but with it, bin/erl segfaults. I tried a bunch of combinations of code sign flags and entitlements, code signing .so files, but no luck either.

Any help would be very appreciated.

@wojtekmach
Copy link
Contributor Author

wojtekmach commented Oct 4, 2021

A quick update on code signing.

As a reminder, I'm trying to build OTP on Mac x86_64 and run it on Mac ARM64 via Rosetta 2. I want to be able to do these two things:

  1. Start Erlang
  2. Start crypto app

Here's what I found:

  1. On clean otp master, erlang segfaults.
  2. without jit and with appropriate entitlement, everything works. patch
  3. with jit and without the disable library validation entitlement, starting erlang works but starting crypto does not. patch
  4. with jit and disable library validation entitlements (also trying other smilingly related entitlements), it segfaults. patch

Any help, especially if familiar with code signing and/or with access to x64/ARM64 hardware, would be appreciated.

@kubkon
Copy link

kubkon commented Oct 6, 2021

A quick update on code signing.

As a reminder, I'm trying to build OTP on Mac x86_64 and run it on Mac ARM64 via Rosetta 2. I want to be able to do these two things:

Unless you are generating an arm64 binary, it doesn't need to be codesigned to be run via the rosetta emulator on an arm64 Mac. If you look at the Zig's linker sources here, we only codesign the produced binary if the target architecture is arm64 and target OS is a macOS:

const requires_adhoc_codesig = cpu_arch == .aarch64 and (os_tag == .macos or abi == .simulator);

Intel-bound binaries are not codesigned and they can be run via rosetta just fine.

  1. Start Erlang
  2. Start crypto app

Here's what I found:

  1. On clean otp master, erlang segfaults.

Lack of code signature should never lead to a segfault in the directly run binary, but trigger a SIGKILL by the kernel.

  1. without jit and with appropriate entitlement, everything works. patch

Does that perhaps mean that it's the JIT that is generating arm64 code on the fly and the output is somehow lacking/needs code signature?

  1. with jit and without the disable library validation entitlement, starting erlang works but starting crypto does not. patch
  2. with jit and disable library validation entitlements (also trying other smilingly related entitlements), it segfaults. patch

Any help, especially if familiar with code signing and/or with access to x64/ARM64 hardware, would be appreciated.

Anyway, I've got access to arm64 Mac, and I'd be happy to help but will require some handholding as I'm not familiar with Erlang at all :-)

@wojtekmach
Copy link
Contributor Author

Unless you are generating an arm64 binary, it doesn't need to be codesigned to be run via the rosetta emulator on an arm64 Mac.

Thank you, that makes sense, and that's what I was missing.

Does that perhaps mean that it's the JIT that is generating arm64 code on the fly and the output is somehow lacking/needs code signature?

Yeah, seems JIT is the culprit here and it sounds like there're JIT bugs that surface under Rosetta, I am told there are wip patches for them.

So I think code signing and rosetta was a red herring. Apologies all! I'm going to go ahead and remove the entitlement stuff from the PR.

I think code signing might come up again with regards to quarantine that I mentioned earlier in the thread, the fact that if someone downloads our binary using the web browser, they'd get a security error, but I'd argue that it is not a show stopper. Once we have binary assets on GitHub Releases, folks could download them using e.g. curl -LO https://github.com/erlang/otp/releases/download/OTP-x/x.zip and they'd just work.

and I'd be happy to help but will require some handholding as I'm not familiar with Erlang at all

@kubkon I'm sure you have your plate way full but I'd remiss not to use this opportunity to plug the excellent The Soul of Erlang and Elixir talk by Sasa Juric. ;) Thanks for the help and as you know, you're gonna be hearing more from us, Zig and Erlang/Elixir are a fantastic pairing!

@jhogberg
Copy link
Contributor

jhogberg commented Oct 6, 2021

Yeah, seems JIT is the culprit here and it sounds like there're JIT bugs that surface under Rosetta, I am told there are wip patches for them.

If it is what I think it is, it's a deficiency in Rosetta and not a bug in the JIT per se. Did you give my branch a try? I can't test it myself.

https://github.com/jhogberg/otp/tree/john/erts%2Fmore-autumn-cleaning

@wojtekmach
Copy link
Contributor Author

@jhogberg got it, I'll test it right now. Btw, how would I build with both emu and jit flavours? I'm thinking that maybe it would be worth to ship with both so the arm users would have an easy workaround. This is a stop gap solution anyway, in the long term we should builds on arm64 machines (GitHub is allegedly working on provisioning those but no ETA yet) in which case the problem goes away.

@wojtekmach
Copy link
Contributor Author

I just tested it and yeah same segfault.

@jhogberg
Copy link
Contributor

jhogberg commented Oct 6, 2021

Pity, I'd need a closer look at the segfault to see if it's at all fixable, but I'm not sure it'll be worth the time. Maybe we should just go with the interpreter in this build?

@wojtekmach
Copy link
Contributor Author

I’d hate for folks to miss out on JIT. I’d personally rather have it not working on arm than punish people on x86. It seems it is really a matter of time until we could have native arm builds and in the meantime building with kerl and friends is a great option, one they had to use all along anyway.

@wojtekmach wojtekmach changed the title ci: Add macOS builds gh-actions: Add macOS builds Oct 7, 2021
@wojtekmach
Copy link
Contributor Author

As far as I am concerned, the PR is done. Looking forward to the feedback!

@paulo-ferraz-oliveira
Copy link
Contributor

Any update on this?

@bjorng
Copy link
Contributor

bjorng commented Jan 14, 2022

@KennethL @garazdawi Since it seems that Github Actions successfully built Erlang/OTP for macOS in this branch, I suggest that we merge this PR now. If it turns out to cause problems, we can revert it. OK?

The build script can be conveniently executed locally:

    % time MAKEFLAGS="-j8" .github/scripts/build-macos.sh

We statically link OpenSSL:

    % otool -L otp/otp/lib/crypto-5.0.2/priv/lib/crypto.so
    otp/otp/lib/crypto-5.0.2/priv/lib/crypto.so:
            /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1281.100.1)

See e.g.: https://rentzsch.tumblr.com/post/33696323211/wherein-i-write-apples-technote-about-openssl-on

We also statically link wxWidgets:

    % otool -L lib/wx-2.1.1/priv/wxe_driver.so
    lib/wx-2.1.1/priv/wxe_driver.so:
            /System/Library/Frameworks/IOKit.framework/Versions/A/IOKit (compatibility version 1.0.0, current version 275.0.0)
            /System/Library/Frameworks/Carbon.framework/Versions/A/Carbon (compatibility version 2.0.0, current version 165.0.0)
            /System/Library/Frameworks/Cocoa.framework/Versions/A/Cocoa (compatibility version 1.0.0, current version 23.0.0)
            /System/Library/Frameworks/QuartzCore.framework/Versions/A/QuartzCore (compatibility version 1.2.0, current version 1.11.0)
            /System/Library/Frameworks/AudioToolbox.framework/Versions/A/AudioToolbox (compatibility version 1.0.0, current version 1000.0.0)
            /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1311.0.0)
            /System/Library/Frameworks/OpenGL.framework/Versions/A/OpenGL (compatibility version 1.0.0, current version 1.0.0)
            /System/Library/Frameworks/AGL.framework/Versions/A/AGL (compatibility version 1.0.0, current version 1.0.0)
            /System/Library/Frameworks/WebKit.framework/Versions/A/WebKit (compatibility version 1.0.0, current version 612.3.6)
            /System/Library/Frameworks/Security.framework/Versions/A/Security (compatibility version 1.0.0, current version 60157.60.19)
            /usr/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)
            /usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1200.3.0)

Worth remembering that when downloading the release from GitHub via Web
browser, macOS will put the file under quarantine:

    % ~/Downloads% xattr -l otp_macos_25.0-rc0.tar
    com.apple.quarantine: 0082;60e71fca;Safari;

and after unpacking the executables will be put under quarantine as
well, meaning they cannot be executed and you'll get a security warning.
Here's how we can remove the tarball from quarantine:

    % xattr -d otp_macos_25.0-rc0.tar

This will not be a problem when downloading the tarball with curl, etc.
The proper solution is to codesign binaries.
@bjorng
Copy link
Contributor

bjorng commented Jan 14, 2022

Thanks!

We have merged this PR so that it can be more thoroughly tested (every time someone creates a pull request based on master).

Note that we still consider this feature experimental for the moment; if it turns to cause a lot of problems (frequent failures of the macOS builds), we will consider reverting it.

@wojtekmach
Copy link
Contributor Author

Thank you! Please feel free to ping me directly if any issues arise with the build.

Just a friendly reminder that this is just step one, the second and final step is to attach these builds to GitHub releases going forward. :) Definitely makes sense to make sure the builds are stable enough before doing that though!

@paulo-ferraz-oliveira
Copy link
Contributor

Thanks for the updates.

This would be a nice addition to setup-beam (which is what prompted me to "wake this up"), once you feel it's more stable.

And thank you @wojtekmach for your work on this.

@bjorng bjorng merged commit 110e690 into erlang:master Jan 17, 2022
@wojtekmach wojtekmach deleted the wm-macos-builds branch February 16, 2022 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team:PO Assigned to OTP team PO
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants