From 873b2ad2ebaad2ca62c92259a02dcadb9201af1e Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Sat, 27 Jan 2024 11:06:38 +1100 Subject: ExternalProject: Remove N^2 add_dependencies() calls ExternalProject_Add_StepDependencies() contained a foreach() loop that had another foreach() loop inside it iterating over the same set of values (the dependencies). This resulted in add_dependencies() calls that added the same dependencies to the step target N^2 times. A single call to add_dependencies() with the list of dependencies provides the necessary relationships without the N^2 behavior, and it removes the inner foreach() loop. --- Modules/ExternalProject.cmake | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Modules/ExternalProject.cmake b/Modules/ExternalProject.cmake index 757b04e..2e82e1c 100644 --- a/Modules/ExternalProject.cmake +++ b/Modules/ExternalProject.cmake @@ -2746,12 +2746,10 @@ function(ExternalProject_Add_StepDependencies name step) OUTPUT ${stamp_file} DEPENDS ${dep} ) - if(TARGET ${name}-${step}) - foreach(dep ${dependencies}) - add_dependencies(${name}-${step} ${dep}) - endforeach() - endif() endforeach() + if(TARGET ${name}-${step}) + add_dependencies(${name}-${step} ${dependencies}) + endif() endfunction() -- cgit v0.12 From e72791ecf69fba58803e0a71b71af5339af98da9 Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Sat, 27 Jan 2024 11:07:36 +1100 Subject: ExternalProject: Update foreach() calls to use IN LISTS and IN ITEMS --- Modules/ExternalProject.cmake | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/Modules/ExternalProject.cmake b/Modules/ExternalProject.cmake index 2e82e1c..77566b3 100644 --- a/Modules/ExternalProject.cmake +++ b/Modules/ExternalProject.cmake @@ -1599,7 +1599,7 @@ function(_ep_write_downloadfile_script set(HTTP_HEADERS_ARGS "") if(NOT http_headers STREQUAL "") - foreach(header ${http_headers}) + foreach(header IN LISTS http_headers) string(PREPEND HTTP_HEADERS_ARGS "HTTPHEADER \"${header}\"\n " ) @@ -1724,7 +1724,7 @@ function(_ep_set_directories name) # Apply defaults and convert to absolute paths. set(places stamp download source binary install tmp) - foreach(var ${places}) + foreach(var IN LISTS places) string(TOUPPER "${var}" VAR) get_property(${var}_dir TARGET ${name} PROPERTY _EP_${VAR}_DIR) if(NOT ${var}_dir) @@ -1796,9 +1796,9 @@ endfunction() # macro(_ep_replace_location_tags target_name) set(vars ${ARGN}) - foreach(var ${vars}) - if(${var}) - foreach(dir + foreach(var IN LISTS vars) + if(var) + foreach(dir IN ITEMS SOURCE_DIR SOURCE_SUBDIR BINARY_DIR @@ -1828,7 +1828,7 @@ function(_ep_command_line_to_initial_cache if(force) set(forceArg "FORCE") endif() - foreach(line ${args}) + foreach(line IN LISTS args) if("${line}" MATCHES "^-D(.*)") set(line "${CMAKE_MATCH_1}") if(NOT "${setArg}" STREQUAL "") @@ -1884,7 +1884,7 @@ endfunction() function(ExternalProject_Get_Property name) - foreach(var ${ARGN}) + foreach(var IN LISTS ARGN) string(TOUPPER "${var}" VAR) get_property(is_set TARGET ${name} PROPERTY _EP_${VAR} SET) if(NOT is_set) @@ -2372,7 +2372,7 @@ function(ExternalProject_Add_StepTargets name) endif() message(AUTHOR_WARNING "${_cmp0114_warning}") endif() - foreach(step ${steps}) + foreach(step IN LISTS steps) _ep_step_add_target("${name}" "${step}" "${no_deps}") endforeach() endfunction() @@ -2553,7 +2553,7 @@ function(ExternalProject_Add_Step name step) get_property(_isMultiConfig GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG) if(_isMultiConfig) _ep_get_configuration_subdir_genex(cfgdir) - foreach(cfg ${CMAKE_CONFIGURATION_TYPES}) + foreach(cfg IN LISTS CMAKE_CONFIGURATION_TYPES) string(REPLACE "${cfgdir}" "/${cfg}" stamp_file_config "${stamp_file}" ) @@ -2628,7 +2628,7 @@ function(ExternalProject_Add_Step name step) PROPERTY EP_STEP_TARGETS ) endif() - foreach(st ${step_targets}) + foreach(st IN LISTS step_targets) if("${st}" STREQUAL "${step}") _ep_step_add_target("${name}" "${step}" "FALSE") break() @@ -2675,7 +2675,7 @@ function(ExternalProject_Add_Step name step) message(AUTHOR_WARNING "${_cmp0114_warning}") endif() endif() - foreach(st ${independent_step_targets}) + foreach(st IN LISTS independent_step_targets) if("${st}" STREQUAL "${step}") _ep_step_add_target("${name}" "${step}" "TRUE") break() @@ -2741,7 +2741,7 @@ function(ExternalProject_Add_StepDependencies name step) # Always add file-level dependency, but add target-level dependency # only if the target exists for that step. _ep_get_step_stampfile(${name} ${step} stamp_file) - foreach(dep ${dependencies}) + foreach(dep IN LISTS dependencies) add_custom_command(APPEND OUTPUT ${stamp_file} DEPENDS ${dep} @@ -3077,7 +3077,7 @@ hash=${hash} list(LENGTH url url_list_length) if(NOT "${url_list_length}" STREQUAL "1") - foreach(entry ${url}) + foreach(entry IN LISTS url) if(NOT "${entry}" MATCHES "^[a-z]+://") message(FATAL_ERROR "At least one entry of URL is a path (invalid in a list)" -- cgit v0.12 From aab6be9aad1e926ca2c0950b0abb7bf3c4b9105f Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Sat, 27 Jan 2024 11:44:01 +1100 Subject: ExternalProject: Catch empty REMOTE or LOCAL earlier If we are given an empty string for URL, or we have a logic error that leads to the file we download to being an empty string, we will now catch this at CMake configure time instead of whenever the download is attempted at build time. --- Modules/ExternalProject.cmake | 7 +++++++ Modules/ExternalProject/download.cmake.in | 8 -------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/Modules/ExternalProject.cmake b/Modules/ExternalProject.cmake index 77566b3..d7af1bd 100644 --- a/Modules/ExternalProject.cmake +++ b/Modules/ExternalProject.cmake @@ -1518,6 +1518,13 @@ function(_ep_write_downloadfile_script netrc netrc_file ) + if("x${REMOTE}" STREQUAL "x") + message(FATAL_ERROR "REMOTE can't be empty") + endif() + if("x${LOCAL}" STREQUAL "x") + message(FATAL_ERROR "LOCAL can't be empty") + endif() + if(timeout) set(TIMEOUT_ARGS TIMEOUT ${timeout}) set(TIMEOUT_MSG "${timeout} seconds") diff --git a/Modules/ExternalProject/download.cmake.in b/Modules/ExternalProject/download.cmake.in index bf7f209..261f782 100644 --- a/Modules/ExternalProject/download.cmake.in +++ b/Modules/ExternalProject/download.cmake.in @@ -71,14 +71,6 @@ function(sleep_before_download attempt) execute_process(COMMAND "${CMAKE_COMMAND}" -E sleep "${sleep_seconds}") endfunction() -if("@LOCAL@" STREQUAL "") - message(FATAL_ERROR "LOCAL can't be empty") -endif() - -if("@REMOTE@" STREQUAL "") - message(FATAL_ERROR "REMOTE can't be empty") -endif() - if(EXISTS "@LOCAL@") check_file_hash(has_hash hash_is_good) if(has_hash) -- cgit v0.12 From 54cb65b19757a15efda9fe29c8c864cbb28cdffe Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Sat, 27 Jan 2024 11:47:09 +1100 Subject: ExternalProject: Prevent URL list-splitting on special characters If a URL contains special characters like parentheses and a few others, they would previously have caused a foreach() call that iterates over the URLs to parse those special characters as separate, unquoted arguments. They would then have effectively split the list of URLs at unexpected places. Prepare the arguments for the foreach() call by using use bracket syntax to robustly handle any URLs that do have unescaped special characters. Issue: #25148 --- Modules/ExternalProject.cmake | 8 ++++++++ Modules/ExternalProject/download.cmake.in | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Modules/ExternalProject.cmake b/Modules/ExternalProject.cmake index d7af1bd..8b10135 100644 --- a/Modules/ExternalProject.cmake +++ b/Modules/ExternalProject.cmake @@ -1525,6 +1525,14 @@ function(_ep_write_downloadfile_script message(FATAL_ERROR "LOCAL can't be empty") endif() + # REMOTE could contain special characters that parse as separate arguments. + # Things like parentheses are legitimate characters in a URL, but would be + # seen as the start of a new unquoted argument by the cmake language parser. + # Avoid those special cases by preparing quoted strings for direct inclusion + # in the foreach() call that iterates over the set of URLs in REMOTE. + set(REMOTE "[====[${REMOTE}]====]") + string(REPLACE ";" "]====] [====[" REMOTE "${REMOTE}") + if(timeout) set(TIMEOUT_ARGS TIMEOUT ${timeout}) set(TIMEOUT_MSG "${timeout} seconds") diff --git a/Modules/ExternalProject/download.cmake.in b/Modules/ExternalProject/download.cmake.in index 261f782..0ad0dd3 100644 --- a/Modules/ExternalProject/download.cmake.in +++ b/Modules/ExternalProject/download.cmake.in @@ -107,7 +107,7 @@ foreach(i RANGE ${retry_number}) if(status_code IN_LIST download_retry_codes) sleep_before_download(${i}) endif() - foreach(url @REMOTE@) + foreach(url IN ITEMS @REMOTE@) if(NOT url IN_LIST skip_url_list) message(STATUS "Using src='${url}'") -- cgit v0.12