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

Resolves #11456.
2025-01-14 14:58:00 +08:00 · 2020-10-07 10:31:42 -07:00 · 2020-10-07 10:31:42 -07:00 · ab86954093
commit ab86954093
parent 690215da95
3 changed files with 120 additions and 114 deletions
--- 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
--- 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");
+}
--- 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");