From 2b68048bdb63903eacd6f61e862654cadfc40b47 Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Wed, 13 Mar 2024 17:26:27 +1100 Subject: FetchContent: Don't leak temporary variable and restore var robustly In FetchContent_MakeAvailable(), the __fcprefix__ previously used when pushing the value of CMAKE_EXPORT_FIND_PACKAGE_NAME to the variable stack is not needed. The stack will never be empty at that point, so pushing an empty value will be handled correctly. By removing the __fcprefix__, we no longer need any temporary variable when restoring CMAKE_EXPORT_FIND_PACKAGE_NAME. But we need to ensure CMAKE_EXPORT_FIND_PACKAGE_NAME is left undefined if it wasn't defined before, and pushing an empty value doesn't let us distinguish between unset and set-but-empty. Therefore, when CMAKE_EXPORT_FIND_PACKAGE_NAME is undefined, push a specific value that can't be used by the project instead and check for that when popping it again. This ensures we can robustly distinguish the two cases and will always restore the right state. Fixes: #25758 --- Modules/FetchContent.cmake | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/Modules/FetchContent.cmake b/Modules/FetchContent.cmake index 48cdaf4..fae51cf 100644 --- a/Modules/FetchContent.cmake +++ b/Modules/FetchContent.cmake @@ -1966,7 +1966,12 @@ macro(FetchContent_MakeAvailable) "${__cmake_contentName}" ) - list(APPEND __cmake_fcCurrentVarsStack "__fcprefix__${CMAKE_EXPORT_FIND_PACKAGE_NAME}") + if(DEFINED CMAKE_EXPORT_FIND_PACKAGE_NAME) + list(APPEND __cmake_fcCurrentVarsStack "${CMAKE_EXPORT_FIND_PACKAGE_NAME}") + else() + # This just needs to be something that can't be a real package name + list(APPEND __cmake_fcCurrentVarsStack "<<::VAR_NOT_SET::>>") + endif() set(CMAKE_EXPORT_FIND_PACKAGE_NAME "${__cmake_contentName}") # It's still valid if there are no saved details. The project may have @@ -2014,7 +2019,11 @@ macro(FetchContent_MakeAvailable) list(POP_BACK __cmake_fcCurrentVarsStack __cmake_contentNameLower __cmake_contentName + CMAKE_EXPORT_FIND_PACKAGE_NAME ) + if(CMAKE_EXPORT_FIND_PACKAGE_NAME STREQUAL "<<::VAR_NOT_SET::>>") + unset(CMAKE_EXPORT_FIND_PACKAGE_NAME) + endif() unset(__cmake_fcProvider_${__cmake_contentNameLower}) unset(__cmake_providerArgs) @@ -2023,12 +2032,6 @@ macro(FetchContent_MakeAvailable) unset(__cmake_item) unset(__cmake_contentDetails) - 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}) - FetchContent_GetProperties(${__cmake_contentName}) if(${__cmake_contentNameLower}_POPULATED) continue() @@ -2098,7 +2101,12 @@ macro(FetchContent_MakeAvailable) endif() if(EXISTS ${__cmake_srcdir}/CMakeLists.txt) - list(APPEND __cmake_fcCurrentVarsStack "__fcprefix__${CMAKE_EXPORT_FIND_PACKAGE_NAME}") + if(DEFINED CMAKE_EXPORT_FIND_PACKAGE_NAME) + list(APPEND __cmake_fcCurrentVarsStack "${CMAKE_EXPORT_FIND_PACKAGE_NAME}") + else() + # This just needs to be something that can't be a real package name + list(APPEND __cmake_fcCurrentVarsStack "<<::VAR_NOT_SET::>>") + endif() set(CMAKE_EXPORT_FIND_PACKAGE_NAME "${__cmake_contentName}") set(__cmake_add_subdirectory_args ${__cmake_srcdir} ${${__cmake_contentNameLower}_BINARY_DIR}) @@ -2110,11 +2118,10 @@ macro(FetchContent_MakeAvailable) endif() add_subdirectory(${__cmake_add_subdirectory_args}) - 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}) + list(POP_BACK __cmake_fcCurrentVarsStack CMAKE_EXPORT_FIND_PACKAGE_NAME) + if(CMAKE_EXPORT_FIND_PACKAGE_NAME STREQUAL "<<::VAR_NOT_SET::>>") + unset(CMAKE_EXPORT_FIND_PACKAGE_NAME) + endif() endif() unset(__cmake_srcdir) -- cgit v0.12