cmake revisited all if() conditions 🙃

Author: silverqx

cmake/CommonModules/CsDebug.cmake

@@ -11,7 +11,7 @@ endfunction()
 # Print all CMake variables (excluding all CMake variables by default)
 # cs_print_vars() == cs_print_vars(yes)
 function(cs_print_vars)
-    set(exclude_cmake yes)
+    set(exclude_cmake yes) # Exclude CMake by default
     if(ARGC GREATER_EQUAL 1 AND DEFINED ARGV0 AND NOT ARGV0)
         set(exclude_cmake no)
         message(STATUS "All variables:")
@@ -21,7 +21,7 @@ function(cs_print_vars)
 
     get_cmake_property(variable_names VARIABLES)
     foreach (variable ${variable_names})
-        if(exclude_cmake AND "${variable}" MATCHES "(^(CMAKE_.*)|^(_.*))")
+        if(exclude_cmake AND variable MATCHES "(^(CMAKE_.*)|^(_.*))")
             continue()
         endif()
 
@@ -50,7 +50,7 @@ endif()
 # Print all target properties
 function(cs_print_target_properties target)
 
-    if(NOT TARGET ${target})
+    if(NOT TARGET ${target}) # Quotes not needed, the target cannot be a list and can't contain spaces
         message(FATAL_ERROR "There is no target named: ${target}")
     endif()
 
@@ -82,7 +82,7 @@ endfunction()
 # Print all source file properties
 function(cs_print_source_properties source)
 
-    if(NOT EXISTS "${source}")
+    if(NOT EXISTS ${source}) # Quotes not needed, the target cannot be a list (it fails on the list which is good, it quotes it doesn't fail with the list) and spaces are handled correctly without quotes
         message(FATAL_ERROR "There is no source file named: ${source}")
     endif()
 
@@ -128,9 +128,9 @@ endfunction()
 
 # Print clearly visible notice message about passed BOOL variable
 function(pb variable)
-    if(NOT DEFINED ${variable})
+    if(NOT DEFINED ${variable}) # Quotes not needed
         message("|||-- ${variable} : ${variable}-NOTFOUND")
-    elseif(${variable})
+    elseif(${variable}) # Quotes not needed because of the DEFINED check above and don't care about lists and strings
         message("|||-- ${variable} : ON")
     else()
         message("|||-- ${variable} : OFF")

cmake/CommonModules/TinyFeatureOptions.cmake

@@ -55,7 +55,7 @@ macro(tiny_dependent_string_option option strings doc default depends force)
 
     # Determine whether the given option should be provided and visible (using the full
     # Condition Syntax (CMP0127 implementation))
-    foreach(depend ${depends})
+    foreach(depend ${depends}) # Don't use ITEMS keyword
         # Don't use the if(NOT ${depend}) without else() block here, the ${depend} can
         # contain complex condition and it can break meaning of this condition
         cmake_language(EVAL CODE "
@@ -66,7 +66,7 @@ macro(tiny_dependent_string_option option strings doc default depends force)
         )
     endforeach()
 
-    if(${option}_AVAILABLE)
+    if(${option}_AVAILABLE) # Quotes not needed, will be _AVAILABLE at least, so FALSE on empty/undefined ${option}
         # Restore the previous option value from the INTERNAL cache variable saved earlier
         if(DEFINED CACHE{${option}})
             set(${option} "${${option}}" CACHE STRING "${doc}" FORCE)

cmake/CommonModules/TinyHelpers.cmake

@@ -45,7 +45,7 @@ macro(tiny_find_package package_name)
 
     find_package(${package_name} ${ARGN})
 
-    if(${package_name}_FOUND)
+    if(${package_name}_FOUND) # Quotes not needed, will be _FOUND at least, so FALSE on empty/undefined ${package_name}
         set(args "${package_name}")
         # These arguments will be forwarded to the find_package() by find_dependency()
         list(APPEND args "${ARGN}")
@@ -61,7 +61,7 @@ macro(tiny_find_package package_name)
         # Check if the given args are in the TINY_PACKAGE_DEPENDENCIES list
         get_property(packageDependencies GLOBAL PROPERTY TINY_PACKAGE_DEPENDENCIES)
 
-        if(NOT args IN_LIST packageDependencies)
+        if(NOT args IN_LIST packageDependencies) # Automatic Variable Expansion applies
             set_property(GLOBAL APPEND PROPERTY TINY_PACKAGE_DEPENDENCIES "${args}")
         endif()
     endif()
@@ -134,7 +134,7 @@ ${TINY_UNPARSED_ARGUMENTS}")
 
     option(${TINY_NAME} "${TINY_DESCRIPTION}" ${TINY_DEFAULT})
 
-    if(${${TINY_NAME}})
+    if(${${TINY_NAME}}) # Quotes not needed, don't care about lists for now
         target_compile_definitions(${target} ${scope} ${TINY_ENABLED})
     else()
         target_compile_definitions(${target} ${scope} ${TINY_DISABLED})
@@ -163,7 +163,7 @@ endfunction()
 # Create an empty SQLite database file if it does not exist
 function(tiny_create_sqlite_db db_filepath)
 
-    if(EXISTS ${db_filepath})
+    if(EXISTS ${db_filepath}) # Quotes not needed, the target cannot be a list (it fails on the list which is good, it quotes it doesn't fail with the list) and spaces are handled correctly without quotes
         return()
     endif()
 
@@ -179,7 +179,7 @@ function(tiny_create_buildtree_tagfiles filepaths)
 
     foreach(filepath ${filepaths})
         # Nothing to do, .build_tree tag already exists
-        if(EXISTS ${filepath})
+        if(EXISTS ${filepath}) # Quotes not needed, the target cannot be a list (it fails on the list which is good, it quotes it doesn't fail with the list) and spaces are handled correctly without quotes
             continue()
         endif()
 
@@ -479,6 +479,9 @@ endfunction()
 # Helper function to replace /Zi and /ZI by /Z7 in the CMAKE_<C|CXX>_FLAGS_<CONFIG> option
 function(tiny_replace_Zi_by_Z7_for option help_string)
 
+    # Don't quote: ${option} MATCHES; it would change the meaning and stop working;
+    # CMake first do the Variable Expansion and then the Automatic Variable Evaluation
+    # on this replaced/expanded value (eg. CMAKE_CXX_FLAGS_DEBUG)
     if(DEFINED ${option} AND ${option} MATCHES "(/|-)(Zi|ZI)")
         string(REGEX REPLACE "(/|-)(Zi|ZI)" "/Z7" ${option} "${${option}}")
 

cmake/CommonModules/TinyToolchainRequirement.cmake

@@ -13,7 +13,7 @@ function(tiny_satisfies_minimum_required_qt_version out_variable)
 
     # Nothing to do, Qt version was already populated (cache hit)
     if(DEFINED TINY_QT_VERSION AND NOT TINY_QT_VERSION STREQUAL "")
-        if(TINY_QT_VERSION VERSION_GREATER_EQUAL minReqQtVersion)
+        if(TINY_QT_VERSION VERSION_GREATER_EQUAL minReqQtVersion) # Automatic Variable Expansion
             set(${out_variable} TRUE PARENT_SCOPE)
 
         # There is a very low chance that this code branch will be invoked, but I can't
@@ -59,7 +59,7 @@ in ${CMAKE_CURRENT_FUNCTION}().")
         "Qt version used to determine whether a minimum required Qt version was \
 satisfied (also used by tiny_configure_test_pch()).")
 
-    if(TINY_QT_VERSION VERSION_GREATER_EQUAL minReqQtVersion)
+    if(TINY_QT_VERSION VERSION_GREATER_EQUAL minReqQtVersion) # Automatic Variable Expansion
         set(${out_variable} TRUE PARENT_SCOPE)
     else()
         set(${out_variable} FALSE PARENT_SCOPE)

cmake/Modules/FindMySQL.cmake

@@ -70,7 +70,7 @@ if(MySQL_FOUND)
     set(MySQL_INCLUDE_DIRS "${MySQL_INCLUDE_DIR}")
     set(MySQL_LIBRARIES "${MySQL_LIBRARY}")
 
-    if(NOT TARGET MySQL::MySQL)
+    if(NOT TARGET MySQL::MySQL) # Quotes not needed (my code style)
         add_library(MySQL::MySQL UNKNOWN IMPORTED)
         set_target_properties(MySQL::MySQL
             PROPERTIES

cmake/Modules/GeneratePkgConfig.cmake

@@ -148,7 +148,7 @@ function(generate_and_install_pkg_config_file _target _packageName)
 
 	# Since CMake 3.18 FindThreads may include a generator expression requiring a target, which gets propagated to us through INTERFACE_OPTIONS.
 	# Before CMake 3.19 there's no way to solve this in a general way, so we work around the specific case. See #4956 and CMake bug #21074.
-	if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.19)
+	if(CMAKE_VERSION VERSION_GREATER_EQUAL "3.19")
 		set(_target_arg TARGET ${_target})
 	else()
 		string(REPLACE "<COMPILE_LANG_AND_ID:CUDA,NVIDIA>" "<COMPILE_LANGUAGE:CUDA>" _interface_compile_options "${_interface_compile_options}")

cmake/Modules/TinyDeployment.cmake

@@ -170,9 +170,9 @@ list(APPEND CMAKE_MODULE_PATH \"\${CMAKE_CURRENT_LIST_DIR}/Modules\")")
 
     # Used in the Package Config and Config Version files
     get_property(cvf_is_multi_config GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG)
-    tiny_to_bool(cvf_is_multi_config ${cvf_is_multi_config})
+    tiny_to_bool(cvf_is_multi_config ${cvf_is_multi_config}) # Don't quote, must fail if undefined
 
-    tiny_to_bool(cvf_is_vcpkg ${TINY_VCPKG})
+    tiny_to_bool(cvf_is_vcpkg ${TINY_VCPKG}) # Don't quote, must fail if undefined
 
     # Generate target includes for the TinyORM package config file
     set(tiny_target_includes)
@@ -291,7 +291,7 @@ list(APPEND CMAKE_MODULE_PATH \"\${CMAKE_CURRENT_LIST_DIR}/cmake/Modules\")")
     endif()
 
     get_property(cvf_is_multi_config GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG)
-    tiny_to_bool(cvf_is_multi_config ${cvf_is_multi_config})
+    tiny_to_bool(cvf_is_multi_config ${cvf_is_multi_config}) # Don't quote, must fail if undefined
 
     # Generate target includes for the TinyORM package config file
     set(tiny_target_includes)

cmake/Modules/TinyTestCommon.cmake

@@ -146,7 +146,7 @@ function(tiny_configure_test_pch name provides_pch)
     # variable, it also affects CI pipelines on GitHub self-hosted runners
     if(TINY_QT_VERSION VERSION_LESS "6.9.0" AND
             NOT (DEFINED ENV{TINY_QT6_TEST_TARGET_PATCHED} AND
-                "$ENV{TINY_QT6_TEST_TARGET_PATCHED}")
+                "$ENV{TINY_QT6_TEST_TARGET_PATCHED}") # Quotes needed to avoid fail if undefined as conditions don't short-circuit!
     )
         target_precompile_headers(${name} PRIVATE
             $<$<COMPILE_LANGUAGE:CXX>:"${${TinyOrm_ns}_SOURCE_DIR}/include/pch.h">
@@ -190,7 +190,7 @@ function(tiny_throw_if_wrong_reuse_from name)
 
     # Nothing to do
     if(NOT DEFINED CACHE{TINY_TESTS_PCH_REUSE_FROM} OR
-            "$CACHE{TINY_TESTS_PCH_REUSE_FROM}" STREQUAL name
+            "$CACHE{TINY_TESTS_PCH_REUSE_FROM}" STREQUAL name # Quotes needed to avoid fail if undefined as conditions don't short-circuit!
     )
         return()
     endif()

cmake/TinyPackageConfigHelpers.cmake

@@ -122,7 +122,7 @@ function(tiny_build_type_requirements_install_tree
         list(LENGTH cvfTargetConfigurations count)
 
         get_property(isMultiConfig GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG)
-        tiny_to_bool(isMultiConfig ${isMultiConfig})
+        tiny_to_bool(isMultiConfig ${isMultiConfig}) # Don't quote, must fail if undefined
 
         # Used in STREQUAL comparisons
         string(TOLOWER "${CMAKE_BUILD_TYPE}" cmakeBuildTypeLower)
@@ -190,7 +190,7 @@ function(tiny_build_type_requirements_build_tree
 
     if(NOT cvf_is_multi_config)
         get_property(isMultiConfig GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG)
-        tiny_to_bool(isMultiConfig ${isMultiConfig})
+        tiny_to_bool(isMultiConfig ${isMultiConfig}) # Don't quote, must fail if undefined
 
         # Used in STREQUAL comparisons
         string(TOLOWER "${CMAKE_BUILD_TYPE}" cmakeBuildTypeLower)