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

Mac: Provide universal binaries for x86_64 & arm64e #2790

Closed
Tracked by #2357
hoffie opened this issue Aug 20, 2022 · 7 comments · Fixed by #2808
Closed
Tracked by #2357

Mac: Provide universal binaries for x86_64 & arm64e #2790

hoffie opened this issue Aug 20, 2022 · 7 comments · Fixed by #2808
Assignees
Labels
feature request Feature request
Milestone

Comments

@hoffie
Copy link
Member

hoffie commented Aug 20, 2022

What is the current behaviour and why should it be changed?

#2357 introduced native M1 releases. It would be more user-friendly if we packaged both x86_64 and arm64e builds into a single universal binary. This is possible using lipo, but the details have to be figured out.

https://developer.apple.com/documentation/apple-silicon/building-a-universal-macos-binary

Describe possible approaches

A first attempt of putting both archs into QMAKE_APPLE_DEVICE_ARCHS has been done in #2357, but it failed due to arch-specific code paths.

If we can't get that to work, it will either need moving both build runs into a single CI job or will require another CI job which takes the artifacts from the previous steps and combines them. The former sounds more realistical, especially considerung the .dmg generation.

Has this feature been discussed and generally agreed?

See #2357

@hoffie hoffie added the feature request Feature request label Aug 20, 2022
@hoffie hoffie added this to the Release 3.10.0 milestone Aug 20, 2022
@ann0see
Copy link
Member

ann0see commented Aug 22, 2022

A first attempt via QMAKE_APPLE_DEVICE_ARCHS has been done in #2357, but it failed due to arch-specific code paths.

So is opus the problem? We should understand if it's a Qt bug or opus doesn't support the config option.

@hoffie
Copy link
Member Author

hoffie commented Aug 22, 2022

opus doesn't know about QMAKE_* variables. The issue is that we sneak in the bundled opus compilation via our qmake build system. Part of this is the decision to include platform-specific performance improvements for opus. The issue is that this detection works once and decides to include certain things (i.e. the x86_64 opus optimizations). This makes the compiler fail when doing the arm compilation, as those headers are not supported there.

For testing if this path is usable at all, it might make sense to drop the opus performance improvements. If this works, we could look at finding a way to include those properly for such multi-arch builds. I don't think qmake has any features for that ("arm-only source files"), but we might be able to come up with something.

@hoffie
Copy link
Member Author

hoffie commented Aug 23, 2022

After having a closer look, I don't see a sane way of passing certain flags/defines for specific archs only (well, clang supports -Xarch_foo, but... we would have to sneak that in via qmake) or specific source or header files for specific archs only.

I suspect the better way forward would be running two standard builds. This could be done in a single CI job which would save all the resources for preparation steps.
I've manually (via SSH) done the following and the resulting binary looks correct:

  • /usr/local/opt/qt/6.3.1/macos/bin/qmake QMAKE_APPLE_DEVICE_ARCHS=x86_64 QT_ARCH=x86_64 TARGET=jamulus.x86_64
  • /usr/local/opt/qt/6.3.1/macos/bin/qmake QMAKE_APPLE_DEVICE_ARCHS=arm64 QT_ARCH=arm64 TARGET=jamulus.arm64
  • lipo -create -output jamulus.universal jamulus.*.app/Contents/MacOS/jamulus.*

Result:

bash-3.2$ file jamulus.universal 
jamulus.universal: Mach-O universal binary with 2 architectures: [x86_64:Mach-O 64-bit executable x86_64Mach-O 64-
bit executable x86_64] [arm64]
jamulus.universal (for architecture x86_64):    Mach-O 64-bit executable x86_64
jamulus.universal (for architecture arm64):     Mach-O 64-bit executable arm64

bash-3.2$ lipo -archs jamulus.universal 
x86_64 arm64

The final step would be to move it to jamulus.app again and ensure that the plist file is correct (it references the binary name). It should be trivial to modify that manually, but maybe we can find a nicer way to do that via Qt tools.

@hoffie hoffie self-assigned this Aug 24, 2022
@hoffie
Copy link
Member Author

hoffie commented Aug 24, 2022

The following build is supposed to contain a universal binary which runs both on Intel and M1 Macs:
jamulus_3.9.0dev-26b8fe3_mac.dmg

Could someone please test that build?

Pinging @ashstrahle @PeRAux @josephschito for M1
Pinging @ann0see for Intel

Please report back the following details:

  • Architecture (M1 or Intel)
  • macOS version
  • Whether the binary was installable and runnable without any workarounds regarding signing which were needed previously on M1 -dev releases (context: Mac: Sign non-release builds with self-signed key #2791 (comment))
  • Whether you've noticed any other warnings or problems
  • Whether you've tested the client, the server or both (which would be optimal, of course)

If we have positive feedback, I'll submit a (cleaned up) PR based on hoffie@autobuild/mac-universal-arm64.

Thanks in advance!

@ashstrahle
Copy link

Hi Christian. I can confirm:
M1 native execution
macOS Monterey
Did require allowing both applications to execute via System Preferences | Security & Privacy
On initial no noticeable issues
Client tested briefly without issue

image

@hoffie
Copy link
Member Author

hoffie commented Aug 24, 2022

@ashstrahle Thanks!

Did require allowing both applications to execute via System Preferences | Security & Privacy

Do you remember if this is just as bad or better compared to previous M1 -dev releases?

@ashstrahle
Copy link

Much easier - less hackery.

hoffie added a commit to hoffie/jamulus that referenced this issue Aug 25, 2022
This combines the x86_64 and arm64e CI jobs into a single job in order
to have both binary results at hand. It then uses `lipo` to combine
those two binaries in a single universal binary which runs on both
architectures.
This increases user-friendliness as users no longer have to worry about
downloading the proper build for their architecture.

Fixes: jamulussoftware#2790
hoffie added a commit to hoffie/jamulus that referenced this issue Aug 28, 2022
This combines the x86_64 and arm64e CI jobs into a single job in order
to have both binary results at hand. It then uses `lipo` to combine
those two binaries in a single universal binary which runs on both
architectures.
This increases user-friendliness as users no longer have to worry about
downloading the proper build for their architecture.

Fixes: jamulussoftware#2790
@pljones pljones modified the milestones: Release 3.10.0, Release 3.9.1 Aug 29, 2022
ann0see pushed a commit to ann0see/jamulus that referenced this issue Nov 9, 2022
This combines the x86_64 and arm64e CI jobs into a single job in order
to have both binary results at hand. It then uses `lipo` to combine
those two binaries in a single universal binary which runs on both
architectures.
This increases user-friendliness as users no longer have to worry about
downloading the proper build for their architecture.

Fixes: jamulussoftware#2790
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants