FetchContent: Enforce FETCHCONTENT_FULLY_DISCONNECTED requirements

FETCHCONTENT_FULLY_DISCONNECTED should only be set to true if each dependency's source directory has already been populated. Previously, this wasn't being checked, but now it is (subject to a new policy).
2025-12-31 02:39:48 -06:00 · 2024-05-28 22:56:25 +10:00
parent 4370fcf750
commit f588421b58
18 changed files with 178 additions and 6 deletions
--- a/Help/manual/cmake-policies.7.rst
+++ b/Help/manual/cmake-policies.7.rst
@@ -57,6 +57,7 @@ Policies Introduced by CMake 3.30
 .. toctree::
   :maxdepth: 1

+   CMP0170: FETCHCONTENT_FULLY_DISCONNECTED requirements are enforced. </policy/CMP0170>
   CMP0169: FetchContent_Populate(depName) single-argument signature is deprecated. </policy/CMP0169>
   CMP0168: FetchContent implements steps directly instead of through a sub-build. </policy/CMP0168>
   CMP0167: The FindBoost module is removed. </policy/CMP0167>
--- a/Help/policy/CMP0170.rst
+++ b/Help/policy/CMP0170.rst
@@ -0,0 +1,30 @@
+CMP0170
+-------
+
+.. versionadded:: 3.30
+
+When ``FETCHCONTENT_FULLY_DISCONNECTED`` is set to true,
+:command:`FetchContent_MakeAvailable` and :command:`FetchContent_Populate`
+enforce the constraint that their source directory must already be populated.
+The requirement has always been documented, but it was not checked or enforced
+with CMake 3.29 or older.  This sometimes led to hard-to-trace errors when a
+project expected a dependency to have been populated, but its population was
+silently skipped.
+
+CMake 3.30 and above prefers to check and enforce the constraint.
+This policy provides compatibility for situations where the user cannot easily
+prevent ``FETCHCONTENT_FULLY_DISCONNECTED`` from being inappropriately set
+to true.
+
+The ``OLD`` behavior of this policy allows ``FETCHCONTENT_FULLY_DISCONNECTED``
+to be set to true even if a dependency's source directory has not been
+populated.
+The ``NEW`` behavior halts with a fatal error if
+``FETCHCONTENT_FULLY_DISCONNECTED`` is set to true and a dependency population
+would be skipped, but that dependency's source directory doesn't exist.
+
+.. |INTRODUCED_IN_CMAKE_VERSION| replace:: 3.30
+.. |WARNS_OR_DOES_NOT_WARN| replace:: warns
+.. include:: STANDARD_ADVICE.txt
+
+.. include:: DEPRECATED.txt
--- a/Help/release/dev/enforce-fc-fully-disconnected-requirements.rst
+++ b/Help/release/dev/enforce-fc-fully-disconnected-requirements.rst
@@ -0,0 +1,9 @@
+enforce-fc-fully-disconnected-requirements
+------------------------------------------
+
+* When :variable:`FETCHCONTENT_FULLY_DISCONNECTED` is set to true,
+  :command:`FetchContent_MakeAvailable` and the single-argument form of
+  :command:`FetchContent_Populate` require that the dependency's source
+  directory has already been populated. CMake 3.29 and earlier did not
+  check this requirement, but it is now enforced, subject to policy
+  :policy:`CMP0170`.
--- a/Modules/FetchContent.cmake
+++ b/Modules/FetchContent.cmake
@@ -484,7 +484,8 @@ Commands
  - ``TEST_COMMAND``

  With this form, the :variable:`FETCHCONTENT_FULLY_DISCONNECTED` and
-  :variable:`FETCHCONTENT_UPDATES_DISCONNECTED` variables are ignored.
+  :variable:`FETCHCONTENT_UPDATES_DISCONNECTED` variables and policy
+  :policy:`CMP0170` are ignored.

  When this form of ``FetchContent_Populate()`` returns, the following
  variables will be set in the scope of the caller:
@@ -699,6 +700,11 @@ A number of cache variables can influence the behavior where details from a
    :ref:`dependency provider <dependency_providers>` and populate the
    dependency from local content instead.

+  .. versionchanged:: 3.30
+    The constraint that the source directory has already been populated when
+    ``FETCHCONTENT_FULLY_DISCONNECTED`` is true is now enforced.
+    See policy :policy:`CMP0170`.
+
 .. variable:: FETCHCONTENT_UPDATES_DISCONNECTED

  This is a less severe download/update control compared to
@@ -1937,8 +1943,12 @@ function(FetchContent_Populate contentName)
    endif()
  endif()

-  cmake_parse_arguments(PARSE_ARGV 0 __arg "" "" "")
-  set(__argsQuoted)
+  cmake_policy(GET CMP0170 cmp0170
+    PARENT_SCOPE # undocumented, do not use outside of CMake
+  )
+
+  cmake_parse_arguments(PARSE_ARGV 1 __arg "" "" "")
+  set(__argsQuoted "[==[${contentName}]==] [==[${cmp0170}]==]")
  foreach(__item IN LISTS __arg_UNPARSED_ARGUMENTS __doDirectArgs)
    string(APPEND __argsQuoted " [==[${__item}]==]")
  endforeach()
@@ -1955,7 +1965,7 @@ function(FetchContent_Populate contentName)

 endfunction()

-function(__FetchContent_Populate contentName)
+function(__FetchContent_Populate contentName cmp0170)

  string(TOLOWER ${contentName} contentNameLower)

@@ -2047,6 +2057,38 @@ function(__FetchContent_Populate contentName)
    else()
      set(${contentNameLower}_SOURCE_DIR "${FETCHCONTENT_BASE_DIR}/${contentNameLower}-src")
    endif()
+    if(NOT IS_ABSOLUTE "${${contentNameLower}_SOURCE_DIR}")
+      message(WARNING
+        "Relative source directory specified. This is not safe, as it depends "
+        "on the calling directory scope.\n"
+        "  ${${contentNameLower}_SOURCE_DIR}"
+      )
+      set(${contentNameLower}_SOURCE_DIR
+        "${CMAKE_CURRENT_BINARY_DIR}/${${contentNameLower}_SOURCE_DIR}"
+      )
+    endif()
+    if(NOT EXISTS "${${contentNameLower}_SOURCE_DIR}")
+      if(cmp0170 STREQUAL "")
+        set(cmp0170 WARN)
+      endif()
+      string(CONCAT msg
+        "FETCHCONTENT_FULLY_DISCONNECTED is set to true, which requires the "
+        "source directory for dependency ${contentName} to already be populated. "
+        "This generally means it must not be set to true the first time CMake "
+        "is run in a build directory. The following source directory should "
+        "already be populated, but it doesn't exist:\n"
+        "  ${${contentNameLower}_SOURCE_DIR}\n"
+        "Policy CMP0170 controls enforcement of this requirement."
+      )
+      if(cmp0170 STREQUAL "NEW")
+        message(FATAL_ERROR "${msg}")
+      elseif(NOT cmp0170 STREQUAL "OLD")
+        # Note that this is a user warning, not a project author warning.
+        # The user has set FETCHCONTENT_FULLY_DISCONNECTED in a scenario
+        # where that is not allowed.
+        message(WARNING "${msg}")
+      endif()
+    endif()

    if(savedDetails_BINARY_DIR)
      set(${contentNameLower}_BINARY_DIR ${savedDetails_BINARY_DIR})
@@ -2305,7 +2347,11 @@ macro(FetchContent_MakeAvailable)

    FetchContent_GetProperties(${__cmake_contentName})
    if(NOT ${__cmake_contentNameLower}_POPULATED)
-      __FetchContent_Populate(${__cmake_contentName})
+      cmake_policy(GET CMP0170 __cmake_fc_cmp0170
+        PARENT_SCOPE # undocumented, do not use outside of CMake
+      )
+      __FetchContent_Populate(${__cmake_contentName} "${__cmake_fc_cmp0170}")
+      unset(__cmake_fc_cmp0170)
      __FetchContent_setupFindPackageRedirection(${__cmake_contentName})

      # Only try to call add_subdirectory() if the populated content
--- a/Source/cmPolicies.h
+++ b/Source/cmPolicies.h
@@ -522,7 +522,10 @@ class cmMakefile;
  SELECT(POLICY, CMP0169,                                                     \
         "FetchContent_Populate(depName) single-argument signature is "       \
         "deprecated.",                                                       \
-         3, 30, 0, cmPolicies::WARN)
+         3, 30, 0, cmPolicies::WARN)                                          \
+  SELECT(POLICY, CMP0170,                                                     \
+         "FETCHCONTENT_FULLY_DISCONNECTED requirements are enforced.", 3, 30, \
+         0, cmPolicies::WARN)

 #define CM_SELECT_ID(F, A1, A2, A3, A4, A5, A6) F(A1)
 #define CM_FOR_EACH_POLICY_ID(POLICY)                                         \
--- a/Tests/RunCMake/CMP0170/CMP0170-NEW-result.txt
+++ b/Tests/RunCMake/CMP0170/CMP0170-NEW-result.txt
@@ -0,0 +1 @@
+1
--- a/Tests/RunCMake/CMP0170/CMP0170-NEW-stderr.txt
+++ b/Tests/RunCMake/CMP0170/CMP0170-NEW-stderr.txt
@@ -0,0 +1,14 @@
+CMake Error at .*/Modules/FetchContent\.cmake:[0-9]+ \(message\):
+  FETCHCONTENT_FULLY_DISCONNECTED is set to true, which requires the source
+  directory for dependency t1 to already be populated\.  This generally means
+  it must not be set to true the first time CMake is run in a build
+  directory\.  The following source directory should already be populated, but
+  it doesn't exist:[
+]+    .*/Tests/RunCMake/CMP0170/IdoNotExist[
+]+  Policy CMP0170 controls enforcement of this requirement\.
+Call Stack \(most recent call first\):
+  .*/Modules/FetchContent\.cmake:[0-9]+:EVAL:1 \(__FetchContent_Populate\)
+  .*/Modules/FetchContent\.cmake:[0-9]+ \(cmake_language\)
+  CMP0170\.cmake:[0-9]+ \(FetchContent_Populate\)
+  CMP0170-NEW\.cmake:[0-9]+ \(include\)
+  CMakeLists\.txt:[0-9]+ \(include\)
--- a/Tests/RunCMake/CMP0170/CMP0170-NEW-stdout.txt
+++ b/Tests/RunCMake/CMP0170/CMP0170-NEW-stdout.txt
@@ -0,0 +1,2 @@
+-- Starting population
+-- Configuring incomplete, errors occurred!
--- a/Tests/RunCMake/CMP0170/CMP0170-NEW.cmake
+++ b/Tests/RunCMake/CMP0170/CMP0170-NEW.cmake
@@ -0,0 +1,2 @@
+cmake_policy(SET CMP0170 NEW)
+include(CMP0170.cmake)
--- a/Tests/RunCMake/CMP0170/CMP0170-OLD-stdout.txt
+++ b/Tests/RunCMake/CMP0170/CMP0170-OLD-stdout.txt
@@ -0,0 +1,3 @@
+-- Starting population
+-- Configuring done \([0-9]+\.[0-9]s\)
+-- Generating done \([0-9]+\.[0-9]s\)
--- a/Tests/RunCMake/CMP0170/CMP0170-OLD.cmake
+++ b/Tests/RunCMake/CMP0170/CMP0170-OLD.cmake
@@ -0,0 +1,2 @@
+cmake_policy(SET CMP0170 OLD)
+include(CMP0170.cmake)
--- a/Tests/RunCMake/CMP0170/CMP0170-WARN-stderr.txt
+++ b/Tests/RunCMake/CMP0170/CMP0170-WARN-stderr.txt
@@ -0,0 +1,28 @@
+CMake Warning at .*/Modules/FetchContent\.cmake:[0-9]+ \(message\):
+  FETCHCONTENT_FULLY_DISCONNECTED is set to true, which requires the source
+  directory for dependency t1 to already be populated\.  This generally means
+  it must not be set to true the first time CMake is run in a build
+  directory\.  The following source directory should already be populated, but
+  it doesn't exist:[
+]+    .*/Tests/RunCMake/CMP0170/IdoNotExist[
+]+  Policy CMP0170 controls enforcement of this requirement\.
+Call Stack \(most recent call first\):
+  .*/Modules/FetchContent\.cmake:[0-9]+:EVAL:1 \(__FetchContent_Populate\)
+  .*/Modules/FetchContent\.cmake:[0-9]+ \(cmake_language\)
+  CMP0170\.cmake:[0-9]+ \(FetchContent_Populate\)
+  CMP0170-WARN\.cmake:[0-9]+ \(include\)
+  CMakeLists\.txt:[0-9]+ \(include\)
+[
+]+CMake Warning at .*/Modules/FetchContent\.cmake:[0-9]+ \(message\):
+  FETCHCONTENT_FULLY_DISCONNECTED is set to true, which requires the source
+  directory for dependency t2 to already be populated\.  This generally means
+  it must not be set to true the first time CMake is run in a build
+  directory\.  The following source directory should already be populated, but
+  it doesn't exist:[
+]+    .*/Tests/RunCMake/CMP0170/IdoNotExist[
+]+  Policy CMP0170 controls enforcement of this requirement\.
+Call Stack \(most recent call first\):
+  .*/Modules/FetchContent\.cmake:[0-9]+ \(__FetchContent_Populate\)
+  CMP0170\.cmake:[0-9]+ \(FetchContent_MakeAvailable\)
+  CMP0170-WARN\.cmake:[0-9]+ \(include\)
+  CMakeLists\.txt:[0-9]+ \(include\)
--- a/Tests/RunCMake/CMP0170/CMP0170-WARN-stdout.txt
+++ b/Tests/RunCMake/CMP0170/CMP0170-WARN-stdout.txt
@@ -0,0 +1,3 @@
+-- Starting population
+-- Configuring done \([0-9]+\.[0-9]s\)
+-- Generating done \([0-9]+\.[0-9]s\)
--- a/Tests/RunCMake/CMP0170/CMP0170-WARN.cmake
+++ b/Tests/RunCMake/CMP0170/CMP0170-WARN.cmake
@@ -0,0 +1 @@
+include(CMP0170.cmake)
--- a/Tests/RunCMake/CMP0170/CMP0170.cmake
+++ b/Tests/RunCMake/CMP0170/CMP0170.cmake
@@ -0,0 +1,18 @@
+cmake_policy(SET CMP0168 NEW)  # Faster, don't need to test with sub-build
+cmake_policy(SET CMP0169 OLD)  # So we can test FetchContent_Populate() directly
+
+set(FETCHCONTENT_FULLY_DISCONNECTED TRUE)
+
+include(FetchContent)
+
+message(STATUS "Starting population")
+
+FetchContent_Declare(t1
+  SOURCE_DIR "${CMAKE_CURRENT_LIST_DIR}/IdoNotExist"
+)
+FetchContent_Populate(t1)
+
+FetchContent_Declare(t2
+  SOURCE_DIR "${CMAKE_CURRENT_LIST_DIR}/IdoNotExist"
+)
+FetchContent_MakeAvailable(t2)
--- a/Tests/RunCMake/CMP0170/CMakeLists.txt
+++ b/Tests/RunCMake/CMP0170/CMakeLists.txt
@@ -0,0 +1,3 @@
+cmake_minimum_required(VERSION 3.29)
+project(${RunCMake_TEST} NONE)
+include(${RunCMake_TEST}.cmake)
--- a/Tests/RunCMake/CMP0170/RunCMakeTest.cmake
+++ b/Tests/RunCMake/CMP0170/RunCMakeTest.cmake
@@ -0,0 +1,5 @@
+include(RunCMake)
+
+run_cmake(CMP0170-WARN)
+run_cmake(CMP0170-OLD)
+run_cmake(CMP0170-NEW)
--- a/Tests/RunCMake/CMakeLists.txt
+++ b/Tests/RunCMake/CMakeLists.txt
@@ -176,6 +176,7 @@ add_RunCMake_test(CMP0160)
 add_RunCMake_test(CMP0163)
 add_RunCMake_test(CMP0165)
 add_RunCMake_test(CMP0169)
+add_RunCMake_test(CMP0170)

 # The test for Policy 65 requires the use of the
 # CMAKE_SHARED_LIBRARY_LINK_CXX_FLAGS variable, which both the VS and Xcode