Always accept = or space as delimiters when parsing common command line parameters. (#13857)

Resolves #11456.

3 files changed, 120 insertions, 114 deletions

diff --git a/scripts/azure-pipelines/end-to-end-tests.ps1 b/scripts/azure-pipelines/end-to-end-tests.ps1
index 3a38a8382..50e7f6afd 100644
--- a/scripts/azure-pipelines/end-to-end-tests.ps1
+++ b/scripts/azure-pipelines/end-to-end-tests.ps1
@@ -43,6 +43,7 @@ $commonArgs = @(
     "--x-install-root=$installRoot",

     "--x-packages-root=$packagesRoot"

 )

+$CurrentTest = 'unassigned'

 

 function Refresh-TestRoot {

     Remove-Item -Recurse -Force $TestingRoot -ErrorAction SilentlyContinue

@@ -59,6 +60,7 @@ function Require-FileExists {
         throw "'$CurrentTest' failed to create file '$File'"

     }

 }

+

 function Require-FileNotExists {

     [CmdletBinding()]

     Param(

@@ -68,14 +70,27 @@ function Require-FileNotExists {
         throw "'$CurrentTest' should not have created file '$File'"

     }

 }

+

 function Throw-IfFailed {

     if ($LASTEXITCODE -ne 0) {

         throw "'$CurrentTest' had a step with a nonzero exit code"

     }

 }

 

-if (-not $IsLinux -and -not $IsMacOS)

-{

+function Throw-IfNotFailed {

+    if ($LASTEXITCODE -eq 0) {

+        throw "'$CurrentTest' had a step with an unexpectedly zero exit code"

+    }

+}

+

+function Run-Vcpkg {

+    param([string[]]$TestArgs)

+    $CurrentTest = "./vcpkg $($testArgs -join ' ')"

+    Write-Host $CurrentTest

+    ./vcpkg @testArgs

+}

+

+if (-not $IsLinux -and -not $IsMacOS) {

     Refresh-TestRoot

     # Test msbuild props and targets

     $CurrentTest = "zlib:x86-windows-static msbuild scripts\testing\integrate-install\..."

@@ -106,118 +121,94 @@ if (-not $IsLinux -and -not $IsMacOS)
 Refresh-TestRoot

 

 # Test simple installation

-$args = $commonArgs + @("install","rapidjson","--binarycaching","--x-binarysource=clear;files,$ArchiveRoot,write;nuget,$NuGetRoot,readwrite")

-$CurrentTest = "./vcpkg $($args -join ' ')"

-Write-Host $CurrentTest

-./vcpkg @args

+Run-Vcpkg -TestArgs ($commonArgs + @("install", "rapidjson", "--binarycaching", "--x-binarysource=clear;files,$ArchiveRoot,write;nuget,$NuGetRoot,readwrite"))

 Throw-IfFailed

-

 Require-FileExists "$installRoot/$Triplet/include/rapidjson/rapidjson.h"

 

 # Test simple removal

-$args = $commonArgs + @("remove", "rapidjson")

-$CurrentTest = "./vcpkg $($args -join ' ')"

-Write-Host $CurrentTest

-./vcpkg @args

+Run-Vcpkg -TestArgs ($commonArgs + @("remove", "rapidjson"))

 Throw-IfFailed

-

 Require-FileNotExists "$installRoot/$Triplet/include/rapidjson/rapidjson.h"

 

 # Test restoring from files archive

-$args = $commonArgs + @("install","rapidjson","--x-binarysource=clear;files,$ArchiveRoot,read")

-$CurrentTest = "./vcpkg $($args -join ' ')"

 Remove-Item -Recurse -Force $installRoot

 Remove-Item -Recurse -Force $buildtreesRoot

-Write-Host $CurrentTest

-./vcpkg @args

+Run-Vcpkg -TestArgs ($commonArgs + @("install","rapidjson","--binarycaching","--x-binarysource=clear;files,$ArchiveRoot,read"))

 Throw-IfFailed

-

 Require-FileExists "$installRoot/$Triplet/include/rapidjson/rapidjson.h"

 Require-FileNotExists "$buildtreesRoot/rapidjson/src"

 Require-FileExists "$buildtreesRoot/detect_compiler"

 

 # Test --no-binarycaching

-$args = $commonArgs + @("install","rapidjson","--no-binarycaching","--x-binarysource=clear;files,$ArchiveRoot,read")

-$CurrentTest = "./vcpkg $($args -join ' ')"

 Remove-Item -Recurse -Force $installRoot

 Remove-Item -Recurse -Force $buildtreesRoot

-Write-Host $CurrentTest

-./vcpkg @args

+Run-Vcpkg -TestArgs ($commonArgs + @("install","rapidjson","--no-binarycaching","--x-binarysource=clear;files,$ArchiveRoot,read"))

 Throw-IfFailed

-

 Require-FileExists "$installRoot/$Triplet/include/rapidjson/rapidjson.h"

 Require-FileExists "$buildtreesRoot/rapidjson/src"

 Require-FileExists "$buildtreesRoot/detect_compiler"

 

 # Test --editable

-$args = $commonArgs + @("install","rapidjson","--editable","--x-binarysource=clear;files,$ArchiveRoot,read")

-$CurrentTest = "./vcpkg $($args -join ' ')"

 Remove-Item -Recurse -Force $installRoot

 Remove-Item -Recurse -Force $buildtreesRoot

-Write-Host $CurrentTest

-./vcpkg @args

+Run-Vcpkg -TestArgs ($commonArgs + @("install","rapidjson","--editable","--x-binarysource=clear;files,$ArchiveRoot,read"))

 Throw-IfFailed

-

 Require-FileExists "$installRoot/$Triplet/include/rapidjson/rapidjson.h"

 Require-FileExists "$buildtreesRoot/rapidjson/src"

 Require-FileNotExists "$buildtreesRoot/detect_compiler"

 

 # Test restoring from nuget

-$args = $commonArgs + @("install","rapidjson","--binarycaching","--x-binarysource=clear;nuget,$NuGetRoot")

-$CurrentTest = "./vcpkg $($args -join ' ')"

 Remove-Item -Recurse -Force $installRoot

 Remove-Item -Recurse -Force $buildtreesRoot

-Write-Host $CurrentTest

-./vcpkg @args

+Run-Vcpkg -TestArgs ($commonArgs + @("install", "rapidjson", "--binarycaching", "--x-binarysource=clear;nuget,$NuGetRoot"))

 Throw-IfFailed

-

 Require-FileExists "$installRoot/$Triplet/include/rapidjson/rapidjson.h"

 Require-FileNotExists "$buildtreesRoot/rapidjson/src"

 

 # Test four-phase flow

-$args = $commonArgs + @("install","rapidjson","--dry-run","--x-write-nuget-packages-config=$TestingRoot/packages.config")

-$CurrentTest = "./vcpkg $($args -join ' ')"

 Remove-Item -Recurse -Force $installRoot -ErrorAction SilentlyContinue

-Write-Host $CurrentTest

-./vcpkg @args

+Run-Vcpkg -TestArgs ($commonArgs + @("install", "rapidjson", "--dry-run", "--x-write-nuget-packages-config=$TestingRoot/packages.config"))

 Throw-IfFailed

 Require-FileNotExists "$installRoot/$Triplet/include/rapidjson/rapidjson.h"

 Require-FileNotExists "$buildtreesRoot/rapidjson/src"

 Require-FileExists "$TestingRoot/packages.config"

-

 if ($IsLinux -or $IsMacOS) {

     mono $(./vcpkg fetch nuget) restore $TestingRoot/packages.config -OutputDirectory "$NuGetRoot2" -Source "$NuGetRoot"

-    Throw-IfFailed

 } else {

     & $(./vcpkg fetch nuget) restore $TestingRoot/packages.config -OutputDirectory "$NuGetRoot2" -Source "$NuGetRoot"

-    Throw-IfFailed

 }

-

+Throw-IfFailed

 Remove-Item -Recurse -Force $NuGetRoot -ErrorAction SilentlyContinue

 mkdir $NuGetRoot

-

-$args = $commonArgs + @("install","rapidjson","tinyxml","--binarycaching","--x-binarysource=clear;nuget,$NuGetRoot2;nuget,$NuGetRoot,write")

-$CurrentTest = "./vcpkg $($args -join ' ')"

-Write-Host $CurrentTest

-./vcpkg @args

+Run-Vcpkg -TestArgs ($commonArgs + @("install", "rapidjson", "tinyxml", "--binarycaching", "--x-binarysource=clear;nuget,$NuGetRoot2;nuget,$NuGetRoot,write"))

 Throw-IfFailed

 Require-FileExists "$installRoot/$Triplet/include/rapidjson/rapidjson.h"

 Require-FileExists "$installRoot/$Triplet/include/tinyxml.h"

 Require-FileNotExists "$buildtreesRoot/rapidjson/src"

 Require-FileExists "$buildtreesRoot/tinyxml/src"

-

 if ((Get-ChildItem $NuGetRoot -Filter '*.nupkg' | Measure-Object).Count -ne 1) {

     throw "In '$CurrentTest': did not create exactly 1 NuGet package"

 }

 

 # Test export

-$args = $commonArgs + @("export","rapidjson","tinyxml","--nuget","--nuget-id=vcpkg-export","--nuget-version=1.0.0","--output=vcpkg-export-output","--raw","--zip","--output-dir=$TestingRoot")

-$CurrentTest = "./vcpkg $($args -join ' ')"

+$CurrentTest = 'Prepare for export test'

 Write-Host $CurrentTest

 Require-FileNotExists "$TestingRoot/vcpkg-export-output"

 Require-FileNotExists "$TestingRoot/vcpkg-export.1.0.0.nupkg"

 Require-FileNotExists "$TestingRoot/vcpkg-export-output.zip"

-./vcpkg @args

+Run-Vcpkg -TestArgs ($commonArgs + @("export", "rapidjson", "tinyxml", "--nuget", "--nuget-id=vcpkg-export", "--nuget-version=1.0.0", "--output=vcpkg-export-output", "--raw", "--zip", "--output-dir=$TestingRoot"))

 Require-FileExists "$TestingRoot/vcpkg-export-output"

 Require-FileExists "$TestingRoot/vcpkg-export.1.0.0.nupkg"

 Require-FileExists "$TestingRoot/vcpkg-export-output.zip"

+

+# Test bad command lines

+Run-Vcpkg -TestArgs ($commonArgs + @("install", "zlib", "--vcpkg-rootttttt", "C:\"))

+Throw-IfNotFailed

+

+Run-Vcpkg -TestArgs ($commonArgs + @("install", "zlib", "--vcpkg-rootttttt=C:\"))

+Throw-IfNotFailed

+

+Run-Vcpkg -TestArgs ($commonArgs + @("install", "zlib", "--fast")) # NB: --fast is not a switch

+Throw-IfNotFailed

+

+$LASTEXITCODE = 0

diff --git a/toolsrc/src/vcpkg-test/arguments.cpp b/toolsrc/src/vcpkg-test/arguments.cpp
index 7ade6aa2a..00a1beb81 100644
--- a/toolsrc/src/vcpkg-test/arguments.cpp
+++ b/toolsrc/src/vcpkg-test/arguments.cpp
@@ -107,3 +107,21 @@ TEST_CASE ("VcpkgCmdArguments from argument sequence with valued options", "[arg
         REQUIRE(v.command_arguments.size() == 0);
     }
 }
+
+TEST_CASE ("vcpkg_root parse with arg separator", "[arguments]")
+{
+    std::vector<std::string> t = {"--vcpkg-root", "C:\\vcpkg"};
+    auto v = VcpkgCmdArguments::create_from_arg_sequence(t.data(), t.data() + t.size());
+    auto& vcpkg_root_dir = v.vcpkg_root_dir;
+    REQUIRE(vcpkg_root_dir);
+    REQUIRE(*vcpkg_root_dir == "C:\\vcpkg");
+}
+
+TEST_CASE ("vcpkg_root parse with equal separator", "[arguments]")
+{
+    std::vector<std::string> t = {"--vcpkg-root=C:\\vcpkg"};
+    auto v = VcpkgCmdArguments::create_from_arg_sequence(t.data(), t.data() + t.size());
+    auto& vcpkg_root_dir = v.vcpkg_root_dir;
+    REQUIRE(vcpkg_root_dir);
+    REQUIRE(*vcpkg_root_dir == "C:\\vcpkg");
+}
diff --git a/toolsrc/src/vcpkg/vcpkgcmdarguments.cpp b/toolsrc/src/vcpkg/vcpkgcmdarguments.cpp
index af755408c..7b4e9c40d 100644
--- a/toolsrc/src/vcpkg/vcpkgcmdarguments.cpp
+++ b/toolsrc/src/vcpkg/vcpkgcmdarguments.cpp
@@ -58,30 +58,6 @@ namespace vcpkg
         }
     }
 
-    static void parse_value(const std::string* arg_begin,
-                            const std::string* arg_end,
-                            StringView option_name,
-                            std::unique_ptr<std::string>& option_field)
-    {
-        if (arg_begin == arg_end)
-        {
-            System::print2(System::Color::error, "Error: expected value after --", option_name, '\n');
-            Metrics::g_metrics.lock()->track_property("error", "error option name");
-            print_usage();
-            Checks::exit_fail(VCPKG_LINE_INFO);
-        }
-
-        if (option_field != nullptr)
-        {
-            System::print2(System::Color::error, "Error: --", option_name, " specified multiple times\n");
-            Metrics::g_metrics.lock()->track_property("error", "error option specified multiple times");
-            print_usage();
-            Checks::exit_fail(VCPKG_LINE_INFO);
-        }
-
-        option_field = std::make_unique<std::string>(*arg_begin);
-    }
-
     static void parse_cojoined_value(StringView new_value,
                                      StringView option_name,
                                      std::unique_ptr<std::string>& option_field)
@@ -176,27 +152,46 @@ namespace vcpkg
         return VcpkgCmdArguments::create_from_arg_sequence(v.data(), v.data() + v.size());
     }
 
-    // returns true if this does parse this argument as this option
-    template<class T, class F>
-    static bool try_parse_argument_as_option(StringView option, StringView arg, T& place, F parser)
+    enum class TryParseArgumentResult
     {
-        if (arg.size() <= option.size() + 1)
-        {
-            // it is impossible for this argument to be this option
-            return false;
-        }
+        NotFound,
+        Found,
+        FoundAndConsumedLookahead
+    };
 
+    template<class T, class F>
+    static TryParseArgumentResult try_parse_argument_as_option(
+        StringView arg, Optional<StringView> lookahead, StringView option, T& place, F parser)
+    {
         if (Strings::starts_with(arg, "x-") && !Strings::starts_with(option, "x-"))
         {
             arg = arg.substr(2);
         }
-        if (Strings::starts_with(arg, option) && arg.byte_at_index(option.size()) == '=')
+
+        if (Strings::starts_with(arg, option))
         {
-            parser(arg.substr(option.size() + 1), option, place);
-            return true;
+            if (arg.size() == option.size())
+            {
+                if (auto next = lookahead.get())
+                {
+                    parser(*next, option, place);
+                    return TryParseArgumentResult::FoundAndConsumedLookahead;
+                }
+
+                System::print2(System::Color::error, "Error: expected value after ", option, '\n');
+                Metrics::g_metrics.lock()->track_property("error", "error option name");
+                print_usage();
+                Checks::exit_fail(VCPKG_LINE_INFO);
+            }
+
+            if (arg.byte_at_index(option.size()) == '=')
+            {
+                parser(arg.substr(option.size() + 1), option, place);
+                return TryParseArgumentResult::Found;
+            }
         }
 
-        return false;
+        return TryParseArgumentResult::NotFound;
     }
 
     static bool equals_modulo_experimental(StringView arg, StringView option)
@@ -230,13 +225,13 @@ namespace vcpkg
         return false;
     }
 
-    VcpkgCmdArguments VcpkgCmdArguments::create_from_arg_sequence(const std::string* arg_begin,
-                                                                  const std::string* arg_end)
+    VcpkgCmdArguments VcpkgCmdArguments::create_from_arg_sequence(const std::string* arg_first,
+                                                                  const std::string* arg_last)
     {
         VcpkgCmdArguments args;
         std::vector<std::string> feature_flags;
 
-        for (auto it = arg_begin; it != arg_end; ++it)
+        for (auto it = arg_first; it != arg_last; ++it)
         {
             std::string basic_arg = *it;
 
@@ -269,23 +264,10 @@ namespace vcpkg
             Strings::ascii_to_lowercase(std::begin(basic_arg), first_eq);
             // basic_arg[0] == '-' && basic_arg[1] == '-'
             StringView arg = StringView(basic_arg).substr(2);
-
-            // command switch
-            if (arg == VCPKG_ROOT_DIR_ARG)
-            {
-                ++it;
-                parse_value(it, arg_end, VCPKG_ROOT_DIR_ARG, args.vcpkg_root_dir);
-                continue;
-            }
-            if (arg == TRIPLET_ARG)
-            {
-                ++it;
-                parse_value(it, arg_end, TRIPLET_ARG, args.triplet);
-                continue;
-            }
-
             constexpr static std::pair<StringView, std::unique_ptr<std::string> VcpkgCmdArguments::*>
                 cojoined_values[] = {
+                    {VCPKG_ROOT_DIR_ARG, &VcpkgCmdArguments::vcpkg_root_dir},
+                    {TRIPLET_ARG, &VcpkgCmdArguments::triplet},
                     {MANIFEST_ROOT_DIR_ARG, &VcpkgCmdArguments::manifest_root_dir},
                     {BUILDTREES_ROOT_DIR_ARG, &VcpkgCmdArguments::buildtrees_root_dir},
                     {DOWNLOADS_ROOT_DIR_ARG, &VcpkgCmdArguments::downloads_root_dir},
@@ -312,30 +294,45 @@ namespace vcpkg
                 {JSON_SWITCH, &VcpkgCmdArguments::json},
             };
 
+            Optional<StringView> lookahead;
+            if (it + 1 != arg_last)
+            {
+                lookahead = it[1];
+            }
+
             bool found = false;
             for (const auto& pr : cojoined_values)
             {
-                if (try_parse_argument_as_option(pr.first, arg, args.*pr.second, parse_cojoined_value))
+                switch (try_parse_argument_as_option(arg, lookahead, pr.first, args.*pr.second, parse_cojoined_value))
                 {
-                    found = true;
-                    break;
+                    case TryParseArgumentResult::FoundAndConsumedLookahead: ++it; [[fallthrough]];
+                    case TryParseArgumentResult::Found: found = true; break;
+                    case TryParseArgumentResult::NotFound: break;
+                    default: Checks::unreachable(VCPKG_LINE_INFO);
                 }
             }
             if (found) continue;
 
             for (const auto& pr : cojoined_multivalues)
             {
-                if (try_parse_argument_as_option(pr.first, arg, args.*pr.second, parse_cojoined_multivalue))
+                switch (
+                    try_parse_argument_as_option(arg, lookahead, pr.first, args.*pr.second, parse_cojoined_multivalue))
                 {
-                    found = true;
-                    break;
+                    case TryParseArgumentResult::FoundAndConsumedLookahead: ++it; [[fallthrough]];
+                    case TryParseArgumentResult::Found: found = true; break;
+                    case TryParseArgumentResult::NotFound: break;
+                    default: Checks::unreachable(VCPKG_LINE_INFO);
                 }
             }
             if (found) continue;
 
-            if (try_parse_argument_as_option(FEATURE_FLAGS_ARG, arg, feature_flags, parse_cojoined_list_multivalue))
+            switch (try_parse_argument_as_option(
+                arg, lookahead, FEATURE_FLAGS_ARG, feature_flags, parse_cojoined_list_multivalue))
             {
-                continue;
+                case TryParseArgumentResult::FoundAndConsumedLookahead: ++it; [[fallthrough]];
+                case TryParseArgumentResult::Found: found = true; break;
+                case TryParseArgumentResult::NotFound: break;
+                default: Checks::unreachable(VCPKG_LINE_INFO);
             }
 
             for (const auto& pr : switches)
@@ -515,7 +512,7 @@ namespace vcpkg
             }
         }
 
-        if (!switches_copy.empty())
+        if (!switches_copy.empty() || !options_copy.empty())
         {
             System::printf(System::Color::error, "Unknown option(s) for command '%s':\n", this->command);
             for (auto&& switch_ : switches_copy)
@@ -609,7 +606,7 @@ namespace vcpkg
             return Strings::concat("--", arg, joiner, value);
         };
 
-        table.format(opt(TRIPLET_ARG, " ", "<t>"), "Specify the target architecture triplet. See 'vcpkg help triplet'");
+        table.format(opt(TRIPLET_ARG, "=", "<t>"), "Specify the target architecture triplet. See 'vcpkg help triplet'");
         table.format("", "(default: " + format_environment_variable("VCPKG_DEFAULT_TRIPLET") + ')');
         table.format(opt(OVERLAY_PORTS_ARG, "=", "<path>"), "Specify directories to be used when searching for ports");
         table.format("", "(also: " + format_environment_variable("VCPKG_OVERLAY_PORTS") + ')');
@@ -619,7 +616,7 @@ namespace vcpkg
                      "Add sources for binary caching. See 'vcpkg help binarycaching'");
         table.format(opt(DOWNLOADS_ROOT_DIR_ARG, "=", "<path>"), "Specify the downloads root directory");
         table.format("", "(default: " + format_environment_variable("VCPKG_DOWNLOADS") + ')');
-        table.format(opt(VCPKG_ROOT_DIR_ARG, " ", "<path>"), "Specify the vcpkg root directory");
+        table.format(opt(VCPKG_ROOT_DIR_ARG, "=", "<path>"), "Specify the vcpkg root directory");
         table.format("", "(default: " + format_environment_variable("VCPKG_ROOT") + ')');
         table.format(opt(BUILDTREES_ROOT_DIR_ARG, "=", "<path>"),
                      "(Experimental) Specify the buildtrees root directory");