ExternalData: Preserve escaped semicolons during argument expansion

The CMake language implicitly flattens lists so a ";" in a list element
must be escaped with a backslash.  List expansion removes backslashes
escaping semicolons to leave raw semicolons in the values.  Teach
ExternalData_Add_Test and ExternalData_Expand_Arguments to re-escape
semicolons found in list elements so the resulting argument lists work
as if constructed directly by the set() command.

For example:

  ExternalData_Add_Test(Data NAME test1 COMMAND ... "a\\;b")
  ExternalData_Expand_Arguments(Data args2 "c\\;d")
  add_test(NAME test2 COMMAND ... ${args2})

should be equivalent to

  set(args1 "a\\;b")
  add_test(NAME test1 COMMAND ... ${args1})
  set(args2 "c\\;d")
  add_test(NAME test2 COMMAND ... ${args2})

which is equivalent to

  add_test(NAME test1 COMMAND ... "a;b")
  add_test(NAME test2 COMMAND ... "c;d")

Note that it is not possible to make ExternalData_Add_Test act exactly
like add_test when quoted arguments contain semicolons because the CMake
language flattens lists when constructing function ARGN values.  This
re-escape approach at least allows test arguments to have semicolons.

While at it, teach ExternalData APIs to not transform "DATA{...;...}"
arguments because the contained semicolons are non-sensical.

Suggested-by: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com>

10 files changed, 69 insertions, 5 deletions

diff --git a/Modules/ExternalData.cmake b/Modules/ExternalData.cmake
index 9d84f8d..187f408 100644
--- a/Modules/ExternalData.cmake
+++ b/Modules/ExternalData.cmake
@@ -156,7 +156,8 @@
 #  License text for the above reference.)
 
 function(ExternalData_add_test target)
-  ExternalData_expand_arguments("${target}" testArgs ${ARGN})
+  # Expand all arguments as a single string to preserve escaped semicolons.
+  ExternalData_expand_arguments("${target}" testArgs "${ARGN}")
   add_test(${testArgs})
 endfunction()
 
@@ -234,13 +235,17 @@ endfunction()
 
 function(ExternalData_expand_arguments target outArgsVar)
   # Replace DATA{} references with real arguments.
-  set(data_regex "DATA{([^{}\r\n]*)}")
+  set(data_regex "DATA{([^;{}\r\n]*)}")
   set(other_regex "([^D]|D[^A]|DA[^T]|DAT[^A]|DATA[^{])+|.")
   set(outArgs "")
+  # This list expansion un-escapes semicolons in list element values so we
+  # must re-escape them below anywhere a new list expansion will occur.
   foreach(arg IN LISTS ARGN)
     if("x${arg}" MATCHES "${data_regex}")
+      # Re-escape in-value semicolons before expansion in foreach below.
+      string(REPLACE ";" "\\;" tmp "${arg}")
       # Split argument into DATA{}-pieces and other pieces.
-      string(REGEX MATCHALL "${data_regex}|${other_regex}" pieces "${arg}")
+      string(REGEX MATCHALL "${data_regex}|${other_regex}" pieces "${tmp}")
       # Compose output argument with DATA{}-pieces replaced.
       set(outArg "")
       foreach(piece IN LISTS pieces)
@@ -254,11 +259,13 @@ function(ExternalData_expand_arguments target outArgsVar)
           set(outArg "${outArg}${piece}")
         endif()
       endforeach()
-      list(APPEND outArgs "${outArg}")
     else()
       # No replacements needed in this argument.
-      list(APPEND outArgs "${arg}")
+      set(outArg "${arg}")
     endif()
+    # Re-escape in-value semicolons in resulting list.
+    string(REPLACE ";" "\\;" outArg "${outArg}")
+    list(APPEND outArgs "${outArg}")
   endforeach()
   set("${outArgsVar}" "${outArgs}" PARENT_SCOPE)
 endfunction()
diff --git a/Tests/Module/ExternalData/CMakeLists.txt b/Tests/Module/ExternalData/CMakeLists.txt
index a379dca..8312dca 100644
--- a/Tests/Module/ExternalData/CMakeLists.txt
+++ b/Tests/Module/ExternalData/CMakeLists.txt
@@ -35,6 +35,7 @@ ExternalData_Add_Test(Data1
     -D Paired=DATA{PairedA.dat,PairedB.dat}
     -D Meta=DATA{MetaTop.dat,REGEX:Meta[ABC].dat}
     -D Directory=DATA{Directory/,A.dat,REGEX:[BC].dat}
+    -D "Semicolons=DATA{Data.dat}\\;DATA{Data.dat}"
     -P ${CMAKE_CURRENT_SOURCE_DIR}/Data1Check.cmake
   )
 ExternalData_Add_Target(Data1)
diff --git a/Tests/Module/ExternalData/Data1Check.cmake b/Tests/Module/ExternalData/Data1Check.cmake
index f40b76c..5770245 100644
--- a/Tests/Module/ExternalData/Data1Check.cmake
+++ b/Tests/Module/ExternalData/Data1Check.cmake
@@ -56,3 +56,13 @@ foreach(n A B C)
     message(SEND_ERROR "Input file:\n  ${file}\ndoes not exist!")
   endif()
 endforeach()
+list(LENGTH Semicolons len)
+if("${len}" EQUAL 2)
+  foreach(file ${Semicolons})
+    if(NOT EXISTS "${file}")
+      message(SEND_ERROR "Input file:\n  ${file}\ndoes not exist!")
+    endif()
+  endforeach()
+else()
+  message(SEND_ERROR "Semicolons value:\n  ${Semicolons}\nis not a list of length 2.")
+endif()
diff --git a/Tests/RunCMake/ExternalData/RunCMakeTest.cmake b/Tests/RunCMake/ExternalData/RunCMakeTest.cmake
index 5ee46c9..ceb2ecf 100644
--- a/Tests/RunCMake/ExternalData/RunCMakeTest.cmake
+++ b/Tests/RunCMake/ExternalData/RunCMakeTest.cmake
@@ -21,4 +21,7 @@ run_cmake(NormalData2)
 run_cmake(NormalData3)
 run_cmake(NormalDataSub1)
 run_cmake(NotUnderRoot)
+run_cmake(Semicolon1)
+run_cmake(Semicolon2)
+run_cmake(Semicolon3)
 run_cmake(SubDirectory1)
diff --git a/Tests/RunCMake/ExternalData/Semicolon1-stdout.txt b/Tests/RunCMake/ExternalData/Semicolon1-stdout.txt
new file mode 100644
index 0000000..361baeb
--- /dev/null
+++ b/Tests/RunCMake/ExternalData/Semicolon1-stdout.txt
@@ -0,0 +1 @@
+-- Data arguments correctly transformed!
diff --git a/Tests/RunCMake/ExternalData/Semicolon1.cmake b/Tests/RunCMake/ExternalData/Semicolon1.cmake
new file mode 100644
index 0000000..c832860
--- /dev/null
+++ b/Tests/RunCMake/ExternalData/Semicolon1.cmake
@@ -0,0 +1,14 @@
+include(ExternalData)
+set(ExternalData_URL_TEMPLATES
+  "file:///${CMAKE_CURRENT_SOURCE_DIR}/%(algo)/%(hash)"
+  )
+set(input Data.txt)
+set(output ${CMAKE_CURRENT_BINARY_DIR}/Data.txt)
+ExternalData_Expand_Arguments(Data args DATA{${input}} "a\\;b" "c;d" DATA{${input}})
+set(expect "${output};a\\;b;c;d;${output}")
+if("x${args}" STREQUAL "x${expect}")
+  message(STATUS "Data arguments correctly transformed!")
+else()
+  message(FATAL_ERROR "Data arguments transformed to:\n  ${args}\n"
+    "but we expected:\n  ${expect}")
+endif()
diff --git a/Tests/RunCMake/ExternalData/Semicolon2-stdout.txt b/Tests/RunCMake/ExternalData/Semicolon2-stdout.txt
new file mode 100644
index 0000000..361baeb
--- /dev/null
+++ b/Tests/RunCMake/ExternalData/Semicolon2-stdout.txt
@@ -0,0 +1 @@
+-- Data arguments correctly transformed!
diff --git a/Tests/RunCMake/ExternalData/Semicolon2.cmake b/Tests/RunCMake/ExternalData/Semicolon2.cmake
new file mode 100644
index 0000000..1a1ae5f
--- /dev/null
+++ b/Tests/RunCMake/ExternalData/Semicolon2.cmake
@@ -0,0 +1,14 @@
+include(ExternalData)
+set(ExternalData_URL_TEMPLATES
+  "file:///${CMAKE_CURRENT_SOURCE_DIR}/%(algo)/%(hash)"
+  )
+set(input Data.txt)
+set(output ${CMAKE_CURRENT_BINARY_DIR}/Data.txt)
+ExternalData_Expand_Arguments(Data args "DATA{${input}};a\\;b;c;d;DATA{${input}}")
+set(expect "${output};a\\;b;c;d;${output}")
+if("x${args}" STREQUAL "x${expect}")
+  message(STATUS "Data arguments correctly transformed!")
+else()
+  message(FATAL_ERROR "Data arguments transformed to:\n  ${args}\n"
+    "but we expected:\n  ${expect}")
+endif()
diff --git a/Tests/RunCMake/ExternalData/Semicolon3-stdout.txt b/Tests/RunCMake/ExternalData/Semicolon3-stdout.txt
new file mode 100644
index 0000000..ca4a360
--- /dev/null
+++ b/Tests/RunCMake/ExternalData/Semicolon3-stdout.txt
@@ -0,0 +1 @@
+-- Data arguments correctly not transformed!
diff --git a/Tests/RunCMake/ExternalData/Semicolon3.cmake b/Tests/RunCMake/ExternalData/Semicolon3.cmake
new file mode 100644
index 0000000..2ae99da
--- /dev/null
+++ b/Tests/RunCMake/ExternalData/Semicolon3.cmake
@@ -0,0 +1,12 @@
+include(ExternalData)
+set(ExternalData_URL_TEMPLATES
+  "file:///${CMAKE_CURRENT_SOURCE_DIR}/%(algo)/%(hash)"
+  )
+set(input "DATA{a;b}")
+ExternalData_Expand_Arguments(Data args "${input}")
+if("x${args}" STREQUAL "x${input}")
+  message(STATUS "Data arguments correctly not transformed!")
+else()
+  message(FATAL_ERROR "Data arguments transformed to:\n  ${args}\n"
+    "but we expected:\n  ${input}")
+endif()