Scope package options to avoid changing the local scope (#235)

* Fix #222

* Fix #222

* move policy change and local options to cpm_add_subdirectory

* change default policy as well

see https://gitlab.kitware.com/cmake/cmake/-/issues/20312

* add test

* update check to not use the NOT operator

(interestingly it works as expected locally)

* simplify test by not using options

* check options in tests

Co-authored-by: Lars Melchior <TheLartians@users.noreply.github.com>

cmake/CPM.cmake

@@ -132,7 +132,6 @@ if(NOT CPM_DONT_CREATE_PACKAGE_LOCK)
 endif()
 
 include(FetchContent)
-include(CMakeParseArguments)
 
 # Try to infer package name from git repository uri (path or url)
 function(cpm_package_name_from_git_uri URI RESULT)
@@ -516,16 +515,6 @@ function(CPMAddPackage)
 
   CPMRegisterPackage("${CPM_ARGS_NAME}" "${CPM_ARGS_VERSION}")
 
-  if(CPM_ARGS_OPTIONS)
-    foreach(OPTION ${CPM_ARGS_OPTIONS})
-      cpm_parse_option(${OPTION})
-      set(${OPTION_KEY}
-          ${OPTION_VALUE}
-          CACHE INTERNAL ""
-      )
-    endforeach()
-  endif()
-
   if(DEFINED CPM_ARGS_GIT_TAG)
     set(PACKAGE_INFO "${CPM_ARGS_GIT_TAG}")
   elseif(DEFINED CPM_ARGS_SOURCE_DIR)
@@ -553,7 +542,7 @@ function(CPMAddPackage)
       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_EXCLUDE_FROM_ALL}" "${CPM_ARGS_OPTIONS}"
       )
       set(CPM_SKIP_FETCH TRUE)
       set(PACKAGE_INFO "${PACKAGE_INFO} at ${download_directory}")
@@ -597,7 +586,7 @@ function(CPMAddPackage)
     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_EXCLUDE_FROM_ALL}" "${CPM_ARGS_OPTIONS}"
     )
     cpm_get_fetch_properties("${CPM_ARGS_NAME}")
   endif()
@@ -722,13 +711,31 @@ function(cpm_get_fetch_properties PACKAGE)
 endfunction()
 
 # adds a package as a subdirectory if viable, according to provided options
-function(cpm_add_subdirectory PACKAGE DOWNLOAD_ONLY SOURCE_DIR BINARY_DIR EXCLUDE)
+function(
+  cpm_add_subdirectory
+  PACKAGE
+  DOWNLOAD_ONLY
+  SOURCE_DIR
+  BINARY_DIR
+  EXCLUDE
+  OPTIONS
+)
   if(NOT DOWNLOAD_ONLY AND EXISTS ${SOURCE_DIR}/CMakeLists.txt)
     if(EXCLUDE)
       set(addSubdirectoryExtraArgs EXCLUDE_FROM_ALL)
     else()
       set(addSubdirectoryExtraArgs "")
     endif()
+    if(OPTIONS)
+      # the policy allows us to change options without caching
+      cmake_policy(SET CMP0077 NEW)
+      set(CMAKE_POLICY_DEFAULT_CMP0077 NEW)
+
+      foreach(OPTION ${OPTIONS})
+        cpm_parse_option(${OPTION})
+        set(${OPTION_KEY} ${OPTION_VALUE})
+      endforeach()
+    endif()
     set(CPM_OLD_INDENT "${CPM_INDENT}")
     set(CPM_INDENT "${CPM_INDENT} ${PACKAGE}:")
     add_subdirectory(${SOURCE_DIR} ${BINARY_DIR} ${addSubdirectoryExtraArgs})

cmake/testing.cmake

@@ -38,7 +38,7 @@ endfunction()
 
 function(ASSERT_NOT_DEFINED KEY)
   if(DEFINED ${KEY})
-    message(FATAL_ERROR "assertion failed: '${KEY}' is defiend")
+    message(FATAL_ERROR "assertion failed: '${KEY}' is defiend (${${KEY}})")
   else()
     message(STATUS "test passed: '${KEY}' is not defined")
   endif()
@@ -48,7 +48,15 @@ function(ASSERT_TRUTHY KEY)
   if(${${KEY}})
     message(STATUS "test passed: '${KEY}' is set truthy")
   else()
-    message(FATAL_ERROR "assertion failed: value of '${KEY}' is not true (${${KEY}})")
+    message(FATAL_ERROR "assertion failed: value of '${KEY}' is not truthy (${${KEY}})")
+  endif()
+endfunction()
+
+function(ASSERT_FALSY KEY)
+  if(${${KEY}})
+    message(FATAL_ERROR "assertion failed: value of '${KEY}' is not falsy (${${KEY}})")
+  else()
+    message(STATUS "test passed: '${KEY}' is set falsy")
   endif()
 endfunction()
 

test/unit/local_dependency/OptionsCMakeLists.txt.in

@@ -0,0 +1,33 @@
+cmake_minimum_required(VERSION 3.14 FATAL_ERROR)
+
+project(CPMTest)
+
+# ---- Options ----
+
+option(ENABLE_TEST_COVERAGE "Enable test coverage" OFF)
+
+# ---- Dependencies ----
+
+include(@CPM_PATH@/CPM.cmake)
+
+CPMAddPackage(
+  NAME Dependency
+  SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/dependency
+  OPTIONS "DEFINE_ALTERNATIVE_FUNCTION YES"
+  EXCLUDE_FROM_ALL YES
+)
+
+# ---- Dependencies ----
+
+alternative_dependency_function()
+
+# ---- Check parameters ----
+
+include(@CPM_PATH@/testing.cmake)
+
+message("DEFINE_ALTERNATIVE_FUNCTION: ${DEFINE_ALTERNATIVE_FUNCTION}")
+
+# this option is overridden by CPM.cmake
+ASSERT_NOT_DEFINED(DEFINE_ALTERNATIVE_FUNCTION)
+# this option is leaked by the dependency
+ASSERT_EQUAL(${LEAKED_OPTION} "OFF")

test/unit/local_dependency/dependency/CMakeLists.txt

@@ -1,3 +1,14 @@
-function(dependency_function)
-  message("called external method")
-endfunction()
+cmake_minimum_required(VERSION 3.0 FATAL_ERROR)
+
+option(DEFINE_ALTERNATIVE_FUNCTION "define the alternative function" OFF)
+option(LEAKED_OPTION "this option will be leaked to the outer scope" OFF)
+
+if(NOT DEFINE_ALTERNATIVE_FUNCTION)
+  function(dependency_function)
+    message("called external method")
+  endfunction()
+else()
+  function(alternative_dependency_function)
+    message("called alternative external method")
+  endfunction()
+endif()

test/unit/options.cmake

@@ -0,0 +1,19 @@
+include(CMakePackageConfigHelpers)
+include(${CPM_PATH}/testing.cmake)
+
+set(TEST_BUILD_DIR ${CMAKE_CURRENT_BINARY_DIR}/source_dir)
+
+set(TEST_DEPENDENCY_NAME Dependency)
+
+configure_package_config_file(
+  "${CMAKE_CURRENT_LIST_DIR}/local_dependency/OptionsCMakeLists.txt.in"
+  "${CMAKE_CURRENT_LIST_DIR}/local_dependency/CMakeLists.txt"
+  INSTALL_DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/junk
+)
+
+execute_process(
+  COMMAND ${CMAKE_COMMAND} "-H${CMAKE_CURRENT_LIST_DIR}/local_dependency" "-B${TEST_BUILD_DIR}"
+  RESULT_VARIABLE ret
+)
+
+assert_equal(${ret} "0")