Support building for architectures other than x86_64 on macOS (Apple Silicon) #185
Conversation
…:noInstall:)' method to construct 'swift build' command that specifies the correct target architecture in order to support building on Apple Silicon
|
Ahh thanks Liam! |
liamnichols
force-pushed
the
ln/apple-silicon
branch
from
74d18fd
to
05fc39f
Compare
| final class ProcessInfoExtensionTests: XCTestCase { | ||
| #if arch(x86_64) | ||
| func testMachineHardwareName_Intel() { | ||
| XCTAssertEqual(ProcessInfo.processInfo.machineHardwareName, "x86_64") | ||
| } | ||
| #endif | ||
|
|
||
| #if arch(arm64) | ||
| func testMachineHardwareName_AppleSilicone() { | ||
| XCTAssertEqual(ProcessInfo.processInfo.machineHardwareName, "arm64") | ||
| } | ||
| #endif | ||
| } |
There was a problem hiding this comment.
Not sure if there is a better way to test this.. While we only need for machineHardwareName for macOS, it should also work for Linux as well.
I was going to use XCTSkipIf(true, "...") but that's Xcode 11.4+ whereas we need to support Xcode 10.2+.
| /// Returns a `String` representing the machine hardware name or nil if there was an error invoking `uname(_:)` or decoding the response. | ||
| /// | ||
| /// Return value is the equivalent to running `$ uname -m` in shell. | ||
| var machineHardwareName: String? { |
There was a problem hiding this comment.
I wonder if I am being too strict here? Could this be non-optional and either just force unwrap or return an empty string in the (unlikely) event of a failure?
There was a problem hiding this comment.
This is great, thank you @liamnichols. Sorry for the delay on this.
I'll assume this is ready to go even if it's in a draft state
|
Yep this was good to go @yonaskolb although I did have a question here: #184 (comment) 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 The above would become the following: (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 |
|
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. |
Closes #184 (cc @amine2233)
As identified in the linked PR, hardcoding the target when forming the
swift buildcommand 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.