Refactor PHP/Set arguments validation and do some cleanup

petk · petk · commit c37d85d15707 · 2024-10-26T22:16:39.000+02:00
diff --git a/cmake/cmake/modules/PHP/SAPIs.cmake b/cmake/cmake/modules/PHP/SAPIs.cmake
@@ -22,10 +22,11 @@ php_sapis_add(subdirectory)
   configuration.
 #]=============================================================================]
 
+# Add subdirectories of PHP SAPIs. Macro enables variable scope of the current
+# CMakeLists.txt and adds ability to pass around directory variables.
 macro(php_sapis_add directory)
   _php_sapis_get(${directory} directories)
 
-  # Add subdirectories of PHP SAPIs.
   foreach(dir ${directories})
     cmake_path(GET dir FILENAME sapi)
     list(APPEND CMAKE_MESSAGE_CONTEXT "sapi/${sapi}")
@@ -38,10 +39,9 @@ macro(php_sapis_add directory)
     _php_sapis_post_configure("${dir}")
   endforeach()
 
-  _php_sapis_validate()
-
   unset(directories)
-  unset(sapis)
+
+  _php_sapis_validate()
 endmacro()
 
 # Get a list of subdirectories related to PHP SAPIs.
diff --git a/cmake/cmake/modules/PHP/SearchLibraries.cmake b/cmake/cmake/modules/PHP/SearchLibraries.cmake
@@ -55,7 +55,7 @@ of `<headers>`) and store the result in an internal cache variable
 * `LIBRARY_VARIABLE`
 
   When symbol is not found in the default libraries, the resulting library that
-  contains the symbol is stored in this regular variable name.
+  contains the symbol is stored in this local variable name.
 
 * `TARGET`
 
@@ -142,7 +142,7 @@ macro(_php_search_libraries_populate)
       )
     endif()
 
-    # Store found library in a regular variable provided by the user.
+    # Store found library in a local variable with name provided by the user.
     if(libraryResultVariable)
       set(
         ${libraryResultVariable}
diff --git a/cmake/cmake/modules/PHP/Set.cmake b/cmake/cmake/modules/PHP/Set.cmake
@@ -1,6 +1,13 @@
 #[=============================================================================[
 Set a CACHE variable that depends on a set of conditions.
 
+> [!WARNING]
+> TODO: This module is still under review to determine its usefulness.
+> Dependent variables may seem convenient for the application but may create
+> difficulties for anyone troubleshooting why a configuration isn't applied,
+> even though a configuration value has been set. In the end, build system
+> configuration isn't aiming to provide a HTML-form-alike functionality.
+
 At the time of writing, there are 3 main ways in CMake to create non-internal
 cache variables that can be also customized from the outside using the `-D`
 command-line option, through CMake presets, or similar:
@@ -26,6 +33,7 @@ php_set(
   [CHOICES <string>...]
   [IF <condition> VALUE <value> [ELSE_VALUE <default>]] | [VALUE <value>]
   DOC <docstring>...
+  [WARNING <warning>]
 )
 ```
 
@@ -60,50 +68,45 @@ It sets a CACHE `<variable>` of `<type>` to a `<value>`.
 
 * `DOC` is a short variable help text visible in the GUIs. Multiple strings are
   joined together.
+
+* `WARNING` is optional text that is emitted when setting a variable from the
+  command line or CMake presets but its condition is not met. Otherwise, a
+  default warning is emitted.
 #]=============================================================================]
 
 include_guard(GLOBAL)
 
 function(php_set)
-  # https://cmake.org/cmake/help/latest/policy/CMP0174.html
-  if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.31)
-    set(valueNew "VALUE")
-    set(elseValueNew "ELSE_VALUE")
-  else()
-    set(valueOld "VALUE")
-    set(elseValueOld "ELSE_VALUE")
-  endif()
-
   cmake_parse_arguments(
     PARSE_ARGV
     1
-    parsed                                    # prefix
-    ""                                        # options
-    "${valueNew};TYPE;IF;${elseValueNew}"     # one-value keywords
-    "${valueOld};CHOICES;DOC;${elseValueOld}" # multi-value keywords
+    parsed                     # prefix
+    ""                         # options
+    "TYPE;IF;VALUE;ELSE_VALUE" # one-value keywords
+    "CHOICES;DOC;WARNING"      # multi-value keywords
   )
 
-  if(parsed_UNPARSED_ARGUMENTS)
-    message(FATAL_ERROR "Bad arguments: ${parsed_UNPARSED_ARGUMENTS}")
-  endif()
-
-  if(NOT DEFINED parsed_VALUE)
-    message(FATAL_ERROR "Missing VALUE argument")
-  endif()
-
-  if(NOT parsed_TYPE)
-    message(FATAL_ERROR "Missing TYPE argument")
-  elseif(NOT parsed_TYPE MATCHES "^(BOOL|FILEPATH|PATH|STRING)$")
-    message(FATAL_ERROR "Unknown TYPE argument: ${parsed_TYPE}")
-  endif()
-
-  if(NOT DEFINED parsed_IF AND DEFINED parsed_ELSE_VALUE)
-    message(FATAL_ERROR "Redundant ELSE_VALUE argument without IF condition")
+  # The cmake_parse_arguments() before 3.31 didn't define one-value keywords
+  # with empty value of "". This fills the gap and behaves the same until it can
+  # be removed. See: https://cmake.org/cmake/help/latest/policy/CMP0174.html
+  if(CMAKE_VERSION VERSION_LESS 3.31)
+    set(i 0)
+    foreach(arg IN LISTS ARGN)
+      math(EXPR i "${i}+1")
+      foreach(keyword VALUE ELSE_VALUE)
+        if(
+          arg STREQUAL "${keyword}"
+          AND NOT DEFINED parsed_${keyword}
+          AND DEFINED ARGV${i}
+          AND "${ARGV${i}}" STREQUAL ""
+        )
+          set(parsed_${keyword} "")
+        endif()
+      endforeach()
+    endforeach()
   endif()
 
-  if(NOT DEFINED parsed_DOC)
-    message(FATAL_ERROR "Missing DOC argument")
-  endif()
+  _php_set_validate_arguments("${ARGN}")
 
   set(doc "")
   foreach(string ${parsed_DOC})
@@ -112,6 +115,9 @@ function(php_set)
 
   set(condition TRUE)
   if(parsed_IF)
+    # Make condition look nice in the possible output strings.
+    string(STRIP "${parsed_IF}" parsed_IF)
+    string(REGEX REPLACE "[ \t]*[\r\n]+[ \t\r\n]*" "\n" parsed_IF "${parsed_IF}")
     foreach(d ${parsed_IF})
       cmake_language(EVAL CODE "
         if(${d})
@@ -122,46 +128,120 @@ function(php_set)
     endforeach()
   endif()
 
-  set(var "${ARGV0}")
-  set(internal ___PHP_SET_${var})
+  set(varName "${ARGV0}")
+  set(bufferVarName ___PHP_SET_${varName})
+  set(bufferDoc "Internal storage for ${varName} variable")
 
-  if(NOT DEFINED ${internal} AND DEFINED ${var})
+  if(NOT DEFINED ${bufferVarName} AND DEFINED ${varName})
     # Initial configuration phase with variable set by the user.
-    set(${internal} "${${var}}" CACHE INTERNAL "Internal storage for ${var}")
-  elseif(NOT DEFINED ${internal})
+    set(${bufferVarName} "${${varName}}" CACHE INTERNAL "${bufferDoc}")
+  elseif(NOT DEFINED ${bufferVarName})
     # Initial configuration phase without variable set by the user.
-    set(${internal} "${parsed_VALUE}" CACHE INTERNAL "Internal storage for ${var}")
+    set(${bufferVarName} "${parsed_VALUE}" CACHE INTERNAL "${bufferDoc}")
   elseif(
-    DEFINED ${internal}
-    AND ${internal}_FORCED
-    AND NOT ${var} STREQUAL "${parsed_ELSE_VALUE}"
+    DEFINED ${bufferVarName}
+    AND ${bufferVarName}_OVERRIDDEN
+    AND NOT ${varName} STREQUAL "${parsed_ELSE_VALUE}"
   )
     # Consecutive configuration phase that changes the variable after being
     # re-enabled.
-    set(${internal} "${${var}}" CACHE INTERNAL "Internal storage for ${var}")
-  elseif(DEFINED ${internal} AND NOT ${internal}_FORCED)
+    set(${bufferVarName} "${${varName}}" CACHE INTERNAL "${bufferDoc}")
+  elseif(DEFINED ${bufferVarName} AND NOT ${bufferVarName}_OVERRIDDEN)
     # Consecutive configuration phase.
-    set(${internal} "${${var}}" CACHE INTERNAL "Internal storage for ${var}")
+    set(${bufferVarName} "${${varName}}" CACHE INTERNAL "${bufferDoc}")
   endif()
 
   if(condition)
-    set(${var} "${${internal}}" CACHE ${parsed_TYPE} "${doc}" FORCE)
+    set(${varName} "${${bufferVarName}}" CACHE ${parsed_TYPE} "${doc}" FORCE)
     if(parsed_TYPE STREQUAL "STRING" AND parsed_CHOICES)
-      set_property(CACHE ${var} PROPERTY STRINGS ${parsed_CHOICES})
+      set_property(CACHE ${varName} PROPERTY STRINGS ${parsed_CHOICES})
     endif()
-    unset(${internal} CACHE)
-    unset(${internal}_FORCED CACHE)
+    unset(${bufferVarName} CACHE)
+    unset(${bufferVarName}_OVERRIDDEN CACHE)
   else()
+    _php_set_validate_input()
+
     if(DEFINED parsed_ELSE_VALUE)
-      set(${var} "${parsed_ELSE_VALUE}" CACHE INTERNAL "${doc}")
+      set(${varName} "${parsed_ELSE_VALUE}" CACHE INTERNAL "${doc}" FORCE)
     else()
-      unset(${var} CACHE)
+      unset(${varName} CACHE)
     endif()
     set(
-      ${internal}_FORCED
+      ${bufferVarName}_OVERRIDDEN
       TRUE
       CACHE INTERNAL
-      "Internal marker that ${var} has a forced value."
+      "Internal marker that ${varName} is overridden."
     )
   endif()
 endfunction()
+
+# Validate parsed arguments.
+function(_php_set_validate_arguments arguments)
+  if(parsed_UNPARSED_ARGUMENTS)
+    message(FATAL_ERROR "Bad arguments: ${parsed_UNPARSED_ARGUMENTS}")
+  endif()
+
+  if(NOT DEFINED parsed_VALUE)
+    message(FATAL_ERROR "Missing VALUE argument")
+  endif()
+
+  if(NOT parsed_TYPE)
+    message(FATAL_ERROR "Missing TYPE argument")
+  elseif(NOT parsed_TYPE MATCHES "^(BOOL|FILEPATH|PATH|STRING)$")
+    message(FATAL_ERROR "Unknown TYPE argument: ${parsed_TYPE}")
+  endif()
+
+  list(FIND arguments ELSE_VALUE elseValueIndex)
+  if(NOT DEFINED parsed_IF AND NOT elseValueIndex EQUAL -1)
+    message(FATAL_ERROR "Redundant ELSE_VALUE argument without IF condition")
+  elseif(
+    DEFINED parsed_IF
+    AND NOT DEFINED parsed_ELSE_VALUE
+    AND NOT elseValueIndex EQUAL -1
+  )
+    message(FATAL_ERROR "Missing ELSE_VALUE argument")
+  endif()
+
+  if(NOT DEFINED parsed_DOC)
+    message(FATAL_ERROR "Missing DOC argument")
+  endif()
+endfunction()
+
+# Output warning when setting conditional variable and condition is not met.
+function(_php_set_validate_input)
+  get_property(helpString CACHE ${varName} PROPERTY HELPSTRING)
+  if(NOT helpString STREQUAL "No help, variable specified on the command line.")
+    return()
+  endif()
+
+  if(NOT parsed_WARNING)
+    set(parsed_WARNING "Variable ${varName}")
+    if(DEFINED parsed_ELSE_VALUE)
+      string(
+        APPEND
+        parsed_WARNING
+        " has been overridden (${varName}=${parsed_ELSE_VALUE})"
+      )
+    else()
+      string(
+        APPEND
+        parsed_WARNING
+        " has been overridden to an undefined state"
+      )
+    endif()
+    string(
+      APPEND
+      parsed_WARNING
+      " as it depends on the condition:\n"
+      "${parsed_IF}\n"
+    )
+  endif()
+  set(warning "")
+  foreach(string ${parsed_WARNING})
+    string(APPEND warning "${string}")
+  endforeach()
+
+  if(NOT ${varName} STREQUAL "${parsed_ELSE_VALUE}")
+    message(WARNING "${warning}")
+  endif()
+endfunction()
diff --git a/cmake/cmake/modules/Zend/MaxExecutionTimers.cmake b/cmake/cmake/modules/Zend/MaxExecutionTimers.cmake
@@ -13,9 +13,9 @@ Check whether to enable Zend max execution timers.
 
 * `ZEND_MAX_EXECUTION_TIMERS`
 
-  A regular variable based on the cache variable and thread safety to be able to
+  A local variable based on the cache variable and thread safety to be able to
   run consecutive configuration runs. When `ZEND_MAX_EXECUTION_TIMERS` cache
-  variable is set to 'auto', regular variable default value is set to the
+  variable is set to 'auto', local variable default value is set to the
   `PHP_THREAD_SAFETY` value.
 
 ## INTERFACE IMPORTED library
@@ -48,7 +48,7 @@ set_property(
   PROPERTY STRINGS "auto" "ON" "OFF"
 )
 
-# Set a regular variable based on the cache variable.
+# Set a local variable based on the cache variable.
 if(ZEND_MAX_EXECUTION_TIMERS STREQUAL "auto")
   set(ZEND_MAX_EXECUTION_TIMERS "${PHP_THREAD_SAFETY}")
 else()
diff --git a/cmake/ext/fileinfo/CMakeLists.txt b/cmake/ext/fileinfo/CMakeLists.txt
@@ -56,20 +56,19 @@ target_sources(
     php_libmagic.c
 )
 
-check_symbol_exists(utimes "sys/time.h" HAVE_UTIMES)
-check_symbol_exists(strndup "string.h" HAVE_STRNDUP)
-
 # The strcasestr is missing on Windows and Solaris <= 10 (Solaris 11 has it).
 cmake_push_check_state(RESET)
   set(CMAKE_REQUIRED_DEFINITIONS -D_GNU_SOURCE)
   check_symbol_exists(strcasestr "string.h" HAVE_STRCASESTR)
 cmake_pop_check_state()
-
 if(NOT HAVE_STRCASESTR)
   message(STATUS "Using libmagic strcasestr implementation")
   target_sources(php_fileinfo PRIVATE libmagic/strcasestr.c)
 endif()
 
+check_symbol_exists(strndup "string.h" HAVE_STRNDUP)
+check_symbol_exists(utimes "sys/time.h" HAVE_UTIMES)
+
 php_check_function_attribute(visibility HAVE_VISIBILITY)
 
 target_compile_definitions(
