search

yonaskolb / Mint

A package manager that installs and runs executable Swift packages
MIT License
2.26k stars 122 forks source link

Support building for architectures other than x86_64 on macOS (Apple Silicon) #185

Closed liamnichols closed 3 years ago

liamnichols commented 3 years ago

Closes #184 (cc @amine2233)

As identified in the linked PR, hardcoding the target when forming the swift build command on macOS has become a limiting factor when supporting Apple Silicon. Thankfully @esteluk already provided a viable work around that preserves some other requirements defined in #58.

I've taken the two suggestions and formed them into a single PR that should hopefully be good to go 🤞

I can confirm that this does the job nicely on Apple Silicon by using Realm's SwiftLint as a test project since your SimplePackage is too simple to throw up any build error in the tests. If it helps I can try and help you to expand that package a bit to reproduce the issue but I'm not sure how important that is.

esteluk commented 3 years ago

Ahh thanks Liam!

liamnichols commented 3 years ago

Yep this was good to go @yonaskolb although I did have a question here: https://github.com/yonaskolb/Mint/pull/184#issuecomment-667293914

Basically, do we still need the deployment target workaround that causes us to have to specify this target in the first place?

In addition, you might also want to update the Makefile to support distributing a universal binary

https://github.com/yonaskolb/XcodeGen/blob/9fdc194771168c9128bf7833edbf16391b1b848d/Makefile#L22

The above would become the following:

swift build --disable-sandbox -c release --arch arm64 --arch x86_64

(see https://liamnichols.github.io/2020/08/01/building-swift-packages-as-a-universal-binary.html)

This will likely be required when creating a Homebrew Bottle for Big Sur, but will require Xcode 12 so I'm not sure what implications that has as I think the project supposedly still supports Xcode 10.2?

Maybe I can move this discussion into a new issue if it requires a bit more thinking though

yonaskolb commented 3 years ago

In terms of requiring the deployment target, I would say technically we don't need it, but practically it might be useful. Many package authors haven't specified their deployment targets, and in this case adding it will fix issues. I may be wrong there though?

In regards to the universal binary, thanks for the heads up. Do you think it will be ok to make the brew formula require 12 but keep the package itself at 10.2? I'm happy to update the whole package as well if needed. As you say a new issue is best.