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
Feature: Add support for Apple Silicon (ARM64) build on macOS #8340
Conversation
a7d2390
to
8562a96
Compare
OK, Homebrew isn't currently building/supplying universal libraries, so this will have to go on hold until it does or I can identify a workaround. |
Tracking issue here Homebrew/brew#7857 I think |
Reading through the issue above, it seems that people have had success building the dependencies from source? All our dependencies have got a "works with ARM" checkbox, perhaps worth a try? |
Yes, building from source is no problem. I suspect the package managers will catch up, and MacPorts has a universal mode that works too. I'm going to do a bit more work on this tonight. |
292f81f
to
6b944ca
Compare
.github/workflows/ci-build.yml
Outdated
run: cd build-x86_64 && make -j2 test | ||
|
||
- name: CMake - arm64 | ||
run: mkdir build-arm64 && cd build-arm64 && cmake .. -DCMAKE_OSX_ARCHITECTURES=arm64 -DVCPKG_TARGET_TRIPLET=arm64-osx -DCMAKE_TOOLCHAIN_FILE=/usr/local/share/vcpkg/scripts/buildsystems/vcpkg.cmake -DHOST_BINARY_DIR=../build-x86_64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As this is just testing, and Mac OS is known for taking its sweet time to compile .. should this be 2 steps? Of course you need to do a quick make tools
on x86-64 to get strgen
and co, but it might speed up the CI drasticly?
CI used to be done in ~10 minutes .. with this change, that is now ~22 minutes :P Hence the question :)
You can use matrix
if you want to make this all pretty and without less code duplication, if you like :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't run make test
on the CI anyway unless/until they introduce arm64 hardware. :) I was using the CI as a testbed but I suspect we will either keep the CI as it is for Mac (perhaps switching to vcpkg for consistency), or add arm64 as a separate job so it can run simultaneously.
It's the release pipeline I'm intending to target for arm64 primarily. Same for Windows at some point too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the release build, I also suggest splitting it up in 2 steps, for exactly the same reason ;) I was more wondering if you still want to combine the binaries or leave it at that? If you want to combine, 2 steps is still possible btw, but would require some more work. If you want to leave it as 2 binaries (you know I prefer that :P), making 2 steps is easy, but might be worth trying out while at it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From a user experience point of view, a Universal binary would be preferable, but we can always try it out and see if it causes a major issue in terms of size. When I get a chance to look at this further I'll see if I can set it up with two separate steps. Building as universal basically involves running one command to glue together the two generated openttd
binaries, before running cpack.
The resulting |
Sometimes, like 1 out of 6 times now, it returns this when creating the
I have no clue yet what causes that ... sorry, not helpful, but I wanted to share it nevertheless :) |
Friendly poke :) Can you integrate this in the release-workflow? (not the CI). If you push to your own fork you can test it out by going to Actions, Release, and on the right is there a Would be nice to have this in for 1.11 :) |
Yes, I hope to have a look at this in the next few days. :) |
8f3c7ae
to
c417636
Compare
5209d25
to
50db531
Compare
0529c41
to
77c18dc
Compare
I think this PR is ready for integration. I've tested it out on my own repository's Actions and the builds do work. As discussed, there are issues with signing which means it won't work out-of-the-box on an M1 Mac without the user running: xattr -r -d com.apple.quarantine /Applications/OpenTTD.app to remove the "quarantine" flag from the application. This will be resolved once we are able to sign the app. Once we have resolved that issue, I will look into building a Universal binary, though we may be happy initially at least to have separate AMD64 and ARM64 builds. Currently, the macOS Actions workflow clones the latest vcpkg to a temporary folder; we can drop this once the default vcpkg in the macOS image is updated. (The version in the image does not include the .git folder, so we can't just update that.) |
Not completely sure if this will work but it should generate a "Universal 2" app on macOS for x86_64 plus Apple Silicon. Hoping that by creating a PR it triggers the CI...