FindPkgConfig: Fix version and prefix with freedesktop.org pkg-config

pkg_check_modules() queries --modversion and --variable=prefix in one
pkg-config call. pkgconf prints both, but freedesktop.org pkg-config
prints only the version and ignores --variable. As a result, <prefix>_PREFIX
was empty, and a module with an empty "Version:" failed with
"list GET given empty list".

Use the prefix from the combined output only when pkg-config printed it,
and otherwise query it separately. While splitting the combined output
into lines, escape semicolons so that a prefix containing one is not
split into a list.

Fixes: #28194

Modules/FindPkgConfig.cmake

@@ -586,6 +586,18 @@ macro(_pkgconfig_set var value)
   set(${var} ${value} CACHE INTERNAL "")
 endmacro()
 
+# pkg-config <0.29.1 and pkgconf <1.5.1 leave surrounding quotes on variables
+macro(_pkgconfig_unquote _var)
+  if((PkgConfig_VERSION VERSION_LESS 0.29.1) OR
+      (PkgConfig_VERSION VERSION_GREATER_EQUAL 1.0 AND PkgConfig_VERSION VERSION_LESS 1.5.1))
+    if(${_var} MATCHES "^\"(.*)\"$")
+      set(${_var} "${CMAKE_MATCH_1}")
+    elseif(${_var} MATCHES "^'(.*)'$")
+      set(${_var} "${CMAKE_MATCH_1}")
+    endif()
+  endif()
+endmacro()
+
 # Invokes pkgconfig, cleans up the result and sets variables
 macro(_pkgconfig_invoke _pkglist _prefix _varname _regexp)
   set(_pkgconfig_invoke_result)
@@ -606,16 +618,7 @@ macro(_pkgconfig_invoke _pkglist _prefix _varname _regexp)
       string(REGEX REPLACE "${_regexp}" " " _pkgconfig_invoke_result "${_pkgconfig_invoke_result}")
     endif()
 
-    # pkg-config <0.29.1 and pkgconf <1.5.1 prints quoted variables without unquoting
-    # unquote only if quotes are first and last characters
-    if((PkgConfig_VERSION VERSION_LESS 0.29.1) OR
-        (PkgConfig_VERSION VERSION_GREATER_EQUAL 1.0 AND PkgConfig_VERSION VERSION_LESS 1.5.1))
-      if (_pkgconfig_invoke_result MATCHES "^\"(.*)\"$")
-        set(_pkgconfig_invoke_result "${CMAKE_MATCH_1}")
-      elseif(_pkgconfig_invoke_result MATCHES "^'(.*)'$")
-        set(_pkgconfig_invoke_result "${CMAKE_MATCH_1}")
-      endif()
-    endif()
+    _pkgconfig_unquote(_pkgconfig_invoke_result)
 
     # pkg-config can represent "spaces within an argument" by backslash-escaping the space.
     # UNIX_COMMAND mode treats backslash-escaped spaces as "not a space that delimits arguments".
@@ -1010,15 +1013,7 @@ endfunction()
 function(_pkgconfig_parse_flags _prefix _flags_output _is_libs _static)
   string(REGEX REPLACE "[\r\n]" " " _pkgconfig_pf_output "${_flags_output}")
 
-  # Unquote if needed (same logic as _pkgconfig_invoke)
-  if((PkgConfig_VERSION VERSION_LESS 0.29.1) OR
-      (PkgConfig_VERSION VERSION_GREATER_EQUAL 1.0 AND PkgConfig_VERSION VERSION_LESS 1.5.1))
-    if(_pkgconfig_pf_output MATCHES "^\"(.*)\"$")
-      set(_pkgconfig_pf_output "${CMAKE_MATCH_1}")
-    elseif(_pkgconfig_pf_output MATCHES "^'(.*)'$")
-      set(_pkgconfig_pf_output "${CMAKE_MATCH_1}")
-    endif()
-  endif()
+  _pkgconfig_unquote(_pkgconfig_pf_output)
 
   separate_arguments(_pkgconfig_pf_list UNIX_COMMAND "${_pkgconfig_pf_output}")
 
@@ -1205,10 +1200,10 @@ macro(_pkg_check_modules_internal _is_required _is_silent _no_cmake_path _no_cma
       _pkgconfig_unset(${_prefix}_${_pkg_check_modules_pkg_name}_INCLUDEDIR)
       _pkgconfig_unset(${_prefix}_${_pkg_check_modules_pkg_name}_LIBDIR)
 
-      # --modversion validates any version constraint and prints the module
-      # version. We can't combine with --variable, since FDO pkg-config treats
-      # these as incompatible output options.
-      list(APPEND _pkg_check_modules_exist_query --print-errors --short-errors --modversion)
+      # --modversion validates any version constraint and prints the module version.
+      # pkgconf also honors --variable=prefix and prints the prefix
+      # on a second line, but freedesktop.org pkg-config ignores it
+      list(APPEND _pkg_check_modules_exist_query --print-errors --short-errors --modversion --variable=prefix)
       if (_pkg_check_modules_pkg_op)
         list(APPEND _pkg_check_modules_exist_query "${_pkg_check_modules_pkg_name} ${_pkg_check_modules_pkg_op} ${_pkg_check_modules_pkg_ver}")
       else()
@@ -1232,6 +1227,23 @@ macro(_pkg_check_modules_internal _is_required _is_silent _no_cmake_path _no_cma
 
         string(APPEND _pkg_check_modules_failed " - ${_pkg_check_modules_pkg}\n")
       else()
+        # An empty output means no version or prefix was printed. Otherwise,
+        # a second line contains the prefix when --variable=prefix was honored
+        string(REPLACE ";" "\\;" _pkgconfig_combined_list "${_pkgconfig_module_version}")
+        string(REGEX REPLACE "\n" ";" _pkgconfig_combined_list "${_pkgconfig_combined_list}")
+        list(LENGTH _pkgconfig_combined_list _pkgconfig_combined_len)
+        if(_pkgconfig_combined_len GREATER 0)
+          list(GET _pkgconfig_combined_list 0 _pkgconfig_module_version)
+        endif()
+        unset(_pkgconfig_${_pkg_check_modules_pkg_name}_PREFIX)
+        if(_pkgconfig_combined_len GREATER 1)
+          list(GET _pkgconfig_combined_list 1 _pkgconfig_module_prefix)
+          _pkgconfig_unquote(_pkgconfig_module_prefix)
+          separate_arguments(_pkgconfig_module_prefix UNIX_COMMAND "${_pkgconfig_module_prefix}")
+          # Keep a literal semicolon from becoming a backslash escape in the cache value
+          string(REPLACE "\\;" ";" _pkgconfig_module_prefix "${_pkgconfig_module_prefix}")
+          set(_pkgconfig_${_pkg_check_modules_pkg_name}_PREFIX "${_pkgconfig_module_prefix}")
+        endif()
         list(APPEND _pkg_check_modules_packages "${_pkg_check_modules_pkg_name}")
         set(_pkgconfig_${_pkg_check_modules_pkg_name}_VERSION "${_pkgconfig_module_version}")
       endif()
@@ -1259,7 +1271,11 @@ macro(_pkg_check_modules_internal _is_required _is_silent _no_cmake_path _no_cma
         endif()
 
         _pkgconfig_set(${_pkg_check_prefix}_VERSION "${_pkgconfig_${_pkg_check_modules_pkg}_VERSION}")
-        pkg_get_variable("${_pkg_check_prefix}_PREFIX" ${_pkg_check_modules_pkg} "prefix")
+        if(DEFINED _pkgconfig_${_pkg_check_modules_pkg}_PREFIX)
+          set("${_pkg_check_prefix}_PREFIX" "${_pkgconfig_${_pkg_check_modules_pkg}_PREFIX}")
+        else()
+          pkg_get_variable("${_pkg_check_prefix}_PREFIX" ${_pkg_check_modules_pkg} "prefix")
+        endif()
         pkg_get_variable("${_pkg_check_prefix}_INCLUDEDIR" ${_pkg_check_modules_pkg} "includedir")
         pkg_get_variable("${_pkg_check_prefix}_LIBDIR" ${_pkg_check_modules_pkg} "libdir")
         foreach (variable IN ITEMS PREFIX INCLUDEDIR LIBDIR)

Tests/RunCMake/FindPkgConfig/FindPkgConfig_MODVERSION_VARIABLE.cmake

@@ -0,0 +1,59 @@
+set(PKG_CONFIG_EXECUTABLE "${CMAKE_COMMAND}")
+set(PKG_CONFIG_ARGN "-P;${CMAKE_CURRENT_SOURCE_DIR}/pkg-config-static-requery.cmake;--")
+set(ENV{FAKE_PKG_CONFIG_FLAVOR} "${flavor}")
+find_package(PkgConfig REQUIRED)
+
+set(log "${CMAKE_CURRENT_BINARY_DIR}/pkg-config.log")
+set(ENV{FAKE_PKG_CONFIG_LOG} "${log}")
+
+function(check var expect)
+  if(NOT "${${var}}" STREQUAL "${expect}")
+    message(SEND_ERROR "Expected ${var} to be \"${expect}\", got \"${${var}}\"")
+  endif()
+endfunction()
+
+# Check whether the prefix was queried separately from --modversion
+function(check_prefix_query expect)
+  file(STRINGS "${log}" calls REGEX " --variable=prefix")
+  list(FILTER calls EXCLUDE REGEX " --modversion")
+  list(LENGTH calls count)
+  if(NOT count EQUAL expect)
+    message(SEND_ERROR "Expected ${expect} separate prefix queries, got ${count}")
+  endif()
+  file(REMOVE "${log}")
+endfunction()
+
+if(flavor STREQUAL "pkgconf")
+  set(separate 0)
+else()
+  set(separate 1)
+endif()
+
+pkg_check_modules(NORMAL REQUIRED normal)
+check(NORMAL_FOUND 1)
+check(NORMAL_VERSION "1.2.3")
+check(NORMAL_PREFIX "/fake/normal")
+check(NORMAL_INCLUDEDIR "/fake/normal/include")
+check(NORMAL_LIBDIR "/fake/normal/lib")
+check(NORMAL_LIBRARIES "normal")
+check_prefix_query(${separate})
+
+pkg_check_modules(CONSTRAINT REQUIRED "normal>=1.0")
+check(CONSTRAINT_VERSION "1.2.3")
+check(CONSTRAINT_PREFIX "/fake/normal")
+check_prefix_query(${separate})
+
+pkg_check_modules(EMPTY REQUIRED emptyversion)
+check(EMPTY_FOUND 1)
+check(EMPTY_VERSION "")
+check(EMPTY_PREFIX "/fake/emptyversion")
+check(EMPTY_LIBRARIES "emptyversion")
+check_prefix_query(${separate})
+
+pkg_check_modules(BOTH REQUIRED normal emptyversion)
+check(BOTH_normal_VERSION "1.2.3")
+check(BOTH_normal_PREFIX "/fake/normal")
+check(BOTH_emptyversion_VERSION "")
+check(BOTH_emptyversion_PREFIX "/fake/emptyversion")
+math(EXPR separate_both "${separate} * 2")
+check_prefix_query(${separate_both})

Tests/RunCMake/FindPkgConfig/FindPkgConfig_STATIC_REQUERY.cmake

@@ -1,11 +1,11 @@
-if(WIN32)
-  set(PKG_CONFIG_EXECUTABLE "${CMAKE_CURRENT_SOURCE_DIR}/pkg-config-static-requery.bat")
-else()
-  set(PKG_CONFIG_EXECUTABLE "${CMAKE_CURRENT_SOURCE_DIR}/pkg-config-static-requery.sh")
-endif()
+set(PKG_CONFIG_EXECUTABLE "${CMAKE_COMMAND}")
+set(PKG_CONFIG_ARGN "-P;${CMAKE_CURRENT_SOURCE_DIR}/pkg-config-static-requery.cmake;--")
 find_package(PkgConfig REQUIRED)
 
 pkg_check_modules(P REQUIRED good)
+if(NOT P_PREFIX STREQUAL "/fake;prefix")
+  message(FATAL_ERROR "Expected semicolon in prefix, got: ${P_PREFIX}")
+endif()
 if(NOT P_STATIC_LIBRARIES STREQUAL "good;dep")
   message(FATAL_ERROR
     "Expected static libraries from first query, got: ${P_STATIC_LIBRARIES}")

Tests/RunCMake/FindPkgConfig/RunCMakeTest.cmake

@@ -19,6 +19,12 @@ if(NOT WIN32)
 endif()
 
 run_cmake(FindPkgConfig_extract_frameworks)
+foreach(flavor IN ITEMS pkg-config pkgconf)
+  set(RunCMake_TEST_VARIANT_DESCRIPTION "-${flavor}")
+  run_cmake_with_options(FindPkgConfig_MODVERSION_VARIABLE -Dflavor=${flavor})
+endforeach()
+unset(RunCMake_TEST_VARIANT_DESCRIPTION)
+run_cmake(FindPkgConfig_STATIC_REQUERY)
 
 if(APPLE)
   run_cmake(FindPkgConfig_extract_frameworks_target)
@@ -57,7 +63,6 @@ Libs: -L\${libdir}
   run_cmake(FindPkgConfig_GET_VARIABLE_MULTIPLE_VALUES)
   run_cmake(FindPkgConfig_cache_variables)
   run_cmake(FindPkgConfig_IMPORTED_TARGET)
-  run_cmake(FindPkgConfig_STATIC_REQUERY)
   run_cmake(FindPkgConfig_VERSION_OPERATORS)
   run_cmake(FindPkgConfig_GET_MATCHING_MODULE_NAME)
   run_cmake(FindPkgConfig_empty_target)

Tests/RunCMake/FindPkgConfig/pkg-config-static-requery.bat

@@ -1,99 +0,0 @@
-@ECHO OFF
-
-set static=false
-set mode=
-set pkg=
-set variable=
-
-:LOOP
-IF "%~1"=="" GOTO RUN
-
-IF "%~1"=="--version" (
-  ECHO 1.0
-  EXIT /B 0
-)
-IF "%~1"=="--static" (
-  set static=true
-  SHIFT
-  GOTO LOOP
-)
-IF "%~1"=="--modversion" (
-  set mode=modversion
-  SHIFT
-  GOTO LOOP
-)
-IF "%~1"=="--cflags" (
-  set mode=cflags
-  SHIFT
-  GOTO LOOP
-)
-IF "%~1"=="--libs" (
-  set mode=libs
-  SHIFT
-  GOTO LOOP
-)
-IF "%~1"=="--print-errors" (
-  SHIFT
-  GOTO LOOP
-)
-IF "%~1"=="--short-errors" (
-  SHIFT
-  GOTO LOOP
-)
-
-set arg=%~1
-IF "%arg:~0,11%"=="--variable=" (
-  set mode=variable
-  set variable=%arg:~11%
-) ELSE (
-  set pkg=%~1
-)
-SHIFT
-GOTO LOOP
-
-:RUN
-IF "%mode%"=="modversion" (
-  IF "%pkg%"=="good" GOTO VERSION
-  IF "%pkg%"=="bad" GOTO VERSION
-)
-IF "%mode%"=="variable" (
-  IF "%pkg%"=="good" GOTO VARIABLE
-  IF "%pkg%"=="bad" GOTO VARIABLE
-)
-IF "%mode%"=="cflags" (
-  IF "%pkg%"=="good" GOTO CFLAGS
-  IF "%pkg%"=="bad" GOTO CFLAGS
-)
-IF "%mode%"=="libs" (
-  IF "%pkg%"=="good" GOTO GOOD_LIBS
-  IF "%pkg%"=="bad" GOTO BAD_LIBS
-)
-EXIT /B 1
-
-:VERSION
-ECHO 1.0
-EXIT /B 0
-
-:VARIABLE
-ECHO /fake-%variable%
-EXIT /B 0
-
-:CFLAGS
-ECHO -I/fake-include
-EXIT /B 0
-
-:GOOD_LIBS
-IF "%static%"=="true" (
-  ECHO -lgood -ldep
-) ELSE (
-  ECHO -lgood
-)
-EXIT /B 0
-
-:BAD_LIBS
-IF "%static%"=="true" (
-  EXIT /B 1
-) ELSE (
-  ECHO -lbad
-)
-EXIT /B 0

Tests/RunCMake/FindPkgConfig/pkg-config-static-requery.cmake

@@ -0,0 +1,113 @@
+# Emulate pkg-config for the static requery and modversion/variable tests.
+# CMAKE_ARGV0..3 are: cmake -P <this file> --
+
+macro(print_stdout value)
+  execute_process(COMMAND "${CMAKE_COMMAND}" -E echo "${value}")
+endmacro()
+
+set(static FALSE)
+set(mode "")
+set(pkg "")
+set(variable "")
+set(call "")
+
+math(EXPR last_arg "${CMAKE_ARGC} - 1")
+foreach(i RANGE 4 ${last_arg})
+  set(arg "${CMAKE_ARGV${i}}")
+  string(APPEND call " ${arg}")
+
+  if(arg STREQUAL "--version")
+    set(mode version)
+  elseif(arg STREQUAL "--static")
+    set(static TRUE)
+  elseif(arg STREQUAL "--modversion")
+    set(mode modversion)
+  elseif(arg MATCHES "^--variable=(.*)$")
+    if(NOT mode STREQUAL "modversion")
+      set(mode variable)
+    endif()
+    set(variable "${CMAKE_MATCH_1}")
+  elseif(arg STREQUAL "--cflags")
+    set(mode cflags)
+  elseif(arg STREQUAL "--libs")
+    set(mode libs)
+  elseif(NOT arg MATCHES "^--(print-errors|short-errors)$")
+    string(REGEX REPLACE " .*" "" pkg "${arg}")
+  endif()
+endforeach()
+
+if(NOT "$ENV{FAKE_PKG_CONFIG_LOG}" STREQUAL "")
+  file(APPEND "$ENV{FAKE_PKG_CONFIG_LOG}" "${call}\n")
+endif()
+
+if(mode STREQUAL "version")
+  if("$ENV{FAKE_PKG_CONFIG_FLAVOR}" STREQUAL "pkg-config")
+    print_stdout("0.29.2")
+  elseif("$ENV{FAKE_PKG_CONFIG_FLAVOR}" STREQUAL "pkgconf")
+    print_stdout("2.1.0")
+  else()
+    print_stdout("1.0")
+  endif()
+  return()
+endif()
+
+if(pkg STREQUAL "good" OR pkg STREQUAL "bad")
+  set(version "1.0")
+  set(prefix "/fake;prefix")
+elseif(pkg STREQUAL "normal")
+  set(version "1.2.3")
+  set(prefix "/fake/normal")
+elseif(pkg STREQUAL "emptyversion")
+  set(version "")
+  set(prefix "/fake/emptyversion")
+else()
+  cmake_language(EXIT 1)
+endif()
+
+if(mode STREQUAL "modversion")
+  print_stdout("${version}")
+  if(NOT variable STREQUAL "")
+    if("$ENV{FAKE_PKG_CONFIG_FLAVOR}" STREQUAL "pkg-config")
+      message("Ignoring incompatible output option \"--variable\"")
+    else()
+      print_stdout("${prefix}")
+    endif()
+  endif()
+  return()
+elseif(mode STREQUAL "variable")
+  if(variable STREQUAL "prefix")
+    print_stdout("${prefix}")
+  elseif(pkg STREQUAL "good" OR pkg STREQUAL "bad")
+    print_stdout("/fake-${variable}")
+  elseif(variable STREQUAL "includedir")
+    print_stdout("/fake/${pkg}/include")
+  elseif(variable STREQUAL "libdir")
+    print_stdout("/fake/${pkg}/lib")
+  endif()
+  return()
+elseif(mode STREQUAL "cflags")
+  if(pkg STREQUAL "good" OR pkg STREQUAL "bad")
+    print_stdout("-I/fake-include")
+  else()
+    print_stdout("-I/fake/${pkg}/include")
+  endif()
+  return()
+elseif(mode STREQUAL "libs")
+  if(pkg STREQUAL "good")
+    if(static)
+      print_stdout("-lgood -ldep")
+    else()
+      print_stdout("-lgood")
+    endif()
+  elseif(pkg STREQUAL "bad")
+    if(static)
+      cmake_language(EXIT 1)
+    endif()
+    print_stdout("-lbad")
+  else()
+    print_stdout("-L/fake/${pkg}/lib -l${pkg}")
+  endif()
+  return()
+else()
+  cmake_language(EXIT 1)
+endif()

Tests/RunCMake/FindPkgConfig/pkg-config-static-requery.sh

@@ -1,64 +0,0 @@
-#!/bin/sh
-
-static=false
-mode=
-pkg=
-
-for arg in "$@"; do
-  case "$arg" in
-    --version)
-      echo "1.0"
-      exit 0
-      ;;
-    --static)
-      static=true
-      ;;
-    --modversion)
-      mode=modversion
-      ;;
-    --variable=*)
-      mode=variable
-      variable="${arg#--variable=}"
-      ;;
-    --cflags)
-      mode=cflags
-      ;;
-    --libs)
-      mode=libs
-      ;;
-    --print-errors|--short-errors)
-      ;;
-    *)
-      pkg="$arg"
-      ;;
-  esac
-done
-
-case "$mode:$pkg" in
-  modversion:good|modversion:bad)
-    echo "1.0"
-    ;;
-  variable:good|variable:bad)
-    echo "/fake-$variable"
-    ;;
-  cflags:good|cflags:bad)
-    echo "-I/fake-include"
-    ;;
-  libs:good)
-    if $static; then
-      echo "-lgood -ldep"
-    else
-      echo "-lgood"
-    fi
-    ;;
-  libs:bad)
-    if $static; then
-      exit 1
-    else
-      echo "-lbad"
-    fi
-    ;;
-  *)
-    exit 1
-    ;;
-esac