From e867d76a62c9185927d85f01fa6620ac459aca21 Mon Sep 17 00:00:00 2001 From: Robert Maynard Date: Wed, 6 Nov 2024 09:40:12 -0500 Subject: [PATCH] rapids_export_write_language no longer generates cmake dev warnings (#712) The `rapids_export_write_language` function would generate CMake code that would try and do `set(var PARENT_SCOPE)` in the root directory. This would cause CMake developer warnings to appear. This resolves the issues by adding root level guards around the `set` commands, and setting up each rapids-cmake test to have developer warnings as errors. Due to https://github.com/rapidsai/rapids-cmake/issues/680 we mark some tests with `NO_DEV_ERRORS` so that they continue to pass while we wait to 2025 to bump the minimum required CMake version. Authors: - Robert Maynard (https://github.com/robertmaynard) Approvers: - Bradley Dice (https://github.com/bdice) URL: https://github.com/rapidsai/rapids-cmake/pull/712 --- rapids-cmake/export/write_language.cmake | 18 ++++++++++-------- testing/cpm/CMakeLists.txt | 10 +++++++--- testing/test/CMakeLists.txt | 2 +- testing/utils/cmake_test.cmake | 8 ++++++-- 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/rapids-cmake/export/write_language.cmake b/rapids-cmake/export/write_language.cmake index 7a046775..4b367a84 100644 --- a/rapids-cmake/export/write_language.cmake +++ b/rapids-cmake/export/write_language.cmake @@ -73,31 +73,33 @@ endif() # # So what we need to do is the following: # -# 1. Transform each `set` in CMake@lang@Compiler to be a `PARENT_SCOPE` -# This allows us to propagate up immediate information that is -# used by commands such target_compile_features. +# For non-root directories: +# 1. Transform each `set` in CMake@lang@Compiler to be a `PARENT_SCOPE` +# This allows us to propagate up immediate information that is +# used by commands such target_compile_features. # -# 2. Make sure that every directory including root also re-executes -# `CMake@lang@Information` This can't be deferred as the contents -# are required if any target is constructed +# 2. Include `CMake@lang@Information` this can't be deferred as the contents +# are required if any target is constructed +# +# For root directories we only need to include `CMake@lang@Information` # # Expose the language at the current scope enable_language(@lang@) - if(NOT EXISTS "${CMAKE_BINARY_DIR}/cmake/PropagateCMake@lang@Compiler.cmake") # 1. # Take everything that `enable_language` generated and transform all sets to PARENT_SCOPE () # This will allow our parent directory to be able to call CMake@lang@Information file(STRINGS "${CMAKE_BINARY_DIR}/CMakeFiles/${CMAKE_VERSION}/CMake@lang@Compiler.cmake" rapids_code_to_transform) - set(rapids_code_to_execute ) + set(rapids_code_to_execute "if(NOT CMAKE_CURRENT_SOURCE_DIR STREQUAL CMAKE_SOURCE_DIR)\n") foreach( line IN LISTS rapids_code_to_transform) if(line MATCHES "[ ]*set") string(REPLACE ")" " PARENT_SCOPE)" line "${line}") endif() string(APPEND rapids_code_to_execute "${line}\n") endforeach() + string(APPEND rapids_code_to_execute "endif()\n") # 2. # Make sure we call "CMake@lang@Information" for the current directory diff --git a/testing/cpm/CMakeLists.txt b/testing/cpm/CMakeLists.txt index 7b770a42..9656335a 100644 --- a/testing/cpm/CMakeLists.txt +++ b/testing/cpm/CMakeLists.txt @@ -15,6 +15,10 @@ #============================================================================= add_cmake_config_test( rapids-cpm.cmake ) +# Do to https://github.com/rapidsai/rapids-cmake/issues/680 we need +# to suppress dev warning as errors for all the rapids_cpm tests +set(RAPIDS_TEST_DISABLE_DEV_ERRORS ON) + add_cmake_config_test( cpm_find-add-pkg-source ) add_cmake_config_test( cpm_find-and-find_package ) add_cmake_config_test( cpm_find-components ) @@ -23,16 +27,16 @@ add_cmake_config_test( cpm_find-existing-target ) add_cmake_config_test( cpm_find-existing-target-to-export-sets ) add_cmake_config_test( cpm_find-gtest-no-gmock ) add_cmake_config_test( cpm_find-options-escaped ) -add_cmake_config_test( cpm_find-patch-command NO_CPM_CACHE) +add_cmake_config_test( cpm_find-patch-command NO_CPM_CACHE ) add_cmake_config_test( cpm_find-patch-command-required NO_CPM_CACHE ) add_cmake_config_test( cpm_find-patch-command-required-fails NO_CPM_CACHE SHOULD_FAIL "rapids-cmake [fmt]: failed to apply patch") add_cmake_config_test( cpm_find-restore-cpm-vars ) add_cmake_config_test( cpm_find-version-explicit-install.cmake ) -add_cmake_build_test( cpm_generate_pins-format-patches NO_CPM_CACHE) +add_cmake_build_test( cpm_generate_pins-format-patches NO_CPM_CACHE ) add_cmake_build_test( cpm_generate_pins-nested ) add_cmake_build_test( cpm_generate_pins-no-src-dir ) -add_cmake_build_test( cpm_generate_pins-override NO_CPM_CACHE) +add_cmake_build_test( cpm_generate_pins-override NO_CPM_CACHE ) add_cmake_build_test( cpm_generate_pins-pure-cpm ) add_cmake_build_test( cpm_generate_pins-simple ) add_cmake_build_test( cpm_generate_pins-simple-via-variable ) diff --git a/testing/test/CMakeLists.txt b/testing/test/CMakeLists.txt index e62d1d6a..1fa7c2b8 100644 --- a/testing/test/CMakeLists.txt +++ b/testing/test/CMakeLists.txt @@ -46,7 +46,7 @@ add_cmake_build_test(install_relocatable-no-tests.cmake) add_cmake_build_test(install_relocatable-simple.cmake) add_cmake_build_test(install_relocatable-installed-tests-run) add_cmake_build_test(install_relocatable-installed-tests-run-lazy-msg) -add_cmake_build_test(install_relocatable-with-gtest_discover_tests.cmake) +add_cmake_build_test(install_relocatable-with-gtest_discover_tests.cmake NO_DEV_ERRORS) add_cmake_config_test(install_relocatable-wrong-component.cmake SHOULD_FAIL "${wrong_component_message}") add_cmake_ctest_test(add-impossible-allocation SHOULD_FAIL "Insufficient resources for test") diff --git a/testing/utils/cmake_test.cmake b/testing/utils/cmake_test.cmake index 8a5bacf2..e679ae43 100644 --- a/testing/utils/cmake_test.cmake +++ b/testing/utils/cmake_test.cmake @@ -35,6 +35,7 @@ adds a test for each generator: add_cmake_build_test( (config|build|test|install) [SERIAL] + [NO_DEV_ERRORS] [NO_CPM_CACHE] [NO_RAPIDS_CMAKE_HOOKS] [SHOULD_FAIL ] @@ -60,7 +61,7 @@ adds a test for each generator: #]=======================================================================] function(add_cmake_test mode source_or_dir) - set(options SERIAL NO_CPM_CACHE NO_RAPIDS_CMAKE_HOOKS) + set(options SERIAL NO_DEV_ERRORS NO_CPM_CACHE NO_RAPIDS_CMAKE_HOOKS) set(one_value SHOULD_FAIL) set(multi_value) cmake_parse_arguments(RAPIDS_TEST "${options}" "${one_value}" "${multi_value}" ${ARGN}) @@ -84,7 +85,10 @@ function(add_cmake_test mode source_or_dir) endif() if(NOT RAPIDS_TEST_NO_RAPIDS_CMAKE_HOOKS) - set(extra_configure_flags "-DCMAKE_PROJECT_INCLUDE_BEFORE=${PROJECT_SOURCE_DIR}/utils/emulate_fetching_rapids_cmake.cmake") + list(APPEND extra_configure_flags "-DCMAKE_PROJECT_INCLUDE_BEFORE=${PROJECT_SOURCE_DIR}/utils/emulate_fetching_rapids_cmake.cmake") + endif() + if(NOT (RAPIDS_TEST_NO_DEV_ERRORS OR RAPIDS_TEST_DISABLE_DEV_ERRORS) ) + list(APPEND extra_configure_flags "-Werror=dev") endif() if(DEFINED CPM_SOURCE_CACHE AND NOT RAPIDS_TEST_NO_CPM_CACHE) list(APPEND extra_configure_flags "-DCPM_SOURCE_CACHE=${CPM_SOURCE_CACHE}")