Skip to content

Commit

Permalink
rapids_export_write_language no longer generates cmake dev warnings (#…
Browse files Browse the repository at this point in the history
…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 #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: #712
  • Loading branch information
robertmaynard authored Nov 6, 2024
1 parent 2f81c15 commit e867d76
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 14 deletions.
18 changes: 10 additions & 8 deletions rapids-cmake/export/write_language.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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@[email protected]")
# 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@[email protected]" 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
Expand Down
10 changes: 7 additions & 3 deletions testing/cpm/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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 )
Expand All @@ -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 )
Expand Down
2 changes: 1 addition & 1 deletion testing/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
8 changes: 6 additions & 2 deletions testing/utils/cmake_test.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ adds a test for each generator:
add_cmake_build_test( (config|build|test|install)
<SourceOrDir>
[SERIAL]
[NO_DEV_ERRORS]
[NO_CPM_CACHE]
[NO_RAPIDS_CMAKE_HOOKS]
[SHOULD_FAIL <expected error message string>]
Expand All @@ -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})
Expand All @@ -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}")
Expand Down

0 comments on commit e867d76

Please sign in to comment.