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

build: add macOS SDL to thirdparty, add .dylib to bundle #1345

Merged
merged 1 commit into from
Dec 28, 2023

Conversation

jcm93
Copy link
Contributor

@jcm93 jcm93 commented Dec 14, 2023

Description

Adds SDL (properly) to Mac builds.

This time, it has been added as a fully fledged optional dependency in thirdparty, à la MoltenVK, and built from source before ares. We build from source because brew installations are both single-architecture and not fully portable.

The .dylib is copied inside the app bundle when linking, at the same stage as MoltenVK.

The initialization of ares.dylibs has been moved from ares's Makefile to the desktop-ui Makefile so that it's initialized when building ruby, which it wasn't previously.

The SDL build is also added to CI, with the same caching methods as used for MVK.

Relevant Details

HEAD points at SDL 2.28.5 currently, which is the most recent stable release. This should probably be periodically incremented as appropriate.

SDL is currently built with the macOS deployment target of 10.13. While SDL's minimum supported macOS version is 10.11, ares CI is using Xcode 14.2, which has a minimum deployment target of 10.13. I am currently testing to see if I can get the deployment target lower without breaking other parts of ares's CI. resolved, per below; SDL supporting 10.11 is built + bundled.

Testing

This has been tested to build properly locally and via GitHub CI. The .dylib is packaged inside the bundle correctly and SDL functions correctly in my testing, even if the user does not have SDL installed elsewhere on their system.

This is seeking tests from other Mac users, especially on older operating systems, to verify that SDL works correctly there.

@shinra-electric
Copy link
Contributor

Currently the macOS minimum version for ares is 10.9, which seems to be automatic as the minimum system version hasn't been set in the plist.

You can set the minimum version by adding the following key to /desktop-ui/resource/ares.plist

<key>LSMinimumSystemVersion</key>
<string>10.11</string>

@jcm93
Copy link
Contributor Author

jcm93 commented Dec 15, 2023

Currently the macOS minimum version for ares is 10.9, which seems to be automatic as the minimum system version hasn't been set in the plist.

You can set the minimum version by adding the following key to /desktop-ui/resource/ares.plist

<key>LSMinimumSystemVersion</key>
<string>10.11</string>

Well, if ares does actually run successfully on 10.9, I wouldn't want to raise the minimum system requirements just for the sake of this change. Ideally ares would just not load the SDL .dylib if it's incompatible.

I've done some testing, and it seems to be pretty simple to compile SDL to support 10.11. I can just set xcode-select to use the runner's 13 SDK, which has a minimum deploy target of 10.11, and set it back afterward and compile the rest of ares as normal. Going to do a bit more testing to see what occurs if the .dylib can't be loaded.

@jcm93
Copy link
Contributor Author

jcm93 commented Dec 15, 2023

If the binary is incompatible (as it would be for users on 10.9-10.10), looks like ares will only crash if the user tries to select SDL. I think this is an acceptable failure mode, especially given how few users there could possibly be left on those OSs.

Since I now have SDL building for 10.11 (the minimum version) and nothing seems broken in my testing, going to take this out of draft mode. Someone may want to trigger CI on this one more time, since I changed the build script for SDL in the most recent push.

@jcm93 jcm93 marked this pull request as ready for review December 15, 2023 18:38
@LukeUsher LukeUsher merged commit 9e4b379 into ares-emulator:master Dec 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants