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}