From 1561748496b10bc74adc731fd1d5e7c733034cba Mon Sep 17 00:00:00 2001
From: Craig Scott <craig.scott@crascit.com>
Date: Wed, 23 Aug 2017 22:40:49 +0800
Subject: ExternalProject: Prevent COMMAND from being treated as a true keyword

The known keywords for each function are obtained by scraping the
documentation for lines matching a particular regular expression. In
commit 8842a027 (ExternalProject: Improve documentation, 2017-07-09),
the docs were overhauled and the COMMAND docs subsequently matched the
regular expression when they shouldn't have. This made COMMAND appear as
a true keyword, which thwarted the special handling logic elsewhere for
the intended use of COMMAND arguments.

This commit contains a workaround for issue #17229 to force a dependency
of the patch step on the update step to ensure a predictable step order.

Fixes: #17198
---
 Modules/ExternalProject.cmake                      |  8 ++++++
 .../ExternalProject/MultiCommand-build-stdout.txt  | 15 +++++++++++
 Tests/RunCMake/ExternalProject/MultiCommand.cmake  | 30 ++++++++++++++++++++++
 Tests/RunCMake/ExternalProject/RunCMakeTest.cmake  | 10 ++++++++
 4 files changed, 63 insertions(+)
 create mode 100644 Tests/RunCMake/ExternalProject/MultiCommand-build-stdout.txt
 create mode 100644 Tests/RunCMake/ExternalProject/MultiCommand.cmake

diff --git a/Modules/ExternalProject.cmake b/Modules/ExternalProject.cmake
index d92eb5f..912c5ac 100644
--- a/Modules/ExternalProject.cmake
+++ b/Modules/ExternalProject.cmake
@@ -870,6 +870,14 @@ foreach(line IN LISTS lines)
     set(_ep_keyword_sep)
   elseif("${line}" MATCHES "^ +``([A-Z0-9_]+) [^`]*``$")
     set(_ep_key "${CMAKE_MATCH_1}")
+    # COMMAND should never be included as a keyword,
+    # for ExternalProject_Add(), as it is treated as a
+    # special case by argument parsing as an extension
+    # of a previous ..._COMMAND
+    if("x${_ep_key}x" STREQUAL "xCOMMANDx" AND
+       "x${_ep_func}x" STREQUAL "xExternalProject_Addx")
+      continue()
+    endif()
     #message("  keyword [${_ep_key}]")
     string(APPEND _ep_keywords_${_ep_func}
       "${_ep_keyword_sep}${_ep_key}")
diff --git a/Tests/RunCMake/ExternalProject/MultiCommand-build-stdout.txt b/Tests/RunCMake/ExternalProject/MultiCommand-build-stdout.txt
new file mode 100644
index 0000000..30ebc7d
--- /dev/null
+++ b/Tests/RunCMake/ExternalProject/MultiCommand-build-stdout.txt
@@ -0,0 +1,15 @@
+.* *download 1
+.* *download 2
+.* *update 1
+.* *update 2
+.* *patch 1
+.* *patch 2
+.* *configure 1
+.* *configure 2
+.* *build 1
+.* *build 2
+.* *install 1
+.* *install 2
+.* *test 1
+.* *test 2
+.*
diff --git a/Tests/RunCMake/ExternalProject/MultiCommand.cmake b/Tests/RunCMake/ExternalProject/MultiCommand.cmake
new file mode 100644
index 0000000..a8dbfea
--- /dev/null
+++ b/Tests/RunCMake/ExternalProject/MultiCommand.cmake
@@ -0,0 +1,30 @@
+cmake_minimum_required(VERSION 3.9)
+
+include(ExternalProject)
+
+# Verify COMMAND keyword is recognised after various *_COMMAND options
+ExternalProject_Add(multiCommand
+  DOWNLOAD_COMMAND  "${CMAKE_COMMAND}" -E echo "download 1"
+           COMMAND  "${CMAKE_COMMAND}" -E echo "download 2"
+  UPDATE_COMMAND    "${CMAKE_COMMAND}" -E echo "update 1"
+         COMMAND    "${CMAKE_COMMAND}" -E echo "update 2"
+  PATCH_COMMAND     "${CMAKE_COMMAND}" -E echo "patch 1"
+        COMMAND     "${CMAKE_COMMAND}" -E echo "patch 2"
+  CONFIGURE_COMMAND "${CMAKE_COMMAND}" -E echo "configure 1"
+            COMMAND "${CMAKE_COMMAND}" -E echo "configure 2"
+  BUILD_COMMAND     "${CMAKE_COMMAND}" -E echo "build 1"
+        COMMAND     "${CMAKE_COMMAND}" -E echo "build 2"
+  TEST_COMMAND      "${CMAKE_COMMAND}" -E echo "test 1"
+       COMMAND      "${CMAKE_COMMAND}" -E echo "test 2"
+  INSTALL_COMMAND   "${CMAKE_COMMAND}" -E echo "install 1"
+          COMMAND   "${CMAKE_COMMAND}" -E echo "install 2"
+)
+
+# Workaround for issue 17229 (missing dependency between update and patch steps)
+ExternalProject_Add_StepTargets(multiCommand NO_DEPENDS update)
+ExternalProject_Add_StepDependencies(multiCommand patch multiCommand-update)
+
+# Force all steps to be re-run by removing timestamps from any previous run
+ExternalProject_Get_Property(multiCommand STAMP_DIR)
+file(REMOVE_RECURSE "${STAMP_DIR}")
+file(MAKE_DIRECTORY "${STAMP_DIR}")
diff --git a/Tests/RunCMake/ExternalProject/RunCMakeTest.cmake b/Tests/RunCMake/ExternalProject/RunCMakeTest.cmake
index 47d6129..994e2aa 100644
--- a/Tests/RunCMake/ExternalProject/RunCMakeTest.cmake
+++ b/Tests/RunCMake/ExternalProject/RunCMakeTest.cmake
@@ -12,3 +12,13 @@ run_cmake(Add_StepDependencies_iface)
 run_cmake(Add_StepDependencies_iface_step)
 run_cmake(Add_StepDependencies_no_target)
 run_cmake(UsesTerminal)
+
+# Run both cmake and build steps. We always do a clean before the
+# build to ensure that the download step re-runs each time.
+set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/MultiCommand-build)
+set(RunCMake_TEST_NO_CLEAN 1)
+file(REMOVE_RECURSE "${RunCMake_TEST_BINARY_DIR}")
+file(MAKE_DIRECTORY "${RunCMake_TEST_BINARY_DIR}")
+run_cmake(MultiCommand)
+run_cmake_command(MultiCommand-clean ${CMAKE_COMMAND} --build . --target clean)
+run_cmake_command(MultiCommand-build ${CMAKE_COMMAND} --build .)
-- 
cgit v0.12