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

Feature: Add support for Apple Silicon (ARM64) build on macOS #8340

Merged
merged 2 commits into from Jan 8, 2021

Conversation

orudge
Copy link
Contributor

@orudge orudge commented Nov 12, 2020

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...

@orudge orudge force-pushed the universal-mac branch 2 times, most recently from a7d2390 to 8562a96 Compare Nov 12, 2020
@orudge
Copy link
Contributor Author

@orudge orudge commented Nov 12, 2020

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.

@LordAro
Copy link
Member

@LordAro LordAro commented Nov 14, 2020

Tracking issue here Homebrew/brew#7857 I think

@LordAro
Copy link
Member

@LordAro LordAro commented Dec 2, 2020

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?

@orudge
Copy link
Contributor Author

@orudge orudge commented Dec 2, 2020

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.

@orudge orudge force-pushed the universal-mac branch 2 times, most recently from 292f81f to 6b944ca Compare Dec 9, 2020
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
Copy link
Member

@TrueBrain TrueBrain Dec 9, 2020

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 :)

Copy link
Contributor Author

@orudge orudge Dec 10, 2020

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.

Copy link
Member

@TrueBrain TrueBrain Dec 10, 2020

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 :)

Copy link
Contributor Author

@orudge orudge Dec 10, 2020

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.

@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Dec 10, 2020

The resulting dmg files do not have a postfix to indicate for what platform they were build. See cmake/InstallAndPackage.cmake:97. Most likely the same fix as Windows will work fine :)

@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Dec 10, 2020

Sometimes, like 1 out of 6 times now, it returns this when creating the dmg:

hdiutil: create failed - Resource busy

I have no clue yet what causes that ... sorry, not helpful, but I wanted to share it nevertheless :)

@TrueBrain TrueBrain added candidate: yes OS: MacOS size: trivial size: small and removed size: trivial labels Dec 14, 2020
@TrueBrain TrueBrain added this to the 1.11.0 milestone Jan 5, 2021
@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Jan 7, 2021

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 Run workflow. Select the branch you pushed it to, type the branch again in the textbox, and hit Run workflow. This will fail btw, on the Upload step (as you don't have the S3 secrets), but you can download the artifact nevertheless to test out if the build is working.

Would be nice to have this in for 1.11 :)

@orudge
Copy link
Contributor Author

@orudge orudge commented Jan 7, 2021

Yes, I hope to have a look at this in the next few days. :)

@orudge orudge force-pushed the universal-mac branch 2 times, most recently from 8f3c7ae to c417636 Compare Jan 7, 2021
.github/workflows/release.yml Outdated Show resolved Hide resolved
@orudge orudge force-pushed the universal-mac branch 2 times, most recently from 5209d25 to 50db531 Compare Jan 8, 2021
@LordAro LordAro changed the title Draft: Feature: Create Universal (x86_64 + Apple Silicon) build on macOS Feature: Create Universal (x86_64 + Apple Silicon) build on macOS Jan 8, 2021
@LordAro LordAro changed the title Feature: Create Universal (x86_64 + Apple Silicon) build on macOS Draft: Feature: Create Universal (x86_64 + Apple Silicon) build on macOS Jan 8, 2021
@orudge orudge force-pushed the universal-mac branch 4 times, most recently from 0529c41 to 77c18dc Compare Jan 8, 2021
@orudge orudge changed the title Draft: Feature: Create Universal (x86_64 + Apple Silicon) build on macOS Feature: Add support for Apple Silicon (ARM64) build on macOS Jan 8, 2021
@orudge
Copy link
Contributor Author

@orudge orudge commented Jan 8, 2021

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.)

@orudge orudge merged commit fec5ce0 into OpenTTD:master Jan 8, 2021
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
candidate: yes OS: MacOS size: small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants