From d9c9aa3a22408d070339e0c1f04f46d311773549 Mon Sep 17 00:00:00 2001 From: ClausKlein Date: Tue, 12 Dec 2023 07:34:13 +0100 Subject: [PATCH] Parsing Arguments Robustly Prevent test errors if CPM_SOURCE_CACHE is set Use cmake_parse_arguments(PARSE_ARGV...) Add CMake workflow presets test --- .github/workflows/test.yaml | 7 +- cmake/CPM.cmake | 93 ++++++++++++++----- examples/fmt/CMakeLists.txt | 4 +- examples/gtest/CMakeLists.txt | 6 +- examples/spdlog/CMakeLists.txt | 18 ++-- test/CMakePresets.json | 88 ++++++++++++++++++ .../local_dependency/OptionsCMakeLists.txt.in | 12 ++- .../dependency/CMakeLists.txt | 10 +- test/unit/options.cmake | 2 +- 9 files changed, 195 insertions(+), 45 deletions(-) create mode 100644 test/CMakePresets.json diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 7b4eef8f..d1e6b579 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -7,6 +7,7 @@ on: pull_request: branches: - master + - develop jobs: tests: @@ -19,11 +20,11 @@ jobs: os: [ubuntu-latest, windows-2022, macos-latest] # we want to ensure compatibility with a recent CMake version as well as the lowest officially supported # legacy version that we define as the default version of the second-latest Ubuntu LTS release currently available - cmake_version: ['3.16.3', '3.27.5'] + cmake_version: ['3.16.9', '3.27.9'] exclude: - # there seems to be an issue with CMake 3.16 not finding a C++ compiler on windows-2022 + # there seems to be an issue with CMake 3.16 not finding a C++ compiler on windows-2022 - os: windows-2022 - cmake_version: '3.16.3' + cmake_version: '3.16.9' steps: - name: clone diff --git a/cmake/CPM.cmake b/cmake/CPM.cmake index f4d0eea4..25d537fd 100644 --- a/cmake/CPM.cmake +++ b/cmake/CPM.cmake @@ -188,7 +188,7 @@ include(FetchContent) # Try to infer package name from git repository uri (path or url) function(cpm_package_name_from_git_uri URI RESULT) - if("${URI}" MATCHES "([^/:]+)/?.git/?$") + if(URI MATCHES "([^/:]+)/?.git/?$") set(${RESULT} ${CMAKE_MATCH_1} PARENT_SCOPE @@ -280,11 +280,15 @@ endfunction() function(CPMFindPackage) set(oneValueArgs NAME VERSION GIT_TAG FIND_PACKAGE_ARGUMENTS) - cmake_parse_arguments(CPM_ARGS "" "${oneValueArgs}" "" ${ARGN}) + cmake_parse_arguments(PARSE_ARGV 0 CPM_ARGS "" "${oneValueArgs}" "") + if(CPM_ARGS_KEYWORDS_MISSING_VALUES) + message(ERROR "No values for ${CPM_ARGS_KEYWORDS_MISSING_VALUES} given!") + endif() 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) + message(TRACE "${CPM_ARGS_VERSION} set") endif() endif() @@ -312,7 +316,6 @@ function(CPMFindPackage) CPMAddPackage(${ARGN}) cpm_export_variables(${CPM_ARGS_NAME}) endif() - endfunction() # checks if a package has been added before @@ -345,18 +348,20 @@ endfunction() # to: GITHUB_REPOSITORY;foo/bar;VERSION;1.2.3 function(cpm_parse_add_package_single_arg arg outArgs) # Look for a scheme - if("${arg}" MATCHES "^([a-zA-Z]+):(.+)$") + message(DEBUG "cpm_parse_add_package_single_arg(${arg}) called") + if(arg MATCHES "^([a-zA-Z]+):(.+)$") string(TOLOWER "${CMAKE_MATCH_1}" scheme) set(uri "${CMAKE_MATCH_2}") + message(DEBUG "${scheme} ${uri} found") # Check for CPM-specific schemes - if(scheme STREQUAL "gh") + if(scheme STREQUAL gh) set(out "GITHUB_REPOSITORY;${uri}") - set(packageType "git") - elseif(scheme STREQUAL "gl") + set(packageType git) + elseif(scheme STREQUAL gl) set(out "GITLAB_REPOSITORY;${uri}") - set(packageType "git") - elseif(scheme STREQUAL "bb") + set(packageType git) + elseif(scheme STREQUAL bb) set(out "BITBUCKET_REPOSITORY;${uri}") set(packageType "git") # A CPM-specific scheme was not found. Looks like this is a generic URL so try to determine @@ -368,6 +373,7 @@ function(cpm_parse_add_package_single_arg arg outArgs) # Fall back to a URL set(out "URL;${arg}") set(packageType "archive") + message(WARNING "${out} Fall back to a URL!") # We could also check for SVN since FetchContent supports it, but SVN is so rare these days. # We just won't bother with the additional complexity it will induce in this function. SVN is @@ -386,14 +392,17 @@ function(cpm_parse_add_package_single_arg arg outArgs) # For all packages we interpret @... as version. Only replace the last occurrence. Thus URIs # containing '@' can be used string(REGEX REPLACE "@([^@]+)$" ";VERSION;\\1" out "${out}") + message(DEBUG "${out} set") # Parse the rest according to package type - if(packageType STREQUAL "git") + if(packageType STREQUAL git) # For git repos we interpret #... as a tag or branch or commit hash string(REGEX REPLACE "#([^#]+)$" ";GIT_TAG;\\1" out "${out}") - elseif(packageType STREQUAL "archive") + message(DEBUG "${out} set") + elseif(packageType STREQUAL archive) # For archives we interpret #... as a URL hash. string(REGEX REPLACE "#([^#]+)$" ";URL_HASH;\\1" out "${out}") + message(DEBUG "${out} set") # We don't try to parse the version if it's not provided explicitly. cpm_get_version_from_url # should do this at a later point else() @@ -467,7 +476,6 @@ function(cpm_check_git_working_dir_is_clean repoPath gitTag isClean) PARENT_SCOPE ) endif() - endfunction() # method to overwrite internal FetchContent properties, to allow using CPM.cmake to overload @@ -515,12 +523,22 @@ endfunction() function(CPMAddPackage) cpm_set_policies() - list(LENGTH ARGN argnLength) - if(argnLength EQUAL 1) - cpm_parse_add_package_single_arg("${ARGN}" ARGN) + if(ARGC EQUAL 1) + cpm_parse_add_package_single_arg("${ARGV}" PARSED_ARGS) # The shorthand syntax implies EXCLUDE_FROM_ALL and SYSTEM - set(ARGN "${ARGN};EXCLUDE_FROM_ALL;YES;SYSTEM;YES;") + unset(ARGV) + set(ARGV "${PARSED_ARGS};EXCLUDE_FROM_ALL;YES;SYSTEM;YES") + list(LENGTH ARGV ARGC) + + message(DEBUG "parsed ARGC: ${ARGC}") + set(n "0") + foreach(arg IN LISTS ARGV) + set(ARGV${n} ${arg}) + message(DEBUG "ARGV${n}: ${arg}") + math(EXPR n "${n}+1") + endforeach() + message(DEBUG "parsed ARGV: ${ARGV}") endif() set(oneValueArgs @@ -544,17 +562,25 @@ function(CPMAddPackage) set(multiValueArgs URL OPTIONS DOWNLOAD_COMMAND) - cmake_parse_arguments(CPM_ARGS "" "${oneValueArgs}" "${multiValueArgs}" "${ARGN}") + cmake_parse_arguments(PARSE_ARGV 0 CPM_ARGS "" "${oneValueArgs}" "${multiValueArgs}") + if(CPM_ARGS_KEYWORDS_MISSING_VALUES) + message(ERROR "No values for ${CPM_ARGS_KEYWORDS_MISSING_VALUES} given!") + endif() # Set default values for arguments + if(CPM_ARGS_UNPARSED_ARGUMENTS) + message(TRACE "unparsed arguments: ${CPM_ARGS_UNPARSED_ARGUMENTS}") + endif() 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) + message(TRACE "${CPM_ARGS_VERSION} set") endif() endif() if(CPM_ARGS_DOWNLOAD_ONLY) + message(TRACE "${CPM_ARGS_DOWNLOAD_ONLY} defined") set(DOWNLOAD_ONLY ${CPM_ARGS_DOWNLOAD_ONLY}) else() set(DOWNLOAD_ONLY NO) @@ -569,9 +595,11 @@ function(CPMAddPackage) endif() if(DEFINED CPM_ARGS_GIT_REPOSITORY) + message(TRACE "${CPM_ARGS_GIT_REPOSITORY} defined") list(APPEND CPM_ARGS_UNPARSED_ARGUMENTS GIT_REPOSITORY ${CPM_ARGS_GIT_REPOSITORY}) if(NOT DEFINED CPM_ARGS_GIT_TAG) set(CPM_ARGS_GIT_TAG v${CPM_ARGS_VERSION}) + message(TRACE "${CPM_ARGS_GIT_TAG} set") endif() # If a name wasn't provided, try to infer it from the git repo @@ -586,11 +614,13 @@ function(CPMAddPackage) list(APPEND CPM_ARGS_UNPARSED_ARGUMENTS GIT_TAG ${CPM_ARGS_GIT_TAG}) # If GIT_SHALLOW is explicitly specified, honor the value. if(DEFINED CPM_ARGS_GIT_SHALLOW) + message(TRACE "${CPM_ARGS_GIT_SHALLOW} defined") list(APPEND CPM_ARGS_UNPARSED_ARGUMENTS GIT_SHALLOW ${CPM_ARGS_GIT_SHALLOW}) endif() endif() if(DEFINED CPM_ARGS_URL) + message(TRACE "${CPM_ARGS_URL} defined") # If a name or version aren't provided, try to infer them from the URL list(GET CPM_ARGS_URL 0 firstUrl) cpm_package_name_and_ver_from_url(${firstUrl} nameFromUrl verFromUrl) @@ -600,9 +630,11 @@ function(CPMAddPackage) # If the caller provided their own name and version, they trump the inferred ones. if(NOT DEFINED CPM_ARGS_NAME) set(CPM_ARGS_NAME ${nameFromUrl}) + message(TRACE "${CPM_ARGS_NAME} set") endif() if(NOT DEFINED CPM_ARGS_VERSION) set(CPM_ARGS_VERSION ${verFromUrl}) + message(TRACE "${CPM_ARGS_VERSION} set") endif() list(APPEND CPM_ARGS_UNPARSED_ARGUMENTS URL "${CPM_ARGS_URL}") @@ -613,7 +645,7 @@ function(CPMAddPackage) if(NOT DEFINED CPM_ARGS_NAME) message( FATAL_ERROR - "${CPM_INDENT} 'NAME' was not provided and couldn't be automatically inferred for package added with arguments: '${ARGN}'" + "${CPM_INDENT} 'NAME' was not provided and couldn't be automatically inferred for package added with arguments: '${ARGV}'" ) endif() @@ -781,15 +813,15 @@ function(CPMAddPackage) endif() endif() - cpm_create_module_file(${CPM_ARGS_NAME} "CPMAddPackage(\"${ARGN}\")") + cpm_create_module_file(${CPM_ARGS_NAME} "CPMAddPackage(\"${ARGV}\")") if(CPM_PACKAGE_LOCK_ENABLED) if((CPM_ARGS_VERSION AND NOT CPM_ARGS_SOURCE_DIR) OR CPM_INCLUDE_ALL_IN_PACKAGE_LOCK) - cpm_add_to_package_lock(${CPM_ARGS_NAME} "${ARGN}") + cpm_add_to_package_lock(${CPM_ARGS_NAME} "${ARGV}") elseif(CPM_ARGS_SOURCE_DIR) cpm_add_comment_to_package_lock(${CPM_ARGS_NAME} "local directory") else() - cpm_add_comment_to_package_lock(${CPM_ARGS_NAME} "${ARGN}") + cpm_add_comment_to_package_lock(${CPM_ARGS_NAME} "${ARGV}") endif() endif() @@ -981,6 +1013,7 @@ function( foreach(OPTION ${OPTIONS}) cpm_parse_option("${OPTION}") set(${OPTION_KEY} "${OPTION_VALUE}") + message(WARNING "${OPTION_KEY} '${OPTION_VALUE}'") endforeach() endif() set(CPM_OLD_INDENT "${CPM_INDENT}") @@ -1028,8 +1061,15 @@ function(cpm_fetch_package PACKAGE populated) ) endfunction() +function(cpm_print_args) + message(DEBUG "ARGC = ${ARGC}") + message(DEBUG "ARGV = ${ARGV}") +endfunction() + # splits a package option function(cpm_parse_option OPTION) + cpm_print_args(OPTION) + string(REGEX MATCH "^[^ ]+" OPTION_KEY "${OPTION}") string(LENGTH "${OPTION}" OPTION_LENGTH) string(LENGTH "${OPTION_KEY}" OPTION_KEY_LENGTH) @@ -1078,7 +1118,7 @@ function(cpm_is_git_tag_commit_hash GIT_TAG RESULT) PARENT_SCOPE ) else() - if(${GIT_TAG} MATCHES "^[a-fA-F0-9]+$") + if(GIT_TAG MATCHES "^[a-fA-F0-9]+$") set(${RESULT} 1 PARENT_SCOPE @@ -1112,7 +1152,13 @@ function(cpm_prettify_package_arguments OUT_VAR IS_IN_COMMENT) SOURCE_SUBDIR ) set(multiValueArgs URL OPTIONS DOWNLOAD_COMMAND) - cmake_parse_arguments(CPM_ARGS "" "${oneValueArgs}" "${multiValueArgs}" ${ARGN}) + + cpm_print_args(${ARGV}) + + cmake_parse_arguments(PARSE_ARGV 2 CPM_ARGS "" "${oneValueArgs}" "${multiValueArgs}") + if(CPM_ARGS_KEYWORDS_MISSING_VALUES) + message(ERROR "No values for ${CPM_ARGS_KEYWORDS_MISSING_VALUES} given!") + endif() foreach(oneArgName ${oneValueArgs}) if(DEFINED CPM_ARGS_${oneArgName}) @@ -1157,5 +1203,4 @@ function(cpm_prettify_package_arguments OUT_VAR IS_IN_COMMENT) ${PRETTY_OUT_VAR} PARENT_SCOPE ) - endfunction() diff --git a/examples/fmt/CMakeLists.txt b/examples/fmt/CMakeLists.txt index 16699979..1de5e289 100644 --- a/examples/fmt/CMakeLists.txt +++ b/examples/fmt/CMakeLists.txt @@ -6,10 +6,10 @@ project(CPMFmtExample) include(../../cmake/CPM.cmake) -CPMAddPackage("gh:fmtlib/fmt#7.1.3") +CPMAddPackage("gh:fmtlib/fmt#10.0.0") # ---- Executable ---- add_executable(CPMFmtExample main.cpp) target_compile_features(CPMFmtExample PRIVATE cxx_std_17) -target_link_libraries(CPMFmtExample fmt) +target_link_libraries(CPMFmtExample fmt::fmt) diff --git a/examples/gtest/CMakeLists.txt b/examples/gtest/CMakeLists.txt index 19cf8c20..386fe968 100644 --- a/examples/gtest/CMakeLists.txt +++ b/examples/gtest/CMakeLists.txt @@ -11,15 +11,15 @@ CPMAddPackage("gh:cpm-cmake/testpack-fibonacci@2.0") CPMAddPackage( NAME googletest GITHUB_REPOSITORY google/googletest - GIT_TAG release-1.12.1 - VERSION 1.12.1 + # GIT_TAG v1.14.0 + VERSION 1.14.0 OPTIONS "INSTALL_GTEST OFF" "gtest_force_shared_crt" ) # ---- Create binary ---- add_executable(CPMExampleGtest main.cpp) -target_link_libraries(CPMExampleGtest fibonacci gtest gtest_main gmock) +target_link_libraries(CPMExampleGtest fibonacci GTest::gtest_main GTest::gmock) target_compile_features(CPMExampleGtest PRIVATE cxx_std_17) # ---- Enable testing ---- diff --git a/examples/spdlog/CMakeLists.txt b/examples/spdlog/CMakeLists.txt index 3777b34d..c36b7e6d 100644 --- a/examples/spdlog/CMakeLists.txt +++ b/examples/spdlog/CMakeLists.txt @@ -6,19 +6,21 @@ project(CPMSpdlogExample) include(../../cmake/CPM.cmake) -CPMAddPackage("gh:gabime/spdlog@1.8.2") +# CPMAddPackage("gh:gabime/spdlog@1.12.0") # spdlog uses fmt and bundles that dependency. If you want to use fmt in your project as well, you # can let spdlog reuse fmt from CPM.cmake like this: # # cmake-format: off # -# CPMAddPackage("gh:fmtlib/fmt#7.1.3") -# CPMAddPackage( -# GITHUB_REPOSITORY gabime/spdlog -# VERSION 1.8.2 -# OPTIONS "SPDLOG_FMT_EXTERNAL 1" -# ) +CPMAddPackage("gh:fmtlib/fmt#10.0.0") +CPMAddPackage( + GITHUB_REPOSITORY gabime/spdlog + VERSION 1.12.0 + OPTIONS "SPDLOG_FMT_EXTERNAL 1" + "SPDLOG_INSTALL 1" + SYSTEM ON +) # # cmake-format: on @@ -26,4 +28,4 @@ CPMAddPackage("gh:gabime/spdlog@1.8.2") add_executable(CPMSpdlogExample main.cpp) target_compile_features(CPMSpdlogExample PRIVATE cxx_std_17) -target_link_libraries(CPMSpdlogExample spdlog) +target_link_libraries(CPMSpdlogExample spdlog::spdlog) diff --git a/test/CMakePresets.json b/test/CMakePresets.json new file mode 100644 index 00000000..0751fdaa --- /dev/null +++ b/test/CMakePresets.json @@ -0,0 +1,88 @@ +{ + "version": 6, + "cmakeMinimumRequired": { + "major": 3, + "minor": 25, + "patch": 0 + }, + "configurePresets": [ + { + "name": "test", + "displayName": "Default test Config", + "description": "Default build using Ninja generator", + "generator": "Ninja", + "binaryDir": "${sourceDir}/build", + "installDir": "${sourceDir}/stagedir", + "cacheVariables": { + "CMAKE_PREFIX_PATH": { + "type": "path", + "value": "${sourceDir}/stagedir" + }, + "CMAKE_BUILD_TYPE": "Release", + "CMAKE_CXX_EXTENSIONS": false, + "CMAKE_CXX_STANDARD": "17", + "CMAKE_CXX_STANDARD_REQUIRED": true + }, + "environment": { + "CMAKE_EXPORT_COMPILE_COMMANDS": "YES", + "CPM_USE_LOCAL_PACKAGES": "NO", + "CPM_SOURCE_CACHE": "NO" + }, + "warnings": { + "deprecated": true, + "uninitialized": true + } + } + ], + "buildPresets": [ + { + "name": "test", + "configurePreset": "test", + "targets": [ + "check-format" + ] + } + ], + "testPresets": [ + { + "name": "test", + "configurePreset": "test", + "output": { + "outputOnFailure": true + }, + "execution": { + "noTestsAction": "error", + "stopOnFailure": false, + "rerun-failed": true + } + } + ], + "packagePresets": [ + { + "name": "test", + "configurePreset": "test", + "generators": [ + "TGZ" + ] + } + ], + "workflowPresets": [ + { + "name": "test", + "steps": [ + { + "type": "configure", + "name": "test" + }, + { + "type": "build", + "name": "test" + }, + { + "type": "test", + "name": "test" + } + ] + } + ] +} diff --git a/test/unit/local_dependency/OptionsCMakeLists.txt.in b/test/unit/local_dependency/OptionsCMakeLists.txt.in index 96accc9a..c9122fbf 100644 --- a/test/unit/local_dependency/OptionsCMakeLists.txt.in +++ b/test/unit/local_dependency/OptionsCMakeLists.txt.in @@ -10,12 +10,22 @@ option(ENABLE_TEST_COVERAGE "Enable test coverage" OFF) include(@CPM_PATH@/CPM.cmake) +function(print) + message(STATUS "ARGC = ${ARGC}") + message(STATUS "ARGV = ${ARGV}") +endfunction() + +set(LIST_VALUES "a;b;c") +set(ARGUMENT "LIST_ARGUMENT ${LIST_VALUES}") +print("${ARGUMENT}") + CPMAddPackage( NAME Dependency SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/dependency OPTIONS "DEFINE_ALTERNATIVE_FUNCTION YES" - "LIST_ARGUMENT a\\\\;b\\\\;c" + # FIXME: "LIST_ARGUMENT a\\\\;b\\\\;c" + "LIST_ARGUMENT ${LIST_VALUES}" EXCLUDE_FROM_ALL YES ) diff --git a/test/unit/local_dependency/dependency/CMakeLists.txt b/test/unit/local_dependency/dependency/CMakeLists.txt index 7d01e6ad..dfbbd279 100644 --- a/test/unit/local_dependency/dependency/CMakeLists.txt +++ b/test/unit/local_dependency/dependency/CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 3.0 FATAL_ERROR) +cmake_minimum_required(VERSION 3.5...3.14 FATAL_ERROR) option(DEFINE_ALTERNATIVE_FUNCTION "define the alternative function" OFF) option(LEAKED_OPTION "this option will be leaked to the outer scope" OFF) @@ -10,8 +10,12 @@ if(NOT DEFINE_ALTERNATIVE_FUNCTION) 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}'") + set(EXPECTED_LIST "a;b;c") + if(NOT LIST_ARGUMENT STREQUAL EXPECTED_LIST) + message( + FATAL_ERROR + "list argument not properly passed to dependency: '${LIST_ARGUMENT}' != '${EXPECTED_LIST}'" + ) endif() function(alternative_dependency_function) diff --git a/test/unit/options.cmake b/test/unit/options.cmake index 3c23e1a7..f6f392b4 100644 --- a/test/unit/options.cmake +++ b/test/unit/options.cmake @@ -13,7 +13,7 @@ configure_package_config_file( execute_process( COMMAND ${CMAKE_COMMAND} "-S${CMAKE_CURRENT_LIST_DIR}/local_dependency" "-B${TEST_BUILD_DIR}" - RESULT_VARIABLE ret + "-DCPM_SOURCE_CACHE=OFF" RESULT_VARIABLE ret ) assert_equal(${ret} "0")