Skip to content

Don't use iOS toolchain on macOS #12578

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Don't use iOS toolchain on macOS #12578

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions CMakePresets.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
"inherits": ["common"],
"generator": "Xcode",
"cacheVariables": {
"CMAKE_TOOLCHAIN_FILE": "${sourceDir}/third-party/ios-cmake/ios.toolchain.cmake",
Copy link
Contributor

@shoumikhin shoumikhin Jul 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

iirc, this is not iOS-only toolchain, we use it to build mac targets too.
It contains a bunch of useful declarations to simplify our live, e.g. the very MAC_ARM64 platform definition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not the normal way that CMake projects expect to build for Mac though. concretely, sentencepiece's build does not like CMAKE_MACOSX_BUNDLE being set on non-iOS platforms, and it wouldn't be set on macOS if not for the use of this toolchain. IMO we should keep everything standard and defaulted unless we have a reason not to. Why do we need to use this to build for Mac?

Copy link
Contributor

Choose a reason for hiding this comment

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

So we rely on that toolchain platform definitions with the extra cmake flags they add to build frameworks for iOS and macOS. So far they worked well. If you believe there's a better approach, let's explore it too.
I guess we can disabled CMAKE_MACOSX_BUNDLE for sentencepiece specifically when building for mac, that's what we do with the custom script build phase in Xcode when building extension/llm third-party deps.
Screenshot 2025-07-16 at 18 40 40

Copy link
Contributor Author

@swolchok swolchok Jul 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I guess we can disabled CMAKE_MACOSX_BUNDLE for sentencepiece specifically when building for mac, that's what we do with the custom script build phase in Xcode when building extension/llm third-party deps.

I agree that we can work around this and it's straightforward to do so. My concern is that the setting of this toolchain cost me half an hour of debugging time to track down, because it was surprising that 1) any toolchain was active for macos, since builds work fine without it when not using presets, and 2) "ios.toolchain.cmake" in particular was active. This will presumably continue to be surprising to others in the future.

What if instead we set the CMAKE_TOOLCHAIN_FILE only in build_apple_frameworks.sh? Do we need it anywhere else? (Long-term I think we should investigate using CMake's built-in support for producing Apple frameworks, but I don't see a particular reason to prioritize that.)

Copy link
Contributor

Choose a reason for hiding this comment

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

I don’t have a strong preference - so long as we can build the frameworks as usual, I’m happy either way.
Just a few quick notes:

  • Despite its name, this toolchain isn’t limited to iOS.
  • The last time I tried building the frameworks by manually setting all the macOS-specific flags myself - it ended up costing way more time 😅
  • Otherwise, this toolchain has worked well so far.

Maybe just disable that particular flag like in #12594

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, maybe #12594 will unblock me. If not I'll see if I can find another way to unblock export since I've found out that this is here for a reason and not just a mistake.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I rebased and it looks like we don't need this.

"EXECUTORCH_BUILD_PRESET_FILE": "${sourceDir}/tools/cmake/preset/macos.cmake",
"PLATFORM": "MAC_ARM64",
"DEPLOYMENT_TARGET": "12.0"
Expand Down Expand Up @@ -47,7 +46,7 @@
"generator": "Xcode",
"cacheVariables": {
"CMAKE_TOOLCHAIN_FILE": "${sourceDir}/third-party/ios-cmake/ios.toolchain.cmake",
"EXECUTORCH_BUILD_PRESET_FILE": "${sourceDir}/tools/cmake/preset/ios.cmake",
"EXECUTORCH_BUILD_PREe SET_FILE": "${sourceDir}/tools/cmake/preset/ios.cmake",
"PLATFORM": "SIMULATORARM64",
"DEPLOYMENT_TARGET": "17.0"
},
Expand Down
Loading