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

Support the "Xcode (iOS)" Projucer exporter #551

Merged
merged 30 commits into from
Nov 9, 2019
Merged

Support the "Xcode (iOS)" Projucer exporter #551

merged 30 commits into from
Nov 9, 2019

Conversation

McMartin
Copy link
Owner

@McMartin McMartin commented Nov 6, 2019

Related to #458.

This PR is made of the following commit "groups":

  • commit 7fe3eaf documents per-exporter CMake requirements
  • commits 6828d9b and 940cf8b only deal with renaming
  • commits from e9fb724 to 169af4d add the core of the iOS support
  • commits from d9495f8 to 50806cd update generated CMakeLists.txt files (most of the changes were done by Jucer2Reprojucer)
  • commits from 1da9069 to 8022fab add support for various settings that are specific to the "Xcode (iOS)" exporter
  • commits 387b70e and 9f88d82 extend CI and polish the README

@MartyLake: please review, thanks!

@MartyLake
Copy link
Collaborator

LGTM, even if it's hard to say I read every single line of it

@McMartin
Copy link
Owner Author

McMartin commented Nov 7, 2019

First-class support for iOS was added in CMake 3.14. When I opened this PR, I didn't want to make that a hard requirement, since the code of FRUT itself only requires CMake version 3.4 minimum. However, after a bit of testing on macOS, and considering that Visual Studio 2019 is also only supported since CMake 3.14, I changed my mind. In this new version of the PR, CMake 3.14 is required for iOS. Instead of calling cmake -G Xcode -DJUCER_IOS=ON and using JUCER_IOS in Reprojucer.cmake, we now call cmake -G Xcode -DCMAKE_SYSTEM_NAME=iOS and we use IOS in Reprojucer.cmake.

@MartyLake: please re-review this PR. To help you understand what changed, you can read the diff between bb0cb70 and 96933d1. I also updated the PR description with the new commit groups. Thanks!

@McMartin McMartin requested a review from MartyLake November 7, 2019 22:45
@McMartin McMartin force-pushed the support-iOS branch 2 times, most recently from 2f048a9 to 96933d1 Compare November 7, 2019 22:51
@McMartin
Copy link
Owner Author

McMartin commented Nov 8, 2019

We hit a new problem. Reprojucer.cmake needs to build its helper executables (BinaryDataBuilder, IconBuilder, PListMerger, and the new XcassetsBuilder), but these executables can only be built and run on macOS, not iOS. Since Reprojucer.cmake uses the try_compile CMake command, CMAKE_SYSTEM_NAME=iOS (and everything that comes with it) is passed down when configuring the helper executables. This obviously fails because the Cocoa framework cannot be found in the iphoneos SDK.

@MartyLake: please don't review yet, fixing this problem might require quite some changes.

@McMartin
Copy link
Owner Author

McMartin commented Nov 8, 2019

After reading more about try_compile I came to the conclusion that it is not the right tool in the first place. try_compile should be used to determine if something can/would compile, it shouldn't be used to just build and install other CMake projects at configure time. I'll open a new PR to replace try_compile by something else in _FRUT_build_and_install_helper_exe, and then I'll rebase this one again. Stay tuned!

The code of FRUT still only requires CMake 3.4 minimum to run.
This matches XcodeProjectExporter::XcodeTarget::xcodeFrameworks and
XcodeProjectExporter::xcodeFrameworks in
JUCE/extras/Projucer/Source/ProjectSaving/jucer_ProjectExport_Xcode.h.

This also makes room for introducing iOS frameworks.
This matches XcodeProjectExporter::XcodeTarget::xcodeLibs and
XcodeProjectExporter::xcodeLibs in
JUCE/extras/Projucer/Source/ProjectSaving/jucer_ProjectExport_Xcode.h.

This also makes room for introducing iOS libs.
It will be used by Reprojucer.cmake to generate an "Images.xcassets"
bundle like Projucer does.
@McMartin
Copy link
Owner Author

McMartin commented Nov 9, 2019

@MartyLake: I've rebased the branch on top of the latest master after merging #553 and force-pushed. This PR is now ready for review again, please have another look, thanks!

@MartyLake
Copy link
Collaborator

MartyLake commented Nov 9, 2019

LGTM, no obvious typo :D

@McMartin McMartin merged commit a21aea7 into master Nov 9, 2019
@McMartin McMartin deleted the support-iOS branch November 9, 2019 19:59
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