Fix <package>_SOURCE_DIR and _BINARY_DIR handling when caching is active (#314)

* fix: use CACHE variables instead relying on FetchContents mechanism

* apply cmake-format

Signed-off-by: DNKpp <DNKpp2011@gmail.com>

README.md

@@ -1,4 +1,3 @@
-[![Build Status](https://travis-ci.com/cpm-cmake/CPM.cmake.svg?branch=master)](https://travis-ci.com/cpm-cmake/CPM.cmake)
 [![Actions Status](https://github.com/cpm-cmake/CPM.cmake/workflows/MacOS/badge.svg)](https://github.com/cpm-cmake/CPM.cmake/actions)
 [![Actions Status](https://github.com/cpm-cmake/CPM.cmake/workflows/Windows/badge.svg)](https://github.com/cpm-cmake/CPM.cmake/actions)
 [![Actions Status](https://github.com/cpm-cmake/CPM.cmake/workflows/Ubuntu/badge.svg)](https://github.com/cpm-cmake/CPM.cmake/actions)
@@ -196,6 +195,11 @@ These options can also be set as environmental variables.
 
 In the case that `find_package` requires additional arguments, the parameter `FIND_PACKAGE_ARGUMENTS` may be specified in the `CPMAddPackage` call. The value of this parameter will be forwarded to `find_package`.
 
+### CPM_USE_NAMED_CACHE_DIRECTORIES
+
+If set, CPM use additional directory level in cache to improve readability of packages names in IDEs like CLion. It changes cache structure, so all dependencies are downloaded again. There is no problem to mix both structures in one cache directory but then there may be 2 copies of some dependencies.
+This can also be set as an environmental variable.
+
 ## Local package override
 
 Library developers are often in the situation where they work on a locally checked out dependency at the same time as on a consumer project.
@@ -270,11 +274,10 @@ See the [wiki](https://github.com/cpm-cmake/CPM.cmake/wiki/More-Snippets) for mo
 CPMAddPackage("gh:catchorg/Catch2@2.5.0")
 ```
 
-### [Boost (via boost-cmake)](https://github.com/Orphis/boost-cmake)
+### [Range-v3](https://github.com/ericniebler/range-v3)
 
-```CMake
+```Cmake
-# boost-cmake currently doesn't tag versions, so we use the according boost version
+CPMAddPackage("gh:ericniebler/range-v3#0.11.0")
-CPMAddPackage("gh:Orphis/boost-cmake#7f97a08b64bd5d2e53e932ddf80c40544cf45edf@1.71.0")
 ```
 
 ### [Yaml-cpp](https://github.com/jbeder/yaml-cpp)
@@ -284,12 +287,6 @@ CPMAddPackage("gh:Orphis/boost-cmake#7f97a08b64bd5d2e53e932ddf80c40544cf45edf@1.
 CPMAddPackage("gh:jbeder/yaml-cpp#yaml-cpp-0.6.3@0.6.3")
 ```
 
-### [Range-v3](https://github.com/ericniebler/range-v3)
-
-```Cmake
-CPMAddPackage("gh:ericniebler/range-v3#0.11.0")
-```
-
 ### [nlohmann/json](https://github.com/nlohmann/json)
 
 ```cmake
@@ -301,6 +298,19 @@ CPMAddPackage(
 )
 ```
 
+### [Boost ](https://github.com/boostorg/boost)
+
+```CMake
+# boost is a huge project and will take a while to download
+# using `CPM_SOURCE_CACHE` is strongly recommended
+CPMAddPackage(
+  NAME Boost
+  VERSION 1.77.0
+  GITHUB_REPOSITORY "boostorg/boost"
+  GIT_TAG "boost-1.77.0"
+)
+```
+
 ### [cxxopts](https://github.com/jarro2783/cxxopts)
 
 ```cmake

cmake/CPM.cmake

@@ -57,6 +57,13 @@ See https://github.com/cpm-cmake/CPM.cmake for more information."
   endif()
 endif()
 
+if(CURRENT_CPM_VERSION MATCHES "development-version")
+  message(WARNING "Your project is using an unstable development version of CPM.cmake. \
+Please update to a recent release if possible. \
+See https://github.com/cpm-cmake/CPM.cmake for details."
+  )
+endif()
+
 set_property(GLOBAL PROPERTY CPM_INITIALIZED true)
 
 option(CPM_USE_LOCAL_PACKAGES "Always try to use `find_package` to get dependencies"
@@ -76,6 +83,10 @@ option(CPM_INCLUDE_ALL_IN_PACKAGE_LOCK
        "Add all packages added through CPM.cmake to the package lock"
        $ENV{CPM_INCLUDE_ALL_IN_PACKAGE_LOCK}
 )
+option(CPM_USE_NAMED_CACHE_DIRECTORIES
+       "Use additional directory of package name in cache on the most nested level."
+       $ENV{CPM_USE_NAMED_CACHE_DIRECTORIES}
+)
 
 set(CPM_VERSION
     ${CURRENT_CPM_VERSION}
@@ -354,6 +365,68 @@ function(cpm_parse_add_package_single_arg arg outArgs)
   )
 endfunction()
 
+# Check that the working directory for a git repo is clean
+function(cpm_check_git_working_dir_is_clean repoPath gitTag isClean)
+
+  find_package(Git REQUIRED)
+
+  if(NOT GIT_EXECUTABLE)
+    # No git executable, assume directory is clean
+    set(${isClean}
+        TRUE
+        PARENT_SCOPE
+    )
+    return()
+  endif()
+
+  # check for uncommited changes
+  execute_process(
+    COMMAND ${GIT_EXECUTABLE} status --porcelain
+    RESULT_VARIABLE resultGitStatus
+    OUTPUT_VARIABLE repoStatus
+    OUTPUT_STRIP_TRAILING_WHITESPACE ERROR_QUIET
+    WORKING_DIRECTORY ${repoPath}
+  )
+  if(resultGitStatus)
+    # not supposed to happen, assume clean anyway
+    message(WARNING "Calling git status on folder ${repoPath} failed")
+    set(${isClean}
+        TRUE
+        PARENT_SCOPE
+    )
+    return()
+  endif()
+
+  if(NOT "${repoStatus}" STREQUAL "")
+    set(${isClean}
+        FALSE
+        PARENT_SCOPE
+    )
+    return()
+  endif()
+
+  # check for commited changes
+  execute_process(
+    COMMAND ${GIT_EXECUTABLE} diff -s --exit-code ${gitTag}
+    RESULT_VARIABLE resultGitDiff
+    OUTPUT_STRIP_TRAILING_WHITESPACE OUTPUT_QUIET
+    WORKING_DIRECTORY ${repoPath}
+  )
+
+  if(${resultGitDiff} EQUAL 0)
+    set(${isClean}
+        TRUE
+        PARENT_SCOPE
+    )
+  else()
+    set(${isClean}
+        FALSE
+        PARENT_SCOPE
+    )
+  endif()
+
+endfunction()
+
 # Download and add a package from source
 function(CPMAddPackage)
   list(LENGTH ARGN argnLength)
@@ -475,6 +548,7 @@ function(CPMAddPackage)
       EXCLUDE_FROM_ALL "${CPM_ARGS_EXCLUDE_FROM_ALL}"
       OPTIONS "${CPM_ARGS_OPTIONS}"
       SOURCE_SUBDIR "${CPM_ARGS_SOURCE_SUBDIR}"
+      DOWNLOAD_ONLY "${DOWNLOAD_ONLY}"
       FORCE True
     )
     cpm_export_variables(${CPM_ARGS_NAME})
@@ -533,17 +607,32 @@ function(CPMAddPackage)
     string(TOLOWER ${CPM_ARGS_NAME} lower_case_name)
     set(origin_parameters ${CPM_ARGS_UNPARSED_ARGUMENTS})
     list(SORT origin_parameters)
-    string(SHA1 origin_hash "${origin_parameters}")
+    if(CPM_USE_NAMED_CACHE_DIRECTORIES)
-    set(download_directory ${CPM_SOURCE_CACHE}/${lower_case_name}/${origin_hash})
+      string(SHA1 origin_hash "${origin_parameters};NEW_CACHE_STRUCTURE_TAG")
+      set(download_directory ${CPM_SOURCE_CACHE}/${lower_case_name}/${origin_hash}/${CPM_ARGS_NAME})
+    else()
+      string(SHA1 origin_hash "${origin_parameters}")
+      set(download_directory ${CPM_SOURCE_CACHE}/${lower_case_name}/${origin_hash})
+    endif()
     # Expand `download_directory` relative path. This is important because EXISTS doesn't work for
     # relative paths.
     get_filename_component(download_directory ${download_directory} ABSOLUTE)
     list(APPEND CPM_ARGS_UNPARSED_ARGUMENTS SOURCE_DIR ${download_directory})
     if(EXISTS ${download_directory})
-      # avoid FetchContent modules to improve performance
+      cpm_store_fetch_properties(
-      set(${CPM_ARGS_NAME}_BINARY_DIR ${CPM_FETCHCONTENT_BASE_DIR}/${lower_case_name}-build)
+        ${CPM_ARGS_NAME} "${download_directory}"
-      set(${CPM_ARGS_NAME}_ADDED YES)
+        "${CPM_FETCHCONTENT_BASE_DIR}/${lower_case_name}-build"
-      set(${CPM_ARGS_NAME}_SOURCE_DIR ${download_directory})
+      )
+      cpm_get_fetch_properties("${CPM_ARGS_NAME}")
+
+      if(DEFINED CPM_ARGS_GIT_TAG)
+        # warn if cache has been changed since checkout
+        cpm_check_git_working_dir_is_clean(${download_directory} ${CPM_ARGS_GIT_TAG} IS_CLEAN)
+        if(NOT ${IS_CLEAN})
+          message(WARNING "Cache for ${CPM_ARGS_NAME} (${download_directory}) is dirty")
+        endif()
+      endif()
+
       cpm_add_subdirectory(
         "${CPM_ARGS_NAME}" "${DOWNLOAD_ONLY}"
         "${${CPM_ARGS_NAME}_SOURCE_DIR}/${CPM_ARGS_SOURCE_SUBDIR}" "${${CPM_ARGS_NAME}_BINARY_DIR}"
@@ -567,7 +656,7 @@ function(CPMAddPackage)
     endif()
   endif()
 
-  cpm_create_module_file(${CPM_ARGS_NAME} "CPMAddPackage(${ARGN})")
+  cpm_create_module_file(${CPM_ARGS_NAME} "CPMAddPackage(\"${ARGN}\")")
 
   if(CPM_PACKAGE_LOCK_ENABLED)
     if((CPM_ARGS_VERSION AND NOT CPM_ARGS_SOURCE_DIR) OR CPM_INCLUDE_ALL_IN_PACKAGE_LOCK)
@@ -705,18 +794,32 @@ function(cpm_get_fetch_properties PACKAGE)
   if(${CPM_DRY_RUN})
     return()
   endif()
-  FetchContent_GetProperties(${PACKAGE})
+
-  string(TOLOWER ${PACKAGE} lpackage)
   set(${PACKAGE}_SOURCE_DIR
-      "${${lpackage}_SOURCE_DIR}"
+      "${CPM_PACKAGE_${PACKAGE}_SOURCE_DIR}"
       PARENT_SCOPE
   )
   set(${PACKAGE}_BINARY_DIR
-      "${${lpackage}_BINARY_DIR}"
+      "${CPM_PACKAGE_${PACKAGE}_BINARY_DIR}"
       PARENT_SCOPE
   )
 endfunction()
 
+function(cpm_store_fetch_properties PACKAGE source_dir binary_dir)
+  if(${CPM_DRY_RUN})
+    return()
+  endif()
+
+  set(CPM_PACKAGE_${PACKAGE}_SOURCE_DIR
+      "${source_dir}"
+      CACHE INTERNAL ""
+  )
+  set(CPM_PACKAGE_${PACKAGE}_BINARY_DIR
+      "${binary_dir}"
+      CACHE INTERNAL ""
+  )
+endfunction()
+
 # adds a package as a subdirectory if viable, according to provided options
 function(
   cpm_add_subdirectory
@@ -739,8 +842,8 @@ function(
       set(CMAKE_POLICY_DEFAULT_CMP0077 NEW)
 
       foreach(OPTION ${OPTIONS})
-        cpm_parse_option(${OPTION})
+        cpm_parse_option("${OPTION}")
-        set(${OPTION_KEY} ${OPTION_VALUE})
+        set(${OPTION_KEY} "${OPTION_VALUE}")
       endforeach()
     endif()
     set(CPM_OLD_INDENT "${CPM_INDENT}")
@@ -774,6 +877,10 @@ function(cpm_fetch_package PACKAGE populated)
     )
   endif()
 
+  cpm_store_fetch_properties(
+    ${CPM_ARGS_NAME} ${${lower_case_name}_SOURCE_DIR} ${${lower_case_name}_BINARY_DIR}
+  )
+
   set(${PACKAGE}_SOURCE_DIR
       ${${lower_case_name}_SOURCE_DIR}
       PARENT_SCOPE
@@ -786,15 +893,15 @@ endfunction()
 
 # splits a package option
 function(cpm_parse_option OPTION)
-  string(REGEX MATCH "^[^ ]+" OPTION_KEY ${OPTION})
+  string(REGEX MATCH "^[^ ]+" OPTION_KEY "${OPTION}")
-  string(LENGTH ${OPTION} OPTION_LENGTH)
+  string(LENGTH "${OPTION}" OPTION_LENGTH)
-  string(LENGTH ${OPTION_KEY} OPTION_KEY_LENGTH)
+  string(LENGTH "${OPTION_KEY}" OPTION_KEY_LENGTH)
   if(OPTION_KEY_LENGTH STREQUAL OPTION_LENGTH)
     # no value for key provided, assume user wants to set option to "ON"
     set(OPTION_VALUE "ON")
   else()
     math(EXPR OPTION_KEY_LENGTH "${OPTION_KEY_LENGTH}+1")
-    string(SUBSTRING ${OPTION} "${OPTION_KEY_LENGTH}" "-1" OPTION_VALUE)
+    string(SUBSTRING "${OPTION}" "${OPTION_KEY_LENGTH}" "-1" OPTION_VALUE)
   endif()
   set(OPTION_KEY
       "${OPTION_KEY}"

examples/boost/CMakeLists.txt

@@ -11,9 +11,11 @@ target_compile_features(CPMExampleBoost PRIVATE cxx_std_17)
 
 include(../../cmake/CPM.cmake)
 
-# boost-cmake currently doesn't tag versions, so we use the according boost version
+CPMAddPackage(
-CPMAddPackage("gh:Orphis/boost-cmake#7f97a08b64bd5d2e53e932ddf80c40544cf45edf@1.71.0")
+  NAME Boost
+  VERSION 1.77.0
+  GITHUB_REPOSITORY "boostorg/boost"
+  GIT_TAG "boost-1.77.0"
+)
 
-find_package(Threads REQUIRED)
+target_link_libraries(CPMExampleBoost PRIVATE Boost::asio)
-
-target_link_libraries(CPMExampleBoost PRIVATE Boost::system Threads::Threads)

test/unit/dirty-cache-check.cmake

@@ -0,0 +1,49 @@
+include(${CPM_PATH}/CPM.cmake)
+include(${CPM_PATH}/testing.cmake)
+
+set(baseDir "${CMAKE_CURRENT_BINARY_DIR}/test_dirty_cache")
+
+find_package(Git REQUIRED)
+
+function(git_do dir)
+  execute_process(
+    COMMAND ${GIT_EXECUTABLE} -c user.name='User' -c user.email='user@email.org' ${ARGN}
+    RESULT_VARIABLE result
+    OUTPUT_VARIABLE status
+    OUTPUT_STRIP_TRAILING_WHITESPACE
+    WORKING_DIRECTORY "${dir}"
+  )
+  if(result)
+    message(FATAL_ERROR "git ${ARGN} fail: ${result} ${status}")
+  endif()
+endfunction()
+
+file(MAKE_DIRECTORY "${baseDir}")
+
+file(WRITE "${baseDir}/draft.txt" "this is a test")
+
+git_do("${baseDir}" init -b main)
+git_do("${baseDir}" commit --allow-empty -m "empty repo")
+message(STATUS "empty repo with file")
+cpm_check_git_working_dir_is_clean(${baseDir} HEAD emptygit_test)
+assert_falsy(emptygit_test)
+
+git_do("${baseDir}" add draft.txt)
+git_do("${baseDir}" commit -m "test change")
+git_do("${baseDir}" tag v0.0.0)
+message(STATUS "commit a change")
+cpm_check_git_working_dir_is_clean(${baseDir} v0.0.0 onecommit_test)
+assert_truthy(onecommit_test)
+
+file(WRITE "${baseDir}/draft.txt" "a modification")
+message(STATUS "dirty repo")
+cpm_check_git_working_dir_is_clean(${baseDir} v0.0.0 nonemptygit_test)
+assert_falsy(nonemptygit_test)
+
+git_do("${baseDir}" add draft.txt)
+git_do("${baseDir}" commit -m "another change")
+message(STATUS "repo clean")
+cpm_check_git_working_dir_is_clean(${baseDir} v0.0.0 twocommit_test)
+assert_falsy(twocommit_test)
+
+file(REMOVE_RECURSE "${baseDir}")

test/unit/local_dependency/OptionsCMakeLists.txt.in

@@ -13,7 +13,9 @@ include(@CPM_PATH@/CPM.cmake)
 CPMAddPackage(
   NAME Dependency
   SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/dependency
-  OPTIONS "DEFINE_ALTERNATIVE_FUNCTION YES"
+  OPTIONS
+    "DEFINE_ALTERNATIVE_FUNCTION YES"
+    "LIST_ARGUMENT a\\\\;b\\\\;c"
   EXCLUDE_FROM_ALL YES
 )
 

test/unit/local_dependency/dependency/CMakeLists.txt

@@ -8,6 +8,12 @@ if(NOT DEFINE_ALTERNATIVE_FUNCTION)
     message("called external method")
   endfunction()
 else()
+
+  # check if list was passed correctly
+  if(NOT "${LIST_ARGUMENT}" STREQUAL "a;b;c")
+    message(FATAL_ERROR "list argument not properly passed to dependency: '${LIST_ARGUMENT}'")
+  endif()
+
   function(alternative_dependency_function)
     message("called alternative external method")
   endfunction()