BUILD(cmake): Set macOS deployment target based on architecture#7066
Conversation
WalkthroughThe change modifies the CMake configuration to introduce architecture-aware macOS deployment target selection. Previously, the deployment target was statically set to 10.15. The modification adds conditional logic to determine the target architecture through CMAKE_OSX_ARCHITECTURES, CMAKE_SYSTEM_PROCESSOR, or CMAKE_HOST_SYSTEM_PROCESSOR, then sets CMAKE_OSX_DEPLOYMENT_TARGET to 11.0 for ARM64 architectures and 10.15 for all other architectures. The temporary variable used for architecture detection is cleaned up after use. This change results in a net +20/-1 lines. Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@CMakeLists.txt`:
- Around line 40-57: The current check treats CMAKE_OSX_ARCHITECTURES as a
single string and compares _target_arch STREQUAL "arm64", which fails for
semicolon-separated multi-arch lists; change the logic that determines the
architecture to test membership (use list(FIND) on CMAKE_OSX_ARCHITECTURES for
"arm64") and base the CMAKE_OSX_DEPLOYMENT_TARGET on that membership check (if
"arm64" found -> set 11.0, else -> set 10.15), keeping the existing fallback
behavior that uses CMAKE_SYSTEM_PROCESSOR or CMAKE_HOST_SYSTEM_PROCESSOR when
CMAKE_OSX_ARCHITECTURES is not set.
| if(APPLE) | ||
| # Determine target architecture for deployment target selection | ||
| # Priority: explicit CMAKE_OSX_ARCHITECTURES > CMAKE_SYSTEM_PROCESSOR > host processor | ||
| if(CMAKE_OSX_ARCHITECTURES) | ||
| set(_target_arch "${CMAKE_OSX_ARCHITECTURES}") | ||
| elseif(CMAKE_SYSTEM_PROCESSOR) | ||
| set(_target_arch "${CMAKE_SYSTEM_PROCESSOR}") | ||
| else() | ||
| set(_target_arch "${CMAKE_HOST_SYSTEM_PROCESSOR}") | ||
| endif() | ||
|
|
||
| # macOS 11.0 is the minimum for ARM64 (Apple Silicon was introduced with Big Sur) | ||
| # macOS 10.15 remains the target for Intel builds | ||
| if(_target_arch STREQUAL "arm64") | ||
| set(CMAKE_OSX_DEPLOYMENT_TARGET 11.0) | ||
| else() | ||
| set(CMAKE_OSX_DEPLOYMENT_TARGET 10.15) | ||
| endif() |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
CMake CMAKE_OSX_ARCHITECTURES multi-architecture list semicolon separated
💡 Result:
CMAKE_OSX_ARCHITECTURES takes a CMake list, and CMake lists are semicolon-separated strings. So a multi-arch (“universal”) build is written like:
set(CMAKE_OSX_ARCHITECTURES "x86_64;arm64")or per-target:
set_property(TARGET myapp PROPERTY OSX_ARCHITECTURES "x86_64;arm64")Important details:
- Set it before the first
project()/enable_language()because it can affect toolchain configuration. It’s also intended to be set as a CACHE entry. [1] - On the command line, quote (or otherwise escape) the semicolon so your shell doesn’t treat it as a command separator:
cmake -S . -B build -DCMAKE_OSX_ARCHITECTURES="x86_64;arm64"CMake’s own behavior here follows the general rule: many CMake variables/properties accept semicolon-separated lists. [2]
Sources:
[1] CMake docs: CMAKE_OSX_ARCHITECTURES variable (latest) https://cmake.org/cmake/help/latest/variable/CMAKE_OSX_ARCHITECTURES.html
[2] CMake docs: Lists are semicolon-delimited / list handling https://cmake.org/cmake/help/v3.26/manual/cmake-language.7.html
🌐 Web query:
CMake STREQUAL behavior with lists and list(FIND) for detecting element membership
💡 Result:
What STREQUAL does with “lists”
In CMake, a “list” is just a single string whose elements are separated by semicolons (;). So STREQUAL is always a plain string equality test—it does not test element membership. If you compare a list variable, you’re comparing its full semicolon-joined representation (e.g., a;b;c). [1], [2]
set(L a b c) # L is the string "a;b;c" [1]
if("${L}" STREQUAL "a;b;c")
message("equal")
endif()Why quoting matters (lists can “explode” into multiple arguments)
If you expand a list unquoted in a command call, CMake splits it on semicolons into multiple arguments (“list expansion”). Quoted arguments preserve semicolons as part of the single argument. [2]
So this is dangerous:
# BAD: ${L} becomes a;b;c, which is split into multiple arguments
if(${L} STREQUAL "a;b;c")
...
endif()Prefer quoting expansions in comparisons:
if("${L}" STREQUAL "a;b;c")
...
endif()Correct way to test whether a list contains an element: list(FIND)
list(FIND <list> <value> <outVar>) returns the index of an element equal to <value>, or -1 if not found. [1]
set(L a b c)
list(FIND L "b" idx)
if(idx GREATER -1)
message("L contains b")
endif()
list(FIND L "x" idx)
if(idx EQUAL -1)
message("L does not contain x")
endif()Notes:
list(FIND ...)checks exact element equality (not substring / pattern matching). [1]- Use quotes around the searched value if it might contain spaces.
Sources: CMake list() documentation (list definition; list(FIND) return value) [1]; CMake language rules for unquoted argument splitting on semicolons vs quoted preservation [2]; if() comparison operator context (STREQUAL is a string comparison operator) [3].
[1] https://cmake.org/cmake/help/latest/command/list.html
[2] https://cmake.org/cmake/help/v3.26/manual/cmake-language.7.html
[3] https://cmake.org/cmake/help/latest/command/if.html
Fix multi-arch deployment target detection.
CMAKE_OSX_ARCHITECTURES is a semicolon-separated CMake list (e.g., arm64;x86_64 for universal builds). Using STREQUAL "arm64" will fail on such lists, incorrectly applying the Intel 10.15 target even when building for ARM64. Use list(FIND) to detect membership instead.
Suggested fix
- if(_target_arch STREQUAL "arm64")
+ set(_arm64_idx -1)
+ list(FIND _target_arch "arm64" _arm64_idx)
+ if(_arm64_idx GREATER -1)
set(CMAKE_OSX_DEPLOYMENT_TARGET 11.0)
else()
set(CMAKE_OSX_DEPLOYMENT_TARGET 10.15)
endif()🤖 Prompt for AI Agents
In `@CMakeLists.txt` around lines 40 - 57, The current check treats
CMAKE_OSX_ARCHITECTURES as a single string and compares _target_arch STREQUAL
"arm64", which fails for semicolon-separated multi-arch lists; change the logic
that determines the architecture to test membership (use list(FIND) on
CMAKE_OSX_ARCHITECTURES for "arm64") and base the CMAKE_OSX_DEPLOYMENT_TARGET on
that membership check (if "arm64" found -> set 11.0, else -> set 10.15), keeping
the existing fallback behavior that uses CMAKE_SYSTEM_PROCESSOR or
CMAKE_HOST_SYSTEM_PROCESSOR when CMAKE_OSX_ARCHITECTURES is not set.
|
I have an ARM based Mac so I wanted to clean up these build warnings. I also wanted to ask about potentially targeting macOS 11 (the version immediately following 10.15 after Apple's number scheming changed). This would simplify the build targets, so that ARM/x86 could be built to the same target. In a subsequent PR I plan to submit changes to the build processes that will build both architectures x86 and ARM and then combine them into a Universal 2 binary. So this change leads in that direction, though build target can still be 10.15 for x86 it would need to be 11 for arm. Would love to get your feedback here. Some data points: 
|
Apple Silicon requires macOS 11.0 minimum (Big Sur) since ARM Macs were introduced with that version. Setting 10.15 as the deployment target on ARM builds causes compiler warnings as it silently overrides to 11.0. This change detects the target architecture and sets the appropriate deployment target: - ARM64 (Apple Silicon): macOS 11.0 - x86_64 (Intel): macOS 10.15 Co-Authored-By: Claude Opus 4.5 <[email protected]>
nicholas-lonsinger
force-pushed
the
macos-arm-build-warnings-os11-vs-os1015
branch
from
ba07881 to
8e0e943
Compare
|
I wanted to increase the deployment version for quite some time. However, not everything compiles (cleanly) with newer deployment versions. And since I don't have a Mac, I couldn't really figure this out thus far. That is to say: If you could make sure we compile cleanly on newer deployment versions, I'm open to increase it all the way to whatever Apple currently still supports. However, iirc this would imply to either rewrite the overlay code to use Metal or to just drop it on macOS. Personally, I would also be fine with the latter. |
|
Let's drop it, for two reasons:
|
|
Let me dig in on Monday when I'm back at a desk and I'll offer a recommendation based on competitive landscape (Discord/Steam) as well as effort. My preliminary search said those 2 dropped support due to issues with macOS security but I want to dig deeper to be sure |
|
Also, I'll need to raise the deployment target and look at what issues arise, I feel like those might be easier to resolve (outside of the overlay issue) |
|
Putting a comment in here in case the other thread didn't notify. I did the target version increase in a different PR: #7069 I'd like to get this approved into the code base along with the Input Monitoring permission #7065 so I can continue working without it getting too messy. I hope to build a universal binary next as a big project, as well as some smaller tweaks as well or anything else you had in mind for macOS. |
3 participants
Apple Silicon requires macOS 11.0 minimum (Big Sur) since ARM Macs were introduced with that version. Setting 10.15 as the deployment target on ARM builds causes compiler warnings as it silently overrides to 11.0.
This change detects the target architecture and sets the appropriate deployment target:
Checks