FetchContent_MakeAvailable() leaks __cmake_original_export_find_package_name variable
The change in !8957 (merged) added blocks like the following to the FetchContent_MakeAvailable()
implementation:
list(APPEND __cmake_fcCurrentVarsStack "__fcprefix__${CMAKE_EXPORT_FIND_PACKAGE_NAME}")
set(CMAKE_EXPORT_FIND_PACKAGE_NAME "${__cmake_contentName}")
followed later by something like:
list(POP_BACK __cmake_fcCurrentVarsStack __cmake_original_export_find_package_name)
string(SUBSTRING "${__cmake_original_export_find_package_name}"
12 -1 __cmake_original_export_find_package_name
)
set(CMAKE_EXPORT_FIND_PACKAGE_NAME ${__cmake_original_export_find_package_name})
The second of the above blocks is also sometimes before or after a bunch of unset()
calls intended to prevent leaking any temporary local variables to the calling scope, but __cmake_original_export_find_package_name
is never unset. Therefore, it leaks out to the FetchContent_MakeAvailable()
caller. If we keep the existing code, we should ensure we unset this variable too, just like the other temporary local variables.
The above pattern is more complicated than it needs to be though. There's actually no need for the __fcprefix__
at all. The only place where that is needed is when pushing the very first variable of the command's implementation, to ensure that an empty __cmake_fcCurrentVarsStack
variable upon entry still results in something being pushed. We know we don't have an empty __cmake_fcCurrentVarsStack
stack whenever we need to push CMAKE_EXPORT_FIND_PACKAGE_NAME
though, and pushing an empty value onto a non-empty stack will always push at least a semicolon. Therefore, we don't even need the temporary __cmake_original_export_find_package_name
variable in the above blocks. We can operate on CMAKE_EXPORT_FIND_PACKAGE_NAME
directly:
list(APPEND __cmake_fcCurrentVarsStack "${CMAKE_EXPORT_FIND_PACKAGE_NAME}")
followed later by:
list(POP_BACK __cmake_fcCurrentVarsStack CMAKE_EXPORT_FIND_PACKAGE_NAME)
Both the current code and the above suggested improvement suffer from not exactly preserving CMAKE_EXPORT_FIND_PACKAGE_NAME
though. The existing code will leave CMAKE_EXPORT_FIND_PACKAGE_NAME
undefined if it was explicitly set to an empty value before, and the suggested improved code above has the opposite problem of leaving CMAKE_EXPORT_FIND_PACKAGE_NAME
defined to an empty string where it may have been undefined before. Currently, the way this variable affects the EXPORT_FIND_PACKAGE_NAME
target property means this distinction doesn't affect the behavior right now, but ideally we would preserve the exact variable in case a future change makes a distinction between CMAKE_EXPORT_FIND_PACKAGE_NAME
being undefined and it being defined but empty.
CC: @kyle.edwards