From 2a9cc3e8e8254eb1703e7e0931fe062814067454 Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Tue, 2 Aug 2022 22:42:08 +1000 Subject: FetchContent: Disable header set verification for dependencies The CMAKE_VERIFY_INTERFACE_HEADER_SETS variable is intended to be under the control of the user. It doesn't discriminate between header sets defined in the main project and those defined by dependencies brought into the build directly via FetchContent. Developers will usually only be interested in verifying the main project's header sets, not those from dependencies. Make the variable effectively only enable header set verification of the main project by turning it off during FetchContent_MakeAvailable() calls. The user still has variables like CMAKE_PROJECT_INCLUDE and CMAKE_PROJECT__INCLUDE available to them if they want to enable verification of all or specific dependencies respectively. Fixes: #23808 --- .../CMAKE_VERIFY_INTERFACE_HEADER_SETS.rst | 24 ++++++++++++++++++- Modules/FetchContent.cmake | 27 ++++++++++++++++++++++ Tests/RunCMake/FetchContent/RunCMakeTest.cmake | 1 + .../FetchContent/VerifyHeaderSet-stdout.txt | 4 ++++ Tests/RunCMake/FetchContent/VerifyHeaderSet.cmake | 16 +++++++++++++ .../FetchContent/VerifyHeaderSet/CMakeLists.txt | 9 ++++++++ Tests/RunCMake/FetchContent/VerifyHeaderSet/blah.h | 1 + 7 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 Tests/RunCMake/FetchContent/VerifyHeaderSet-stdout.txt create mode 100644 Tests/RunCMake/FetchContent/VerifyHeaderSet.cmake create mode 100644 Tests/RunCMake/FetchContent/VerifyHeaderSet/CMakeLists.txt create mode 100644 Tests/RunCMake/FetchContent/VerifyHeaderSet/blah.h diff --git a/Help/variable/CMAKE_VERIFY_INTERFACE_HEADER_SETS.rst b/Help/variable/CMAKE_VERIFY_INTERFACE_HEADER_SETS.rst index 6f14e6f..3fb8817 100644 --- a/Help/variable/CMAKE_VERIFY_INTERFACE_HEADER_SETS.rst +++ b/Help/variable/CMAKE_VERIFY_INTERFACE_HEADER_SETS.rst @@ -7,11 +7,33 @@ This variable is used to initialize the :prop_tgt:`VERIFY_INTERFACE_HEADER_SETS` property of targets when they are created. Setting it to true enables header set verification. -Projects should not set this variable, it is intended as a developer +Projects should not normally set this variable, it is intended as a developer control to be set on the :manual:`cmake(1)` command line or other equivalent methods. The developer must have the ability to enable or disable header set verification according to the capabilities of their own machine and compiler. +Verification of a dependency's header sets is not typically of interest +to developers. Therefore, :command:`FetchContent_MakeAvailable` explicitly +sets ``CMAKE_VERIFY_INTERFACE_HEADER_SETS`` to false for the duration of its +call, but restores its original value before returning. If a project brings +a dependency directly into the main build (e.g. calling +:command:`add_subdirectory` on a vendored project from a git submodule), it +should also do likewise. For example: + +.. code:: cmake + + # Save original setting so we can restore it later + set(want_header_set_verification ${CMAKE_VERIFY_INTERFACE_HEADER_SETS}) + + # Include the vendored dependency with header set verification disabled + set(CMAKE_VERIFY_INTERFACE_HEADER_SETS OFF) + add_subdirectory(...) # Vendored sources, e.g. from git submodules + + # Add the project's own sources. Restore the developer's original choice + # for whether to enable header set verification. + set(CMAKE_VERIFY_INTERFACE_HEADER_SETS ${want_header_set_verification}) + add_subdirectory(src) + By default, this variable is not set, which will result in header set verification being disabled. diff --git a/Modules/FetchContent.cmake b/Modules/FetchContent.cmake index 27070c0..d016fb5 100644 --- a/Modules/FetchContent.cmake +++ b/Modules/FetchContent.cmake @@ -364,6 +364,18 @@ Commands FetchContent_Declare(other ...) FetchContent_MakeAvailable(uses_other other) + Note that :variable:`CMAKE_VERIFY_INTERFACE_HEADER_SETS` is explicitly set + to false upon entry to ``FetchContent_MakeAvailable()``, and is restored to + its original value before the command returns. Developers typically only + want to verify header sets from the main project, not those from any + dependencies. This local manipulation of the + :variable:`CMAKE_VERIFY_INTERFACE_HEADER_SETS` variable provides that + intuitive behavior. You can use variables like + :variable:`CMAKE_PROJECT_INCLUDE` or + :variable:`CMAKE_PROJECT__INCLUDE` to turn verification back + on for all or some dependencies. You can also set the + :prop_tgt:`VERIFY_INTERFACE_HEADER_SETS` property of individual targets. + .. command:: FetchContent_Populate .. note:: @@ -1814,6 +1826,13 @@ endfunction() # calls will be available to the caller. macro(FetchContent_MakeAvailable) + # We must append an item, even if the variable is unset, so prefix its value. + # We will strip that prefix when we pop the value at the end of the macro. + list(APPEND __cmake_fcCurrentVarsStack + "__fcprefix__${CMAKE_VERIFY_INTERFACE_HEADER_SETS}" + ) + set(CMAKE_VERIFY_INTERFACE_HEADER_SETS FALSE) + get_property(__cmake_providerCommand GLOBAL PROPERTY __FETCHCONTENT_MAKEAVAILABLE_SERIAL_PROVIDER ) @@ -1965,10 +1984,18 @@ macro(FetchContent_MakeAvailable) endif() endforeach() + # Prefix will be "__fcprefix__" + list(POP_BACK __cmake_fcCurrentVarsStack __cmake_original_verify_setting) + string(SUBSTRING "${__cmake_original_verify_setting}" + 12 -1 __cmake_original_verify_setting + ) + set(CMAKE_VERIFY_INTERFACE_HEADER_SETS ${__cmake_original_verify_setting}) + # clear local variables to prevent leaking into the caller's scope unset(__cmake_contentName) unset(__cmake_contentNameLower) unset(__cmake_contentNameUpper) unset(__cmake_providerCommand) + unset(__cmake_original_verify_setting) endmacro() diff --git a/Tests/RunCMake/FetchContent/RunCMakeTest.cmake b/Tests/RunCMake/FetchContent/RunCMakeTest.cmake index e83c45e..a7ccf83 100644 --- a/Tests/RunCMake/FetchContent/RunCMakeTest.cmake +++ b/Tests/RunCMake/FetchContent/RunCMakeTest.cmake @@ -15,6 +15,7 @@ run_cmake(UsesTerminalOverride) run_cmake(MakeAvailable) run_cmake(MakeAvailableTwice) run_cmake(MakeAvailableUndeclared) +run_cmake(VerifyHeaderSet) run_cmake_with_options(ManualSourceDirectory -D "FETCHCONTENT_SOURCE_DIR_WITHPROJECT=${CMAKE_CURRENT_LIST_DIR}/WithProject" diff --git a/Tests/RunCMake/FetchContent/VerifyHeaderSet-stdout.txt b/Tests/RunCMake/FetchContent/VerifyHeaderSet-stdout.txt new file mode 100644 index 0000000..354529d --- /dev/null +++ b/Tests/RunCMake/FetchContent/VerifyHeaderSet-stdout.txt @@ -0,0 +1,4 @@ +-- Before subproject, var = 'TRUE' +-- Inside subproject, var = 'FALSE' +-- After subproject, var = 'TRUE' +-- Subproject target property VERIFY_INTERFACE_HEADER_SETS='FALSE' diff --git a/Tests/RunCMake/FetchContent/VerifyHeaderSet.cmake b/Tests/RunCMake/FetchContent/VerifyHeaderSet.cmake new file mode 100644 index 0000000..8adfcea --- /dev/null +++ b/Tests/RunCMake/FetchContent/VerifyHeaderSet.cmake @@ -0,0 +1,16 @@ +enable_language(C) + +set(CMAKE_VERIFY_INTERFACE_HEADER_SETS TRUE) + +include(FetchContent) +FetchContent_Declare(verify_subproj + SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/VerifyHeaderSet +) +message(STATUS "Before subproject, var = '${CMAKE_VERIFY_INTERFACE_HEADER_SETS}'") +FetchContent_MakeAvailable(verify_subproj) + +# Provide a way to verify the variable was reset back to its original value +message(STATUS "After subproject, var = '${CMAKE_VERIFY_INTERFACE_HEADER_SETS}'") + +get_property(verify TARGET Blah PROPERTY VERIFY_INTERFACE_HEADER_SETS) +message(STATUS "Subproject target property VERIFY_INTERFACE_HEADER_SETS='${verify}'") diff --git a/Tests/RunCMake/FetchContent/VerifyHeaderSet/CMakeLists.txt b/Tests/RunCMake/FetchContent/VerifyHeaderSet/CMakeLists.txt new file mode 100644 index 0000000..c6ba37e --- /dev/null +++ b/Tests/RunCMake/FetchContent/VerifyHeaderSet/CMakeLists.txt @@ -0,0 +1,9 @@ +cmake_minimum_required(VERSION 3.24) +project(VerifyHeaderSet LANGUAGES C) + +message(STATUS "Inside subproject, var = '${CMAKE_VERIFY_INTERFACE_HEADER_SETS}'") + +add_library(Blah INTERFACE) +target_sources(Blah + INTERFACE FILE_SET HEADERS FILES blah.h +) diff --git a/Tests/RunCMake/FetchContent/VerifyHeaderSet/blah.h b/Tests/RunCMake/FetchContent/VerifyHeaderSet/blah.h new file mode 100644 index 0000000..5b47e14 --- /dev/null +++ b/Tests/RunCMake/FetchContent/VerifyHeaderSet/blah.h @@ -0,0 +1 @@ +#error Header was used -- cgit v0.12