[scripts-audit] vcpkg_fixup_pkgconfig (microsoft#19658)

* [scripts-audit] vcpkg_fixup_pkgconfig

* Neumann-A CR, fix docs

* vcpkg_fixup_pkgconfig bugs

* fix group

* moar fixing

* be more clever around pkg_config_path

* add `vcpkg_host_path_list` so that we can unit test

* move stuff around a bit

* fix bug in vcpkg_host_path_list.cmake

* ras0219 CRs

docs/maintainers/portfile-functions.md

@@ -44,6 +44,7 @@
 - [vcpkg\_from\_sourceforge](vcpkg_from_sourceforge.md)
 - [vcpkg\_get\_program\_files\_platform\_bitness](vcpkg_get_program_files_platform_bitness.md)
 - [vcpkg\_get\_windows\_sdk](vcpkg_get_windows_sdk.md)
+- [vcpkg\_host\_path\_list](vcpkg_host_path_list.md)
 - [vcpkg\_install\_cmake](vcpkg_install_cmake.md) (deprecated, use [vcpkg\_cmake\_install](ports/vcpkg-cmake/vcpkg_cmake_install.md))
 - [vcpkg\_install\_gn](vcpkg_install_gn.md)
 - [vcpkg\_install\_make](vcpkg_install_make.md)

docs/maintainers/vcpkg_host_path_list.md

@@ -0,0 +1,26 @@
+# vcpkg_host_path_list
+
+The latest version of this document lives in the [vcpkg repo](https://github.com/Microsoft/vcpkg/blob/master/docs/maintainers/vcpkg_host_path_list.md).
+
+Modify a host path list variable (PATH, INCLUDE, LIBPATH, etc.)
+
+```cmake
+vcpkg_host_path_list(PREPEND <list-var> [<path>...])
+vcpkg_host_path_list(APPEND <list-var> [<path>...])
+```
+
+`<list-var>` may be either a regular variable name, or `ENV{variable-name}`,
+in which case `vcpkg_host_path_list` will modify the environment.
+
+`vcpkg_host_path_list` adds all of the paths passed to it to `<list-var>`;
+`PREPEND` puts them before the existing list, so that they are searched first;
+`APPEND` places them after the existing list,
+so they would be searched after the paths which are already in the variable.
+
+For both `APPEND` and `PREPEND`,
+the paths are added (and thus searched) in the order received.
+
+If no paths are passed, then nothing will be done.
+
+## Source
+[scripts/cmake/vcpkg\_host\_path\_list.cmake](https://github.com/Microsoft/vcpkg/blob/master/scripts/cmake/vcpkg_host_path_list.cmake)

scripts/cmake/vcpkg_add_to_path.cmake

@@ -24,14 +24,11 @@ If no paths are passed, then nothing will be done.
 #]===]
 function(vcpkg_add_to_path)
     cmake_parse_arguments(PARSE_ARGV 0 "arg" "PREPEND" "" "")
-    if(NOT DEFINED arg_UNPARSED_ARGUMENTS)
-        return()
-    endif()
-
-    list(JOIN arg_UNPARSED_ARGUMENTS "${VCPKG_HOST_PATH_SEPARATOR}" add_to_path)
     if(arg_PREPEND)
-        set(ENV{PATH} "${add_to_path}${VCPKG_HOST_PATH_SEPARATOR}$ENV{PATH}")
+        set(operation PREPEND)
     else()
-        set(ENV{PATH} "$ENV{PATH}${VCPKG_HOST_PATH_SEPARATOR}${add_to_path}")
+        set(operation APPEND)
     endif()
+
+    vcpkg_host_path_list("${operation}" ENV{PATH} ${arg_UNPARSED_ARGUMENTS})
 endfunction()

scripts/cmake/vcpkg_fixup_pkgconfig.cmake

@@ -45,154 +45,166 @@ Still work in progress. If there are more cases which can be handled here feel f
 * [brotli](https://github.com/Microsoft/vcpkg/blob/master/ports/brotli/portfile.cmake)
 #]===]
 
-function(vcpkg_fixup_pkgconfig_check_files pkg_cfg_cmd _file _config)
-    set(PATH_SUFFIX_DEBUG /debug)
-    set(PATH_SUFFIX_RELEASE)
-    set(PKGCONFIG_INSTALLED_DIR "${CURRENT_INSTALLED_DIR}${PATH_SUFFIX_${_config}}/lib/pkgconfig")
-    set(PKGCONFIG_INSTALLED_SHARE_DIR "${CURRENT_INSTALLED_DIR}/share/pkgconfig")
-    set(PKGCONFIG_PACKAGES_DIR "${CURRENT_PACKAGES_DIR}${PATH_SUFFIX_${_config}}/lib/pkgconfig")
-    set(PKGCONFIG_PACKAGES_SHARE_DIR "${CURRENT_PACKAGES_DIR}/share/pkgconfig")
+function(z_vcpkg_fixup_pkgconfig_check_files file config)
+    set(path_suffix_DEBUG /debug)
+    set(path_suffix_RELEASE "")
 
     if(DEFINED ENV{PKG_CONFIG_PATH})
-        set(BACKUP_ENV_PKG_CONFIG_PATH "$ENV{PKG_CONFIG_PATH}")
+        set(backup_env_pkg_config_path "$ENV{PKG_CONFIG_PATH}")
     else()
-        unset(BACKUP_ENV_PKG_CONFIG_PATH)
-    endif()
-    if(DEFINED ENV{PKG_CONFIG_PATH} AND NOT ENV{PKG_CONFIG_PATH} STREQUAL "")
-        set(ENV{PKG_CONFIG_PATH} "${PKGCONFIG_INSTALLED_DIR}${VCPKG_HOST_PATH_SEPARATOR}${PKGCONFIG_INSTALLED_SHARE_DIR}${VCPKG_HOST_PATH_SEPARATOR}${PKGCONFIG_PACKAGES_DIR}${VCPKG_HOST_PATH_SEPARATOR}${PKGCONFIG_PACKAGES_SHARE_DIR}${VCPKG_HOST_PATH_SEPARATOR}$ENV{PKG_CONFIG_PATH}")
-    else()
-        set(ENV{PKG_CONFIG_PATH} "${PKGCONFIG_INSTALLED_DIR}${VCPKG_HOST_PATH_SEPARATOR}${PKGCONFIG_INSTALLED_SHARE_DIR}${VCPKG_HOST_PATH_SEPARATOR}${PKGCONFIG_PACKAGES_DIR}${VCPKG_HOST_PATH_SEPARATOR}${PKGCONFIG_PACKAGES_SHARE_DIR}")
+        unset(backup_env_pkg_config_path)
     endif()
 
+    vcpkg_host_path_list(PREPEND ENV{PKG_CONFIG_PATH}
+        "${CURRENT_PACKAGES_DIR}${path_suffix_${config}}/lib/pkgconfig"
+        "${CURRENT_PACKAGES_DIR}/share/pkgconfig"
+        "${CURRENT_INSTALLED_DIR}${path_suffix_${config}}/lib/pkgconfig"
+        "${CURRENT_INSTALLED_DIR}/share/pkgconfig"
+    )
+
     # First make sure everything is ok with the package and its deps
-    get_filename_component(_package_name "${_file}" NAME_WLE)
-    debug_message("Checking package (${_config}): ${_package_name}")
-    execute_process(COMMAND "${pkg_cfg_cmd}" --print-errors --exists ${_package_name}
-                    WORKING_DIRECTORY "${CURRENT_BUILDTREES_DIR}"
-                    RESULT_VARIABLE _pkg_error_var
-                    OUTPUT_VARIABLE _pkg_output
-                    ERROR_VARIABLE  _pkg_error_out
-                    OUTPUT_STRIP_TRAILING_WHITESPACE
-                    ERROR_STRIP_TRAILING_WHITESPACE
-                    )
-    if(NOT _pkg_error_var EQUAL 0)
-        message(STATUS "pkg_cfg_cmd call with:${pkg_cfg_cmd} --exists ${_package_name} failed")
-        message(STATUS "ENV{PKG_CONFIG_PATH}:$ENV{PKG_CONFIG_PATH}")
-        message(STATUS "pkg-config call failed with error code:${_pkg_error_var}")
-        message(STATUS "pkg-config output:${_pkg_output}")
-        message(FATAL_ERROR "pkg-config error output:${_pkg_error_out}")
+    cmake_path(GET file STEM LAST_ONLY package_name)
+    debug_message("Checking package (${config}): ${package_name}")
+    execute_process(
+        COMMAND "${PKGCONFIG}" --print-errors --exists "${package_name}"
+        WORKING_DIRECTORY "${CURRENT_BUILDTREES_DIR}"
+        RESULT_VARIABLE error_var
+        OUTPUT_VARIABLE output
+        ERROR_VARIABLE  output
+        OUTPUT_STRIP_TRAILING_WHITESPACE
+        ERROR_STRIP_TRAILING_WHITESPACE
+    )
+    if(NOT "${error_var}" EQUAL "0")
+        message(FATAL_ERROR "${PKGCONFIG} --exists ${package_name} failed with error code: ${error_var}
+    ENV{PKG_CONFIG_PATH}: \"$ENV{PKG_CONFIG_PATH}\"
+    output: ${output}"
+        )
     else()
-        debug_message("pkg-config returned:${_pkg_error_var}")
-        debug_message("pkg-config output:${_pkg_output}")
-        debug_message("pkg-config error output:${_pkg_error_out}")
+        debug_message("pkg-config --exists ${package_name} output: ${output}")
     endif()
-    if(DEFINED BACKUP_ENV_PKG_CONFIG_PATH)
-        set(ENV{PKG_CONFIG_PATH} "${BACKUP_ENV_PKG_CONFIG_PATH}")
+    if(DEFINED backup_env_pkg_config_path)
+        set(ENV{PKG_CONFIG_PATH} "${backup_env_pkg_config_path}")
     else()
         unset(ENV{PKG_CONFIG_PATH})
     endif()
 endfunction()
 
 function(vcpkg_fixup_pkgconfig)
-    # parse parameters such that semicolons in options arguments to COMMAND don't get erased
-    cmake_parse_arguments(PARSE_ARGV 0 _vfpkg "SKIP_CHECK" "" "RELEASE_FILES;DEBUG_FILES;SYSTEM_LIBRARIES;SYSTEM_PACKAGES;IGNORE_FLAGS")
-
-    if(_vfpkg_UNPARSED_ARGUMENTS)
-        message(FATAL_ERROR "vcpkg_fixup_pkgconfig() was passed extra arguments: ${_vfct_UNPARSED_ARGUMENTS}")
+    cmake_parse_arguments(PARSE_ARGV 0 arg 
+        "SKIP_CHECK"
+        ""
+        "RELEASE_FILES;DEBUG_FILES;SYSTEM_LIBRARIES;SYSTEM_PACKAGES;IGNORE_FLAGS"
+    )
+
+    if(DEFINED arg_UNPARSED_ARGUMENTS)
+        message(FATAL_ERROR "${CMAKE_CURRENT_FUNCTION} was passed extra arguments: ${arg_UNPARSED_ARGUMENTS}")
     endif()
 
-    if((DEFINED _vfpkg_RELEASE_FILES AND NOT DEFINED _vfpkg_DEBUG_FILES) OR (NOT DEFINED _vfpkg_RELEASE_FILES AND DEFINED _vfpkg_DEBUG_FILES))
-        message(FATAL_ERROR "vcpkg_fixup_pkgconfig() requires both or neither of DEBUG_FILES and RELEASE_FILES")
+    if(DEFINED arg_RELEASE_FILES AND NOT DEFINED arg_DEBUG_FILES)
+        message(FATAL_ERROR "DEBUG_FILES must be specified if RELEASE_FILES was specified.")
+    endif()
+    if(NOT DEFINED arg_RELEASE_FILES AND DEFINED arg_DEBUG_FILES)
+        message(FATAL_ERROR "RELEASE_FILES must be specified if DEBUG_FILES was specified.")
     endif()
 
-    if(NOT DEFINED _vfpkg_RELEASE_FILES)
-        file(GLOB_RECURSE _vfpkg_RELEASE_FILES "${CURRENT_PACKAGES_DIR}/**/*.pc")
-        file(GLOB_RECURSE _vfpkg_DEBUG_FILES "${CURRENT_PACKAGES_DIR}/debug/**/*.pc")
-        if(_vfpkg_DEBUG_FILES)
-            list(REMOVE_ITEM _vfpkg_RELEASE_FILES ${_vfpkg_DEBUG_FILES})
-        endif()
+    if(NOT DEFINED arg_RELEASE_FILES)
+        file(GLOB_RECURSE arg_RELEASE_FILES "${CURRENT_PACKAGES_DIR}/**/*.pc")
+        file(GLOB_RECURSE arg_DEBUG_FILES "${CURRENT_PACKAGES_DIR}/debug/**/*.pc")
+        foreach(debug_file IN LISTS arg_DEBUG_FILES)
+            vcpkg_list(REMOVE_ITEM arg_RELEASE_FILES "${debug_file}")
+        endforeach()
     endif()
 
-    #Absolute Unix like paths 
-    string(REGEX REPLACE "([a-zA-Z]):/" "/\\1/" _VCPKG_PACKAGES_DIR "${CURRENT_PACKAGES_DIR}")
-    string(REGEX REPLACE "([a-zA-Z]):/" "/\\1/" _VCPKG_INSTALLED_DIR "${CURRENT_INSTALLED_DIR}")
+    string(REGEX REPLACE "^([a-zA-Z]):/" [[/\1/]] unix_packages_dir "${CURRENT_PACKAGES_DIR}")
+    string(REGEX REPLACE "^([a-zA-Z]):/" [[/\1/]] unix_installed_dir "${CURRENT_INSTALLED_DIR}")
 
-    foreach(CONFIG RELEASE DEBUG)
-        debug_message("${CONFIG} Files: ${_vfpkg_${CONFIG}_FILES}")
-        if(VCPKG_BUILD_TYPE STREQUAL "debug" AND CONFIG STREQUAL "RELEASE")
+    foreach(config IN ITEMS RELEASE DEBUG)
+        debug_message("${config} Files: ${arg_${config}_FILES}")
+        if("${VCPKG_BUILD_TYPE}" STREQUAL "debug" AND "${config}" STREQUAL "RELEASE")
             continue()
         endif()
-        if(VCPKG_BUILD_TYPE STREQUAL "release" AND CONFIG STREQUAL "DEBUG")
+        if("${VCPKG_BUILD_TYPE}" STREQUAL "release" AND "${config}" STREQUAL "DEBUG")
             continue()
         endif()
-        foreach(_file ${_vfpkg_${CONFIG}_FILES})
-            message(STATUS "Fixing pkgconfig file: ${_file}")
-            get_filename_component(PKG_LIB_SEARCH_PATH "${_file}" DIRECTORY)
-            if(CONFIG STREQUAL "DEBUG")
-                file(RELATIVE_PATH RELATIVE_PC_PATH "${PKG_LIB_SEARCH_PATH}" "${CURRENT_PACKAGES_DIR}/debug/")
+        foreach(file IN LISTS "arg_${config}_FILES")
+            message(STATUS "Fixing pkgconfig file: ${file}")
+            cmake_path(GET file PARENT_PATH pkg_lib_search_path)
+            if("${config}" STREQUAL "DEBUG")
+                set(relative_pc_path "${CURRENT_PACKAGES_DIR}/debug")
+                cmake_path(RELATIVE_PATH relative_pc_path BASE_DIRECTORY "${pkg_lib_search_path}")
             else()
-                file(RELATIVE_PATH RELATIVE_PC_PATH "${PKG_LIB_SEARCH_PATH}" "${CURRENT_PACKAGES_DIR}")
+                set(relative_pc_path "${CURRENT_PACKAGES_DIR}")
+                cmake_path(RELATIVE_PATH relative_pc_path BASE_DIRECTORY "${pkg_lib_search_path}")
             endif()
-            # strip trailing slash
-            string(REGEX REPLACE "/$" "" RELATIVE_PC_PATH "${RELATIVE_PC_PATH}")
             #Correct *.pc file
-            file(READ "${_file}" _contents)
-            string(REPLACE "${CURRENT_PACKAGES_DIR}" "\${prefix}" _contents "${_contents}")
-            string(REPLACE "${CURRENT_INSTALLED_DIR}" "\${prefix}" _contents "${_contents}")
-            string(REPLACE "${_VCPKG_PACKAGES_DIR}" "\${prefix}" _contents "${_contents}")
-            string(REPLACE "${_VCPKG_INSTALLED_DIR}" "\${prefix}" _contents "${_contents}")
-            string(REGEX REPLACE "(^|\n)prefix[\t ]*=[^\n]*" "" _contents "${_contents}")
-            if(CONFIG STREQUAL "DEBUG")
-                string(REPLACE "}/debug" "}" _contents "${_contents}")
-                # Prefix points at the debug subfolder
-                string(REPLACE "\${prefix}/include" "\${prefix}/../include" _contents "${_contents}")
-                string(REPLACE "\${prefix}/share" "\${prefix}/../share" _contents "${_contents}")
+            file(READ "${file}" contents)
+
+            # this normalizes all files to end with a newline, and use LF instead of CRLF;
+            # this allows us to use regex matches easier to modify these files.
+            if(NOT "${contents}" MATCHES "\n$")
+                string(APPEND contents "\n")
+            endif()
+            string(REPLACE "\r\n" "\n" contents "${contents}")
+
+            string(REPLACE "${CURRENT_PACKAGES_DIR}" [[${prefix}]] contents "${contents}")
+            string(REPLACE "${CURRENT_INSTALLED_DIR}" [[${prefix}]] contents "${contents}")
+            string(REPLACE "${unix_packages_dir}" [[${prefix}]] contents "${contents}")
+            string(REPLACE "${unix_installed_dir}" [[${prefix}]] contents "${contents}")
+
+            string(REGEX REPLACE "(^|\n)prefix[\t ]*=[^\n]*" "" contents "${contents}")
+            if("${config}" STREQUAL "DEBUG")
+                # prefix points at the debug subfolder
+                string(REPLACE [[${prefix}/debug]] [[${prefix}]] contents "${contents}")
+                string(REPLACE [[${prefix}/include]] [[${prefix}/../include]] contents "${contents}")
+                string(REPLACE [[${prefix}/share]] [[${prefix}/../share]] contents "${contents}")
             endif()
-            string(REGEX REPLACE " -L(\\\${[^}]*}[^ \n\t]*)" " -L\"\\1\"" _contents "${_contents}")
-            string(REGEX REPLACE " -I(\\\${[^}]*}[^ \n\t]*)" " -I\"\\1\"" _contents "${_contents}")
-            string(REGEX REPLACE " -l(\\\${[^}]*}[^ \n\t]*)" " -l\"\\1\"" _contents "${_contents}")
+            # quote -L, -I, and -l paths starting with `${blah}`
+            string(REGEX REPLACE " -([LIl])(\\\${[^}]*}[^ \n\t]*)" [[ -\1"\2"]] contents "${contents}")
             # This section fuses XYZ.private and XYZ according to VCPKG_LIBRARY_LINKAGE
             #
             # Pkgconfig searches Requires.private transitively for Cflags in the dynamic case,
             # which prevents us from removing it.
             #
             # Once this transformation is complete, users of vcpkg should never need to pass
             # --static.
-            if(VCPKG_LIBRARY_LINKAGE STREQUAL "static")
-                # Libs comes before Libs.private
-                string(REGEX REPLACE "(^|\n)(Libs: *[^\n]*)(.*)\nLibs.private:( *[^\n]*)" "\\1\\2\\4\\3" _contents "${_contents}")
-                # Libs.private comes before Libs
-                string(REGEX REPLACE "(^|\n)Libs.private:( *[^\n]*)(.*\nLibs: *[^\n]*)" "\\3\\2" _contents "${_contents}")
-                # Only Libs.private
-                string(REGEX REPLACE "(^|\n)Libs.private: *" "\\1Libs: " _contents "${_contents}")
-                # Cflags comes before Cflags.private
-                string(REGEX REPLACE "(^|\n)(Cflags: *[^\n]*)(.*)\nCflags.private:( *[^\n]*)" "\\1\\2\\4\\3" _contents "${_contents}")
-                # Cflags.private comes before Cflags
-                string(REGEX REPLACE "(^|\n)Cflags.private:( *[^\n]*)(.*\nCflags: *[^\n]*)" "\\3\\2" _contents "${_contents}")
-                # Only Cflags.private
-                string(REGEX REPLACE "(^|\n)Cflags.private: *" "\\1Cflags: " _contents "${_contents}")
-                # Requires comes before Requires.private
-                string(REGEX REPLACE "(^|\n)(Requires: *[^\n]*)(.*)\nRequires.private:( *[^\n]*)" "\\1\\2\\4\\3" _contents "${_contents}")
-                # Requires.private comes before Requires
-                string(REGEX REPLACE "(^|\n)Requires.private:( *[^\n]*)(.*\nRequires: *[^\n]*)" "\\3\\2" _contents "${_contents}")
-                # Only Requires.private
-                string(REGEX REPLACE "(^|\n)Requires.private: *" "\\1Requires: " _contents "${_contents}")
+            if("${VCPKG_LIBRARY_LINKAGE}" STREQUAL "static")
+                # how this works:
+                # we want to transform:
+                #   Libs: $1
+                #   Libs.private: $2
+                # into
+                #    Libs: $1 $2
+                # and the same thing for Requires and Requires.private
+
+                foreach(item IN ITEMS "Libs" "Requires" "Cflags")
+                    set(line "")
+                    if("${contents}" MATCHES "(^|\n)${item}: *([^\n]*)")
+                        string(APPEND line " ${CMAKE_MATCH_2}")
+                    endif()
+                    if("${contents}" MATCHES "(^|\n)${item}\\.private: *([^\n]*)")
+                        string(APPEND line " ${CMAKE_MATCH_2}")
+                    endif()
+
+                    string(REGEX REPLACE "(^|\n)${item}(\\.private)?:[^\n]*\n" [[\1]] contents "${contents}")
+                    if(NOT "${line}" STREQUAL "")
+                        string(APPEND contents "${item}:${line}\n")
+                    endif()
+                endforeach()
             endif()
-            file(WRITE "${_file}" "prefix=\${pcfiledir}/${RELATIVE_PC_PATH}\n${_contents}")
-            unset(PKG_LIB_SEARCH_PATH)
+            file(WRITE "${file}" "prefix=\${pcfiledir}/${relative_pc_path}\n${contents}")
         endforeach()
 
-        if(NOT _vfpkg_SKIP_CHECK) # The check can only run after all files have been corrected!
+        if(NOT arg_SKIP_CHECK) # The check can only run after all files have been corrected!
             vcpkg_find_acquire_program(PKGCONFIG)
             debug_message("Using pkg-config from: ${PKGCONFIG}")
-            foreach(_file ${_vfpkg_${CONFIG}_FILES})
-                vcpkg_fixup_pkgconfig_check_files("${PKGCONFIG}" "${_file}" "${CONFIG}")
+            foreach(file IN LISTS "arg_${config}_FILES")
+                z_vcpkg_fixup_pkgconfig_check_files("${file}" "${config}")
             endforeach()
         endif()
     endforeach()
     debug_message("Fixing pkgconfig --- finished")
 
-    set(VCPKG_FIXUP_PKGCONFIG_CALLED TRUE CACHE INTERNAL "See below" FORCE)
+    set(Z_VCPKG_FIXUP_PKGCONFIG_CALLED TRUE CACHE INTERNAL "See below" FORCE)
     # Variable to check if this function has been called!
     # Theoreotically vcpkg could look for *.pc files and automatically call this function
     # or check if this function has been called if *.pc files are detected.