From 361e5331bf4c790ff70c5c36a13d20d17795ac72 Mon Sep 17 00:00:00 2001 From: Robert Maynard Date: Wed, 4 Aug 2021 13:06:31 -0400 Subject: [PATCH] rapids_find_package propagates variables from find_package Fixes #53 The conversion of `rapids_find_package` to being a macro instead of a function allows it to properly propagate both common `find_package` variables and uncommon ones ( CUDAToolkit_LIBRARY_ROOT ). --- rapids-cmake/find/package.cmake | 48 +++++++++---- testing/find/CMakeLists.txt | 2 + .../find/find_package-no-variable-leak.cmake | 69 +++++++++++++++++++ 3 files changed, 105 insertions(+), 14 deletions(-) create mode 100644 testing/find/find_package-no-variable-leak.cmake diff --git a/rapids-cmake/find/package.cmake b/rapids-cmake/find/package.cmake index 2f15e9187..4de567e21 100644 --- a/rapids-cmake/find/package.cmake +++ b/rapids-cmake/find/package.cmake @@ -67,19 +67,26 @@ so users have consistency. List all targets used by your project in `GLOBAL_TARG #]=======================================================================] -function(rapids_find_package name) +macro(rapids_find_package name) + # + # rapids_find_package breaks convention and is a macro on purpose. Since + # `find_package` allows abitrary variable creation ( `` ) we + # need to have `rapids_find_package` do the same. If it doesn't it would + # make drop in replacements impossible + # list(APPEND CMAKE_MESSAGE_CONTEXT "rapids.find.package") - set(options FIND_ARGS REQUIRED) - set(one_value BUILD_EXPORT_SET INSTALL_EXPORT_SET) - set(multi_value GLOBAL_TARGETS) - cmake_parse_arguments(RAPIDS "${options}" "${one_value}" "${multi_value}" ${ARGN}) + set(_rapids_options FIND_ARGS REQUIRED) + set(_rapids_one_value BUILD_EXPORT_SET INSTALL_EXPORT_SET) + set(_rapids_multi_value GLOBAL_TARGETS) + cmake_parse_arguments(RAPIDS "${_rapids_options}" "${_rapids_one_value}" "${_rapids_multi_value}" ${ARGN}) - unset(_required_flag) + set(_rapids_required_flag) if(RAPIDS_REQUIRED) - set(_required_flag REQUIRED) + set(_rapids_required_flag REQUIRED) endif() - find_package(${name} ${_required_flag} ${RAPIDS_UNPARSED_ARGUMENTS}) + find_package(${name} ${_rapids_required_flag} ${RAPIDS_UNPARSED_ARGUMENTS}) + unset(_rapids_required_flag) if(RAPIDS_GLOBAL_TARGETS) include("${rapids-cmake-dir}/cmake/make_global.cmake") @@ -90,21 +97,34 @@ function(rapids_find_package name) # OPTIONAL find packages if(${${name}_FOUND}) - set(extra_info) + set(_rapids_extra_info) if(RAPIDS_GLOBAL_TARGETS) - set(extra_info "GLOBAL_TARGETS") - list(APPEND extra_info ${RAPIDS_GLOBAL_TARGETS}) + set(_rapids_extra_info "GLOBAL_TARGETS") + list(APPEND _rapids_extra_info ${RAPIDS_GLOBAL_TARGETS}) endif() if(RAPIDS_BUILD_EXPORT_SET) include("${rapids-cmake-dir}/export/package.cmake") - rapids_export_package(BUILD ${name} ${RAPIDS_BUILD_EXPORT_SET} ${extra_info}) + rapids_export_package(BUILD ${name} ${RAPIDS_BUILD_EXPORT_SET} ${_rapids_extra_info}) endif() if(RAPIDS_INSTALL_EXPORT_SET) include("${rapids-cmake-dir}/export/package.cmake") - rapids_export_package(INSTALL ${name} ${RAPIDS_INSTALL_EXPORT_SET} ${extra_info}) + rapids_export_package(INSTALL ${name} ${RAPIDS_INSTALL_EXPORT_SET} ${_rapids_extra_info}) endif() + + unset(_rapids_extra_info) endif() -endfunction() + # Cleanup all our local variables + foreach(_rapids_local_var IN LISTS _rapids_options _rapids_one_value _rapids_multi_value) + if(DEFINED RAPIDS_${_rapids_local_var}) + unset(RAPIDS_${_rapids_local_var}) + endif() + endforeach() + unset(_rapids_local_var) + unset(_rapids_options) + unset(_rapids_one_value) + unset(_rapids_multi_value) + list(POP_BACK CMAKE_MESSAGE_CONTEXT) +endmacro() diff --git a/testing/find/CMakeLists.txt b/testing/find/CMakeLists.txt index f2dcba61e..1d6f2a642 100644 --- a/testing/find/CMakeLists.txt +++ b/testing/find/CMakeLists.txt @@ -15,6 +15,8 @@ #============================================================================= add_cmake_config_test( rapids-find.cmake ) +add_cmake_config_test( find_package-no-variable-leak.cmake ) + add_cmake_config_test( find_package-build.cmake ) add_cmake_config_test( find_package-install.cmake ) diff --git a/testing/find/find_package-no-variable-leak.cmake b/testing/find/find_package-no-variable-leak.cmake new file mode 100644 index 000000000..6ce9e8c48 --- /dev/null +++ b/testing/find/find_package-no-variable-leak.cmake @@ -0,0 +1,69 @@ +#============================================================================= +# Copyright (c) 2018-2021, NVIDIA CORPORATION. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +#============================================================================= +include(${rapids-cmake-dir}/find/package.cmake) + + +function(track_normal_find_package count_var) + # We need to establish CMAKE_MESSAGE_CONTEXT before we count + # as it is expected to leak + set(CMAKE_MESSAGE_CONTEXT "test") + + find_package(${ARGN}) + + get_cmake_property(_all_local_variables VARIABLES) + list(LENGTH _all_local_variables count_var) + set(count_var ${count_var} PARENT_SCOPE) +endfunction() + +function(track_rapids_find_package count_var) + # We need to establish CMAKE_MESSAGE_CONTEXT before we count + # as it is expected to leak + set(CMAKE_MESSAGE_CONTEXT "test") + rapids_find_package(${ARGN}) + + get_cmake_property(_all_local_variables VARIABLES) + list(LENGTH _all_local_variables count_var) + set(count_var ${count_var} PARENT_SCOPE) + + # verify CMAKE_MESSAGE_CONTEXT has been properly popped + list(LENGTH CMAKE_MESSAGE_CONTEXT context_len) + if(context_len GREATER 1) + message(FATAL_ERROR "CMAKE_MESSAGE_CONTEXT hasn't been properly reset") + endif() +endfunction() + + +# Need to create both of the length variables ahead of time so +# that they are included in the counts and track_rapids_find_package +set(normal_len 0) +set(rapids_len 0) +track_normal_find_package(normal_len PNG) +track_rapids_find_package(rapids_len PNG) + +if(NOT normal_len EQUAL rapids_len) + message(FATAL_ERROR "A simple rapids_find_package leaked variables!") +endif() + +track_normal_find_package(normal_len ZLIB) +track_rapids_find_package(rapids_len ZLIB + INSTALL_EXPORT_SET test_export_set + BUILD_EXPORT_SET test_export_set + GLOBAL_TARGETS ZLIB::ZLIB + ) + +if(NOT normal_len EQUAL rapids_len) + message(FATAL_ERROR "A complex rapids_find_package leaked variables!") +endif()