[vcpkg_setup_pkgconfig_path] Fix backup/restore (#25361)

* Unit-test vcpkg_backup/restore_env_vars

* Unit-test z_vcpkg_setup/restore_pkgconfig_path

* Fix z_vcpkg_setup_pkgconfig_path env var backup

* Move parent-scoping into vcpkg_backup_env_variables command

* Test repeated restore

* Unset z_vcpkg_backup_env_variables_arg_vars

* Documentation updates

* Inline pkgconfig env vars backup

* Revert obsolete changes

* Fix typo

* Remove duplicate doc file

docs/maintainers/internal/z_vcpkg_setup_pkgconfig_path.cmake.md

@@ -1,11 +0,0 @@
-# z_vcpkg_setup_pkgconfig_path
-
-Setup the generated pkgconfig file path to PKG_CONFIG_PATH environment variable or restore PKG_CONFIG_PATH environment variable.
-
-```cmake
-z_vcpkg_setup_pkgconfig_path(BASE_DIRS <"${CURRENT_INSTALLED_DIR}" ...>)
-z_vcpkg_restore_pkgconfig_path()
-```
-
-`z_vcpkg_setup_pkgconfig_path` prepends `lib/pkgconfig` and `share/pkgconfig` directories for the given `BASE_DIRS` to the `PKG_CONFIG_PATH` environment variable. It creates or updates a backup of the previous value.
-`z_vcpkg_restore_pkgconfig_path` shall be called when leaving the scope which called `z_vcpkg_setup_pkgconfig_path` in order to restore the original value from the backup.

scripts/cmake/z_vcpkg_setup_pkgconfig_path.cmake

@@ -8,7 +8,13 @@ function(z_vcpkg_setup_pkgconfig_path)
         message(FATAL_ERROR "${CMAKE_CURRENT_FUNCTION} was passed extra arguments: ${arg_UNPARSED_ARGUMENTS}")
     endif()
 
-    vcpkg_backup_env_variables(VARS PKG_CONFIG PKG_CONFIG_PATH)
+    foreach(envvar IN ITEMS PKG_CONFIG PKG_CONFIG_PATH)
+        if(DEFINED ENV{${envvar}})
+            set("z_vcpkg_env_backup_${envvar}" "$ENV{${envvar}}" PARENT_SCOPE)
+        else()
+            unset("z_vcpkg_env_backup_${envvar}" PARENT_SCOPE)
+        endif()
+    endforeach()
 
     vcpkg_find_acquire_program(PKGCONFIG)
     get_filename_component(pkgconfig_path "${PKGCONFIG}" DIRECTORY)
@@ -31,5 +37,11 @@ function(z_vcpkg_restore_pkgconfig_path)
         message(FATAL_ERROR "${CMAKE_CURRENT_FUNCTION} was passed extra arguments: ${arg_UNPARSED_ARGUMENTS}")
     endif()
 
-    vcpkg_restore_env_variables(VARS PKG_CONFIG PKG_CONFIG_PATH)
+    foreach(envvar IN ITEMS PKG_CONFIG PKG_CONFIG_PATH)
+        if(DEFINED z_vcpkg_env_backup_${envvar})
+            set("ENV{${envvar}}" "${z_vcpkg_env_backup_${envvar}}")
+        else()
+            unset("ENV{${envvar}}")
+        endif()
+    endforeach()
 endfunction()

scripts/test_ports/unit-test-cmake/portfile.cmake

@@ -113,6 +113,38 @@ function(unit_test_check_variable_equal utcve_test utcve_variable utcve_value)
     endif()
 endfunction()
 
+function(unit_test_check_variable_not_equal utcve_test utcve_variable utcve_value)
+    cmake_language(EVAL CODE "${utcve_test}")
+    if(Z_VCPKG_UNIT_TEST_HAS_FATAL_ERROR)
+        unset_fatal_error()
+        set_has_error()
+        message(STATUS "${utcve_test} had an unexpected FATAL_ERROR;
+    expected: \"${utcve_value}\"")
+        message(STATUS "FATAL_ERROR: ${Z_VCPKG_UNIT_TEST_FATAL_ERROR}")
+        return()
+    endif()
+
+    unit_test_match(utcve "${utcve_variable}" [[^(ENV|CACHE)\{(.*)\}$]])
+    if(utcve_MATCHED)
+        if("${utcve_CMAKE_MATCH_1}" STREQUAL "ENV")
+            set(utcve_actual_value "$ENV{${utcve_CMAKE_MATCH_2}}")
+        elseif("${utcve_CMAKE_MATCH_1}" STREQUAL "CACHE")
+            set(utcve_actual_value "$CACHE{${utcve_CMAKE_MATCH_2}}")
+        else()
+            _message(FATAL_ERROR "unexpected value for CMAKE_MATCH_1: ${utcve_CMAKE_MATCH_1}")
+        endif()
+    else()
+        set(utcve_actual_value "${${utcve_variable}}")
+    endif()
+
+    if("${utcve_actual_value}" STREQUAL "${utcve_value}")
+        message(STATUS "${utcve_test} failed to change ${utcve_variable};
+    unchanged: \"${utcve_value}\"")
+        set_has_error()
+        return()
+    endif()
+endfunction()
+
 function(unit_test_ensure_success utcve_test)
     cmake_language(EVAL CODE "${utcve_test}")
     if(Z_VCPKG_UNIT_TEST_HAS_FATAL_ERROR)
@@ -147,6 +179,12 @@ endif()
 if("merge-libs" IN_LIST FEATURES)
     include("${CMAKE_CURRENT_LIST_DIR}/test-z_vcpkg_cmake_config_fixup_merge.cmake")
 endif()
+if("backup-restore-env-vars" IN_LIST FEATURES)
+    include("${CMAKE_CURRENT_LIST_DIR}/test-vcpkg_backup_restore_env_vars.cmake")
+endif()
+if("setup-pkgconfig-path" IN_LIST FEATURES)
+    include("${CMAKE_CURRENT_LIST_DIR}/test-z_vcpkg_setup_pkgconfig_path.cmake")
+endif()
 
 if(Z_VCPKG_UNIT_TEST_HAS_ERROR)
     _message(FATAL_ERROR "At least one test failed")

scripts/test_ports/unit-test-cmake/test-vcpkg_backup_restore_env_vars.cmake

@@ -0,0 +1,38 @@
+# vcpkg_backup_env_variables(VARS <list>)
+# vcpkg_restore_env_variables(VARS <list>)
+# These functions used scoped variables and cannot be called in unit_test_check_*.
+
+set(ENV{A} [[::a;::b]])
+set(ENV{B} [[]])
+
+# Backup doesn't change variables.
+vcpkg_backup_env_variables(VARS A B)
+unit_test_check_variable_equal([[]] ENV{A} [[::a;::b]])
+unit_test_check_variable_equal([[]] ENV{B} [[]])
+
+# Restore restores.
+set(ENV{A} [[::a;::b;::c]])
+set(ENV{B} [[::1]])
+vcpkg_restore_env_variables(VARS A B)
+unit_test_check_variable_equal([[]] ENV{A} [[::a;::b]])
+unit_test_check_variable_equal([[]] ENV{B} [[]])
+
+# Restore can be called more than once.
+set(ENV{A} [[::a;::b;::c]])
+set(ENV{B} [[::1]])
+vcpkg_restore_env_variables(VARS A B)
+unit_test_check_variable_equal([[]] ENV{A} [[::a;::b]])
+unit_test_check_variable_equal([[]] ENV{B} [[]])
+
+# Backups are scoped.
+function(change_and_backup)
+    set(ENV{A} [[::a;::b;::c]])
+    set(ENV{B} [[::1]])
+    vcpkg_backup_env_variables(VARS A B)
+    # no further change, no restore, in this scope
+endfunction()
+vcpkg_backup_env_variables(VARS A B)
+change_and_backup()
+vcpkg_restore_env_variables(VARS A B)
+unit_test_check_variable_equal([[]] ENV{A} [[::a;::b]])
+unit_test_check_variable_equal([[]] ENV{B} [[]])

scripts/test_ports/unit-test-cmake/test-z_vcpkg_setup_pkgconfig_path.cmake

@@ -0,0 +1,22 @@
+# z_vcpkg_setup_pkgconfig_path(BASE_DIR <list>)
+# z_vcpkg_restore_pkgconfig_path()
+
+# These functions use vcpkg_backup/restore_env_variables which use scoped variables
+# and cannot be called in unit_test_check_*.
+
+set(ENV{PKG_CONFIG} "/a/pkgconf")
+set(ENV{PKG_CONFIG_PATH} "1")
+set(saved_path "$ENV{PATH}")
+
+z_vcpkg_setup_pkgconfig_path(BASE_DIRS "/2")
+unit_test_check_variable_equal([[]] ENV{PKG_CONFIG} [[/a/pkgconf]])
+unit_test_check_variable_not_equal([[]] ENV{PKG_CONFIG_PATH} "1")
+
+z_vcpkg_restore_pkgconfig_path()
+unit_test_check_variable_equal([[]] ENV{PKG_CONFIG} [[/a/pkgconf]])
+unit_test_check_variable_equal([[]] ENV{PKG_CONFIG_PATH} "1")
+
+# z_vcpkg_setup_pkgconfig_path changes PATH but it is not restored.
+# It is hard to see which side effects a restore would have, so
+# this is expected behaviour for now.
+unit_test_check_variable_not_equal([[]] ENV{PATH} "${saved_path}")

scripts/test_ports/unit-test-cmake/vcpkg.json

@@ -4,13 +4,18 @@
   "description": "Ensures that the CMake scripts are unit tested.",
   "supports": "x64",
   "default-features": [
+    "backup-restore-env-vars",
     "function-arguments",
     "host-path-list",
     "list",
     "merge-libs",
-    "minimum-required"
+    "minimum-required",
+    "setup-pkgconfig-path"
   ],
   "features": {
+    "backup-restore-env-vars": {
+      "description": "Test the vcpkg_backup/restore_env_vars functions"
+    },
     "function-arguments": {
       "description": "Test the z_vcpkg_function_arguments function"
     },
@@ -31,6 +36,9 @@
     },
     "minimum-required": {
       "description": "Test the vcpkg_minimum_required function"
+    },
+    "setup-pkgconfig-path": {
+      "description": "Test the z_vcpkg_setup/restore_pkgconfig_path functions"
     }
   }
 }