[vcpkg_setup_pkgconfig_path] Fix backup/restore

[dg0yt]

• What does your PR fix?

  Add unit testing for for vcpkg_backup/restore_env_variables and vcpkg_setup/restore_pkgconfig_path.
  Fixes the broken backup of pkg-config path variables. Cf. [vcpkg_backup_env_variables] use CACHE INTERNAL instead of PARENT_SCOPE #25118 (comment), and https://github.com/microsoft/vcpkg/blob/d732e2f628269c15026e1cdda228cc4f527e81e9/ports/dpdk/portfile.cmake#L63-L69

  Unhanged:
  Due to CMake's special handling of <unset> and scopes, there might still be room for unexpected behaviour.
  And vcpkg_setup_pkgconfig_path still adds to ENV{PATH} with no backup.

• Which triplets are supported/not supported? Have you updated the CI baseline?

  all, no

• Does your PR follow the maintainer guide?

  yes

• If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

  not needed.

[github-actions]

You have modified or added at least one vcpkg.json where you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

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

Valid values for the license field can be found in the documentation

[github-actions]

You have modified or added at least one vcpkg.json where you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

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

Valid values for the license field can be found in the documentation

[github-actions]

You have modified or added at least one vcpkg.json where you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

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

Valid values for the license field can be found in the documentation

[dg0yt]

NB: CI run only the unit tests.

[github-actions]

You have modified or added at least one vcpkg.json where you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

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

Valid values for the license field can be found in the documentation

[JackBoosY]

We should provide a function to other modules instead of a macro.

[dg0yt]

A function cannot write to the parent scope of the parent scope. That's why I changed the user-facing command into a macro. The main work is still implemented in a function, as an implementation detail of the macro.

[JackBoosY]

Is there any other way we can solve this?
Consider this: it would be a disaster if a C++ module provided a macro containing a function for other modules to call.

[dg0yt]

Is there any other way we can solve this?

Not within this command. The only alternative is yet another tighly coupled command. See the individual commits.

Consider this: it would be a disaster if a C++ module provided a macro containing a function for other modules to call.

The script mode command names vcpkg_backup_env_variables/z_vcpkg_backup_env_variables are clearly owned by vcpkg, no matter if macro or function. No room for collision.
There is only one variable always overwritten in calling scope: z_vcpkg_backup_env_variables_arg_vars. Ownership should be clear here, too. What should be added is unsetting the variable before leaving the macro.

[github-actions]

You have modified or added at least one vcpkg.json where you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

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

Valid values for the license field can be found in the documentation

[JackBoosY]

I personally don't agree with this, need other guys to review it further.

[ras0219-msft]

Given the triviality of the function and the fact that the only use case with this issue is in an internal z_ function, I think adding an additional public switch is not warranted. It would be much, much simpler for z_vcpkg_setup_pkgconfig_path to just inline the function for its particular use.

If we're intending to solve the general problem of scope management, this isn't a great solution because it only works for exactly one layer. A general solution should work for arbitrary scoping, which probably means it would add a way to get the list of variables required such that the caller could propagate them appropriately. For composition, it would also likely need to add a custom prefix to the variable names as well, so that multiple callers can be correctly nested.

All of this said, I don't think we should attempt to solve this general problem at this time -- instead we should simply fix z_vcpkg_setup_pkgconfig_path in the simple way.

Thanks for the PR!

[github-actions]

You have modified or added at least one vcpkg.json where you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

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

Valid values for the license field can be found in the documentation

[dg0yt]

just inline the function for its particular use.

Done. Using the same variable names as vcpkg_backup_env_variables/z_vcpkg_restore_env_variables.