From c0787eddf9ad2c3992e6d19fa1f330ed1f462150 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Fri, 2 Oct 2020 16:08:12 -0700 Subject: [PATCH 1/8] Always accept = or space as delimiters when parsing command line parameters. Resolves #11456. --- toolsrc/src/vcpkg-test/arguments.cpp | 18 ++++ toolsrc/src/vcpkg/vcpkgcmdarguments.cpp | 126 ++++++++++++------------ 2 files changed, 80 insertions(+), 64 deletions(-) diff --git a/toolsrc/src/vcpkg-test/arguments.cpp b/toolsrc/src/vcpkg-test/arguments.cpp index 7ade6aa2a95ac1..00a1beb8174016 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 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 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 bd428e8b3e37a7..05b802441a7212 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& 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(*arg_begin); - } - static void parse_cojoined_value(StringView new_value, StringView option_name, std::unique_ptr& option_field) @@ -176,27 +152,43 @@ 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 + // returns the number of consumed arguments: + // 0: parse unsuccessful + // 1: parse successful and option was consumed + // 2: parse successful and both option and lookahead were consumed template - static bool try_parse_argument_as_option(StringView option, StringView arg, T& place, F parser) + static size_t try_parse_argument_as_option( + StringView arg, Optional lookahead, StringView option, T& place, F parser) { - if (arg.size() <= option.size() + 1) - { - // it is impossible for this argument to be this option - return false; - } - 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 2; + } + + 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 1; + } } - return false; + return 0; } static bool equals_modulo_experimental(StringView arg, StringView option) @@ -230,13 +222,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 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 +261,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 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 +291,49 @@ namespace vcpkg {JSON_SWITCH, &VcpkgCmdArguments::json}, }; + Optional lookahead; + { + auto next = it; + ++next; + if (next != arg_last) + { + lookahead = *next; + } + } + 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 2: ++it; // fallthrough + case 1: found = true; break; + case 0: 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 2: ++it; // fallthrough + case 1: found = true; break; + case 0: 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 2: ++it; // fallthrough + case 1: continue; + case 0: break; + default: Checks::unreachable(VCPKG_LINE_INFO); } for (const auto& pr : switches) @@ -609,7 +607,7 @@ namespace vcpkg return Strings::concat("--", arg, joiner, value); }; - table.format(opt(TRIPLET_ARG, " ", ""), "Specify the target architecture triplet. See 'vcpkg help triplet'"); + table.format(opt(TRIPLET_ARG, "=", ""), "Specify the target architecture triplet. See 'vcpkg help triplet'"); table.format("", "(default: " + format_environment_variable("VCPKG_DEFAULT_TRIPLET") + ')'); table.format(opt(OVERLAY_PORTS_ARG, "=", ""), "Specify directories to be used when searching for ports"); table.format("", "(also: " + format_environment_variable("VCPKG_OVERLAY_PORTS") + ')'); @@ -619,7 +617,7 @@ namespace vcpkg "Add sources for binary caching. See 'vcpkg help binarycaching'"); table.format(opt(DOWNLOADS_ROOT_DIR_ARG, "=", ""), "Specify the downloads root directory"); table.format("", "(default: " + format_environment_variable("VCPKG_DOWNLOADS") + ')'); - table.format(opt(VCPKG_ROOT_DIR_ARG, " ", ""), "Specify the vcpkg root directory"); + table.format(opt(VCPKG_ROOT_DIR_ARG, "=", ""), "Specify the vcpkg root directory"); table.format("", "(default: " + format_environment_variable("VCPKG_ROOT") + ')'); table.format(opt(BUILDTREES_ROOT_DIR_ARG, "=", ""), "(Experimental) Specify the buildtrees root directory"); From 00a1d8a3f8c415e4bfa5edd1e33c4054b1e27e22 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Fri, 2 Oct 2020 16:17:49 -0700 Subject: [PATCH 2/8] Fix vcpkgcmdarguments allowing bogus switches. Also rename $args to $testArgs because the former is a PowerShell builtin variable. --- scripts/azure-pipelines/end-to-end-tests.ps1 | 58 ++++++++++++++------ toolsrc/src/vcpkg/vcpkgcmdarguments.cpp | 2 +- 2 files changed, 42 insertions(+), 18 deletions(-) diff --git a/scripts/azure-pipelines/end-to-end-tests.ps1 b/scripts/azure-pipelines/end-to-end-tests.ps1 index 43ec7aa73ede28..7d16fdaf9352b9 100644 --- a/scripts/azure-pipelines/end-to-end-tests.ps1 +++ b/scripts/azure-pipelines/end-to-end-tests.ps1 @@ -73,9 +73,13 @@ function Throw-IfFailed { throw "'$CurrentTest' had a step with a nonzero exit code" } } +function Throw-IfNotFailed { + if ($LASTEXITCODE -ne 0) { + throw "'$CurrentTest' had a step with an unexpectedly zero exit code" + } +} -if (-not $IsLinux -and -not $IsMacOS) -{ +if (-not $IsLinux -and -not $IsMacOS) { Refresh-TestRoot # Test msbuild props and targets $CurrentTest = "zlib:x86-windows-static msbuild scripts\testing\integrate-install\..." @@ -106,8 +110,8 @@ 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 ' ')" +$testArgs = $commonArgs + @("install", "rapidjson", "--binarycaching", "--x-binarysource=clear;files,$ArchiveRoot,write;nuget,$NuGetRoot,readwrite") +$CurrentTest = "./vcpkg $($testArgs -join ' ')" Write-Host $CurrentTest ./vcpkg @args Throw-IfFailed @@ -115,8 +119,8 @@ Throw-IfFailed Require-FileExists "$installRoot/$Triplet/include/rapidjson/rapidjson.h" # Test simple removal -$args = $commonArgs + @("remove", "rapidjson") -$CurrentTest = "./vcpkg $($args -join ' ')" +$testArgs = $commonArgs + @("remove", "rapidjson") +$CurrentTest = "./vcpkg $($testArgs -join ' ')" Write-Host $CurrentTest ./vcpkg @args Throw-IfFailed @@ -124,8 +128,8 @@ Throw-IfFailed Require-FileNotExists "$installRoot/$Triplet/include/rapidjson/rapidjson.h" # Test restoring from files archive -$args = $commonArgs + @("install","rapidjson","--binarycaching","--x-binarysource=clear;files,$ArchiveRoot,read") -$CurrentTest = "./vcpkg $($args -join ' ')" +$testArgs = $commonArgs + @("install", "rapidjson", "--binarycaching", "--x-binarysource=clear;files,$ArchiveRoot,read") +$CurrentTest = "./vcpkg $($testArgs -join ' ')" Remove-Item -Recurse -Force $installRoot Remove-Item -Recurse -Force $buildtreesRoot Write-Host $CurrentTest @@ -136,8 +140,8 @@ Require-FileExists "$installRoot/$Triplet/include/rapidjson/rapidjson.h" Require-FileNotExists "$buildtreesRoot/rapidjson/src" # Test restoring from nuget -$args = $commonArgs + @("install","rapidjson","--binarycaching","--x-binarysource=clear;nuget,$NuGetRoot") -$CurrentTest = "./vcpkg $($args -join ' ')" +$testArgs = $commonArgs + @("install", "rapidjson", "--binarycaching", "--x-binarysource=clear;nuget,$NuGetRoot") +$CurrentTest = "./vcpkg $($testArgs -join ' ')" Remove-Item -Recurse -Force $installRoot Remove-Item -Recurse -Force $buildtreesRoot Write-Host $CurrentTest @@ -148,8 +152,8 @@ 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 ' ')" +$testArgs = $commonArgs + @("install", "rapidjson", "--dry-run", "--x-write-nuget-packages-config=$TestingRoot/packages.config") +$CurrentTest = "./vcpkg $($testArgs -join ' ')" Remove-Item -Recurse -Force $installRoot -ErrorAction SilentlyContinue Write-Host $CurrentTest ./vcpkg @args @@ -161,7 +165,8 @@ Require-FileExists "$TestingRoot/packages.config" if ($IsLinux -or $IsMacOS) { mono $(./vcpkg fetch nuget) restore $TestingRoot/packages.config -OutputDirectory "$NuGetRoot2" -Source "$NuGetRoot" Throw-IfFailed -} else { +} +else { & $(./vcpkg fetch nuget) restore $TestingRoot/packages.config -OutputDirectory "$NuGetRoot2" -Source "$NuGetRoot" Throw-IfFailed } @@ -169,8 +174,8 @@ if ($IsLinux -or $IsMacOS) { 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 ' ')" +$testArgs = $commonArgs + @("install", "rapidjson", "tinyxml", "--binarycaching", "--x-binarysource=clear;nuget,$NuGetRoot2;nuget,$NuGetRoot,write") +$CurrentTest = "./vcpkg $($testArgs -join ' ')" Write-Host $CurrentTest ./vcpkg @args Throw-IfFailed @@ -184,8 +189,8 @@ if ((Get-ChildItem $NuGetRoot -Filter '*.nupkg' | Measure-Object).Count -ne 1) { } # 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 ' ')" +$testArgs = $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 $($testArgs -join ' ')" Write-Host $CurrentTest Require-FileNotExists "$TestingRoot/vcpkg-export-output" Require-FileNotExists "$TestingRoot/vcpkg-export.1.0.0.nupkg" @@ -194,3 +199,22 @@ Require-FileNotExists "$TestingRoot/vcpkg-export-output.zip" 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 +$testArgs = $commonArgs + @("install", "zlib", "--vcpkg-rootttttt", "C:\") +$CurrentTest = "./vcpkg $($testArgs -join ' ')" +Write-Host $CurrentTest +./vcpkg @args +Throw-IfNotFailed + +$testArgs = $commonArgs + @("install", "zlib", "--vcpkg-rootttttt=C:\") +$CurrentTest = "./vcpkg $($testArgs -join ' ')" +Write-Host $CurrentTest +./vcpkg @args +Throw-IfNotFailed + +$testArgs = $commonArgs + @("install", "zlib", "--fast") # NB: --fast is not a switch +$CurrentTest = "./vcpkg $($testArgs -join ' ')" +Write-Host $CurrentTest +./vcpkg @args +Throw-IfNotFailed diff --git a/toolsrc/src/vcpkg/vcpkgcmdarguments.cpp b/toolsrc/src/vcpkg/vcpkgcmdarguments.cpp index 05b802441a7212..e020698ccf9511 100644 --- a/toolsrc/src/vcpkg/vcpkgcmdarguments.cpp +++ b/toolsrc/src/vcpkg/vcpkgcmdarguments.cpp @@ -513,7 +513,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) From ae6431fa644699c12024d3abff50823df0a07650 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 7 Oct 2020 01:25:34 -0700 Subject: [PATCH 3/8] CR feedback. --- toolsrc/src/vcpkg/vcpkgcmdarguments.cpp | 45 ++++++++++++------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/toolsrc/src/vcpkg/vcpkgcmdarguments.cpp b/toolsrc/src/vcpkg/vcpkgcmdarguments.cpp index 9a92b292e613f9..7b4e9c40dc99ca 100644 --- a/toolsrc/src/vcpkg/vcpkgcmdarguments.cpp +++ b/toolsrc/src/vcpkg/vcpkgcmdarguments.cpp @@ -152,12 +152,15 @@ namespace vcpkg return VcpkgCmdArguments::create_from_arg_sequence(v.data(), v.data() + v.size()); } - // returns the number of consumed arguments: - // 0: parse unsuccessful - // 1: parse successful and option was consumed - // 2: parse successful and both option and lookahead were consumed + enum class TryParseArgumentResult + { + NotFound, + Found, + FoundAndConsumedLookahead + }; + template - static size_t try_parse_argument_as_option( + static TryParseArgumentResult try_parse_argument_as_option( StringView arg, Optional lookahead, StringView option, T& place, F parser) { if (Strings::starts_with(arg, "x-") && !Strings::starts_with(option, "x-")) @@ -172,7 +175,7 @@ namespace vcpkg if (auto next = lookahead.get()) { parser(*next, option, place); - return 2; + return TryParseArgumentResult::FoundAndConsumedLookahead; } System::print2(System::Color::error, "Error: expected value after ", option, '\n'); @@ -184,11 +187,11 @@ namespace vcpkg if (arg.byte_at_index(option.size()) == '=') { parser(arg.substr(option.size() + 1), option, place); - return 1; + return TryParseArgumentResult::Found; } } - return 0; + return TryParseArgumentResult::NotFound; } static bool equals_modulo_experimental(StringView arg, StringView option) @@ -292,13 +295,9 @@ namespace vcpkg }; Optional lookahead; + if (it + 1 != arg_last) { - auto next = it; - ++next; - if (next != arg_last) - { - lookahead = *next; - } + lookahead = it[1]; } bool found = false; @@ -306,9 +305,9 @@ namespace vcpkg { switch (try_parse_argument_as_option(arg, lookahead, pr.first, args.*pr.second, parse_cojoined_value)) { - case 2: ++it; // fallthrough - case 1: found = true; break; - case 0: break; + case TryParseArgumentResult::FoundAndConsumedLookahead: ++it; [[fallthrough]]; + case TryParseArgumentResult::Found: found = true; break; + case TryParseArgumentResult::NotFound: break; default: Checks::unreachable(VCPKG_LINE_INFO); } } @@ -319,9 +318,9 @@ namespace vcpkg switch ( try_parse_argument_as_option(arg, lookahead, pr.first, args.*pr.second, parse_cojoined_multivalue)) { - case 2: ++it; // fallthrough - case 1: found = true; break; - case 0: break; + case TryParseArgumentResult::FoundAndConsumedLookahead: ++it; [[fallthrough]]; + case TryParseArgumentResult::Found: found = true; break; + case TryParseArgumentResult::NotFound: break; default: Checks::unreachable(VCPKG_LINE_INFO); } } @@ -330,9 +329,9 @@ namespace vcpkg switch (try_parse_argument_as_option( arg, lookahead, FEATURE_FLAGS_ARG, feature_flags, parse_cojoined_list_multivalue)) { - case 2: ++it; // fallthrough - case 1: continue; - case 0: break; + case TryParseArgumentResult::FoundAndConsumedLookahead: ++it; [[fallthrough]]; + case TryParseArgumentResult::Found: found = true; break; + case TryParseArgumentResult::NotFound: break; default: Checks::unreachable(VCPKG_LINE_INFO); } From b4a90014a4ed9775ea07c116176432854e575fbd Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 7 Oct 2020 02:37:50 -0700 Subject: [PATCH 4/8] More args=>testArgs fixes. --- scripts/azure-pipelines/end-to-end-tests.ps1 | 32 ++++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/scripts/azure-pipelines/end-to-end-tests.ps1 b/scripts/azure-pipelines/end-to-end-tests.ps1 index 9df5f96ccfdc48..ee86b2de55f87e 100644 --- a/scripts/azure-pipelines/end-to-end-tests.ps1 +++ b/scripts/azure-pipelines/end-to-end-tests.ps1 @@ -113,7 +113,7 @@ Refresh-TestRoot $testArgs = $commonArgs + @("install", "rapidjson", "--binarycaching", "--x-binarysource=clear;files,$ArchiveRoot,write;nuget,$NuGetRoot,readwrite") $CurrentTest = "./vcpkg $($testArgs -join ' ')" Write-Host $CurrentTest -./vcpkg @args +./vcpkg @testArgs Throw-IfFailed Require-FileExists "$installRoot/$Triplet/include/rapidjson/rapidjson.h" @@ -122,7 +122,7 @@ Require-FileExists "$installRoot/$Triplet/include/rapidjson/rapidjson.h" $testArgs = $commonArgs + @("remove", "rapidjson") $CurrentTest = "./vcpkg $($testArgs -join ' ')" Write-Host $CurrentTest -./vcpkg @args +./vcpkg @testArgs Throw-IfFailed Require-FileNotExists "$installRoot/$Triplet/include/rapidjson/rapidjson.h" @@ -133,7 +133,7 @@ $CurrentTest = "./vcpkg $($testArgs -join ' ')" Remove-Item -Recurse -Force $installRoot Remove-Item -Recurse -Force $buildtreesRoot Write-Host $CurrentTest -./vcpkg @args +./vcpkg @testArgs Throw-IfFailed Require-FileExists "$installRoot/$Triplet/include/rapidjson/rapidjson.h" @@ -141,12 +141,12 @@ 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 ' ')" +$testArgs = $commonArgs + @("install","rapidjson","--no-binarycaching","--x-binarysource=clear;files,$ArchiveRoot,read") +$CurrentTest = "./vcpkg $($testArgs -join ' ')" Remove-Item -Recurse -Force $installRoot Remove-Item -Recurse -Force $buildtreesRoot Write-Host $CurrentTest -./vcpkg @args +./vcpkg @testArgs Throw-IfFailed Require-FileExists "$installRoot/$Triplet/include/rapidjson/rapidjson.h" @@ -154,12 +154,12 @@ 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 ' ')" +$testArgs = $commonArgs + @("install","rapidjson","--editable","--x-binarysource=clear;files,$ArchiveRoot,read") +$CurrentTest = "./vcpkg $($testArgs -join ' ')" Remove-Item -Recurse -Force $installRoot Remove-Item -Recurse -Force $buildtreesRoot Write-Host $CurrentTest -./vcpkg @args +./vcpkg @testArgs Throw-IfFailed Require-FileExists "$installRoot/$Triplet/include/rapidjson/rapidjson.h" @@ -172,7 +172,7 @@ $CurrentTest = "./vcpkg $($testArgs -join ' ')" Remove-Item -Recurse -Force $installRoot Remove-Item -Recurse -Force $buildtreesRoot Write-Host $CurrentTest -./vcpkg @args +./vcpkg @testArgs Throw-IfFailed Require-FileExists "$installRoot/$Triplet/include/rapidjson/rapidjson.h" @@ -183,7 +183,7 @@ $testArgs = $commonArgs + @("install", "rapidjson", "--dry-run", "--x-write-nuge $CurrentTest = "./vcpkg $($testArgs -join ' ')" Remove-Item -Recurse -Force $installRoot -ErrorAction SilentlyContinue Write-Host $CurrentTest -./vcpkg @args +./vcpkg @testArgs Throw-IfFailed Require-FileNotExists "$installRoot/$Triplet/include/rapidjson/rapidjson.h" Require-FileNotExists "$buildtreesRoot/rapidjson/src" @@ -204,7 +204,7 @@ mkdir $NuGetRoot $testArgs = $commonArgs + @("install", "rapidjson", "tinyxml", "--binarycaching", "--x-binarysource=clear;nuget,$NuGetRoot2;nuget,$NuGetRoot,write") $CurrentTest = "./vcpkg $($testArgs -join ' ')" Write-Host $CurrentTest -./vcpkg @args +./vcpkg @testArgs Throw-IfFailed Require-FileExists "$installRoot/$Triplet/include/rapidjson/rapidjson.h" Require-FileExists "$installRoot/$Triplet/include/tinyxml.h" @@ -222,7 +222,7 @@ 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 +./vcpkg @testArgs Require-FileExists "$TestingRoot/vcpkg-export-output" Require-FileExists "$TestingRoot/vcpkg-export.1.0.0.nupkg" Require-FileExists "$TestingRoot/vcpkg-export-output.zip" @@ -231,17 +231,17 @@ Require-FileExists "$TestingRoot/vcpkg-export-output.zip" $testArgs = $commonArgs + @("install", "zlib", "--vcpkg-rootttttt", "C:\") $CurrentTest = "./vcpkg $($testArgs -join ' ')" Write-Host $CurrentTest -./vcpkg @args +./vcpkg @testArgs Throw-IfNotFailed $testArgs = $commonArgs + @("install", "zlib", "--vcpkg-rootttttt=C:\") $CurrentTest = "./vcpkg $($testArgs -join ' ')" Write-Host $CurrentTest -./vcpkg @args +./vcpkg @testArgs Throw-IfNotFailed $testArgs = $commonArgs + @("install", "zlib", "--fast") # NB: --fast is not a switch $CurrentTest = "./vcpkg $($testArgs -join ' ')" Write-Host $CurrentTest -./vcpkg @args +./vcpkg @testArgs Throw-IfNotFailed From 3e8975bb5096b24e910de42bacb147256b0f6d66 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 7 Oct 2020 02:42:24 -0700 Subject: [PATCH 5/8] Fix inverted if test in e2e tests. --- scripts/azure-pipelines/end-to-end-tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/azure-pipelines/end-to-end-tests.ps1 b/scripts/azure-pipelines/end-to-end-tests.ps1 index ee86b2de55f87e..eb001c39c1c43c 100644 --- a/scripts/azure-pipelines/end-to-end-tests.ps1 +++ b/scripts/azure-pipelines/end-to-end-tests.ps1 @@ -74,7 +74,7 @@ function Throw-IfFailed { } } function Throw-IfNotFailed { - if ($LASTEXITCODE -ne 0) { + if ($LASTEXITCODE -eq 0) { throw "'$CurrentTest' had a step with an unexpectedly zero exit code" } } From 2a47028777381513566673548561047b818321a7 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 7 Oct 2020 09:37:20 -0700 Subject: [PATCH 6/8] Add return 0 to fix bad Azure Pipelines failure. --- scripts/azure-pipelines/end-to-end-tests.ps1 | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/azure-pipelines/end-to-end-tests.ps1 b/scripts/azure-pipelines/end-to-end-tests.ps1 index eb001c39c1c43c..97ad83309a4f5f 100644 --- a/scripts/azure-pipelines/end-to-end-tests.ps1 +++ b/scripts/azure-pipelines/end-to-end-tests.ps1 @@ -245,3 +245,6 @@ $CurrentTest = "./vcpkg $($testArgs -join ' ')" Write-Host $CurrentTest ./vcpkg @testArgs Throw-IfNotFailed + +Write-Host 'All tests passed.' +return 0 # Ensure Pipelines thinks we passed if we got here. From 7046e6f6f5149f4c6a2c597ed16f284396ba167c Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 7 Oct 2020 09:45:42 -0700 Subject: [PATCH 7/8] Extract Run-Vcpkg in e2e tests. --- scripts/azure-pipelines/end-to-end-tests.ps1 | 87 ++++++-------------- 1 file changed, 26 insertions(+), 61 deletions(-) diff --git a/scripts/azure-pipelines/end-to-end-tests.ps1 b/scripts/azure-pipelines/end-to-end-tests.ps1 index 97ad83309a4f5f..f120b895cf0dbb 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,17 +70,26 @@ 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" } } + 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 @@ -110,140 +121,94 @@ if (-not $IsLinux -and -not $IsMacOS) { Refresh-TestRoot # Test simple installation -$testArgs = $commonArgs + @("install", "rapidjson", "--binarycaching", "--x-binarysource=clear;files,$ArchiveRoot,write;nuget,$NuGetRoot,readwrite") -$CurrentTest = "./vcpkg $($testArgs -join ' ')" -Write-Host $CurrentTest -./vcpkg @testArgs +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 -$testArgs = $commonArgs + @("remove", "rapidjson") -$CurrentTest = "./vcpkg $($testArgs -join ' ')" -Write-Host $CurrentTest -./vcpkg @testArgs +Run-Vcpkg -TestArgs ($commonArgs + @("remove", "rapidjson")) Throw-IfFailed - Require-FileNotExists "$installRoot/$Triplet/include/rapidjson/rapidjson.h" # Test restoring from files archive -$testArgs = $commonArgs + @("install","rapidjson","--binarycaching","--x-binarysource=clear;files,$ArchiveRoot,read") -$CurrentTest = "./vcpkg $($testArgs -join ' ')" Remove-Item -Recurse -Force $installRoot Remove-Item -Recurse -Force $buildtreesRoot -Write-Host $CurrentTest -./vcpkg @testArgs +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 -$testArgs = $commonArgs + @("install","rapidjson","--no-binarycaching","--x-binarysource=clear;files,$ArchiveRoot,read") -$CurrentTest = "./vcpkg $($testArgs -join ' ')" Remove-Item -Recurse -Force $installRoot Remove-Item -Recurse -Force $buildtreesRoot -Write-Host $CurrentTest -./vcpkg @testArgs +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 -$testArgs = $commonArgs + @("install","rapidjson","--editable","--x-binarysource=clear;files,$ArchiveRoot,read") -$CurrentTest = "./vcpkg $($testArgs -join ' ')" Remove-Item -Recurse -Force $installRoot Remove-Item -Recurse -Force $buildtreesRoot -Write-Host $CurrentTest -./vcpkg @testArgs +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 -$testArgs = $commonArgs + @("install", "rapidjson", "--binarycaching", "--x-binarysource=clear;nuget,$NuGetRoot") -$CurrentTest = "./vcpkg $($testArgs -join ' ')" Remove-Item -Recurse -Force $installRoot Remove-Item -Recurse -Force $buildtreesRoot -Write-Host $CurrentTest -./vcpkg @testArgs +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 -$testArgs = $commonArgs + @("install", "rapidjson", "--dry-run", "--x-write-nuget-packages-config=$TestingRoot/packages.config") -$CurrentTest = "./vcpkg $($testArgs -join ' ')" Remove-Item -Recurse -Force $installRoot -ErrorAction SilentlyContinue -Write-Host $CurrentTest -./vcpkg @testArgs +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 { +} 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 - -$testArgs = $commonArgs + @("install", "rapidjson", "tinyxml", "--binarycaching", "--x-binarysource=clear;nuget,$NuGetRoot2;nuget,$NuGetRoot,write") -$CurrentTest = "./vcpkg $($testArgs -join ' ')" -Write-Host $CurrentTest -./vcpkg @testArgs +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 -$testArgs = $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 $($testArgs -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 @testArgs +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 -$testArgs = $commonArgs + @("install", "zlib", "--vcpkg-rootttttt", "C:\") -$CurrentTest = "./vcpkg $($testArgs -join ' ')" -Write-Host $CurrentTest -./vcpkg @testArgs +Run-Vcpkg -TestArgs ($commonArgs + @("install", "zlib", "--vcpkg-rootttttt", "C:\")) Throw-IfNotFailed -$testArgs = $commonArgs + @("install", "zlib", "--vcpkg-rootttttt=C:\") -$CurrentTest = "./vcpkg $($testArgs -join ' ')" -Write-Host $CurrentTest -./vcpkg @testArgs +Run-Vcpkg -TestArgs ($commonArgs + @("install", "zlib", "--vcpkg-rootttttt=C:\")) Throw-IfNotFailed -$testArgs = $commonArgs + @("install", "zlib", "--fast") # NB: --fast is not a switch -$CurrentTest = "./vcpkg $($testArgs -join ' ')" -Write-Host $CurrentTest -./vcpkg @testArgs +Run-Vcpkg -TestArgs ($commonArgs + @("install", "zlib", "--fast")) # NB: --fast is not a switch Throw-IfNotFailed Write-Host 'All tests passed.' From fd809f7ba6e002cda6458eb0ce0fddb859eb8591 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 7 Oct 2020 10:00:00 -0700 Subject: [PATCH 8/8] Try to fix $LASTEXITCODE again.... --- scripts/azure-pipelines/end-to-end-tests.ps1 | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scripts/azure-pipelines/end-to-end-tests.ps1 b/scripts/azure-pipelines/end-to-end-tests.ps1 index f120b895cf0dbb..50e7f6afd19c3e 100644 --- a/scripts/azure-pipelines/end-to-end-tests.ps1 +++ b/scripts/azure-pipelines/end-to-end-tests.ps1 @@ -211,5 +211,4 @@ Throw-IfNotFailed Run-Vcpkg -TestArgs ($commonArgs + @("install", "zlib", "--fast")) # NB: --fast is not a switch Throw-IfNotFailed -Write-Host 'All tests passed.' -return 0 # Ensure Pipelines thinks we passed if we got here. +$LASTEXITCODE = 0