From ca6d48dab510a55cf3193298136639cb79ff50d3 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Tue, 9 Jan 2024 21:47:52 -0800 Subject: [PATCH] Fix unit test failures. --- include/vcpkg/base/diagnostics.h | 1 + include/vcpkg/base/system.mac.h | 5 +- src/vcpkg-test/cgroup-parser.cpp | 6 ++- src/vcpkg-test/manifests.cpp | 2 +- src/vcpkg-test/system.mac.cpp | 13 +++-- src/vcpkg/base/diagnostics.cpp | 18 +++++++ src/vcpkg/base/git.cpp | 1 + src/vcpkg/base/parse.cpp | 20 +++++--- src/vcpkg/base/system.mac.cpp | 88 ++++++++++++++++++++++++-------- src/vcpkg/ci-baseline.cpp | 8 +-- src/vcpkg/packagespec.cpp | 11 +++- src/vcpkg/paragraphs.cpp | 18 ++++--- src/vcpkg/sourceparagraph.cpp | 3 +- 13 files changed, 147 insertions(+), 47 deletions(-) diff --git a/include/vcpkg/base/diagnostics.h b/include/vcpkg/base/diagnostics.h index bbcc3db176..01bef61aa1 100644 --- a/include/vcpkg/base/diagnostics.h +++ b/include/vcpkg/base/diagnostics.h @@ -115,6 +115,7 @@ namespace vcpkg }; extern DiagnosticContext& console_diagnostic_context; + extern DiagnosticContext& null_diagnostic_context; // If Ty is an Optional, typename AdaptContextUnwrapOptional::type is the type necessary to return U, and fwd // is the type necessary to forward U. Otherwise, there are no members ::type or ::fwd diff --git a/include/vcpkg/base/system.mac.h b/include/vcpkg/base/system.mac.h index 9e36f8aa72..c776a057b3 100644 --- a/include/vcpkg/base/system.mac.h +++ b/include/vcpkg/base/system.mac.h @@ -3,6 +3,9 @@ #include #include +#include +#include + #include namespace vcpkg @@ -13,5 +16,5 @@ namespace vcpkg bool validate_mac_address_format(StringView mac); bool is_valid_mac_for_telemetry(StringView mac); std::string mac_bytes_to_string(const Span& bytes); - bool extract_mac_from_getmac_output_line(StringView line, std::string& out); + Optional extract_mac_from_getmac_output_line(DiagnosticContext& context, StringView line); } diff --git a/src/vcpkg-test/cgroup-parser.cpp b/src/vcpkg-test/cgroup-parser.cpp index fc7784fb84..bf4ea26ff4 100644 --- a/src/vcpkg-test/cgroup-parser.cpp +++ b/src/vcpkg-test/cgroup-parser.cpp @@ -132,7 +132,11 @@ TEST_CASE ("parse proc/pid/stat file", "[cgroup-parser]") std::string contents = R"(4281 (0123456789abcdefg) S 4099 1676 1676 0 -1 1077936384 51165 303 472 0 81 25 0 0 20 0 10 0 829158 4924583936 39830 18446744073709551615 4194304 14147733 140725993620736 0 0 0 0 16781312 16386 0 0 0 17 1 0 0 5 0 0 16247120 16519160 29999104 140725993622792 140725993622920 140725993622920 140725993627556 0)"; - auto maybe_stat = try_parse_process_stat_file(console_diagnostic_context, {contents, "test"}); + BufferedDiagnosticContext bdc; + auto maybe_stat = try_parse_process_stat_file(bdc, {contents, "test"}); REQUIRE(!maybe_stat.has_value()); + REQUIRE(bdc.to_string() == R"(test:1:7: error: expected ')' here + on expression: 4281 (0123456789abcdefg) S 4099 1676 1676 0 -1 1077936384 51165 303 472 0 81 25 0 0 20 0 10 0 829158 4924583936 39830 18446744073709551615 4194304 14147733 140725993620736 0 0 0 0 16781312 16386 0 0 0 17 1 0 0 5 0 0 16247120 16519160 29999104 140725993622792 140725993622920 140725993622920 140725993627556 0 + ^)"); } } diff --git a/src/vcpkg-test/manifests.cpp b/src/vcpkg-test/manifests.cpp index df71153545..9e771f8a60 100644 --- a/src/vcpkg-test/manifests.cpp +++ b/src/vcpkg-test/manifests.cpp @@ -1285,7 +1285,7 @@ TEST_CASE ("license error messages", "[manifests][license]") } { BufferedDiagnosticContext bdc; - CHECK(!parse_spdx_license_expression(bdc, "MIT AND unknownlicense").has_value()); + CHECK(parse_spdx_license_expression(bdc, "MIT AND unknownlicense").has_value()); CHECK( bdc.to_string() == R"(:1:9: warning: Unknown license identifier 'unknownlicense'. Known values are listed at https://spdx.org/licenses/ diff --git a/src/vcpkg-test/system.mac.cpp b/src/vcpkg-test/system.mac.cpp index 48f982592b..29834cee0b 100644 --- a/src/vcpkg-test/system.mac.cpp +++ b/src/vcpkg-test/system.mac.cpp @@ -51,16 +51,19 @@ TEST_CASE ("MAC bytes to string", "[metrics.mac]") CHECK(long_mac_str.empty()); } -TEST_CASE ("test getmac ouptut parse", "[metrics.mac]") +TEST_CASE ("test getmac output parse", "[metrics.mac]") { std::string mac_str; static constexpr StringLiteral good_line = R"csv("Wi-Fi","Wi-Fi 6, maybe","00-11-22-DD-EE-FF","\Device\Tcip_{GUID}")csv"; - CHECK(extract_mac_from_getmac_output_line(good_line, mac_str)); - CHECK(mac_str == "00:11:22:dd:ee:ff"); + BufferedDiagnosticContext bdc; + REQUIRE(extract_mac_from_getmac_output_line(bdc, good_line).value_or_exit(VCPKG_LINE_INFO) == "00:11:22:dd:ee:ff"); + REQUIRE(bdc.lines.empty()); static constexpr StringLiteral bad_line = "00-11-22-DD-EE-FF \\Device\\Tcip_{GUID}"; - CHECK(!extract_mac_from_getmac_output_line(bad_line, mac_str)); - CHECK(mac_str.empty()); + REQUIRE(!extract_mac_from_getmac_output_line(bdc, bad_line)); + REQUIRE(bdc.to_string() == R"(getmac output:1:1: error: expected '"' here + on expression: 00-11-22-DD-EE-FF \Device\Tcip_{GUID} + ^)"); } diff --git a/src/vcpkg/base/diagnostics.cpp b/src/vcpkg/base/diagnostics.cpp index 3351084a1e..27aba53636 100644 --- a/src/vcpkg/base/diagnostics.cpp +++ b/src/vcpkg/base/diagnostics.cpp @@ -179,3 +179,21 @@ namespace vcpkg { DiagnosticContext& console_diagnostic_context = console_diagnostic_context_instance; } + +namespace +{ + struct NullDiagnosticContext : DiagnosticContext + { + virtual void report(const DiagnosticLine&) override + { + // intentionally empty + } + }; + + NullDiagnosticContext null_diagnostic_context_instance; +} + +namespace vcpkg +{ + DiagnosticContext& null_diagnostic_context = null_diagnostic_context_instance; +} diff --git a/src/vcpkg/base/git.cpp b/src/vcpkg/base/git.cpp index 64a327ba39..2afae50e25 100644 --- a/src/vcpkg/base/git.cpp +++ b/src/vcpkg/base/git.cpp @@ -147,6 +147,7 @@ namespace vcpkg { context.report(DiagnosticLine{DiagKind::Note, msg::format(msgGitUnexpectedCommandOutputCmd, msg::command_line = cmd_line)}); + results_storage.clear(); } return results_storage; diff --git a/src/vcpkg/base/parse.cpp b/src/vcpkg/base/parse.cpp index 80e01f2790..9afac167fc 100644 --- a/src/vcpkg/base/parse.cpp +++ b/src/vcpkg/base/parse.cpp @@ -185,19 +185,25 @@ namespace vcpkg void ParserBase::add_error(LocalizedString&& message, const SourceLoc& loc) { - message.append_raw('\n'); - append_caret_line(message, loc); - m_context.report( - DiagnosticLine{DiagKind::Error, m_origin, TextPosition{loc.row, loc.column}, std::move(message)}); - m_any_errors = true; - // Avoid error loops by skipping to the end - skip_to_eof(); + // avoid cascading errors by only saving the first + if (!m_any_errors) + { + message.append_raw('\n'); + append_caret_line(message, loc); + m_context.report( + DiagnosticLine{DiagKind::Error, m_origin, TextPosition{loc.row, loc.column}, std::move(message)}); + m_any_errors = true; + // Avoid error loops by skipping to the end + skip_to_eof(); + } } void ParserBase::add_warning(LocalizedString&& message) { add_warning(std::move(message), cur_loc()); } void ParserBase::add_warning(LocalizedString&& message, const SourceLoc& loc) { + message.append_raw('\n'); + append_caret_line(message, loc); m_context.report( DiagnosticLine{DiagKind::Warning, m_origin, TextPosition{loc.row, loc.column}, std::move(message)}); } diff --git a/src/vcpkg/base/system.mac.cpp b/src/vcpkg/base/system.mac.cpp index 781068b466..03ba40deb8 100644 --- a/src/vcpkg/base/system.mac.cpp +++ b/src/vcpkg/base/system.mac.cpp @@ -84,51 +84,96 @@ namespace vcpkg return std::string(mac_address, non_zero_mac ? MAC_STRING_LENGTH : 0); } - bool extract_mac_from_getmac_output_line(StringView line, std::string& out) + Optional extract_mac_from_getmac_output_line(DiagnosticContext& context, StringView line) { // getmac /V /NH /FO CSV // outputs each interface on its own comma-separated line // "connection name","network adapter","physical address","transport name" auto is_quote = [](auto ch) -> bool { return ch == '"'; }; - auto parser = ParserBase(console_diagnostic_context, line, "getmac ouptut"); + auto parser = ParserBase(context, line, "getmac output"); - out.clear(); + Optional result; + auto& out = result.emplace(); // ignore "connection name" - if (parser.require_character('"')) return false; + if (parser.require_character('"')) + { + result.clear(); + return result; + } parser.match_until(is_quote); - if (parser.require_character('"')) return false; - if (parser.require_character(',')) return false; + if (parser.require_character('"')) + { + result.clear(); + return result; + } + if (parser.require_character(',')) + { + result.clear(); + return result; + } // ignore "network adapter" - if (parser.require_character('"')) return false; + if (parser.require_character('"')) + { + result.clear(); + return result; + } parser.match_until(is_quote); - if (parser.require_character('"')) return false; - if (parser.require_character(',')) return false; + if (parser.require_character('"')) + { + result.clear(); + return result; + } + if (parser.require_character(',')) + { + result.clear(); + return result; + } // get "physical address" - if (parser.require_character('"')) return false; + if (parser.require_character('"')) + { + result.clear(); + return result; + } out = parser.match_until(is_quote).to_string(); - if (parser.require_character('"')) return false; - if (parser.require_character(',')) return false; + if (parser.require_character('"')) + { + result.clear(); + return result; + } + if (parser.require_character(',')) + { + result.clear(); + return result; + } // ignore "transport name" - if (parser.require_character('"')) return false; + if (parser.require_character('"')) + { + result.clear(); + return result; + } parser.match_until(is_quote); - if (parser.require_character('"')) return false; + if (parser.require_character('"')) + { + result.clear(); + return result; + } parser.skip_whitespace(); if (!parser.at_eof()) { - out.clear(); - return false; + result.clear(); + return result; } // output line was properly formatted std::replace(out.begin(), out.end(), '-', ':'); Strings::inplace_ascii_to_lowercase(out); - return true; + return result; } std::string get_user_mac_hash() @@ -143,10 +188,13 @@ namespace vcpkg { for (auto&& line : Strings::split(getmac->output, '\n')) { - std::string mac; - if (extract_mac_from_getmac_output_line(line, mac) && is_valid_mac_for_telemetry(mac)) + auto maybe_mac = extract_mac_from_getmac_output_line(null_diagnostic_context, line); + if (auto mac = maybe_mac.get()) { - return Hash::get_string_hash(mac, Hash::Algorithm::Sha256); + if (is_valid_mac_for_telemetry(*mac)) + { + return Hash::get_string_hash(*mac, Hash::Algorithm::Sha256); + } } } } diff --git a/src/vcpkg/ci-baseline.cpp b/src/vcpkg/ci-baseline.cpp index fc32f88fda..3acbe15198 100644 --- a/src/vcpkg/ci-baseline.cpp +++ b/src/vcpkg/ci-baseline.cpp @@ -60,7 +60,8 @@ namespace vcpkg StringView text, StringView origin) { - std::vector result; + Optional> result_storage; + auto& result = result_storage.emplace(); ParserBase parser(context, text, origin); for (;;) { @@ -68,7 +69,7 @@ namespace vcpkg if (parser.at_eof()) { // success - return result; + return result_storage; } if (parser.cur() == '#') @@ -148,7 +149,8 @@ namespace vcpkg } // failure - return nullopt; + result_storage.clear(); + return result_storage; } CiBaselineData parse_and_apply_ci_baseline(View lines, diff --git a/src/vcpkg/packagespec.cpp b/src/vcpkg/packagespec.cpp index 207c0caca1..4c76916f91 100644 --- a/src/vcpkg/packagespec.cpp +++ b/src/vcpkg/packagespec.cpp @@ -130,7 +130,16 @@ namespace vcpkg { auto parser = ParserBase(context, input, ""); auto maybe_pqs = parse_qualified_specifier(parser); - if (!parser.at_eof()) parser.add_error(msg::format(msgExpectedEof)); + if (!parser.at_eof()) + { + parser.add_error(msg::format(msgExpectedEof)); + } + + if (parser.any_errors()) + { + maybe_pqs.clear(); + } + return maybe_pqs; } diff --git a/src/vcpkg/paragraphs.cpp b/src/vcpkg/paragraphs.cpp index 1ea6e8baa6..d0642cc4d1 100644 --- a/src/vcpkg/paragraphs.cpp +++ b/src/vcpkg/paragraphs.cpp @@ -145,24 +145,28 @@ namespace vcpkg template static Optional> parse_list_until_eof(Message bad_comma_message, ParserBase& parser, F f) { - std::vector ret; + Optional> ret; + auto& vec = ret.emplace(); parser.skip_whitespace(); if (parser.at_eof()) return std::vector{}; - do + for (;;) { auto item = f(parser); - if (!item) return nullopt; - ret.push_back(std::move(item).value_or_exit(VCPKG_LINE_INFO)); + if (!item) break; + vec.push_back(std::move(item).value_or_exit(VCPKG_LINE_INFO)); parser.skip_whitespace(); - if (parser.at_eof()) return {std::move(ret)}; + if (parser.at_eof()) return ret; if (parser.cur() != ',') { parser.add_error(msg::format(bad_comma_message)); - return nullopt; + break; } parser.next(); parser.skip_whitespace(); - } while (true); + } + + ret.clear(); + return ret; } Optional> parse_default_features_list_context(DiagnosticContext& context, diff --git a/src/vcpkg/sourceparagraph.cpp b/src/vcpkg/sourceparagraph.cpp index e8b0d515e5..62306317f1 100644 --- a/src/vcpkg/sourceparagraph.cpp +++ b/src/vcpkg/sourceparagraph.cpp @@ -889,7 +889,8 @@ namespace vcpkg if (cur() == Unicode::end_of_file) { add_error(msg::format(msgEmptyLicenseExpression)); - return nullopt; + result.clear(); + return result; } Expecting expecting = Expecting::License;