fix issue: wrong Darwin value for iOS

[kgbook]

fix issue: wrong Darwin value for iOS; CMake MATCHES v.s STREQUAL is different, variable should not be caculated its value for MATCHES

[kgbook]

All issues and warnings have been fixed. The project is now compiling well for iOS!

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (88c6214) 67.04% compared to head (2343e3e) 66.87%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2846      +/-   ##
==========================================
- Coverage   67.04%   66.87%   -0.18%     
==========================================
  Files         102      102              
  Lines       20368    20368              
==========================================
- Hits        13656    13621      -35     
- Misses       6712     6747      +35     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[maxsharabayko]

We can't raise the minimum required CMake version in a patch release1.5.4 scheduled to be the next one.
In any case there should be a strong motivation for raising this requirement, as it affects portability of SRT a bit (requires additional effort to update CMake on some older platforms).

[maxsharabayko]

I am always confused with those ${var} and var notations in CMake. With this change you are passing the variable CMAKE_SYSTEM_NAME itself instead of its value to the MATCHES comparator. What's the difference with passing the value?
Also following set_if statements are using a different notation to what is proposed.

[ethouris]

CMake documentation defines it this way:

if(<variable|string> MATCHES <regex>)

I'm not sure how CMake handles it in every single case, however by using ${var} you risk that if this resolves to an empty string, you effectively cause a syntax error. OTOH I have no idea what exactly happens if it resolves to a name of an existing variable. Probably the direct string and an effective string resolved from a variable are also distinguished. Not sure how it exactly works provided that the conditional statement is simply placed as argument for if:

MACRO(set_if varname)
	IF(${ARGN})
		SET(${varname} 1)
	ELSE(${ARGN})
		SET(${varname} 0)
	ENDIF(${ARGN})
ENDMACRO(set_if)

Anyway, if you are certain that a particular variable exists, you can use just as well ${CMAKE_SYSTEM_NAME}.

Frankly I don't understand, what this change is here and actually when I compare it to the latest version, it just changes ${var} into var. This change is completely unnecessary. I understand the problem was with wrong values being compared against. This just solves the problem of a case when CMAKE_SYSTEM_NAME variable doesn't exist or contains an empty string. But they write here that it's not the case.

[kgbook]

set_if(DARWIN1	   (CMAKE_SYSTEM_NAME MATCHES "Darwin")
					OR (CMAKE_SYSTEM_NAME MATCHES "iOS")
					OR (CMAKE_SYSTEM_NAME MATCHES "tvOS")
					OR (CMAKE_SYSTEM_NAME MATCHES "watchOS"))
set_if(DARWIN2	   (${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
	OR (${CMAKE_SYSTEM_NAME} MATCHES "iOS")
	OR (${CMAKE_SYSTEM_NAME} MATCHES "tvOS")
	OR (${CMAKE_SYSTEM_NAME} MATCHES "watchOS"))
message("CMAKE_SYSTEM_NAME: ${CMAKE_SYSTEM_NAME}, DARWIN1: ${DARWIN1}, DARWIN2: ${DARWIN2}")

It prints:

CMAKE_SYSTEM_NAME: iOS, DARWIN1: 1, DARWIN2: 0

[ethouris]

Does that mean that the behavior of the if() command is platform-dependent?

Which version are you using? Mine is 3.20.2, system: Oracle Linux 9.2

I screwed the original code with additional OR so that it at least uses the same "tricks".

set_if(LINUX       (${CMAKE_SYSTEM_NAME} MATCHES "linux") OR (${CMAKE_SYSTEM_NAME} MATCHES "Linux"))
set_if(LINUX2      ("${CMAKE_SYSTEM_NAME}" MATCHES "linux") OR ("${CMAKE_SYSTEM_NAME}" MATCHES "Linux"))
set_if(LINUX3      (CMAKE_SYSTEM_NAME MATCHES "linux") OR (CMAKE_SYSTEM_NAME MATCHES "Linux"))
message("CMAKE_SYSTEM_NAME='${CMAKE_SYSTEM_NAME}' LINUX=${LINUX} LINUX2=${LINUX2} LINUX3=${LINUX3}")

prints:

CMAKE_SYSTEM_NAME='Linux' LINUX=1 LINUX2=1 LINUX3=1

[kgbook]

I figure it out and reproduce the issue.

cmake_minimum_required(VERSION 3.28)

project(test)

if (${CMAKE_SYSTEM_NAME} MATCHES "iOS")
    message("1. MATCHES iOS")
else()
    message("1. ${CMAKE_SYSTEM_NAME} not MATCHES iOS")
endif ()
if (CMAKE_SYSTEM_NAME MATCHES "iOS")
    message("2. MATCHES iOS")
else()
    message("2. ${CMAKE_SYSTEM_NAME} not MATCHES iOS")
endif ()

if (CMAKE_SYSTEM_NAME MATCHES "iOS")
    set(iOS 1) # this line should be remove 
endif()

if (${CMAKE_SYSTEM_NAME} MATCHES "iOS")
    message("3. MATCHES iOS")
else()
    message("3. ${CMAKE_SYSTEM_NAME} not MATCHES iOS")
endif ()
if (CMAKE_SYSTEM_NAME MATCHES "iOS")
    message("4. MATCHES iOS")
else()
    message("4. ${CMAKE_SYSTEM_NAME} not MATCHES iOS")
endif ()

After execute cmake command below:

cmake -B build -DCMAKE_SYSTEM_NAME=iOS 

prints:

1. MATCHES iOS
2. MATCHES iOS
3. iOS not MATCHES iOS
4. MATCHES iOS

if (${CMAKE_SYSTEM_NAME} MATCHES "iOS") will be evaluated again to if (1 MATCHES "iOS") finally after set(iOS 1) in our project, see CMake policy:CMP0054.

I will remove set(iOS 1) from our project.

This change is completely unnecessary.

[maxsharabayko]

It's weird that setting iOS to 1 changes the value of the CMAKE_SYSTEM_NAME. 🤯

[kgbook]

It's weird that setting iOS to 1 changes the value of the CMAKE_SYSTEM_NAME. 🤯

No, setting iOS to 1 not change the value of the CMAKE_SYSTEM_NAME, but the if() command would treat iOS as a variable and interpret again.

CMake may interpret the if statements as below:

if (${CMAKE_SYSTEM_NAME} MATCHES "iOS") ---> if (iOS MATCHES "iOS") ---> if (1 MATCHES "iOS")

[ethouris]

Nice that the problem has been resolved. I was suspecting that this is the case, but I didn't believe that cmake is that stupid.