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 instructions for building Universal2 on macOS via CMake #3568

Merged
merged 4 commits into from
Mar 28, 2023

Conversation

rickmark
Copy link
Contributor

This includes instructions for building as a Universal2 binary.

We could consider making this default for CMake builds, which would make homebrew install fat versions of zstd by default preventing numerous issues when using the binary on M1 / AppleSilicon by making both the arm64 and x86_64 versions availiable.

@Cyan4973
Copy link
Contributor

Cyan4973 commented Mar 23, 2023

The instructions look correct to me.

Worth mentioning though: on my local laptop (M1), the generated binaries crash immediately.
But this could be due to any number of external reasons, maybe some strict security policy which refuses fat binaries, or maybe the local compiler lacks something. Anyway, it's not clear what the problem could be on my local computer.

If it works fine on another M1 macos laptop, then I believe it's still worth documenting.

@rickmark
Copy link
Contributor Author

Just discovered, while macOS will successfully build arm64e, it will not run those binaries without some kernel switch as the ABI is not fixed. This affects the bin but not the lib, but out of simplicity I simply removed the arm64e bits.

The open question is do we make this configuration default for CMAKE, which would make brew install zstd FAT (I don't think we ought to since that's not how homebrew has worked historically?) Current discussion is here: Homebrew/brew#10307

README.md Outdated
```bash
cmake -B build-cmake-debug -S build/cmake -G Ninja -DCMAKE_OSX_ARCHITECTURES="x86_64;x86_64h;arm64"
cd build-cmake-debug
ninja build
Copy link
Contributor

Choose a reason for hiding this comment

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

note: ninja build will fail here : ninja: error: unknown target 'build'.

I think what you mean is ninja (only), this will employ the default target, which is the release build of both the library and CLI.

@Cyan4973
Copy link
Contributor

Cyan4973 commented Mar 23, 2023

Yes, this fixed the issue. Now the produced fat binary will run fine on my local M1 laptop.

The open question is do we make this configuration default for CMAKE

I don't think there is any need to rush to that conclusion. Maybe later,
For the time being, the instructions look clear to me. It seems brew could simply adopt the same solution to generate fat binaries on its side, without the need to change the build recipe for everyone.

fix minor doc mistake (`ninja build` doesn't work)
@rickmark
Copy link
Contributor Author

Yes, this fixed the issue. Now the produced fat binary will run fine on my local M1 laptop.

The open question is do we make this configuration default for CMAKE

I don't think there is any need to rush to that conclusion. Maybe later, For the time being, the instructions look clear to me. It seems brew could simply adopt the same solution to generate fat binaries on its side, without the need to change the build recipe for everyone.

I think for now it might be wise to make "fat builds" default, since it's not a huge library and will improve quality of life for anyone who depends on the library (as both will be available). If a particular CMake is too old, it will simply ignore it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants