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

CMake < 3.14: set IOS variable to 1? #4635

Closed
SpaceIm opened this issue Aug 11, 2021 · 3 comments
Closed

CMake < 3.14: set IOS variable to 1? #4635

SpaceIm opened this issue Aug 11, 2021 · 3 comments
Assignees
Milestone

Comments

@SpaceIm
Copy link

SpaceIm commented Aug 11, 2021

In CMakeLists, there is a lot of logic based on IOS variable, but this variable is never set to 1 when CMAKE_SYSTEM_NAME is iOS.
CMake does that automatically, but only since 3.14: https://cmake.org/cmake/help/latest/variable/IOS.html

In the meantime, CMake min version in sdl CMakeLists is 3.0.0, so I think that it should still be handled manually in CMakeLists.

@SpaceIm
Copy link
Author

SpaceIm commented Aug 11, 2021

Maybe something like this, even if it's a little bit cumbersome:

...
elseif(APPLE)
  if(CMAKE_SYSTEM_NAME MATCHES ".*Darwin.*")
    set(DARWIN TRUE)
  elseif(CMAKE_SYSTEM_NAME MATCHES ".*MacOS.*")
    set(MACOSX TRUE)
  elseif(CMAKE_SYSTEM_NAME MATCHES ".*tvOS.*")
    set(TVOS TRUE)
  elseif(CMAKE_SYSTEM_NAME MATCHES ".*iOS.*")
    # could be set unconditionally, it's just a reminder to remove this logic if CMake min version is bumped to 3.14
    if(CMAKE_VERSION VERSION_LESS 3.14)
      set(IOS TRUE)
    endif()
  endif()
  # what about watchOS?
...

@slouken slouken added this to the 2.0.18 milestone Aug 12, 2021
@icculus
Copy link
Collaborator

icculus commented Sep 23, 2021

if(CMAKE_SYSTEM_NAME MATCHES ".*Darwin.*")
   set(DARWIN TRUE)
 elseif(CMAKE_SYSTEM_NAME MATCHES ".*MacOS.*")
   set(MACOSX TRUE)
 elseif(CMAKE_SYSTEM_NAME MATCHES ".*tvOS.*")
   set(TVOS TRUE)

...are all of these necessary? If this isn't causing a problem beyond the missing IOS variable, I'd rather not bulk this up as a defensive measure.

(if we need them explicitly set, though, then yeah, let's do it.)

@okuoku
Copy link
Contributor

okuoku commented Sep 23, 2021

Generally speaking, we should not dispatch platforms inside CMake project since Apple's recommendation is feature detection macros (Macro itself still have versioning issue though: #4475 ). It seems current SDL2 codebase largely follows this.

...are all of these necessary?

DARWIN, MACOSX and TVOS exist current SDL2 codebase. History:

IMHO we should follow CMake upstream thus we can depend on APPLE and IOS. DARWIN and most of MACOSX can be safely translated into APPLE and some of them might be if(APPLE AND NOT IOS). Every TVOS occurrences can be translated into IOS. Since CMAKE_SYSTEM_NAME is not utilized in unofficial(ios-cmake) CMake support, I don't think it worked CMake prior to 3.14 anyway for iOS/tvOS.

@icculus icculus closed this as completed in f306662 Oct 5, 2021
dimhotepus pushed a commit to The-White-Box/SDL that referenced this issue Oct 9, 2021
dimhotepus pushed a commit to The-White-Box/SDL that referenced this issue Oct 9, 2021
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

4 participants