Merge topic 'fetchcontent-externalproject-empty-args'

8dca6bd04b FetchContent: Preserve empty string arguments
cbf2daeed0 ExternalProject: Preserve empty string arguments

Acked-by: Kitware Robot <[email protected]>
Merge-request: !4729

Modules/ExternalProject.cmake

@@ -1787,7 +1787,11 @@ function(_ep_get_build_command name step cmd_var)
     get_target_property(args ${name} _EP_${step}_ARGS)
   endif()
 
-  list(APPEND cmd ${args})
+  if(NOT "${args}" STREQUAL "")
+    # args could have empty items, so we must quote it to prevent them
+    # from being silently removed
+    list(APPEND cmd "${args}")
+  endif()
   set(${cmd_var} "${cmd}" PARENT_SCOPE)
 endfunction()
 
@@ -2103,17 +2107,23 @@ function(ExternalProject_Add_Step name step)
     set(command ${CMAKE_COMMAND} -E echo_append)
   endif()
 
-  add_custom_command(
-    OUTPUT ${stamp_file}
-    BYPRODUCTS ${byproducts}
-    COMMENT ${comment}
-    COMMAND ${command}
-    COMMAND ${touch}
-    DEPENDS ${depends}
-    WORKING_DIRECTORY ${work_dir}
-    VERBATIM
-    ${uses_terminal}
-    )
+  set(__cmdQuoted)
+  foreach(__item IN LISTS command)
+    string(APPEND __cmdQuoted " [==[${__item}]==]")
+  endforeach()
+  cmake_language(EVAL CODE "
+    add_custom_command(
+      OUTPUT \${stamp_file}
+      BYPRODUCTS \${byproducts}
+      COMMENT \${comment}
+      COMMAND ${__cmdQuoted}
+      COMMAND \${touch}
+      DEPENDS \${depends}
+      WORKING_DIRECTORY \${work_dir}
+      VERBATIM
+      ${uses_terminal}
+    )"
+  )
   set_property(TARGET ${name} APPEND PROPERTY _EP_STEPS ${step})
 
   # Add custom "step target"?
@@ -2568,15 +2578,21 @@ function(_ep_add_download_command name)
     set(uses_terminal "")
   endif()
 
-  ExternalProject_Add_Step(${name} download
-    COMMENT ${comment}
-    COMMAND ${cmd}
-    WORKING_DIRECTORY ${work_dir}
-    DEPENDS ${depends}
-    DEPENDEES mkdir
-    ${log}
-    ${uses_terminal}
-    )
+  set(__cmdQuoted)
+  foreach(__item IN LISTS cmd)
+    string(APPEND __cmdQuoted " [==[${__item}]==]")
+  endforeach()
+  cmake_language(EVAL CODE "
+    ExternalProject_Add_Step(\${name} download
+      COMMENT \${comment}
+      COMMAND ${__cmdQuoted}
+      WORKING_DIRECTORY \${work_dir}
+      DEPENDS \${depends}
+      DEPENDEES mkdir
+      ${log}
+      ${uses_terminal}
+      )"
+  )
 endfunction()
 
 function(_ep_get_update_disconnected var name)
@@ -2725,16 +2741,22 @@ Update to Mercurial >= 2.1.1.
     set(uses_terminal "")
   endif()
 
-  ExternalProject_Add_Step(${name} update
-    COMMENT ${comment}
-    COMMAND ${cmd}
-    ALWAYS ${always}
-    EXCLUDE_FROM_MAIN ${update_disconnected}
-    WORKING_DIRECTORY ${work_dir}
-    DEPENDEES download
-    ${log}
-    ${uses_terminal}
-    )
+  set(__cmdQuoted)
+  foreach(__item IN LISTS cmd)
+    string(APPEND __cmdQuoted " [==[${__item}]==]")
+  endforeach()
+  cmake_language(EVAL CODE "
+    ExternalProject_Add_Step(${name} update
+      COMMENT \${comment}
+      COMMAND ${__cmdQuoted}
+      ALWAYS \${always}
+      EXCLUDE_FROM_MAIN \${update_disconnected}
+      WORKING_DIRECTORY \${work_dir}
+      DEPENDEES download
+      ${log}
+      ${uses_terminal}
+      )"
+  )
 
   if(update_disconnected)
     _ep_get_step_stampfile(${name} skip-update skip-update_stamp_file)
@@ -2780,12 +2802,18 @@ function(_ep_add_patch_command name)
     set(update_dep update)
   endif()
 
-  ExternalProject_Add_Step(${name} patch
-    COMMAND ${cmd}
-    WORKING_DIRECTORY ${work_dir}
-    DEPENDEES download ${update_dep}
-    ${log}
-    )
+  set(__cmdQuoted)
+  foreach(__item IN LISTS cmd)
+    string(APPEND __cmdQuoted " [==[${__item}]==]")
+  endforeach()
+  cmake_language(EVAL CODE "
+    ExternalProject_Add_Step(${name} patch
+      COMMAND ${__cmdQuoted}
+      WORKING_DIRECTORY \${work_dir}
+      DEPENDEES download \${update_dep}
+      ${log}
+      )"
+  )
 endfunction()
 
 
@@ -2945,14 +2973,20 @@ function(_ep_add_configure_command name)
     set(update_dep update)
   endif()
 
-  ExternalProject_Add_Step(${name} configure
-    COMMAND ${cmd}
-    WORKING_DIRECTORY ${binary_dir}
-    DEPENDEES ${update_dep} patch
-    DEPENDS ${file_deps}
-    ${log}
-    ${uses_terminal}
-    )
+  set(__cmdQuoted)
+  foreach(__item IN LISTS cmd)
+    string(APPEND __cmdQuoted " [==[${__item}]==]")
+  endforeach()
+  cmake_language(EVAL CODE "
+    ExternalProject_Add_Step(${name} configure
+      COMMAND ${__cmdQuoted}
+      WORKING_DIRECTORY \${binary_dir}
+      DEPENDEES \${update_dep} patch
+      DEPENDS \${file_deps}
+      ${log}
+      ${uses_terminal}
+      )"
+  )
 endfunction()
 
 
@@ -2990,15 +3024,21 @@ function(_ep_add_build_command name)
 
   get_property(build_byproducts TARGET ${name} PROPERTY _EP_BUILD_BYPRODUCTS)
 
-  ExternalProject_Add_Step(${name} build
-    COMMAND ${cmd}
-    BYPRODUCTS ${build_byproducts}
-    WORKING_DIRECTORY ${binary_dir}
-    DEPENDEES configure
-    ALWAYS ${always}
-    ${log}
-    ${uses_terminal}
-    )
+  set(__cmdQuoted)
+  foreach(__item IN LISTS cmd)
+    string(APPEND __cmdQuoted " [==[${__item}]==]")
+  endforeach()
+  cmake_language(EVAL CODE "
+    ExternalProject_Add_Step(${name} build
+      COMMAND ${__cmdQuoted}
+      BYPRODUCTS \${build_byproducts}
+      WORKING_DIRECTORY \${binary_dir}
+      DEPENDEES configure
+      ALWAYS \${always}
+      ${log}
+      ${uses_terminal}
+      )"
+  )
 endfunction()
 
 
@@ -3027,13 +3067,19 @@ function(_ep_add_install_command name)
     set(uses_terminal "")
   endif()
 
-  ExternalProject_Add_Step(${name} install
-    COMMAND ${cmd}
-    WORKING_DIRECTORY ${binary_dir}
-    DEPENDEES build
-    ${log}
-    ${uses_terminal}
-    )
+  set(__cmdQuoted)
+  foreach(__item IN LISTS cmd)
+    string(APPEND __cmdQuoted " [==[${__item}]==]")
+  endforeach()
+  cmake_language(EVAL CODE "
+    ExternalProject_Add_Step(${name} install
+      COMMAND ${__cmdQuoted}
+      WORKING_DIRECTORY \${binary_dir}
+      DEPENDEES build
+      ${log}
+      ${uses_terminal}
+      )"
+  )
 endfunction()
 
 
@@ -3088,15 +3134,21 @@ function(_ep_add_test_command name)
       set(uses_terminal "")
     endif()
 
-    ExternalProject_Add_Step(${name} test
-      COMMAND ${cmd}
-      WORKING_DIRECTORY ${binary_dir}
-      ${dependees_args}
-      ${dependers_args}
-      ${exclude_args}
-      ${log}
-      ${uses_terminal}
-      )
+    set(__cmdQuoted)
+    foreach(__item IN LISTS cmd)
+      string(APPEND __cmdQuoted " [==[${__item}]==]")
+    endforeach()
+    cmake_language(EVAL CODE "
+      ExternalProject_Add_Step(${name} test
+        COMMAND ${__cmdQuoted}
+        WORKING_DIRECTORY \${binary_dir}
+        ${dependees_args}
+        ${dependers_args}
+        ${exclude_args}
+        ${log}
+        ${uses_terminal}
+        )"
+    )
   endif()
 endfunction()
 

Modules/FetchContent.cmake

@@ -656,7 +656,12 @@ function(__FetchContent_declareDetails contentName)
       BRIEF_DOCS "Internal implementation detail of FetchContent_Populate()"
       FULL_DOCS  "Details used by FetchContent_Populate() for ${contentName}"
     )
-    set_property(GLOBAL PROPERTY ${propertyName} ${ARGN})
+    set(__cmdArgs)
+    foreach(__item IN LISTS ARGN)
+      string(APPEND __cmdArgs " [==[${__item}]==]")
+    endforeach()
+    cmake_language(EVAL CODE
+      "set_property(GLOBAL PROPERTY ${propertyName} ${__cmdArgs})")
   endif()
 
 endfunction()
@@ -689,7 +694,8 @@ function(FetchContent_Declare contentName)
   set(oneValueArgs SVN_REPOSITORY)
   set(multiValueArgs "")
 
-  cmake_parse_arguments(ARG "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
+  cmake_parse_arguments(PARSE_ARGV 1 ARG
+    "${options}" "${oneValueArgs}" "${multiValueArgs}")
 
   unset(srcDirSuffix)
   unset(svnRepoArgs)
@@ -707,13 +713,20 @@ function(FetchContent_Declare contentName)
   endif()
 
   string(TOLOWER ${contentName} contentNameLower)
-  __FetchContent_declareDetails(
-    ${contentNameLower}
-    SOURCE_DIR "${FETCHCONTENT_BASE_DIR}/${contentNameLower}-src${srcDirSuffix}"
-    BINARY_DIR "${FETCHCONTENT_BASE_DIR}/${contentNameLower}-build"
-    ${svnRepoArgs}
-    # List these last so they can override things we set above
-    ${ARG_UNPARSED_ARGUMENTS}
+
+  set(__argsQuoted)
+  foreach(__item IN LISTS ARG_UNPARSED_ARGUMENTS)
+    string(APPEND __argsQuoted " [==[${__item}]==]")
+  endforeach()
+  cmake_language(EVAL CODE "
+    __FetchContent_declareDetails(
+      ${contentNameLower}
+      SOURCE_DIR \"${FETCHCONTENT_BASE_DIR}/${contentNameLower}-src${srcDirSuffix}\"
+      BINARY_DIR \"${FETCHCONTENT_BASE_DIR}/${contentNameLower}-build\"
+      \${svnRepoArgs}
+      # List these last so they can override things we set above
+      ${__argsQuoted}
+    )"
   )
 
 endfunction()
@@ -844,7 +857,8 @@ function(__FetchContent_directPopulate contentName)
   )
   set(multiValueArgs "")
 
-  cmake_parse_arguments(ARG "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
+  cmake_parse_arguments(PARSE_ARGV 1 ARG
+    "${options}" "${oneValueArgs}" "${multiValueArgs}")
 
   if(NOT ARG_SUBBUILD_DIR)
     message(FATAL_ERROR "Internal error: SUBBUILD_DIR not set")
@@ -1056,17 +1070,23 @@ function(FetchContent_Populate contentName)
       message(FATAL_ERROR "No details have been set for content: ${contentName}")
     endif()
 
-    __FetchContent_directPopulate(
-      ${contentNameLower}
-      ${quietFlag}
-      UPDATE_DISCONNECTED ${disconnectUpdates}
-      SUBBUILD_DIR "${FETCHCONTENT_BASE_DIR}/${contentNameLower}-subbuild"
-      SOURCE_DIR   "${FETCHCONTENT_BASE_DIR}/${contentNameLower}-src"
-      BINARY_DIR   "${FETCHCONTENT_BASE_DIR}/${contentNameLower}-build"
-      # Put the saved details last so they can override any of the
-      # the options we set above (this can include SOURCE_DIR or
-      # BUILD_DIR)
-      ${contentDetails}
+    set(__detailsQuoted)
+    foreach(__item IN LISTS contentDetails)
+      string(APPEND __detailsQuoted " [==[${__item}]==]")
+    endforeach()
+    cmake_language(EVAL CODE "
+      __FetchContent_directPopulate(
+        ${contentNameLower}
+        ${quietFlag}
+        UPDATE_DISCONNECTED ${disconnectUpdates}
+        SUBBUILD_DIR \"${FETCHCONTENT_BASE_DIR}/${contentNameLower}-subbuild\"
+        SOURCE_DIR   \"${FETCHCONTENT_BASE_DIR}/${contentNameLower}-src\"
+        BINARY_DIR   \"${FETCHCONTENT_BASE_DIR}/${contentNameLower}-build\"
+        # Put the saved details last so they can override any of the
+        # the options we set above (this can include SOURCE_DIR or
+        # BUILD_DIR)
+        ${__detailsQuoted}
+      )"
     )
   endif()
 

Tests/RunCMake/ExternalProject/NO_DEPENDS-stderr.txt

@@ -2,7 +2,7 @@ CMake Warning \(dev\) at .*/Modules/ExternalProject.cmake:[0-9]+. \(message\):
   Using NO_DEPENDS for "configure" step might break parallel builds
 Call Stack \(most recent call first\):
   .*/Modules/ExternalProject.cmake:[0-9]+ \(ExternalProject_Add_StepTargets\)
-  .*/Modules/ExternalProject.cmake:[0-9]+ \(ExternalProject_Add_Step\)
+  .*/Modules/ExternalProject.cmake:[0-9]+:EVAL:2 \(ExternalProject_Add_Step\)
   .*/Modules/ExternalProject.cmake:[0-9]+ \(_ep_add_configure_command\)
   NO_DEPENDS.cmake:[0-9]+ \(ExternalProject_Add\)
   CMakeLists.txt:[0-9]+ \(include\)
@@ -12,7 +12,7 @@ CMake Warning \(dev\) at .*/Modules/ExternalProject.cmake:[0-9]+. \(message\):
   Using NO_DEPENDS for "build" step might break parallel builds
 Call Stack \(most recent call first\):
   .*/Modules/ExternalProject.cmake:[0-9]+ \(ExternalProject_Add_StepTargets\)
-  .*/Modules/ExternalProject.cmake:[0-9]+ \(ExternalProject_Add_Step\)
+  .*/Modules/ExternalProject.cmake:[0-9]+:EVAL:2 \(ExternalProject_Add_Step\)
   .*/Modules/ExternalProject.cmake:[0-9]+ \(_ep_add_build_command\)
   NO_DEPENDS.cmake:[0-9]+ \(ExternalProject_Add\)
   CMakeLists.txt:[0-9]+ \(include\)
@@ -22,7 +22,7 @@ CMake Warning \(dev\) at .*/Modules/ExternalProject.cmake:[0-9]+. \(message\):
   Using NO_DEPENDS for "install" step might break parallel builds
 Call Stack \(most recent call first\):
   .*/Modules/ExternalProject.cmake:[0-9]+ \(ExternalProject_Add_StepTargets\)
-  .*/Modules/ExternalProject.cmake:[0-9]+ \(ExternalProject_Add_Step\)
+  .*/Modules/ExternalProject.cmake:[0-9]+:EVAL:2 \(ExternalProject_Add_Step\)
   .*/Modules/ExternalProject.cmake:[0-9]+ \(_ep_add_install_command\)
   NO_DEPENDS.cmake:[0-9]+ \(ExternalProject_Add\)
   CMakeLists.txt:[0-9]+ \(include\)

Tests/RunCMake/ExternalProject/PreserveEmptyArgs-build-stdout.txt

@@ -0,0 +1,21 @@
+.*-- Number of arguments for download: 6
+.*-- download argument 4: ''
+.*-- download argument 5: 'after'
+.*-- Number of arguments for update: 6
+.*-- update argument 4: ''
+.*-- update argument 5: 'after'
+.*-- Number of arguments for patch: 6
+.*-- patch argument 4: ''
+.*-- patch argument 5: 'after'
+.*-- Number of arguments for configure: 6
+.*-- configure argument 4: ''
+.*-- configure argument 5: 'after'
+.*-- Number of arguments for build: 6
+.*-- build argument 4: ''
+.*-- build argument 5: 'after'
+.*-- Number of arguments for install: 6
+.*-- install argument 4: ''
+.*-- install argument 5: 'after'
+.*-- Number of arguments for test: 6
+.*-- test argument 4: ''
+.*-- test argument 5: 'after'

Tests/RunCMake/ExternalProject/PreserveEmptyArgs.cmake

@@ -0,0 +1,13 @@
+include(ExternalProject)
+
+set(script "${CMAKE_CURRENT_LIST_DIR}/countArgs.cmake")
+ExternalProject_Add(
+  blankChecker
+  DOWNLOAD_COMMAND  ${CMAKE_COMMAND} -P "${script}" download  "" after
+  UPDATE_COMMAND    ${CMAKE_COMMAND} -P "${script}" update    "" after
+  PATCH_COMMAND     ${CMAKE_COMMAND} -P "${script}" patch     "" after
+  CONFIGURE_COMMAND ${CMAKE_COMMAND} -P "${script}" configure "" after
+  BUILD_COMMAND     ${CMAKE_COMMAND} -P "${script}" build     "" after
+  INSTALL_COMMAND   ${CMAKE_COMMAND} -P "${script}" install   "" after
+  TEST_COMMAND      ${CMAKE_COMMAND} -P "${script}" test      "" after
+)

Tests/RunCMake/ExternalProject/RunCMakeTest.cmake

@@ -29,6 +29,10 @@ endfunction()
 
 __ep_test_with_build(MultiCommand)
 
+set(RunCMake_TEST_OUTPUT_MERGE 1)
+__ep_test_with_build(PreserveEmptyArgs)
+set(RunCMake_TEST_OUTPUT_MERGE 0)
+
 # Output is not predictable enough to be able to verify it reliably
 # when using the various different Visual Studio generators
 if(NOT RunCMake_GENERATOR MATCHES "Visual Studio")

Tests/RunCMake/ExternalProject/countArgs.cmake

@@ -0,0 +1,5 @@
+message(STATUS "Number of arguments for ${CMAKE_ARGV3}: ${CMAKE_ARGC}")
+math(EXPR last "${CMAKE_ARGC} - 1")
+foreach(n RANGE 4 ${last})
+  message(STATUS "${CMAKE_ARGV3} argument ${n}: '${CMAKE_ARGV${n}}'")
+endforeach()

Tests/RunCMake/FetchContent/PreserveEmptyArgs-stdout.txt

@@ -0,0 +1,4 @@
+.*-- Number of arguments: 6
+.*-- Argument 3: 'before'
+.*-- Argument 4: ''
+.*-- Argument 5: 'after'

Tests/RunCMake/FetchContent/PreserveEmptyArgs.cmake

@@ -0,0 +1,13 @@
+include(FetchContent)
+
+# Need to see the download command output
+set(FETCHCONTENT_QUIET OFF)
+
+FetchContent_Declare(
+  t1
+  DOWNLOAD_COMMAND ${CMAKE_COMMAND} -P
+                   ${CMAKE_CURRENT_LIST_DIR}/countArgs.cmake
+                   before "" after
+)
+
+FetchContent_Populate(t1)

Tests/RunCMake/FetchContent/RunCMakeTest.cmake

@@ -16,6 +16,10 @@ run_cmake(MakeAvailable)
 run_cmake(MakeAvailableTwice)
 run_cmake(MakeAvailableUndeclared)
 
+set(RunCMake_TEST_OUTPUT_MERGE 1)
+run_cmake(PreserveEmptyArgs)
+set(RunCMake_TEST_OUTPUT_MERGE 0)
+
 # We need to pass through CMAKE_GENERATOR and CMAKE_MAKE_PROGRAM
 # to ensure the test can run on machines where the build tool
 # isn't on the PATH. Some build slaves explicitly test with such

Tests/RunCMake/FetchContent/countArgs.cmake

@@ -0,0 +1,5 @@
+message(STATUS "Number of arguments: ${CMAKE_ARGC}")
+math(EXPR last "${CMAKE_ARGC} - 1")
+foreach(n RANGE 3 ${last})
+  message(STATUS "Argument ${n}: '${CMAKE_ARGV${n}}'")
+endforeach()