Improve ODBC driver selection

petk · petk · commit c40c821cd66a · 2024-11-09T17:47:09.000+01:00
- MySQL_CONFIG_EXECUTABLE marked as advanced to not show in regular list
  of options in GUI
- PHP/Set module adjusted to have choices validated by default
- Fixed odbc extension build as these are still STATIC libraries for
  simplicity of usage but should probably be OBJECT otherwise PHP
  doesn't provide some symbols when linking.
diff --git a/cmake/cmake/modules/FindMySQL.cmake b/cmake/cmake/modules/FindMySQL.cmake
@@ -55,6 +55,7 @@ find_program(
   NAMES mysql_config
   DOC "The mysql_config command-line tool for getting MySQL installation info"
 )
+mark_as_advanced(MySQL_CONFIG_EXECUTABLE)
 
 # Find the Socket component.
 if("Socket" IN_LIST MySQL_FIND_COMPONENTS)
diff --git a/cmake/cmake/modules/PHP/Set.cmake b/cmake/cmake/modules/PHP/Set.cmake
@@ -31,9 +31,10 @@ php_set(
   <variable>
   TYPE <type>
   [CHOICES <string>...]
+  [CHOICES_OPTIONAL]
+  [CHOICES_CASE_SENSITIVE]
   [IF <condition> VALUE <value> [ELSE_VALUE <default>]] | [VALUE <value>]
   DOC <docstring>...
-  [WARNING <warning>]
 )
 ```
 
@@ -45,6 +46,52 @@ It sets a CACHE `<variable>` of `<type>` to a `<value>`.
   a list of supported options to pick in the GUI. Under the hood, it sets the
   `STRINGS` CACHE variable property.
 
+  * When `CHOICES_OPTIONAL` is used, variable value will not be validated to
+    match one of the list items. By default when using `CHOICES` variable value
+    must match one of the list items otherwise a fatal error is thrown.
+
+  * When `CHOICES_CASE_SENSITIVE` is used, variable value passed by the user
+    will need to be of the same case as defined in the `CHOICES` list. By
+    default, choices are case insensitive.
+
+    For example:
+
+    ```cmake
+    php_set(
+      VAR
+      TYPE STRING
+      CHOICES auto unixODBC iODBC
+      IF TRUE
+      VALUE auto
+      DOC "Variable with a case insensitive list of choices"
+    )
+    message(STATUS "VAR=${VAR}")
+    ```
+
+    ```sh
+    cmake -S <source-dir> -B <build-dir> -DVAR=unixodbc
+    ```
+
+    Will output `VAR=unixODBC` and not `VAR=unixodbc`.
+
+    With `CHOICES_CASE_SENSITIVE`:
+
+    ```cmake
+    php_set(
+      VAR
+      TYPE STRING
+      CHOICES auto unixODBC iODBC
+      CHOICES_CASE_SENSITIVE
+      IF TRUE
+      VALUE auto
+      DOC "Variable with a case sensitive list of choices"
+    )
+    message(STATUS "VAR=${VAR}")
+    ```
+
+    A fatal error will be thrown, if VAR is set to a case-sensitive value
+    `unixodbc`, which is not defined in the `CHOICES` list.
+
 * `VALUE` is the default variable value. There are two ways to set default
   value.
 
@@ -68,10 +115,6 @@ 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)
@@ -80,10 +123,10 @@ function(php_set)
   cmake_parse_arguments(
     PARSE_ARGV
     1
-    parsed                     # prefix
-    ""                         # options
+    parsed # prefix
+    "CHOICES_OPTIONAL;CHOICES_CASE_SENSITIVE" # options
     "TYPE;IF;VALUE;ELSE_VALUE" # one-value keywords
-    "CHOICES;DOC;WARNING"      # multi-value keywords
+    "CHOICES;DOC" # multi-value keywords
   )
 
   # The cmake_parse_arguments() before 3.31 didn't define one-value keywords
@@ -155,11 +198,17 @@ function(php_set)
     set(${varName} "${${bufferVarName}}" CACHE ${parsed_TYPE} "${doc}" FORCE)
     if(parsed_TYPE STREQUAL "STRING" AND parsed_CHOICES)
       set_property(CACHE ${varName} PROPERTY STRINGS ${parsed_CHOICES})
+      if(NOT parsed_CHOICES_CASE_SENSITIVE)
+        _php_set_fix_value(${varName})
+      endif()
+      if(NOT parsed_CHOICES_OPTIONAL)
+        _php_set_validate_choices(${varName} ${parsed_CHOICES_CASE_SENSITIVE})
+      endif()
     endif()
     unset(${bufferVarName} CACHE)
     unset(${bufferVarName}_OVERRIDDEN CACHE)
   else()
-    _php_set_validate_input()
+    _php_set_validate_input(${varName})
 
     if(DEFINED parsed_ELSE_VALUE)
       set(${varName} "${parsed_ELSE_VALUE}" CACHE INTERNAL "${doc}" FORCE)
@@ -207,41 +256,78 @@ function(_php_set_validate_arguments arguments)
   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)
+# Validate variable and output warning when a conditional variable is set by the
+# user (on command line or CMake presets) and the condition is not met.
+# This is for diagnostics purpose for user to be aware that some configuration
+# value was not taken into account.
+function(_php_set_validate_input var)
+  get_property(helpString CACHE ${var} 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"
-    )
+  if(${var} STREQUAL "${parsed_ELSE_VALUE}")
+    return()
   endif()
-  set(warning "")
-  foreach(string ${parsed_WARNING})
-    string(APPEND warning "${string}")
+
+  set(warning "Variable ${var}")
+  if(DEFINED parsed_ELSE_VALUE)
+    string(APPEND warning " has been overridden (${var}=${parsed_ELSE_VALUE})")
+  else()
+    string(APPEND warning " has been undefined")
+  endif()
+  string(
+    APPEND
+    warning
+    " as it depends on the condition:\n"
+    "${parsed_IF}\n"
+    "You can probably remove the ${var} configuration value from the build "
+    "command as it won't be utilized."
+  )
+
+  message(WARNING "${warning}")
+endfunction()
+
+# Reset the value passed by the user according to the case sensitivity given in
+# the choices list.
+function(_php_set_fix_value var)
+  get_property(value CACHE ${var} PROPERTY VALUE)
+  get_property(choices CACHE ${var} PROPERTY STRINGS)
+
+  string(TOLOWER "${value}" valueLower)
+  list(TRANSFORM choices TOLOWER OUTPUT_VARIABLE choicesLower)
+
+  set(index 0)
+  foreach(item IN LISTS choicesLower)
+    if(valueLower STREQUAL "${item}")
+      list(GET choices ${index} itemOriginal)
+      if(NOT value STREQUAL "${itemOriginal}")
+        set_property(CACHE ${var} PROPERTY VALUE ${itemOriginal})
+        break()
+      endif()
+    endif()
+    math(EXPR index "${index}+1")
   endforeach()
+endfunction()
+
+# When CHOICES argument is set, validate variable value to match one of the
+# choices.
+function(_php_set_validate_choices var caseSensitive)
+  get_property(value CACHE ${var} PROPERTY VALUE)
+  get_property(choices CACHE ${var} PROPERTY STRINGS)
 
-  if(NOT ${varName} STREQUAL "${parsed_ELSE_VALUE}")
-    message(WARNING "${warning}")
+  if(NOT caseSensitive)
+    string(TOLOWER "${value}" value)
+    list(TRANSFORM choices TOLOWER)
+  endif()
+
+  if(NOT value IN_LIST choices)
+    list(JOIN choices ", " choices)
+
+    message(
+      FATAL_ERROR
+      "Unknown value: ${var}=${value}\n"
+      "Please select one of: ${choices}."
+    )
   endif()
 endfunction()
diff --git a/cmake/ext/odbc/CMakeLists.txt b/cmake/ext/odbc/CMakeLists.txt
@@ -22,66 +22,55 @@ cmake_dependent_option(
   OFF
 )
 
-block()
-  set(
-    types
-      "auto"
-      "adabas"
-      "dbmaker"
-      "empress-bcs"
-      "empress"
-      "esoob"
-      "ibm-db2"
-      "iODBC"
-      "sapdb"
-      "solid"
-      "unixODBC"
-      "custom"
-  )
+php_set(
+  EXT_ODBC_TYPE
+  TYPE STRING
+  CHOICES
+    "auto"
+    "adabas"
+    "dbmaker"
+    "empress-bcs"
+    "empress"
+    "esoob"
+    "ibm-db2"
+    "iODBC"
+    "sapdb"
+    "solid"
+    "unixODBC"
+    "custom"
+  IF EXT_ODBC
+  VALUE auto
+  DOC "The ODBC type (library driving the ODBC)"
+)
 
-  php_set(
-    EXT_ODBC_TYPE
-    TYPE STRING
-    CHOICES ${types}
-    IF EXT_ODBC
-    VALUE auto
-    DOC "The ODBC type (library driving the ODBC)"
+# Validate ODBC type for odbc and pdo_odbc extensions. Ideally, the same ODBC
+# driver library should be used for both extensions.
+if(EXT_ODBC AND EXT_PDO_ODBC AND NOT EXT_ODBC_TYPE STREQUAL EXT_PDO_ODBC_TYPE)
+  message(
+    FATAL_ERROR
+    "Different ODBC types detected for 'odbc' and 'pdo_odbc' PHP "
+    "extensions:\n"
+    "EXT_ODBC_TYPE=${EXT_ODBC_TYPE}\n"
+    "EXT_PDO_ODBC_TYPE=${EXT_PDO_ODBC_TYPE}\n"
+    "To prevent library symbol collisions when building both 'odbc' and "
+    "'pdo_odbc' please link with the same ODBC library. Or disable one of "
+    "the extensions by setting EXT_ODBC or EXT_PDO_ODBC to 'OFF'."
   )
+endif()
 
-  if(EXT_ODBC_TYPE AND NOT EXT_ODBC_TYPE IN_LIST types)
-    list(JOIN types ", " typesString)
-
-    message(
-      FATAL_ERROR
-      "Unknown ODBC type value EXT_ODBC_TYPE=${EXT_ODBC_TYPE}. Please "
-      "select one of: ${typeString}."
-    )
-  endif()
-
-  # Validate ODBC type.
-  if(EXT_ODBC AND EXT_PDO_ODBC AND NOT EXT_ODBC_TYPE STREQUAL EXT_PDO_ODBC_TYPE)
-    message(
-      FATAL_ERROR
-      "Different ODBC types detected for 'odbc' and 'pdo_odbc' PHP "
-      "extensions:\n"
-      "EXT_ODBC_TYPE=${EXT_ODBC_TYPE}\n"
-      "EXT_PDO_ODBC_TYPE=${EXT_PDO_ODBC_TYPE}\n"
-      "To prevent library symbol collisions when building both 'odbc' and "
-      "'pdo_odbc' please link with the same ODBC library. Or disable one of "
-      "the extensions by setting EXT_ODBC or EXT_PDO_ODBC to 'OFF'."
-    )
-  endif()
-
-  # Some ODBC drivers require setting ODBC_LIBRARY manually to find package.
-  if(NOT EXT_ODBC_TYPE MATCHES "auto|unixODBC|iODBC" AND NOT ODBC_LIBRARY)
-    message(
-      FATAL_ERROR
-      "When using EXT_ODBC_TYPE=${EXT_ODBC_TYPE}, please set also "
-      "ODBC_LIBRARY and optionally ODBC_INCLUDE_DIR to locate the ODBC "
-      "installation."
-    )
-  endif()
-endblock()
+# Some ODBC drivers require setting ODBC_LIBRARY manually to find package.
+if(
+  EXT_ODBC
+  AND NOT EXT_ODBC_TYPE MATCHES "auto|unixODBC|iODBC"
+  AND NOT ODBC_LIBRARY
+)
+  message(
+    FATAL_ERROR
+    "When using EXT_ODBC_TYPE=${EXT_ODBC_TYPE}, please set also "
+    "ODBC_LIBRARY and optionally ODBC_INCLUDE_DIR to locate the ODBC "
+    "installation."
+  )
+endif()
 
 php_set(
   EXT_ODBC_VERSION
@@ -128,6 +117,12 @@ target_sources(
 
 target_compile_definitions(php_odbc PRIVATE ZEND_ENABLE_STATIC_TSRMLS_CACHE=1)
 
+# TODO: Can this be fixed better?
+get_target_property(extension_type php_odbc TYPE)
+if(NOT extension_type MATCHES "^(MODULE|SHARED)_LIBRARY$")
+  target_link_libraries(php_odbc PRIVATE PHP::main)
+endif()
+
 if(NOT EXT_ODBC_TYPE STREQUAL "auto")
   set(ODBC_USE_DRIVER "${EXT_ODBC_TYPE}")
 endif()
diff --git a/cmake/ext/pdo_odbc/CMakeLists.txt b/cmake/ext/pdo_odbc/CMakeLists.txt

Original file line number	Diff line number	Diff line change
`@@ -55,6 +55,7 @@ find_program(`
`55`	`55`	`NAMES mysql_config`
`56`	`56`	`DOC "The mysql_config command-line tool for getting MySQL installation info"`
`57`	`57`	`)`
	`58`	`+mark_as_advanced(MySQL_CONFIG_EXECUTABLE)`
`58`	`59`
`59`	`60`	`# Find the Socket component.`
`60`	`61`	`if("Socket" IN_LIST MySQL_FIND_COMPONENTS)`