From 0cc1b550ddc7cb677ce45fd54f6bbb92f3ff2268 Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Fri, 31 May 2024 17:38:10 +1000 Subject: ExternalProject,FetchContent: Avoid CMAKE_DISABLE_SOURCE_CHANGES error The file(MAKE_DIRECTORY) implementation checks whether a path is allowed to be written to before it checks if it already exists. For the scenario where a SOURCE_DIR is an existing directory within the main project's source directory, this triggers a fatal error if CMAKE_DISABLE_SOURCE_CHANGES is set to true for ExternalProject, and some FetchContent scenarios. Therefore, add an explicit check for existence first to avoid making such error-triggering calls. Fixes: #21872 --- Modules/ExternalProject/mkdirs.cmake.in | 7 ++++++- Modules/FetchContent.cmake | 7 ++++++- Tests/RunCMake/ExternalProject/RunCMakeTest.cmake | 1 + Tests/RunCMake/ExternalProject/SourceDirExisting.cmake | 16 ++++++++++++++++ Tests/RunCMake/FetchContent/DisableSourceChanges.cmake | 18 ++++++++++++++++++ Tests/RunCMake/FetchContent/RunCMakeTest.cmake | 2 ++ 6 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 Tests/RunCMake/ExternalProject/SourceDirExisting.cmake create mode 100644 Tests/RunCMake/FetchContent/DisableSourceChanges.cmake diff --git a/Modules/ExternalProject/mkdirs.cmake.in b/Modules/ExternalProject/mkdirs.cmake.in index bb835cf..03676a2 100644 --- a/Modules/ExternalProject/mkdirs.cmake.in +++ b/Modules/ExternalProject/mkdirs.cmake.in @@ -3,8 +3,13 @@ cmake_minimum_required(VERSION 3.5) +# If CMAKE_DISABLE_SOURCE_CHANGES is set to true and the source directory is an +# existing directory in our source tree, calling file(MAKE_DIRECTORY) on it +# would cause a fatal error, even though it would be a no-op. +if(NOT EXISTS "@source_dir@") + file(MAKE_DIRECTORY "@source_dir@") +endif() file(MAKE_DIRECTORY - "@source_dir@" "@binary_dir@" "@install_dir@" "@tmp_dir@" diff --git a/Modules/FetchContent.cmake b/Modules/FetchContent.cmake index 28541ed..01c9877 100644 --- a/Modules/FetchContent.cmake +++ b/Modules/FetchContent.cmake @@ -1657,8 +1657,13 @@ function(__FetchContent_populateDirect) set(_EP_TMP_DIR "${FETCHCONTENT_BASE_DIR}/${contentNameLower}-tmp") set(_EP_DOWNLOAD_DIR "${_EP_TMP_DIR}") + # If CMAKE_DISABLE_SOURCE_CHANGES is set to true and _EP_SOURCE_DIR is an + # existing directory in our source tree, calling file(MAKE_DIRECTORY) on it + # would cause a fatal error, even though it would be a no-op. + if(NOT EXISTS "${_EP_SOURCE_DIR}") + file(MAKE_DIRECTORY "${_EP_SOURCE_DIR}") + endif() file(MAKE_DIRECTORY - "${_EP_SOURCE_DIR}" "${_EP_BINARY_DIR}" "${_EP_STAMP_DIR}" "${_EP_TMP_DIR}" diff --git a/Tests/RunCMake/ExternalProject/RunCMakeTest.cmake b/Tests/RunCMake/ExternalProject/RunCMakeTest.cmake index 44c6f74..3c7cd68 100644 --- a/Tests/RunCMake/ExternalProject/RunCMakeTest.cmake +++ b/Tests/RunCMake/ExternalProject/RunCMakeTest.cmake @@ -21,6 +21,7 @@ run_cmake(TLSVersionBadEnv) run_cmake(NoOptions) run_cmake(SourceEmpty) run_cmake(SourceMissing) +run_cmake(SourceDirExisting) run_cmake(CMAKE_CACHE_ARGS) run_cmake(CMAKE_CACHE_DEFAULT_ARGS) run_cmake(CMAKE_CACHE_mix) diff --git a/Tests/RunCMake/ExternalProject/SourceDirExisting.cmake b/Tests/RunCMake/ExternalProject/SourceDirExisting.cmake new file mode 100644 index 0000000..761bb8e --- /dev/null +++ b/Tests/RunCMake/ExternalProject/SourceDirExisting.cmake @@ -0,0 +1,16 @@ +# We're providing a pre-existing source directory. Make sure we don't trigger +# an error if the undocumented but used-in-the-wild CMAKE_DISABLE_SOURCE_CHANGES +# variable is set. +set(CMAKE_DISABLE_SOURCE_CHANGES TRUE) + +include(ExternalProject) + +ExternalProject_Add(source_dir_existing + SOURCE_DIR "${CMAKE_CURRENT_LIST_DIR}/Foo" + DOWNLOAD_COMMAND "${CMAKE_COMMAND}" -E echo "Download command executed" + UPDATE_COMMAND "" + CONFIGURE_COMMAND "" + BUILD_COMMAND "" + TEST_COMMAND "" + INSTALL_COMMAND "" +) diff --git a/Tests/RunCMake/FetchContent/DisableSourceChanges.cmake b/Tests/RunCMake/FetchContent/DisableSourceChanges.cmake new file mode 100644 index 0000000..a144005 --- /dev/null +++ b/Tests/RunCMake/FetchContent/DisableSourceChanges.cmake @@ -0,0 +1,18 @@ +cmake_policy(SET CMP0168 NEW) + +# Undocumented variable used to catch attempts to write to anywhere under the +# source directory that isn't under the build directory. In order for this +# code path to be checked for direct population mode, we need a non-empty +# download, update, or patch command so that the population code path is used. +# Custom commands might not write to the source directory and instead just +# print messages or other non-modifying tasks, like is done here. +set(CMAKE_DISABLE_SOURCE_CHANGES TRUE) + +include(FetchContent) + +FetchContent_Declare( + WithProject + SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/WithProject # This exists + DOWNLOAD_COMMAND ${CMAKE_COMMAND} -E echo "Download command executed" +) +FetchContent_MakeAvailable(WithProject) diff --git a/Tests/RunCMake/FetchContent/RunCMakeTest.cmake b/Tests/RunCMake/FetchContent/RunCMakeTest.cmake index 72a458c..d0ec638 100644 --- a/Tests/RunCMake/FetchContent/RunCMakeTest.cmake +++ b/Tests/RunCMake/FetchContent/RunCMakeTest.cmake @@ -106,3 +106,5 @@ run_cmake_command(ScriptMode-direct -DCMP0168=NEW -P ${CMAKE_CURRENT_LIST_DIR}/ScriptMode.cmake ) + +run_cmake(DisableSourceChanges) -- cgit v0.12