From a49358ef3489cec222b28f92bfe5b99b2c63626d Mon Sep 17 00:00:00 2001 From: Lars Melchior Date: Thu, 13 Apr 2023 17:49:13 +0200 Subject: [PATCH 1/2] refactor empty argument passing to not require eval --- cmake/CPM.cmake | 123 ++++++++++++++++++------------ test/unit/forward_arguments.cmake | 27 +++---- test/unit/options.cmake | 2 + 3 files changed, 90 insertions(+), 62 deletions(-) diff --git a/cmake/CPM.cmake b/cmake/CPM.cmake index 68da0897..8ca270f0 100644 --- a/cmake/CPM.cmake +++ b/cmake/CPM.cmake @@ -505,51 +505,71 @@ function(cpm_override_fetchcontent contentName) set_property(GLOBAL PROPERTY ${propertyName} TRUE) endfunction() -macro(cpm_cmake_eval) - set(__ARGN "${ARGN}") - if(COMMAND cmake_language) - cmake_language(EVAL CODE "${__ARGN}") - else() - file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/eval.cmake "${__ARGN}") - include(${CMAKE_CURRENT_BINARY_DIR}/eval.cmake) - endif() -endmacro() +# replaces empty arguments with a placeholder to compensate CMake issues with handling empty +# arguments +function(cpm_encode_empty_arguments args outVar) + set(out "") + # note: we don't use string replacement for ';;' -> ';__CPM_EMPTY_ARG;' here, as it would + # interfere with nested arguments + foreach(ARG IN LISTS args) + if(NOT out STREQUAL "") + string(APPEND out ";") + endif() + if(ARG STREQUAL "") + string(APPEND out "__CPM_EMPTY_ARG") + else() + # prevent escaped characters from getting resolved early + string(REPLACE "\\" "\\\\\\" ARG "${ARG}") + string(APPEND out "${ARG}") + endif() + endforeach() + set("${outVar}" + "${out}" + PARENT_SCOPE + ) +endfunction() -# Download and add a package from source -macro(CPMAddPackage) - set(__ARGN "${ARGN}") - list(LENGTH __ARGN __ARGN_Length) - if(__ARGN_Length EQUAL 1) - cpm_add_package_single_arg(${ARGN}) +function(cpm_decode_empty_argument arg outVar) + if("${arg}" STREQUAL "__CPM_EMPTY_ARG") + set("${outVar}" + "" + PARENT_SCOPE + ) else() - # Forward preserving empty string arguments - # (https://gitlab.kitware.com/cmake/cmake/-/merge_requests/4729) - set(__ARGN_Quoted) - foreach(__ARG IN LISTS __ARGN) - string(APPEND __ARGN_Quoted " [==[${__ARG}]==]") - endforeach() - cpm_cmake_eval("cpm_add_package_multi_arg( ${__ARGN_Quoted} )") + set("${outVar}" + "${arg}" + PARENT_SCOPE + ) endif() -endmacro() - -macro(cpm_add_package_single_arg arg) - cpm_set_policies() - cpm_parse_add_package_single_arg("${arg}" __ARGN_multi) +endfunction() - # The shorthand syntax implies EXCLUDE_FROM_ALL - # cmake-format: off - list(APPEND __ARGN_multi - EXCLUDE_FROM_ALL YES - SYSTEM YES +# replaces placeholder arguments from `cpm_encode_empty_arguments` with empty arguments +function(cpm_decode_empty_arguments args outVar) + set(out "") + foreach(ARG IN LISTS args) + if(NOT out STREQUAL "") + string(APPEND out ";") + endif() + cpm_decode_empty_argument("${ARG}" ARG) + string(APPEND out "${ARG}") + endforeach() + set("${outVar}" + "${out}" + PARENT_SCOPE ) - # cmake-format: on - - cpm_add_package_multi_arg(${__ARGN_multi}) # Forward function arguments to CPMAddPackage() -endmacro() +endfunction() -function(cpm_add_package_multi_arg) +# Download and add a package from source +function(CPMAddPackage) cpm_set_policies() + list(LENGTH ARGN argnLength) + if(argnLength EQUAL 1) + cpm_parse_add_package_single_arg("${ARGN}" ARGN) + # The shorthand syntax implies EXCLUDE_FROM_ALL and SYSTEM + set(ARGN "${ARGN};EXCLUDE_FROM_ALL;YES;SYSTEM;YES;") + endif() + set(oneValueArgs NAME FORCE @@ -572,10 +592,26 @@ function(cpm_add_package_multi_arg) set(multiValueArgs URL OPTIONS) - cmake_parse_arguments(PARSE_ARGV 0 CPM_ARGS "" "${oneValueArgs}" "${multiValueArgs}") + # Encode arguments for `cmake_parse_arguments` + cpm_encode_empty_arguments("${ARGN}" "PARSE_ARGS") - # Set default values for arguments + # Parse arguments + cmake_parse_arguments(CPM_ARGS "" "${oneValueArgs}" "${multiValueArgs}" "${PARSE_ARGS}") + # Decode arguments + foreach(ARG IN LISTS oneValueArgs) + if(DEFINED CPM_ARGS_${ARG}) + cpm_decode_empty_argument("${CPM_ARGS_${ARG}}" CPM_ARGS_${ARG}) + endif() + endforeach() + foreach(ARG IN LISTS multiValueArgs) + if(DEFINED CPM_ARGS_${ARG}) + cpm_decode_empty_arguments("${CPM_ARGS_${ARG}}" CPM_ARGS_${ARG}) + endif() + endforeach() + cpm_decode_empty_arguments("${CPM_ARGS_UNPARSED_ARGUMENTS}" CPM_ARGS_UNPARSED_ARGUMENTS) + + # Set default values for arguments if(NOT DEFINED CPM_ARGS_VERSION) if(DEFINED CPM_ARGS_GIT_TAG) cpm_get_version_from_git_tag("${CPM_ARGS_GIT_TAG}" CPM_ARGS_VERSION) @@ -949,14 +985,7 @@ function(cpm_declare_fetch PACKAGE VERSION INFO) cpm_message(STATUS "${CPM_INDENT} Package not declared (dry run)") return() endif() - - # Forward preserving empty string arguments - # (https://gitlab.kitware.com/cmake/cmake/-/merge_requests/4729) - set(__argsQuoted) - foreach(__item IN LISTS ARGN) - string(APPEND __argsQuoted " [==[${__item}]==]") - endforeach() - cpm_cmake_eval("FetchContent_Declare(${PACKAGE} ${__argsQuoted} )") + FetchContent_Declare(${PACKAGE} "${ARGN}") endfunction() # returns properties for a package previously defined by cpm_declare_fetch diff --git a/test/unit/forward_arguments.cmake b/test/unit/forward_arguments.cmake index 90580f4c..47f3f12e 100644 --- a/test/unit/forward_arguments.cmake +++ b/test/unit/forward_arguments.cmake @@ -3,6 +3,18 @@ cmake_minimum_required(VERSION 3.14 FATAL_ERROR) include(${CPM_PATH}/CPM.cmake) include(${CPM_PATH}/testing.cmake) +set(input "a;;b;c;;;;def;g;;") +cpm_encode_empty_arguments("${input}" encoded) +foreach(arg IN LISTS encoded) + assert_not_equal("${arg}" "") +endforeach() +assert_equal("${contains_empty_arg}" "") +cpm_decode_empty_arguments("${encoded}" decoded) +assert_equal("${decoded}" "${input}") + +# ignore source cache if set +set(CPM_SOURCE_CACHE "") + # Intercept underlying `FetchContent_Declare` function(FetchContent_Declare) set_property(GLOBAL PROPERTY last_FetchContent_Declare_ARGN "${ARGN}") @@ -38,21 +50,6 @@ assert_equal( "fibonacci;EMPTY_OPTION;;COMMAND_WITH_EMPTY_ARG;foo;;bar;GIT_REPOSITORY;https://github.com/cpm-cmake/testpack-fibonacci.git;GIT_TAG;v1.2.3" ) -# Intercept underlying `cpm_add_package_multi_arg` -function(cpm_add_package_multi_arg) - set_property(GLOBAL PROPERTY last_cpm_add_package_multi_arg_ARGN "${ARGN}") -endfunction() - -# TEST: CPM Module file shall store all arguments including empty strings -include(${CPM_MODULE_PATH}/Findfibonacci.cmake) -get_property( - last_cpm_add_package_multi_arg_ARGN GLOBAL PROPERTY last_cpm_add_package_multi_arg_ARGN -) -assert_equal( - "${last_cpm_add_package_multi_arg_ARGN}" - "NAME;fibonacci;GIT_REPOSITORY;https://github.com/cpm-cmake/testpack-fibonacci.git;VERSION;1.2.3;EMPTY_OPTION;;COMMAND_WITH_EMPTY_ARG;foo;;bar" -) - # remove generated files file(REMOVE_RECURSE ${CPM_MODULE_PATH}) file(REMOVE ${CPM_PACKAGE_LOCK_FILE}) diff --git a/test/unit/options.cmake b/test/unit/options.cmake index 3c23e1a7..62ffc369 100644 --- a/test/unit/options.cmake +++ b/test/unit/options.cmake @@ -2,6 +2,8 @@ include(CMakePackageConfigHelpers) include(${CPM_PATH}/testing.cmake) set(TEST_BUILD_DIR ${CMAKE_CURRENT_BINARY_DIR}/source_dir) +# clean existing build if it exists +file(REMOVE_RECURSE "${TEST_BUILD_DIR}") set(TEST_DEPENDENCY_NAME Dependency) From 6b5cd6f19063ad37da93f1b465c8be27fa8311d7 Mon Sep 17 00:00:00 2001 From: Lars Melchior Date: Thu, 13 Apr 2023 17:58:34 +0200 Subject: [PATCH 2/2] add removed test cases --- test/unit/forward_arguments.cmake | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/unit/forward_arguments.cmake b/test/unit/forward_arguments.cmake index 47f3f12e..54d96d39 100644 --- a/test/unit/forward_arguments.cmake +++ b/test/unit/forward_arguments.cmake @@ -50,6 +50,20 @@ assert_equal( "fibonacci;EMPTY_OPTION;;COMMAND_WITH_EMPTY_ARG;foo;;bar;GIT_REPOSITORY;https://github.com/cpm-cmake/testpack-fibonacci.git;GIT_TAG;v1.2.3" ) +# Intercept underlying `cpm_add_package_multi_arg` +function(CPMAddPackage) + set_property(GLOBAL PROPERTY last_cpmaddpackage_argn "${ARGN}") +endfunction() + +# TEST: CPM Module file shall store all arguments including empty strings +include(${CPM_MODULE_PATH}/Findfibonacci.cmake) + +get_property(last_cpmaddpackage_argn GLOBAL PROPERTY last_cpmaddpackage_argn) +assert_equal( + "${last_cpmaddpackage_argn}" + "NAME;fibonacci;GIT_REPOSITORY;https://github.com/cpm-cmake/testpack-fibonacci.git;VERSION;1.2.3;EMPTY_OPTION;;COMMAND_WITH_EMPTY_ARG;foo;;bar" +) + # remove generated files file(REMOVE_RECURSE ${CPM_MODULE_PATH}) file(REMOVE ${CPM_PACKAGE_LOCK_FILE})