From 41291b20f3881cac781e5e628f8b892b29c7b39c Mon Sep 17 00:00:00 2001 From: Matthew Woehlke Date: Wed, 28 Sep 2016 12:07:39 -0400 Subject: [PATCH 1/2] cmake_parse_arguments: Fix PARSE_ARGV multi-value argument handling The `PARSE_ARGV` mode was recently added to help functions properly parse their arguments even when those arguments may be quoted and contain literal `;` in their values. Fix the implementation to encode `;`s in reported multi-value arguments and in `UNPARSED_ARGUMENTS` so that `;`s in the individual values are preserved in the lists. This allows clients to access all their argument values correctly. --- Source/cmParseArgumentsCommand.cxx | 25 +++++++++++++++++-- .../cmake_parse_arguments/ArgvN.cmake | 22 ++++++++++++++-- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/Source/cmParseArgumentsCommand.cxx b/Source/cmParseArgumentsCommand.cxx index e8de5b6f9..55d71ea1f 100644 --- a/Source/cmParseArgumentsCommand.cxx +++ b/Source/cmParseArgumentsCommand.cxx @@ -4,6 +4,19 @@ #include "cmAlgorithms.h" +static std::string escape_arg(const std::string& arg) +{ + // replace ";" with "\;" so output argument lists will split correctly + std::string escapedArg; + for (size_t i = 0; i < arg.size(); ++i) { + if (arg[i] == ';') { + escapedArg += '\\'; + } + escapedArg += arg[i]; + } + return escapedArg; +} + bool cmParseArgumentsCommand::InitialPass(std::vector const& args, cmExecutionStatus&) { @@ -165,10 +178,18 @@ bool cmParseArgumentsCommand::InitialPass(std::vector const& args, insideValues = NONE; break; case MULTI: - multi[currentArgName].push_back(*argIter); + if (parseFromArgV) { + multi[currentArgName].push_back(escape_arg(*argIter)); + } else { + multi[currentArgName].push_back(*argIter); + } break; default: - unparsed.push_back(*argIter); + if (parseFromArgV) { + unparsed.push_back(escape_arg(*argIter)); + } else { + unparsed.push_back(*argIter); + } break; } } diff --git a/Tests/RunCMake/cmake_parse_arguments/ArgvN.cmake b/Tests/RunCMake/cmake_parse_arguments/ArgvN.cmake index 61bde0363..63a1b0112 100644 --- a/Tests/RunCMake/cmake_parse_arguments/ArgvN.cmake +++ b/Tests/RunCMake/cmake_parse_arguments/ArgvN.cmake @@ -1,5 +1,15 @@ include(${CMAKE_CURRENT_LIST_DIR}/test_utils.cmake) +function(test_multi list) + set(i 0) + foreach(value IN LISTS ${list}) + math(EXPR j "${i} + 1") + set(${list}[${i}] "${value}") + TEST(${list}[${i}] "${ARGV${j}}") + set(i ${j}) + endforeach() +endfunction() + function(test1) cmake_parse_arguments(PARSE_ARGV 0 pref "OPT1;OPT2" "SINGLE1;SINGLE2" "MULTI1;MULTI2") @@ -23,8 +33,16 @@ function(test2 arg1) TEST(pref_OPT2 FALSE) TEST(pref_SINGLE1 "foo;bar") TEST(pref_SINGLE2 UNDEFINED) - TEST(pref_MULTI1 bar foo bar) + test_multi(pref_MULTI1 bar "foo;bar") TEST(pref_MULTI2 UNDEFINED) TEST(pref_UNPARSED_ARGUMENTS UNDEFINED) endfunction() -test2("first named" OPT1 SINGLE1 "foo;bar" MULTI1 bar foo bar) +test2("first named" OPT1 SINGLE1 "foo;bar" MULTI1 bar "foo;bar") + +function(test3 arg1) + cmake_parse_arguments(PARSE_ARGV 0 + pref "" "" "") + + test_multi(pref_UNPARSED_ARGUMENTS "foo;bar" dog cat) +endfunction() +test3("foo;bar" dog cat) From 66c70cd9f1eb69b03cefe7fbe8e238aaa4630f47 Mon Sep 17 00:00:00 2001 From: Matthew Woehlke Date: Wed, 28 Sep 2016 15:20:42 -0400 Subject: [PATCH 2/2] cmake_parse_arguments: Add additional unit tests Add additional unit tests for some corner cases in argument splitting. --- .../cmake_parse_arguments/ArgvN.cmake | 14 +---- .../CornerCasesArgvN.cmake | 53 +++++++++++++++++++ .../cmake_parse_arguments/RunCMakeTest.cmake | 1 + .../cmake_parse_arguments/Utils.cmake | 1 + .../cmake_parse_arguments/test_utils.cmake | 44 +++++++++------ 5 files changed, 84 insertions(+), 29 deletions(-) create mode 100644 Tests/RunCMake/cmake_parse_arguments/CornerCasesArgvN.cmake diff --git a/Tests/RunCMake/cmake_parse_arguments/ArgvN.cmake b/Tests/RunCMake/cmake_parse_arguments/ArgvN.cmake index 63a1b0112..96a373d1f 100644 --- a/Tests/RunCMake/cmake_parse_arguments/ArgvN.cmake +++ b/Tests/RunCMake/cmake_parse_arguments/ArgvN.cmake @@ -1,15 +1,5 @@ include(${CMAKE_CURRENT_LIST_DIR}/test_utils.cmake) -function(test_multi list) - set(i 0) - foreach(value IN LISTS ${list}) - math(EXPR j "${i} + 1") - set(${list}[${i}] "${value}") - TEST(${list}[${i}] "${ARGV${j}}") - set(i ${j}) - endforeach() -endfunction() - function(test1) cmake_parse_arguments(PARSE_ARGV 0 pref "OPT1;OPT2" "SINGLE1;SINGLE2" "MULTI1;MULTI2") @@ -33,7 +23,7 @@ function(test2 arg1) TEST(pref_OPT2 FALSE) TEST(pref_SINGLE1 "foo;bar") TEST(pref_SINGLE2 UNDEFINED) - test_multi(pref_MULTI1 bar "foo;bar") + TEST(pref_MULTI1 bar "foo;bar") TEST(pref_MULTI2 UNDEFINED) TEST(pref_UNPARSED_ARGUMENTS UNDEFINED) endfunction() @@ -43,6 +33,6 @@ function(test3 arg1) cmake_parse_arguments(PARSE_ARGV 0 pref "" "" "") - test_multi(pref_UNPARSED_ARGUMENTS "foo;bar" dog cat) + TEST(pref_UNPARSED_ARGUMENTS "foo;bar" dog cat) endfunction() test3("foo;bar" dog cat) diff --git a/Tests/RunCMake/cmake_parse_arguments/CornerCasesArgvN.cmake b/Tests/RunCMake/cmake_parse_arguments/CornerCasesArgvN.cmake new file mode 100644 index 000000000..807ed03e1 --- /dev/null +++ b/Tests/RunCMake/cmake_parse_arguments/CornerCasesArgvN.cmake @@ -0,0 +1,53 @@ +include(${CMAKE_CURRENT_LIST_DIR}/test_utils.cmake) + +function(test1) + cmake_parse_arguments(PARSE_ARGV 0 + mpref "" "" "MULTI") + + TEST(mpref_MULTI foo "foo\;bar") + + cmake_parse_arguments(PARSE_ARGV 1 + upref "" "" "MULTI") + + TEST(upref_UNPARSED_ARGUMENTS foo "foo\;bar") +endfunction() +test1(MULTI foo "foo\;bar") + +function(test2) + cmake_parse_arguments(PARSE_ARGV 0 + mpref "" "" "MULTI") + + TEST(mpref_MULTI "foo;" "bar;") + + cmake_parse_arguments(PARSE_ARGV 1 + upref "" "" "MULTI") + + TEST(upref_UNPARSED_ARGUMENTS "foo;" "bar;") +endfunction() +test2(MULTI "foo;" "bar;") + +function(test3) + cmake_parse_arguments(PARSE_ARGV 0 + mpref "" "" "MULTI") + + TEST(mpref_MULTI "[foo;]" "bar\\") + + cmake_parse_arguments(PARSE_ARGV 1 + upref "" "" "MULTI") + + TEST(upref_UNPARSED_ARGUMENTS "[foo;]" "bar\\") +endfunction() +test3(MULTI "[foo;]" "bar\\") + +function(test4) + cmake_parse_arguments(PARSE_ARGV 0 + mpref "" "" "MULTI") + + TEST(mpref_MULTI foo "bar;none") + + cmake_parse_arguments(PARSE_ARGV 1 + upref "" "" "MULTI") + + TEST(upref_UNPARSED_ARGUMENTS foo "bar;none") +endfunction() +test4(MULTI foo bar\\ none) diff --git a/Tests/RunCMake/cmake_parse_arguments/RunCMakeTest.cmake b/Tests/RunCMake/cmake_parse_arguments/RunCMakeTest.cmake index 22d7ee42d..1e15b3b2c 100644 --- a/Tests/RunCMake/cmake_parse_arguments/RunCMakeTest.cmake +++ b/Tests/RunCMake/cmake_parse_arguments/RunCMakeTest.cmake @@ -10,3 +10,4 @@ run_cmake(BadArgvN1) run_cmake(BadArgvN2) run_cmake(BadArgvN3) run_cmake(BadArgvN4) +run_cmake(CornerCasesArgvN) diff --git a/Tests/RunCMake/cmake_parse_arguments/Utils.cmake b/Tests/RunCMake/cmake_parse_arguments/Utils.cmake index 3bbf115e8..f2001f2c0 100644 --- a/Tests/RunCMake/cmake_parse_arguments/Utils.cmake +++ b/Tests/RunCMake/cmake_parse_arguments/Utils.cmake @@ -17,4 +17,5 @@ SET (asdf "some value") TEST(asdf "some value") SET (asdf some list) +TEST(asdf some list) TEST(asdf "some;list") diff --git a/Tests/RunCMake/cmake_parse_arguments/test_utils.cmake b/Tests/RunCMake/cmake_parse_arguments/test_utils.cmake index f5425c2bc..9ce99b8a4 100644 --- a/Tests/RunCMake/cmake_parse_arguments/test_utils.cmake +++ b/Tests/RunCMake/cmake_parse_arguments/test_utils.cmake @@ -1,20 +1,30 @@ -macro(TEST variable) - SET(expected "${ARGN}") - if ( "${expected}" STREQUAL "UNDEFINED" ) - if (DEFINED ${variable}) - message(FATAL_ERROR "'${variable}' shall be undefined but has value '${${variable}}'") - endif() - elseif( "${expected}" STREQUAL "FALSE" ) - if (NOT ${variable} STREQUAL "FALSE") - message(FATAL_ERROR "'${variable}' shall be FALSE") - endif() - elseif( "${expected}" STREQUAL "TRUE" ) - if (NOT ${variable} STREQUAL "TRUE") - message(FATAL_ERROR "'${variable}' shall be TRUE") - endif() +function(TEST variable) + if(ARGC GREATER 2) + set(i 0) + foreach(value IN LISTS ${variable}) + math(EXPR j "${i} + 1") + set(${variable}[${i}] "${value}") + TEST(${variable}[${i}] "${ARGV${j}}") + set(i ${j}) + endforeach() else() - if (NOT ${variable} STREQUAL "${expected}") - message(FATAL_ERROR "'${variable}' shall be '${expected}'") + set(expected "${ARGN}") + if("${expected}" STREQUAL "UNDEFINED") + if(DEFINED ${variable}) + message(FATAL_ERROR "'${variable}' shall be undefined but has value '${${variable}}'") + endif() + elseif("${expected}" STREQUAL "FALSE") + if(NOT ${variable} STREQUAL "FALSE") + message(FATAL_ERROR "'${variable}' shall be FALSE") + endif() + elseif("${expected}" STREQUAL "TRUE") + if(NOT ${variable} STREQUAL "TRUE") + message(FATAL_ERROR "'${variable}' shall be TRUE") + endif() + else() + if(NOT ${variable} STREQUAL "${expected}") + message(FATAL_ERROR "'${variable}' shall be '${expected}'") + endif() endif() endif() -endmacro() +endfunction()