Merge topic 'ep-url-special-characters'

54cb65b197 ExternalProject: Prevent URL list-splitting on special characters
aab6be9aad ExternalProject: Catch empty REMOTE or LOCAL earlier
e72791ecf6 ExternalProject: Update foreach() calls to use IN LISTS and IN ITEMS
873b2ad2eb ExternalProject: Remove N^2 add_dependencies() calls

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !9193

2 files changed, 32 insertions, 27 deletions

diff --git a/Modules/ExternalProject.cmake b/Modules/ExternalProject.cmake
index 757b04e..8b10135 100644
--- a/Modules/ExternalProject.cmake
+++ b/Modules/ExternalProject.cmake
@@ -1518,6 +1518,21 @@ 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()
+
+  # 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")
@@ -1599,7 +1614,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 +1739,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 +1811,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 +1843,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 +1899,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 +2387,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 +2568,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 +2643,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 +2690,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,17 +2756,15 @@ 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}
     )
-    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()
 
@@ -3079,7 +3092,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)"
diff --git a/Modules/ExternalProject/download.cmake.in b/Modules/ExternalProject/download.cmake.in
index bf7f209..0ad0dd3 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)
@@ -115,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}'")