From ea6a8eb8959e06a576c1843750f03ce28c0b0121 Mon Sep 17 00:00:00 2001 From: Olivier Le Doeuff Date: Sun, 29 Aug 2021 22:02:18 +0200 Subject: [PATCH] Fix CPMAddPackage when FetchContent_MakeAvailable already added package (#288) Add fetchcontent_dependency unit test: * This test should highlight the fact that cpm_add_subdirectory is always called, even when cpm_fetch_package isn't populating the dependency * NO_CACHE YES highlight a bug introduced in 32b063eba5c754f833725ed4b9e5f352bc3ca959 where cpm_fetch_package was checking undefined ${lower_case_name}_POPULATED variable https://github.com/cpm-cmake/CPM.cmake/issues/287 --- cmake/CPM.cmake | 27 +++++++++---- test/unit/fetchcontent_dependency.cmake | 38 +++++++++++++++++++ test/unit/fetchcontent_dependency/.gitignore | 2 + .../fetchcontent_dependency/CMakeLists.txt.in | 38 +++++++++++++++++++ .../dependency/CMakeLists.txt | 15 ++++++++ 5 files changed, 112 insertions(+), 8 deletions(-) create mode 100644 test/unit/fetchcontent_dependency.cmake create mode 100644 test/unit/fetchcontent_dependency/.gitignore create mode 100644 test/unit/fetchcontent_dependency/CMakeLists.txt.in create mode 100644 test/unit/fetchcontent_dependency/dependency/CMakeLists.txt diff --git a/cmake/CPM.cmake b/cmake/CPM.cmake index 53592b8..6957543 100644 --- a/cmake/CPM.cmake +++ b/cmake/CPM.cmake @@ -587,12 +587,14 @@ function(CPMAddPackage) cpm_declare_fetch( "${CPM_ARGS_NAME}" "${CPM_ARGS_VERSION}" "${PACKAGE_INFO}" "${CPM_ARGS_UNPARSED_ARGUMENTS}" ) - cpm_fetch_package("${CPM_ARGS_NAME}") - cpm_add_subdirectory( - "${CPM_ARGS_NAME}" "${DOWNLOAD_ONLY}" - "${${CPM_ARGS_NAME}_SOURCE_DIR}/${CPM_ARGS_SOURCE_SUBDIR}" "${${CPM_ARGS_NAME}_BINARY_DIR}" - "${CPM_ARGS_EXCLUDE_FROM_ALL}" "${CPM_ARGS_OPTIONS}" - ) + cpm_fetch_package("${CPM_ARGS_NAME}" populated) + if(${populated}) + cpm_add_subdirectory( + "${CPM_ARGS_NAME}" "${DOWNLOAD_ONLY}" + "${${CPM_ARGS_NAME}_SOURCE_DIR}/${CPM_ARGS_SOURCE_SUBDIR}" "${${CPM_ARGS_NAME}_BINARY_DIR}" + "${CPM_ARGS_EXCLUDE_FROM_ALL}" "${CPM_ARGS_OPTIONS}" + ) + endif() cpm_get_fetch_properties("${CPM_ARGS_NAME}") endif() @@ -750,7 +752,11 @@ endfunction() # downloads a previously declared package via FetchContent and exports the variables # `${PACKAGE}_SOURCE_DIR` and `${PACKAGE}_BINARY_DIR` to the parent scope -function(cpm_fetch_package PACKAGE) +function(cpm_fetch_package PACKAGE populated) + set(${populated} + FALSE + PARENT_SCOPE + ) if(${CPM_DRY_RUN}) message(STATUS "${CPM_INDENT} package ${PACKAGE} not fetched (dry run)") return() @@ -758,11 +764,16 @@ function(cpm_fetch_package PACKAGE) FetchContent_GetProperties(${PACKAGE}) + string(TOLOWER "${PACKAGE}" lower_case_name) + if(NOT ${lower_case_name}_POPULATED) FetchContent_Populate(${PACKAGE}) + set(${populated} + TRUE + PARENT_SCOPE + ) endif() - string(TOLOWER "${PACKAGE}" lower_case_name) set(${PACKAGE}_SOURCE_DIR ${${lower_case_name}_SOURCE_DIR} PARENT_SCOPE diff --git a/test/unit/fetchcontent_dependency.cmake b/test/unit/fetchcontent_dependency.cmake new file mode 100644 index 0000000..f7ef150 --- /dev/null +++ b/test/unit/fetchcontent_dependency.cmake @@ -0,0 +1,38 @@ +cmake_minimum_required(VERSION 3.14 FATAL_ERROR) + +include(${CPM_PATH}/testing.cmake) +include(CMakePackageConfigHelpers) + +set(CPM_SOURCE_CACHE_DIR "${CMAKE_CURRENT_BINARY_DIR}/CPM") +set(TEST_BUILD_DIR ${CMAKE_CURRENT_BINARY_DIR}/fetchcontent_dependency) + +function(clear_cache) + message(STATUS "clearing CPM cache") + file(REMOVE_RECURSE ${CPM_SOURCE_CACHE_DIR}) + assert_not_exists("${CPM_SOURCE_CACHE_DIR}") +endfunction() + +function(update_cmake_lists) + configure_package_config_file( + "${CMAKE_CURRENT_LIST_DIR}/fetchcontent_dependency/CMakeLists.txt.in" + "${CMAKE_CURRENT_LIST_DIR}/fetchcontent_dependency/CMakeLists.txt" + INSTALL_DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/junk + ) +endfunction() + +function(reset_test) + clear_cache() + file(REMOVE_RECURSE ${TEST_BUILD_DIR}) + update_cmake_lists() +endfunction() + +# Read CPM_SOURCE_CACHE from arguments + +reset_test() + +execute_process( + COMMAND ${CMAKE_COMMAND} "-H${CMAKE_CURRENT_LIST_DIR}/fetchcontent_dependency" + "-B${TEST_BUILD_DIR}" "-DCPM_SOURCE_CACHE=${CPM_SOURCE_CACHE_DIR}" RESULT_VARIABLE ret +) + +assert_equal(${ret} "0") diff --git a/test/unit/fetchcontent_dependency/.gitignore b/test/unit/fetchcontent_dependency/.gitignore new file mode 100644 index 0000000..fe35cb3 --- /dev/null +++ b/test/unit/fetchcontent_dependency/.gitignore @@ -0,0 +1,2 @@ +/CMakeLists.txt +/package-lock.cmake diff --git a/test/unit/fetchcontent_dependency/CMakeLists.txt.in b/test/unit/fetchcontent_dependency/CMakeLists.txt.in new file mode 100644 index 0000000..9863b8f --- /dev/null +++ b/test/unit/fetchcontent_dependency/CMakeLists.txt.in @@ -0,0 +1,38 @@ +# ~~~ +# ┌────────────────────────┐ +# │ FetchContentDependency │ +# └─────┬────────────┬─────┘ +# │1. │3. +# │ │ +# ┌────────▼────┐ ┌───▼─────────┐ +# │ Dependency ├───► Fibonacci │ +# └─────────────┘2. └─────────────┘ +# +# 1. Add Project with CPMAddPackage +# 2. Dependency will add Fibonacci with FetchContent +# 3. Our project add Fibonacci with CPMAddPackage +# ~~~ + +cmake_minimum_required(VERSION 3.14 FATAL_ERROR) + +project(CPMTest_FetchContentDependency) + +# ---- Dependencies ---- + +include(@CPM_PATH@/CPM.cmake) + +# 1 & 2 Dependency will add Fibonacci using FetchContent (1 & 2) +CPMAddPackage(NAME Dependency SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/dependency) + +# 3 Add again Fibonacci that have already been populated with FetchContent_MakeAvailable +# +# * This test should highlight the fact that cpm_add_subdirectory is always called, even when +# cpm_fetch_package isn't populating the dependency +# * NO_CACHE YES highlight a bug introduced in 32b063eba5c754f833725ed4b9e5f352bc3ca959 where +# cpm_fetch_package was checking undefined ${lower_case_name}_POPULATED variable +CPMAddPackage( + NAME Fibonacci + GIT_REPOSITORY https://github.com/cpm-cmake/testpack-fibonacci.git + VERSION 2.0 + NO_CACHE YES +) diff --git a/test/unit/fetchcontent_dependency/dependency/CMakeLists.txt b/test/unit/fetchcontent_dependency/dependency/CMakeLists.txt new file mode 100644 index 0000000..b496fab --- /dev/null +++ b/test/unit/fetchcontent_dependency/dependency/CMakeLists.txt @@ -0,0 +1,15 @@ +cmake_minimum_required(VERSION 3.14 FATAL_ERROR) + +project(CPMTest_Dependency) + +# ---- Dependencies ---- + +include(FetchContent) + +FetchContent_Declare( + Fibonacci + GIT_REPOSITORY https://github.com/cpm-cmake/testpack-fibonacci.git + GIT_TAG v2.0 +) + +FetchContent_MakeAvailable(Fibonacci)