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

Add native ARM macOS build #707

Merged
merged 2 commits into from
Feb 3, 2024
Merged

Conversation

tech-ticks
Copy link
Contributor

Adds a new GitHub Actions job to generate native binaries for ARM Macs using the new macos-14 runner.

The most annoying difference is that all app bundles must be signed to run on Apple Silicon Macs (self-signed certificates are fine). PyInstaller already takes care of signing, but macOS refuses to load app bundles that were tampered with. Thus, we can't copy shell scripts or other files into the bundle after its creation anymore (I tried re-signing it, but I couldn't get it to work). The easiest workaround to get things running was adding code to setup environment variables in main.py instead, so that's what I did. I didn't include code from the wrapper script that reads the system language because SkyTemple seems to detect the language without it.

I didn't test every single feature (consider this a "beta version"), but the general UI and debugger seemed to work fine. The only issue I've noticed is an error while loading the spellchecker (which might also be present in the Intel version because barely anyone uses this feature):

2024-01-31 17:35:49,637 - skytemple_ssb_debugger.controller.script_editor - ERROR - Failed toggling/loading spellchecker:
Traceback (most recent call last):
  File "skytemple_ssb_debugger/controller/script_editor.py", line 709, in toggle_spellchecker
  File "gtkspellcheck/spellcheck.py", line 307, in __init__
gtkspellcheck.spellcheck.NoDictionariesFound

@theCapypara
Copy link
Member

Hi & thanks a lot!

I will review later, but for now it seems this somehow broke the MacOS Intel based build? I'm not entirely sure I understand the error.

The formatting error is unrelated, Black had a major update, I'll open a separate PR to address that.

@tech-ticks
Copy link
Contributor Author

it seems this somehow broke the MacOS Intel based build? I'm not entirely sure I understand the error.

That's odd, I didn't really change anything that would cause issues in the Intel build. My own test run shows a timeout while downloading dependencies from Homebrew, so I guess the build is probably just flaky.

@tech-ticks
Copy link
Contributor Author

tech-ticks commented Feb 1, 2024

The log archive shows that it's a compile error:

2024-01-31T18:29:13.9834110Z �[32m==>�[0m �[1mInstalling gtksourceview4 dependency: �[32mlibvmaf�[39m�[0m
2024-01-31T18:29:18.1205380Z �[34m==>�[0m �[1mmeson build�[0m
2024-01-31T18:29:22.4009130Z �[34m==>�[0m �[1mninja -vC build�[0m
2024-01-31T18:29:26.5725850Z Last 15 lines from /Users/runner/Library/Logs/Homebrew/libvmaf/02.ninja:
2024-01-31T18:29:26.6311370Z clang -Isrc/libvmaf.3.dylib.p -Isrc -I../src -Iinclude -I../include -I../src/feature -I../src/feature/common -fdiagnostics-color=always -Wall -Winvalid-pch -Wextra -std=c11 -O3 -D_DARWIN_C_SOURCE -pedantic -DOC_NEW_STYLE_INCLUDES -MD -MQ src/libvmaf.3.dylib.p/meson-generated_.._vmaf_v0.6.1neg.json.c.o -MF src/libvmaf.3.dylib.p/meson-generated_.._vmaf_v0.6.1neg.json.c.o.d -o src/libvmaf.3.dylib.p/meson-generated_.._vmaf_v0.6.1neg.json.c.o -c src/vmaf_v0.6.1neg.json.c
2024-01-31T18:29:26.6320390Z src/vmaf_v0.6.1neg.json.c:1630:1: error: unknown type name 'size_t'
2024-01-31T18:29:26.6321200Z size_t src_vmaf_v0_6_1neg_json_len = 19519;
2024-01-31T18:29:26.6321620Z ^
2024-01-31T18:29:26.6321860Z 1 error generated.
2024-01-31T18:29:26.6329280Z [44/165] clang -Isrc/libvmaf.3.dylib.p -Isrc -I../src -Iinclude -I../include -I../src/feature -I../src/feature/common -fdiagnostics-color=always -Wall -Winvalid-pch -Wextra -std=c11 -O3 -D_DARWIN_C_SOURCE -pedantic -DOC_NEW_STYLE_INCLUDES -MD -MQ src/libvmaf.3.dylib.p/meson-generated_.._vmaf_b_v0.6.3.json.c.o -MF src/libvmaf.3.dylib.p/meson-generated_.._vmaf_b_v0.6.3.json.c.o.d -o src/libvmaf.3.dylib.p/meson-generated_.._vmaf_b_v0.6.3.json.c.o -c src/vmaf_b_v0.6.3.json.c
2024-01-31T18:29:26.6334400Z FAILED: src/libvmaf.3.dylib.p/meson-generated_.._vmaf_b_v0.6.3.json.c.o 
2024-01-31T18:29:26.6346580Z clang -Isrc/libvmaf.3.dylib.p -Isrc -I../src -Iinclude -I../include -I../src/feature -I../src/feature/common -fdiagnostics-color=always -Wall -Winvalid-pch -Wextra -std=c11 -O3 -D_DARWIN_C_SOURCE -pedantic -DOC_NEW_STYLE_INCLUDES -MD -MQ src/libvmaf.3.dylib.p/meson-generated_.._vmaf_b_v0.6.3.json.c.o -MF src/libvmaf.3.dylib.p/meson-generated_.._vmaf_b_v0.6.3.json.c.o.d -o src/libvmaf.3.dylib.p/meson-generated_.._vmaf_b_v0.6.3.json.c.o -c src/vmaf_b_v0.6.3.json.c
2024-01-31T18:29:26.6350960Z src/vmaf_b_v0.6.3.json.c:34077:1: error: unknown type name 'size_t'
2024-01-31T18:29:26.6351980Z size_t src_vmaf_b_v0_6_3_json_len = 408883;
2024-01-31T18:29:26.6352690Z ^
2024-01-31T18:29:26.6353310Z 1 error generated.
2024-01-31T18:29:26.6358160Z [45/165] clang -Isrc/liblibvmaf_feature.a.p -Isrc -I../src -Iinclude -I../include -I../src/feature -I../src/feature/common -I../src/cuda -fdiagnostics-color=always -Wall -Winvalid-pch -Wextra -std=c11 -O3 -D_DARWIN_C_SOURCE -MD -MQ src/liblibvmaf_feature.a.p/feature_cambi.c.o -MF src/liblibvmaf_feature.a.p/feature_cambi.c.o.d -o src/liblibvmaf_feature.a.p/feature_cambi.c.o -c ../src/feature/cambi.c
2024-01-31T18:29:26.6371660Z [46/165] clang -Isrc/liblibvmaf_feature.a.p -Isrc -I../src -Iinclude -I../include -I../src/feature -I../src/feature/common -I../src/cuda -fdiagnostics-color=always -Wall -Winvalid-pch -Wextra -std=c11 -O3 -D_DARWIN_C_SOURCE -MD -MQ src/liblibvmaf_feature.a.p/feature_integer_adm.c.o -MF src/liblibvmaf_feature.a.p/feature_integer_adm.c.o.d -o src/liblibvmaf_feature.a.p/feature_integer_adm.c.o -c ../src/feature/integer_adm.c
2024-01-31T18:29:26.6374200Z ninja: build stopped: subcommand failed.

According to brew.sh, this formula isn't compatible with Big Sur, so I guess I'll just update from macos-11 to macos-12. The former will be unavailable starting in June anyway.

@theCapypara
Copy link
Member

@tech-ticks Can you run your changes to main.py through the latest version of the black formatter. Rebasing not needed, I'll squash the changes later. Thanks!

@tech-ticks
Copy link
Contributor Author

I've reformatted the file with black and rebased while I was at it.

Copy link
Member

@theCapypara theCapypara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Again, thank you!

@theCapypara theCapypara merged commit c289676 into SkyTemple:master Feb 3, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants