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 32b063eba5 where
  cpm_fetch_package was checking undefined ${lower_case_name}_POPULATED variable

https://github.com/cpm-cmake/CPM.cmake/issues/287

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_fetch_package("${CPM_ARGS_NAME}" populated)
-    cpm_add_subdirectory(
+    if(${populated})
-      "${CPM_ARGS_NAME}" "${DOWNLOAD_ONLY}"
+      cpm_add_subdirectory(
-      "${${CPM_ARGS_NAME}_SOURCE_DIR}/${CPM_ARGS_SOURCE_SUBDIR}" "${${CPM_ARGS_NAME}_BINARY_DIR}"
+        "${CPM_ARGS_NAME}" "${DOWNLOAD_ONLY}"
-      "${CPM_ARGS_EXCLUDE_FROM_ALL}" "${CPM_ARGS_OPTIONS}"
+        "${${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

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")

test/unit/fetchcontent_dependency/.gitignore

@@ -0,0 +1,2 @@
+/CMakeLists.txt
+/package-lock.cmake

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
+)

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)