From 729db484efac18194076c4020fe9b6a87f24ed22 Mon Sep 17 00:00:00 2001
From: Brad King <brad.king@kitware.com>
Date: Tue, 11 Jan 2011 17:10:28 -0500
Subject: [PATCH] Fix ArgumentExpansion test expected results

Teach the ArgumentExpansion test to expect flattened lists as has always
been the case in the CMake language.  Now that the test should pass
enable the failure regex even when CMAKE_STRICT is not on.  Replace the
reference to the old ArgumentExpansion test behavior in the workaround
comment in cmMakefile::TryCompile with a full inline explanation.
---
 Source/cmMakefile.cxx                  | 24 ++++++++++++++++++++++--
 Tests/ArgumentExpansion/CMakeLists.txt | 13 +++++++------
 Tests/CMakeLists.txt                   |  6 ++----
 3 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index 704404926..c6e34f88b 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -2845,8 +2845,28 @@ int cmMakefile::TryCompile(const char *srcdir, const char *bindir,
   // if cmake args were provided then pass them in
   if (cmakeArgs)
     {
-    // FIXME: Workaround to ignore unused CLI variables until the
-    // 'ArgumentExpansion' test succeeds with CMAKE_STRICT on
+    // FIXME: Workaround to ignore unused CLI variables in try-compile.
+    //
+    // Ideally we should use SetArgs to honor options like --warn-unused-vars.
+    // However, there is a subtle problem when certain arguments are passed to
+    // a macro wrapping around try_compile or try_run that does not escape
+    // semicolons in its parameters but just passes ${ARGV} or ${ARGN}.  In
+    // this case a list argument like "-DVAR=a;b" gets split into multiple
+    // cmake arguments "-DVAR=a" and "b".  Currently SetCacheArgs ignores
+    // argument "b" and uses just "-DVAR=a", leading to a subtle bug in that
+    // the try_compile or try_run does not get the proper value of VAR.  If we
+    // call SetArgs here then it would treat "b" as the source directory and
+    // cause an error such as "The source directory .../CMakeFiles/CMakeTmp/b
+    // does not exist", thus breaking the try_compile or try_run completely.
+    //
+    // Strictly speaking the bug is in the wrapper macro because the CMake
+    // language has always flattened nested lists and the macro should escape
+    // the semicolons in its arguments before forwarding them.  However, this
+    // bug is so subtle that projects typically work anyway, usually because
+    // the value VAR=a is sufficient for the try_compile or try_run to get the
+    // correct result.  Calling SetArgs here would break such projects that
+    // previously built.  Instead we work around the issue by never reporting
+    // unused arguments and ignoring options such as --warn-unused-vars.
     cm.SetWarnUnusedCli(false);
     //cm.SetArgs(*cmakeArgs, true);
 
diff --git a/Tests/ArgumentExpansion/CMakeLists.txt b/Tests/ArgumentExpansion/CMakeLists.txt
index 62017067a..a24636f58 100644
--- a/Tests/ArgumentExpansion/CMakeLists.txt
+++ b/Tests/ArgumentExpansion/CMakeLists.txt
@@ -16,11 +16,11 @@ function (argument_tester expected expected_len)
             list(GET ARGN ${i} argn_value)
             list(GET ${expected} ${i} expected_value)
 
-            if (NOT ${argn_value} STREQUAL ${expected_value})
+            if (NOT "${argn_value}" STREQUAL "${expected_value}")
                 message(STATUS "Unexpected: Argument ${i} doesn't match")
                 message(STATUS "    Expected: ${expected_value}")
                 message(STATUS "    Received: ${argn_value}")
-            endif (NOT ${argn_value} STREQUAL ${expected_value})
+            endif ()
 
             math(EXPR i "${i} + 1")
         endwhile (i LESS ${argn_len})
@@ -50,10 +50,11 @@ set(nested_list_arg_test
     "${multiple_arg_test}"
     "first arg"
     "second arg")
-message(STATUS "Test: Nested list argument")
-argument_tester(nested_list_arg_test 3 ${nested_list_arg_test})
+message(STATUS "Test: Nested list argument flattens")
+argument_tester(nested_list_arg_test 4 ${nested_list_arg_test})
 
 set(semicolon_arg_test
     "pre\;post")
-message(STATUS "Test: Semicolon argument")
-argument_tester(semicolon_arg_test 1 ${semicolon_arg_test})
+set(semicolon_arg_test_flat "pre;post")
+message(STATUS "Test: Semicolon argument flattens")
+argument_tester(semicolon_arg_test_flat 2 ${semicolon_arg_test})
diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt
index 02d393bb5..27cff3f8f 100644
--- a/Tests/CMakeLists.txt
+++ b/Tests/CMakeLists.txt
@@ -383,10 +383,8 @@ ${CMake_BINARY_DIR}/bin/cmake -DVERSION=master -P ${CMake_SOURCE_DIR}/Utilities/
     --build-makeprogram ${CMAKE_TEST_MAKEPROGRAM}
     --build-exe-dir "${CMake_BINARY_DIR}/Tests/ArgumentExpansion/bin"
     )
-  IF(CMAKE_STRICT)
-    SET_TESTS_PROPERTIES(ArgumentExpansion PROPERTIES
-      FAIL_REGULAR_EXPRESSION "Unexpected: ")
-  ENDIF(CMAKE_STRICT)
+  SET_TESTS_PROPERTIES(ArgumentExpansion PROPERTIES
+    FAIL_REGULAR_EXPRESSION "Unexpected: ")
   LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/ArgumentExpansion")
 
   ADD_TEST(CustomCommand  ${CMAKE_CTEST_COMMAND}