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

How would people feel about overhauling the iOS support to remove polly magic? #625

Open
hjmallon opened this issue Sep 29, 2022 · 6 comments

Comments

@hjmallon
Copy link


Building with Hunter for iOS involves a bit of Polly magic variable mangling.

IOS_SDK_VERSION -> set to the current SDK version used with Xcode
IOS_DEPLOYMENT_SDK_VERSION -> set to the deployment target for iOS
IPHONEOS_ARCHS -> requested architectures for the iOS device part of the output
IPHONESIMULATOR_ARCHS -> requested architectures for the iOS simulator part of the output
IPHONEOS_ROOT / IPHONEOS_SDK_ROOT / IPHONESIMULATOR_SDK_ROOT
There are probably others

Also various parts of Hunter use the variable IOS to determine things.

  • A lot of this infrastructure was built before CMake had decent iOS support, and some of that has been linked back in
  • Pretty much all of it can be persuaded to work for iOS, but will need changes for tvOS or watchOS (where IOS is not defined by default etc).
  • A lot of it assumes things like arm64 -> device, which are not true any more.

I'm looking at all this stuff and it seems like it might need overhauling, but to do so without breaking existing workflows might be quite difficult, and probably requires more time than I have to devote to it. The only way I can think of to do it bit by bit would be to add slimmed down, modernised, toolchains to polly for the following (which do not set any of the hunter/polly specific magic variables), then gradually fix all the schemes to work correctly (using as much cmake as we can, and increasing the minimum cmake version where required).

  • iOS device, arm64, CMAKE_OSX_DEPLOYMENT_TARGET set
  • iOS simulator, arm64 + x86_64, CMAKE_OSX_DEPLOYMENT_TARGET set
  • iOS combined, arm64 device + x86_64 simulator, CMAKE_OSX_DEPLOYMENT_TARGET set
  • tvOS device, tvOS simulator as above
  • watchOS device, watchOS simulator (we may need to alter the architectures for this as I think there are more device archs)

Does anyone have any better ideas for how to address this stuff?

@hjmallon
Copy link
Author

hjmallon commented Oct 3, 2022

Concrete suggestions:

  • Remove IOS_DEPLOYMENT_SDK_VERSION and use CMAKE_OSX_DEPLOYMENT_TARGET since modern CMake uses that for all Apple platforms
  • Move the logic for calculating IPHONEOS_ROOT / IPHONEOS_SDK_ROOT / IPHONESIMULATOR_SDK_ROOT into Hunter from polly, they are still used for non-CMake schemes. We could try to persuede CMake upstream to make variables containing these values public, since they already find them too.
  • Remove uses of IOS_SDK_VERSION in Hunter, I think they can all be replaced with something more appropriate.

@rbsheth
Copy link
Member

rbsheth commented Oct 5, 2022

Perhaps the new Xcode 14+ toolchains can use the new scheme? And avoid breaking old workflows.

@hjmallon
Copy link
Author

hjmallon commented Oct 6, 2022

I would hope we can do this without breaking old/normal workflows, by which I mean recent-ish CMake versions, recent-ish iOS SDK / Xcode versions.

We can do things like

  • Use CMAKE_OSX_DEPLOYMENT_TARGET, but have a fall back for old workflows
if (NOT CMAKE_OSX_DEPLOYMENT_TARGET AND IOS_DEPLOYMENT_SDK_VERSION)
set(CMAKE_OSX_DEPLOYMENT_TARGET "${IOS_DEPLOYMENT_SDK_VERSION}"
endif()

There are loads of edge cases of old Xcode and/or old CMake that we just wouldn't be able to test without input from whoever is using those workflows.

On the plus side we would be able to:

  • Fix behaviour on macOS ARM64 machines building iOS
  • Fix tvOS building
  • Stop requiring any polly-specific behaviour for iOS builds

@rbsheth
Copy link
Member

rbsheth commented Oct 12, 2022

I would hope we can do this without breaking old/normal workflows, by which I mean recent-ish CMake versions, recent-ish iOS SDK / Xcode versions.

We can do things like

  • Use CMAKE_OSX_DEPLOYMENT_TARGET, but have a fall back for old workflows
if (NOT CMAKE_OSX_DEPLOYMENT_TARGET AND IOS_DEPLOYMENT_SDK_VERSION)
set(CMAKE_OSX_DEPLOYMENT_TARGET "${IOS_DEPLOYMENT_SDK_VERSION}"
endif()

There are loads of edge cases of old Xcode and/or old CMake that we just wouldn't be able to test without input from whoever is using those workflows.

On the plus side we would be able to:

  • Fix behaviour on macOS ARM64 machines building iOS
  • Fix tvOS building
  • Stop requiring any polly-specific behaviour for iOS builds

Would you be volunteering to take a stab at this? 😄

@hjmallon
Copy link
Author

I can certainly start a couple of the different areas. Here at work we might just use the backwards incompatible 'fix just enough to make it work' version I have now in this branch:
https://github.com/hjmallon/hunter/commits/_hm_ios_overhaul

I will keep trying to keep the changes small without messing with too much at once. I will however probably break things unless other iOS-interested people can test changes as we move forwards.

Step 1 would be merging this MR if possible: #610

@rbsheth
Copy link
Member

rbsheth commented Oct 21, 2022

Merged!

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

No branches or pull requests

2 participants