From 2cb22129905614cdc663126eb379ab2ac5734883 Mon Sep 17 00:00:00 2001 From: Kai Pastor Date: Wed, 2 Nov 2022 07:54:07 +0100 Subject: [PATCH 1/7] Eliminate full_results --- src/vcpkg/commands.ci.cpp | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index 78abf69c7b..9f14fce64c 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -300,7 +300,8 @@ namespace vcpkg::Commands::CI return result; } - static void print_baseline_regressions(const std::map& results, + static void print_baseline_regressions(const std::vector& results, + const std::map& known, const CiBaselineData& cidata, const std::string& ci_baseline_file_name, bool allow_unexpected_passing) @@ -309,6 +310,16 @@ namespace vcpkg::Commands::CI LocalizedString output = msg::format(msgCiBaselineRegressionHeader); output.append_raw('\n'); for (auto&& r : results) + { + auto result = r.build_result.value_or_exit(VCPKG_LINE_INFO).code; + auto msg = format_ci_result(r.get_spec(), result, cidata, ci_baseline_file_name, allow_unexpected_passing); + if (!msg.empty()) + { + has_error = true; + output.append(msg).append_raw('\n'); + } + } + for (auto&& r : known) { auto msg = format_ci_result(r.first, r.second, cidata, ci_baseline_file_name, allow_unexpected_passing); if (!msg.empty()) @@ -497,8 +508,6 @@ namespace vcpkg::Commands::CI auto summary = Install::perform( args, action_plan, KeepGoing::YES, paths, status_db, binary_cache, build_logs_recorder, var_provider); - std::map full_results; - // Adding results for ports that were built or pulled from an archive for (auto&& result : summary.results) { @@ -508,9 +517,7 @@ namespace vcpkg::Commands::CI auto code = result.build_result.value_or_exit(VCPKG_LINE_INFO).code; xunitTestResults.add_test_results( spec, code, result.timing, result.start_time, split_specs->abi_map.at(spec), port_features); - full_results.emplace(spec, code); } - full_results.insert(split_specs->known.begin(), split_specs->known.end()); // Adding results for ports that were not built because they have known states if (Util::Sets::contains(options.switches, OPTION_XUNIT_ALL)) @@ -534,7 +541,7 @@ namespace vcpkg::Commands::CI if (baseline_iter != settings.end()) { - print_baseline_regressions(full_results, cidata, baseline_iter->second, allow_unexpected_passing); + print_baseline_regressions(summary.results, split_specs->known, cidata, baseline_iter->second, allow_unexpected_passing); } auto it_xunit = settings.find(OPTION_XUNIT); From 97de1dc0a10bcfdf926c9c320b997aeb4feaeb4b Mon Sep 17 00:00:00 2001 From: Kai Pastor Date: Thu, 3 Nov 2022 06:33:13 +0100 Subject: [PATCH 2/7] Eliminate TripletSummary --- src/vcpkg/commands.ci.cpp | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index 9f14fce64c..4965bb8de0 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -82,12 +82,6 @@ namespace namespace vcpkg::Commands::CI { - struct TripletAndSummary - { - Triplet triplet; - InstallSummary summary; - }; - static constexpr StringLiteral OPTION_DRY_RUN = "dry-run"; static constexpr StringLiteral OPTION_EXCLUDE = "exclude"; static constexpr StringLiteral OPTION_HOST_EXCLUDE = "host-exclude"; @@ -534,10 +528,8 @@ namespace vcpkg::Commands::CI } } - TripletAndSummary result{target_triplet, std::move(summary)}; - - msg::write_unlocalized_text_to_stdout(Color::none, fmt::format("\nTriplet: {}\n", result.triplet)); - result.summary.print(); + msg::write_unlocalized_text_to_stdout(Color::none, fmt::format("\nTriplet: {}\n", target_triplet)); + summary.print(); if (baseline_iter != settings.end()) { From 915a15c39a95756de716d71618442fac5752a84b Mon Sep 17 00:00:00 2001 From: Kai Pastor Date: Thu, 3 Nov 2022 07:12:39 +0100 Subject: [PATCH 3/7] Move ci xunit processing into single block --- src/vcpkg/commands.ci.cpp | 57 ++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index 4965bb8de0..222a4d6c3c 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -393,8 +393,6 @@ namespace vcpkg::Commands::CI auto var_provider_storage = CMakeVars::make_triplet_cmake_var_provider(paths); auto& var_provider = *var_provider_storage; - XunitWriter xunitTestResults; - const ElapsedTimer timer; std::vector all_port_names = Util::fmap(provider.load_all_control_files(), Paragraphs::get_name_of_control_file); @@ -502,30 +500,9 @@ namespace vcpkg::Commands::CI auto summary = Install::perform( args, action_plan, KeepGoing::YES, paths, status_db, binary_cache, build_logs_recorder, var_provider); - // Adding results for ports that were built or pulled from an archive for (auto&& result : summary.results) { - const auto& spec = result.get_spec(); - auto& port_features = split_specs->features.at(spec); - split_specs->known.erase(spec); - auto code = result.build_result.value_or_exit(VCPKG_LINE_INFO).code; - xunitTestResults.add_test_results( - spec, code, result.timing, result.start_time, split_specs->abi_map.at(spec), port_features); - } - - // Adding results for ports that were not built because they have known states - if (Util::Sets::contains(options.switches, OPTION_XUNIT_ALL)) - { - for (auto&& port : split_specs->known) - { - auto& port_features = split_specs->features.at(port.first); - xunitTestResults.add_test_results(port.first, - port.second, - ElapsedTime{}, - std::chrono::system_clock::time_point{}, - split_specs->abi_map.at(port.first), - port_features); - } + split_specs->known.erase(result.get_spec()); } msg::write_unlocalized_text_to_stdout(Color::none, fmt::format("\nTriplet: {}\n", target_triplet)); @@ -539,6 +516,38 @@ namespace vcpkg::Commands::CI auto it_xunit = settings.find(OPTION_XUNIT); if (it_xunit != settings.end()) { + XunitWriter xunitTestResults; + + // Adding results for ports that were built or pulled from an archive + for (auto&& result : summary.results) + { + const auto& spec = result.get_spec(); + auto& port_features = split_specs->features.at(spec); + auto code = result.build_result.value_or_exit(VCPKG_LINE_INFO).code; + xunitTestResults.add_test_results(spec, + code, + result.timing, + result.start_time, + split_specs->abi_map.at(spec), + port_features); + } + + // Adding results for ports that were not built because they have known states + if (Util::Sets::contains(options.switches, OPTION_XUNIT_ALL)) + { + for (auto&& port : split_specs->known) + { + const auto& spec = port.first; + auto& port_features = split_specs->features.at(spec); + xunitTestResults.add_test_results(spec, + port.second, + ElapsedTime{}, + std::chrono::system_clock::time_point{}, + split_specs->abi_map.at(spec), + port_features); + } + } + filesystem.write_contents( it_xunit->second, xunitTestResults.build_xml(target_triplet), VCPKG_LINE_INFO); } From 8955652ba6fdfae8fee9df28588b8845861e7d13 Mon Sep 17 00:00:00 2001 From: Kai Pastor Date: Fri, 4 Nov 2022 07:56:57 +0100 Subject: [PATCH 4/7] Always print regressions From a ci command perspective, errors are regressions by default. The baseline file is only for skips, expected failures and unexpected cascades. --- src/vcpkg/commands.ci.cpp | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index e300a8a8e4..b3c6d9136e 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -294,11 +294,11 @@ namespace vcpkg::Commands::CI return result; } - static void print_baseline_regressions(const std::vector& results, - const std::map& known, - const CiBaselineData& cidata, - const std::string& ci_baseline_file_name, - bool allow_unexpected_passing) + static void print_regressions(const std::vector& results, + const std::map& known, + const CiBaselineData& cidata, + const std::string& ci_baseline_file_name, + bool allow_unexpected_passing) { bool has_error = false; LocalizedString output = msg::format(msgCiBaselineRegressionHeader); @@ -507,11 +507,7 @@ namespace vcpkg::Commands::CI msg::write_unlocalized_text_to_stdout(Color::none, fmt::format("\nTriplet: {}\n", target_triplet)); summary.print(); - - if (baseline_iter != settings.end()) - { - print_baseline_regressions(summary.results, split_specs->known, cidata, baseline_iter->second, allow_unexpected_passing); - } + print_regressions(summary.results, split_specs->known, cidata, baseline_iter->second, allow_unexpected_passing); auto it_xunit = settings.find(OPTION_XUNIT); if (it_xunit != settings.end()) From f0824593dd6c10575713b4768c621f4291278cb9 Mon Sep 17 00:00:00 2001 From: Kai Pastor Date: Tue, 1 Nov 2022 17:46:17 +0100 Subject: [PATCH 5/7] Mark modified ports wrt to parent hashes If a port is not changed with regard to its parent hash but fails to build, it is a real baseline regression. --- src/vcpkg/commands.ci.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index b3c6d9136e..e75427eaae 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -240,9 +240,13 @@ namespace vcpkg::Commands::CI auto it_known = known.find(it->spec); const auto& abi = it->abi_info.value_or_exit(VCPKG_LINE_INFO).package_abi; auto it_parent = std::find(parent_hashes.begin(), parent_hashes.end(), abi); - if (it_known == known.end() && it_parent == parent_hashes.end()) + if (it_parent == parent_hashes.end()) { - to_keep.insert(it->spec); + it->request_type = RequestType::USER_REQUESTED; + if (it_known == known.end()) + { + to_keep.insert(it->spec); + } } if (Util::Sets::contains(to_keep, it->spec)) From c52f9e346209ef38c45455f4bffbbf1b4c301eca Mon Sep 17 00:00:00 2001 From: Kai Pastor Date: Fri, 4 Nov 2022 08:37:59 +0100 Subject: [PATCH 6/7] Change output for regression of independent ports --- include/vcpkg/base/messages.h | 4 ++++ include/vcpkg/ci-baseline.h | 3 ++- src/vcpkg-test/ci-baseline.cpp | 6 +++--- src/vcpkg/base/messages.cpp | 1 + src/vcpkg/ci-baseline.cpp | 20 +++++++++++++++----- src/vcpkg/commands.ci.cpp | 4 ++-- 6 files changed, 27 insertions(+), 11 deletions(-) diff --git a/include/vcpkg/base/messages.h b/include/vcpkg/base/messages.h index 784c153e07..362e7e265f 100644 --- a/include/vcpkg/base/messages.h +++ b/include/vcpkg/base/messages.h @@ -722,6 +722,10 @@ namespace vcpkg (msg::spec, msg::path), "", "REGRESSION: {spec} cascaded, but it is required to pass. ({path})."); + DECLARE_MESSAGE(CiBaselineIndependentRegression, + (msg::spec, msg::build_result), + "", + "REGRESSION: Independent {spec} failed with {build_result}."); DECLARE_MESSAGE(CiBaselineRegression, (msg::spec, msg::build_result, msg::path), "", diff --git a/include/vcpkg/ci-baseline.h b/include/vcpkg/ci-baseline.h index 39fd0b01c1..a6bfcc7237 100644 --- a/include/vcpkg/ci-baseline.h +++ b/include/vcpkg/ci-baseline.h @@ -69,5 +69,6 @@ namespace vcpkg BuildResult result, const CiBaselineData& cidata, StringView cifile, - bool allow_unexpected_passing); + bool allow_unexpected_passing, + bool is_independent); } diff --git a/src/vcpkg-test/ci-baseline.cpp b/src/vcpkg-test/ci-baseline.cpp index 1244fa00fb..887a41df6c 100644 --- a/src/vcpkg-test/ci-baseline.cpp +++ b/src/vcpkg-test/ci-baseline.cpp @@ -336,7 +336,7 @@ TEST_CASE ("format_ci_result 1", "[ci-baseline]") SECTION ("SUCCEEDED") { const auto test = [&](PackageSpec s, bool allow_unexpected_passing) { - return format_ci_result(s, BuildResult::SUCCEEDED, cidata, "cifile", allow_unexpected_passing); + return format_ci_result(s, BuildResult::SUCCEEDED, cidata, "cifile", allow_unexpected_passing, false); }; CHECK(test({"pass", Test::X64_UWP}, true) == ""); CHECK(test({"pass", Test::X64_UWP}, false) == ""); @@ -350,7 +350,7 @@ TEST_CASE ("format_ci_result 1", "[ci-baseline]") SECTION ("BUILD_FAILED") { const auto test = [&](PackageSpec s) { - return format_ci_result(s, BuildResult::BUILD_FAILED, cidata, "cifile", false); + return format_ci_result(s, BuildResult::BUILD_FAILED, cidata, "cifile", false, false); }; CHECK(test({"pass", Test::X64_UWP}) == fmt::format(failmsg, "pass:x64-uwp")); CHECK(test({"fail", Test::X64_UWP}) == ""); @@ -360,7 +360,7 @@ TEST_CASE ("format_ci_result 1", "[ci-baseline]") SECTION ("CASCADED_DUE_TO_MISSING_DEPENDENCIES") { const auto test = [&](PackageSpec s) { - return format_ci_result(s, BuildResult::CASCADED_DUE_TO_MISSING_DEPENDENCIES, cidata, "cifile", false); + return format_ci_result(s, BuildResult::CASCADED_DUE_TO_MISSING_DEPENDENCIES, cidata, "cifile", false, false); }; CHECK(test({"pass", Test::X64_UWP}) == fmt::format(cascademsg, "pass:x64-uwp")); CHECK(test({"fail", Test::X64_UWP}) == ""); diff --git a/src/vcpkg/base/messages.cpp b/src/vcpkg/base/messages.cpp index 3483282b95..1134fd7e80 100644 --- a/src/vcpkg/base/messages.cpp +++ b/src/vcpkg/base/messages.cpp @@ -488,6 +488,7 @@ namespace vcpkg REGISTER_MESSAGE(ChecksUpdateVcpkg); REGISTER_MESSAGE(CiBaselineAllowUnexpectedPassingRequiresBaseline); REGISTER_MESSAGE(CiBaselineDisallowedCascade); + REGISTER_MESSAGE(CiBaselineIndependentRegression); REGISTER_MESSAGE(CiBaselineRegression); REGISTER_MESSAGE(CiBaselineRegressionHeader); REGISTER_MESSAGE(CiBaselineUnexpectedPass); diff --git a/src/vcpkg/ci-baseline.cpp b/src/vcpkg/ci-baseline.cpp index 324e91d2d1..40fba32acc 100644 --- a/src/vcpkg/ci-baseline.cpp +++ b/src/vcpkg/ci-baseline.cpp @@ -204,7 +204,8 @@ namespace vcpkg BuildResult result, const CiBaselineData& cidata, StringView cifile, - bool allow_unexpected_passing) + bool allow_unexpected_passing, + bool is_independent) { switch (result) { @@ -213,10 +214,19 @@ namespace vcpkg case BuildResult::FILE_CONFLICTS: if (!cidata.expected_failures.contains(spec)) { - return msg::format(msgCiBaselineRegression, - msg::spec = spec, - msg::build_result = to_string_locale_invariant(result), - msg::path = cifile); + if (is_independent) + { + return msg::format(msgCiBaselineIndependentRegression, + msg::spec = spec, + msg::build_result = to_string_locale_invariant(result)); + } + else + { + return msg::format(msgCiBaselineRegression, + msg::spec = spec, + msg::build_result = to_string_locale_invariant(result), + msg::path = cifile); + } } break; case BuildResult::SUCCEEDED: diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index e75427eaae..0beed9ed04 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -310,7 +310,7 @@ namespace vcpkg::Commands::CI for (auto&& r : results) { auto result = r.build_result.value_or_exit(VCPKG_LINE_INFO).code; - auto msg = format_ci_result(r.get_spec(), result, cidata, ci_baseline_file_name, allow_unexpected_passing); + auto msg = format_ci_result(r.get_spec(), result, cidata, ci_baseline_file_name, allow_unexpected_passing, !r.is_user_requested_install()); if (!msg.empty()) { has_error = true; @@ -319,7 +319,7 @@ namespace vcpkg::Commands::CI } for (auto&& r : known) { - auto msg = format_ci_result(r.first, r.second, cidata, ci_baseline_file_name, allow_unexpected_passing); + auto msg = format_ci_result(r.first, r.second, cidata, ci_baseline_file_name, allow_unexpected_passing, true); if (!msg.empty()) { has_error = true; From 20552f046b0b11edb0a6b6873b7c8d62e9c41764 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 16 Nov 2022 17:39:03 -0800 Subject: [PATCH 7/7] clang-format --- locales/messages.json | 2 ++ src/vcpkg-test/ci-baseline.cpp | 3 ++- src/vcpkg/commands.ci.cpp | 21 ++++++++++++--------- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/locales/messages.json b/locales/messages.json index c245a672f7..b255999e8a 100644 --- a/locales/messages.json +++ b/locales/messages.json @@ -153,6 +153,8 @@ "CiBaselineAllowUnexpectedPassingRequiresBaseline": "--allow-unexpected-passing can only be used if a baseline is provided via --ci-baseline.", "CiBaselineDisallowedCascade": "REGRESSION: {spec} cascaded, but it is required to pass. ({path}).", "_CiBaselineDisallowedCascade.comment": "An example of {spec} is zlib:x64-windows. An example of {path} is /foo/bar.", + "CiBaselineIndependentRegression": "REGRESSION: Independent {spec} failed with {build_result}.", + "_CiBaselineIndependentRegression.comment": "An example of {spec} is zlib:x64-windows. An example of {build_result} is One of the BuildResultXxx messages (such as BuildResultSucceeded/SUCCEEDED).", "CiBaselineRegression": "REGRESSION: {spec} failed with {build_result}. If expected, add {spec}=fail to {path}.", "_CiBaselineRegression.comment": "An example of {spec} is zlib:x64-windows. An example of {build_result} is One of the BuildResultXxx messages (such as BuildResultSucceeded/SUCCEEDED). An example of {path} is /foo/bar.", "CiBaselineRegressionHeader": "REGRESSIONS:", diff --git a/src/vcpkg-test/ci-baseline.cpp b/src/vcpkg-test/ci-baseline.cpp index 887a41df6c..e75c4bd25b 100644 --- a/src/vcpkg-test/ci-baseline.cpp +++ b/src/vcpkg-test/ci-baseline.cpp @@ -360,7 +360,8 @@ TEST_CASE ("format_ci_result 1", "[ci-baseline]") SECTION ("CASCADED_DUE_TO_MISSING_DEPENDENCIES") { const auto test = [&](PackageSpec s) { - return format_ci_result(s, BuildResult::CASCADED_DUE_TO_MISSING_DEPENDENCIES, cidata, "cifile", false, false); + return format_ci_result( + s, BuildResult::CASCADED_DUE_TO_MISSING_DEPENDENCIES, cidata, "cifile", false, false); }; CHECK(test({"pass", Test::X64_UWP}) == fmt::format(cascademsg, "pass:x64-uwp")); CHECK(test({"fail", Test::X64_UWP}) == ""); diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index 0beed9ed04..350a62c049 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -310,7 +310,12 @@ namespace vcpkg::Commands::CI for (auto&& r : results) { auto result = r.build_result.value_or_exit(VCPKG_LINE_INFO).code; - auto msg = format_ci_result(r.get_spec(), result, cidata, ci_baseline_file_name, allow_unexpected_passing, !r.is_user_requested_install()); + auto msg = format_ci_result(r.get_spec(), + result, + cidata, + ci_baseline_file_name, + allow_unexpected_passing, + !r.is_user_requested_install()); if (!msg.empty()) { has_error = true; @@ -319,7 +324,8 @@ namespace vcpkg::Commands::CI } for (auto&& r : known) { - auto msg = format_ci_result(r.first, r.second, cidata, ci_baseline_file_name, allow_unexpected_passing, true); + auto msg = + format_ci_result(r.first, r.second, cidata, ci_baseline_file_name, allow_unexpected_passing, true); if (!msg.empty()) { has_error = true; @@ -511,7 +517,8 @@ namespace vcpkg::Commands::CI msg::write_unlocalized_text_to_stdout(Color::none, fmt::format("\nTriplet: {}\n", target_triplet)); summary.print(); - print_regressions(summary.results, split_specs->known, cidata, baseline_iter->second, allow_unexpected_passing); + print_regressions( + summary.results, split_specs->known, cidata, baseline_iter->second, allow_unexpected_passing); auto it_xunit = settings.find(OPTION_XUNIT); if (it_xunit != settings.end()) @@ -524,12 +531,8 @@ namespace vcpkg::Commands::CI const auto& spec = result.get_spec(); auto& port_features = split_specs->features.at(spec); auto code = result.build_result.value_or_exit(VCPKG_LINE_INFO).code; - xunitTestResults.add_test_results(spec, - code, - result.timing, - result.start_time, - split_specs->abi_map.at(spec), - port_features); + xunitTestResults.add_test_results( + spec, code, result.timing, result.start_time, split_specs->abi_map.at(spec), port_features); } // Adding results for ports that were not built because they have known states