From ce00d41ed5c0b3d389478b051d95e6990cc70c81 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Tue, 21 Oct 2025 14:12:02 -0700 Subject: [PATCH 01/20] Add a test for not removing features from skipped ports, see related https://github.com/microsoft/vcpkg/pull/47735 OK, so here's the situation: `ci.baseline.txt` has: ```console # Missing system libraries qtwayland:arm64-osx=skip qtwayland:x64-osx=skip ``` In https://github.com/microsoft/vcpkg/pull/47735, `qtwayland` depends on `qtbase[wayland]`, which implicates the same missing system libraries. This causes `qtbase` to contain the feature `wayland` when we try to build it, even though `qtwayland` is skipped. --- .../e2e-assets/ci-skipped-features/baseline.txt | 5 +++++ .../skipped-depends/portfile.cmake | 1 + .../ci-skipped-features/skipped-depends/vcpkg.json | 13 +++++++++++++ .../skipped-features/portfile.cmake | 4 ++++ .../ci-skipped-features/skipped-features/vcpkg.json | 9 +++++++++ azure-pipelines/end-to-end-tests-dir/ci.ps1 | 9 ++++++++- 6 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 azure-pipelines/e2e-assets/ci-skipped-features/baseline.txt create mode 100644 azure-pipelines/e2e-assets/ci-skipped-features/skipped-depends/portfile.cmake create mode 100644 azure-pipelines/e2e-assets/ci-skipped-features/skipped-depends/vcpkg.json create mode 100644 azure-pipelines/e2e-assets/ci-skipped-features/skipped-features/portfile.cmake create mode 100644 azure-pipelines/e2e-assets/ci-skipped-features/skipped-features/vcpkg.json diff --git a/azure-pipelines/e2e-assets/ci-skipped-features/baseline.txt b/azure-pipelines/e2e-assets/ci-skipped-features/baseline.txt new file mode 100644 index 0000000000..305d93a05e --- /dev/null +++ b/azure-pipelines/e2e-assets/ci-skipped-features/baseline.txt @@ -0,0 +1,5 @@ +skipped-depends:arm64-osx=skip +skipped-depends:x64-linux=skip +skipped-depends:x64-osx=skip +skipped-depends:x64-windows=skip +skipped-depends:x86-windows=skip diff --git a/azure-pipelines/e2e-assets/ci-skipped-features/skipped-depends/portfile.cmake b/azure-pipelines/e2e-assets/ci-skipped-features/skipped-depends/portfile.cmake new file mode 100644 index 0000000000..291aee2e6d --- /dev/null +++ b/azure-pipelines/e2e-assets/ci-skipped-features/skipped-depends/portfile.cmake @@ -0,0 +1 @@ +message(FATAL_ERROR "This port is skipped and should not be attempted.") diff --git a/azure-pipelines/e2e-assets/ci-skipped-features/skipped-depends/vcpkg.json b/azure-pipelines/e2e-assets/ci-skipped-features/skipped-depends/vcpkg.json new file mode 100644 index 0000000000..f99edd42ef --- /dev/null +++ b/azure-pipelines/e2e-assets/ci-skipped-features/skipped-depends/vcpkg.json @@ -0,0 +1,13 @@ +{ + "name": "skipped-depends", + "version": "1.0.0", + "dependencies": [ + { + "name": "skipped-features", + "default-features": false, + "features": [ + "fail-if-included" + ] + } + ] +} diff --git a/azure-pipelines/e2e-assets/ci-skipped-features/skipped-features/portfile.cmake b/azure-pipelines/e2e-assets/ci-skipped-features/skipped-features/portfile.cmake new file mode 100644 index 0000000000..ab9e276e23 --- /dev/null +++ b/azure-pipelines/e2e-assets/ci-skipped-features/skipped-features/portfile.cmake @@ -0,0 +1,4 @@ +set(VCPKG_POLICY_EMPTY_PACKAGE enabled) +if ("fail-if-included" IN_LIST FEATURES) + message(FATAL_ERROR "The feature 'fail-if-included' should not be included.") +endif() diff --git a/azure-pipelines/e2e-assets/ci-skipped-features/skipped-features/vcpkg.json b/azure-pipelines/e2e-assets/ci-skipped-features/skipped-features/vcpkg.json new file mode 100644 index 0000000000..685ba378c0 --- /dev/null +++ b/azure-pipelines/e2e-assets/ci-skipped-features/skipped-features/vcpkg.json @@ -0,0 +1,9 @@ +{ + "name": "skipped-features", + "version": "1.0.0", + "features": { + "fail-if-included": { + "description": "This feature should cause a failure if it is included in the build." + } + } +} diff --git a/azure-pipelines/end-to-end-tests-dir/ci.ps1 b/azure-pipelines/end-to-end-tests-dir/ci.ps1 index 7b1b38d0d2..7af65834ba 100644 --- a/azure-pipelines/end-to-end-tests-dir/ci.ps1 +++ b/azure-pipelines/end-to-end-tests-dir/ci.ps1 @@ -53,6 +53,13 @@ if (-not ($Output.Contains("vcpkg.json:3:17: error: Trailing comma"))) { Remove-Problem-Matchers $emptyDir = "$TestingRoot/empty" New-Item -ItemType Directory -Path $emptyDir -Force | Out-Null -$Output = Run-VcpkgAndCaptureOutput ci --triplet=$Triplet --x-builtin-ports-root="$emptyDir" --binarysource=clear --overlay-ports="$PSScriptRoot/../e2e-ports/duplicate-file-a" --overlay-ports="$PSScriptRoot/../e2e-ports/duplicate-file-b" +$Output = Run-VcpkgAndCaptureOutput ci @commonArgs --x-builtin-ports-root="$emptyDir" --binarysource=clear --overlay-ports="$PSScriptRoot/../e2e-ports/duplicate-file-a" --overlay-ports="$PSScriptRoot/../e2e-ports/duplicate-file-b" Throw-IfNotFailed Restore-Problem-Matchers + +# test that features included only by skipped ports are not included +Remove-Problem-Matchers +Refresh-TestRoot +$Output = Run-VcpkgAndCaptureOutput ci @commonArgs --x-builtin-ports-root="$PSScriptRoot/../e2e-assets/ci-skipped-features" --binarysource=clear --ci-baseline="$PSScriptRoot/../e2e-assets/ci-skipped-features/baseline.txt" +Throw-IfFailed +Restore-Problem-Matchers From ddf8c35eeca0cb88446518b6f56a1064798b55af Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Tue, 21 Oct 2025 14:30:46 -0700 Subject: [PATCH 02/20] Extract calculate_ci_requested_specs function and fix the bug. --- src/vcpkg/commands.ci.cpp | 44 ++++++++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index 63d975cf8c..f19254ddfb 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -263,6 +263,36 @@ namespace return has_error; } + std::vector calculate_ci_requested_specs(const ExclusionsMap& exclusions_map, + const Triplet& target_triplet, + PortFileProvider& provider) + { + // Generate a spec for the default features for every package, except for those explicitly skipped. + // While `reduce_action_plan` tries to remove skipped packages as expected failures, there + // it is too late as we have already calculated an action plan with feature dependencies from + // the skipped ports. + std::vector result; + const TripletExclusions* target_triplet_exclusions = nullptr; + for (auto&& te : exclusions_map.triplets) + { + if (te.triplet == target_triplet) + { + target_triplet_exclusions = &te; + break; + } + } + + for (auto scfl : provider.load_all_control_files()) + { + if (!target_triplet_exclusions || !target_triplet_exclusions->exclusions.contains(scfl->to_name())) + { + result.emplace_back(PackageSpec{scfl->to_name(), target_triplet}, + InternalFeatureSet{FeatureNameCore.to_string(), FeatureNameDefault.to_string()}); + } + } + + return result; + } } // unnamed namespace namespace vcpkg @@ -357,14 +387,8 @@ namespace vcpkg auto& var_provider = *var_provider_storage; const ElapsedTimer timer; - // Install the default features for every package - std::vector all_default_full_specs; - for (auto scfl : provider.load_all_control_files()) - { - all_default_full_specs.emplace_back( - PackageSpec{scfl->to_name(), target_triplet}, - InternalFeatureSet{FeatureNameCore.to_string(), FeatureNameDefault.to_string()}); - } + std::vector ci_requested_specs = + calculate_ci_requested_specs(exclusions_map, target_triplet, provider); struct RandomizerInstance : GraphRandomizer { @@ -387,7 +411,7 @@ namespace vcpkg CreateInstallPlanOptions create_install_plan_options( randomizer, host_triplet, UnsupportedPortAction::Warn, UseHeadVersion::No, Editable::No); auto action_plan = compute_full_plan( - paths, provider, var_provider, all_default_full_specs, packages_dir_assigner, create_install_plan_options); + paths, provider, var_provider, ci_requested_specs, packages_dir_assigner, create_install_plan_options); BinaryCache binary_cache(fs); if (!binary_cache.install_providers(args, paths, out_sink)) { @@ -400,7 +424,7 @@ namespace vcpkg LocalizedString not_supported_regressions; { std::string msg; - for (const auto& spec : all_default_full_specs) + for (const auto& spec : ci_requested_specs) { if (!Util::Sets::contains(split_specs->abi_map, spec.package_spec)) { From 39579e5458dfe147d4399db22257eec9b89a2493 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Tue, 21 Oct 2025 14:33:49 -0700 Subject: [PATCH 03/20] Added test sanity check. --- azure-pipelines/end-to-end-tests-dir/ci.ps1 | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/azure-pipelines/end-to-end-tests-dir/ci.ps1 b/azure-pipelines/end-to-end-tests-dir/ci.ps1 index 7af65834ba..91c2754149 100644 --- a/azure-pipelines/end-to-end-tests-dir/ci.ps1 +++ b/azure-pipelines/end-to-end-tests-dir/ci.ps1 @@ -62,4 +62,8 @@ Remove-Problem-Matchers Refresh-TestRoot $Output = Run-VcpkgAndCaptureOutput ci @commonArgs --x-builtin-ports-root="$PSScriptRoot/../e2e-assets/ci-skipped-features" --binarysource=clear --ci-baseline="$PSScriptRoot/../e2e-assets/ci-skipped-features/baseline.txt" Throw-IfFailed +if (-not ($Output -match 'skipped-features:[^:]+: \*' -and $Output -match 'Building skipped-features:[^@]+@1.0.0...')) { + throw 'did not attempt to build skipped-features' +} + Restore-Problem-Matchers From 13fa6bcb754d19e8c7a4d3e930869b8ac315cb32 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Thu, 23 Oct 2025 15:23:32 -0700 Subject: [PATCH 04/20] Eliminate ExclusionsPredicate and extract RandomizerInstance. --- include/vcpkg/ci-baseline.h | 8 +--- include/vcpkg/fwd/ci-baseline.h | 1 - src/vcpkg/ci-baseline.cpp | 4 +- src/vcpkg/commands.ci.cpp | 71 +++++++++++++++++---------------- 4 files changed, 39 insertions(+), 45 deletions(-) diff --git a/include/vcpkg/ci-baseline.h b/include/vcpkg/ci-baseline.h index c58053e003..0b664bc5c7 100644 --- a/include/vcpkg/ci-baseline.h +++ b/include/vcpkg/ci-baseline.h @@ -44,13 +44,7 @@ namespace vcpkg void insert(Triplet triplet); void insert(Triplet triplet, SortedVector&& exclusions); - }; - - struct ExclusionPredicate - { - const ExclusionsMap* data; - - bool operator()(const PackageSpec& spec) const; + bool is_excluded(const PackageSpec& spec) const; }; std::vector parse_ci_baseline(StringView text, StringView origin, ParseMessages& messages); diff --git a/include/vcpkg/fwd/ci-baseline.h b/include/vcpkg/fwd/ci-baseline.h index 79b80cad3f..7ee92bfc4c 100644 --- a/include/vcpkg/fwd/ci-baseline.h +++ b/include/vcpkg/fwd/ci-baseline.h @@ -6,7 +6,6 @@ namespace vcpkg struct CiBaselineLine; struct TripletExclusions; struct ExclusionsMap; - struct ExclusionPredicate; enum class CiBaselineState { diff --git a/src/vcpkg/ci-baseline.cpp b/src/vcpkg/ci-baseline.cpp index 0f8b99ba75..d1d6b26690 100644 --- a/src/vcpkg/ci-baseline.cpp +++ b/src/vcpkg/ci-baseline.cpp @@ -43,9 +43,9 @@ namespace vcpkg triplets.emplace_back(triplet, std::move(exclusions)); } - bool ExclusionPredicate::operator()(const PackageSpec& spec) const + bool ExclusionsMap::is_excluded(const PackageSpec& spec) const { - for (const auto& triplet_exclusions : data->triplets) + for (const auto& triplet_exclusions : triplets) { if (triplet_exclusions.triplet == spec.triplet()) { diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index f19254ddfb..31b6ff4f8b 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -119,7 +119,7 @@ namespace } std::unique_ptr compute_action_statuses( - ExclusionPredicate is_excluded, + const ExclusionsMap& exclusions_map, const std::vector& precheck_results, const std::unordered_set& known_failures, const ActionPlan& action_plan) @@ -136,7 +136,7 @@ namespace auto p = &action; ret->abi_map.emplace(action.spec, action.abi_info.value_or_exit(VCPKG_LINE_INFO).package_abi); ret->features.emplace(action.spec, action.feature_list); - if (is_excluded(p->spec)) + if (exclusions_map.is_excluded(p->spec)) { ret->action_state_string.emplace_back("skip"); ret->known.emplace(p->spec, BuildResult::Excluded); @@ -293,6 +293,18 @@ namespace return result; } + + struct CiRandomizer : GraphRandomizer + { + virtual int random(int i) override + { + if (i <= 1) return 0; + std::uniform_int_distribution d(0, i - 1); + return d(e); + } + + std::random_device e; + }; } // unnamed namespace namespace vcpkg @@ -390,26 +402,15 @@ namespace vcpkg std::vector ci_requested_specs = calculate_ci_requested_specs(exclusions_map, target_triplet, provider); - struct RandomizerInstance : GraphRandomizer - { - virtual int random(int i) override - { - if (i <= 1) return 0; - std::uniform_int_distribution d(0, i - 1); - return d(e); - } - - std::random_device e; - } randomizer_instance; - GraphRandomizer* randomizer = nullptr; + Optional randomizer; if (Util::Sets::contains(options.switches, SwitchXRandomize)) { - randomizer = &randomizer_instance; + randomizer.emplace(); } PackagesDirAssigner packages_dir_assigner{paths.packages()}; CreateInstallPlanOptions create_install_plan_options( - randomizer, host_triplet, UnsupportedPortAction::Warn, UseHeadVersion::No, Editable::No); + randomizer.get(), host_triplet, UnsupportedPortAction::Warn, UseHeadVersion::No, Editable::No); auto action_plan = compute_full_plan( paths, provider, var_provider, ci_requested_specs, packages_dir_assigner, create_install_plan_options); BinaryCache binary_cache(fs); @@ -417,10 +418,10 @@ namespace vcpkg { Checks::exit_fail(VCPKG_LINE_INFO); } - auto install_actions = Util::fmap(action_plan.install_actions, [](const auto& action) { return &action; }); + auto install_actions = + Util::fmap(action_plan.install_actions, [](const InstallPlanAction& action) { return &action; }); const auto precheck_results = binary_cache.precheck(install_actions); - auto split_specs = - compute_action_statuses(ExclusionPredicate{&exclusions_map}, precheck_results, known_failures, action_plan); + auto split_specs = compute_action_statuses(exclusions_map, precheck_results, known_failures, action_plan); LocalizedString not_supported_regressions; { std::string msg; @@ -454,24 +455,24 @@ namespace vcpkg } msg::write_unlocalized_text(Color::none, msg); - auto it_output_hashes = settings.find(SwitchOutputHashes); - if (it_output_hashes != settings.end()) + } + + auto it_output_hashes = settings.find(SwitchOutputHashes); + if (it_output_hashes != settings.end()) + { + const Path output_hash_json = paths.original_cwd / it_output_hashes->second; + Json::Array arr; + for (size_t i = 0; i < action_plan.install_actions.size(); ++i) { - const Path output_hash_json = paths.original_cwd / it_output_hashes->second; - Json::Array arr; - for (size_t i = 0; i < action_plan.install_actions.size(); ++i) - { - auto&& action = action_plan.install_actions[i]; - Json::Object obj; - obj.insert(JsonIdName, Json::Value::string(action.spec.name())); - obj.insert(JsonIdTriplet, Json::Value::string(action.spec.triplet().canonical_name())); - obj.insert(JsonIdState, Json::Value::string(split_specs->action_state_string[i])); - obj.insert(JsonIdAbi, - Json::Value::string(action.abi_info.value_or_exit(VCPKG_LINE_INFO).package_abi)); - arr.push_back(std::move(obj)); - } - fs.write_contents(output_hash_json, Json::stringify(arr), VCPKG_LINE_INFO); + auto&& action = action_plan.install_actions[i]; + Json::Object obj; + obj.insert(JsonIdName, Json::Value::string(action.spec.name())); + obj.insert(JsonIdTriplet, Json::Value::string(action.spec.triplet().canonical_name())); + obj.insert(JsonIdState, Json::Value::string(split_specs->action_state_string[i])); + obj.insert(JsonIdAbi, Json::Value::string(action.abi_info.value_or_exit(VCPKG_LINE_INFO).package_abi)); + arr.push_back(std::move(obj)); } + fs.write_contents(output_hash_json, Json::stringify(arr), VCPKG_LINE_INFO); } std::vector parent_hashes; From 7da5f07e1d96d5e4c1f44bb19f7b3e2a435d23e9 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Thu, 23 Oct 2025 15:53:48 -0700 Subject: [PATCH 05/20] Deoptionalize package_abi() --- include/vcpkg/base/checks.h | 1 + include/vcpkg/commands.install.h | 19 +++++++++++++++-- include/vcpkg/dependencies.h | 3 ++- src/vcpkg/base/checks.cpp | 1 + src/vcpkg/binarycaching.cpp | 31 ++++++++++++++-------------- src/vcpkg/commands.test-features.cpp | 9 ++++---- src/vcpkg/dependencies.cpp | 16 +++++++++++--- src/vcpkg/spdx.cpp | 2 +- 8 files changed, 55 insertions(+), 27 deletions(-) diff --git a/include/vcpkg/base/checks.h b/include/vcpkg/base/checks.h index 6d470cdd45..f13167ba21 100644 --- a/include/vcpkg/base/checks.h +++ b/include/vcpkg/base/checks.h @@ -36,6 +36,7 @@ namespace vcpkg::Checks } // If expression is false, call exit_fail. + VCPKG_SAL_ANNOTATION(_Post_satisfies_(_Old_(expression))) void check_exit(const LineInfo& line_info, bool expression); // if expression is false, call exit_with_message. diff --git a/include/vcpkg/commands.install.h b/include/vcpkg/commands.install.h index fd85d58573..53abaddf9e 100644 --- a/include/vcpkg/commands.install.h +++ b/include/vcpkg/commands.install.h @@ -27,9 +27,24 @@ namespace vcpkg const BinaryParagraph* get_binary_paragraph() const; const PackageSpec& get_spec() const { return m_spec; } - Optional get_abi() const + const std::string* package_abi() const { - return m_install_action ? m_install_action->package_abi() : nullopt; + if (m_install_action) + { + return m_install_action->package_abi(); + } + + return nullptr; + } + const std::string& package_abi_or_exit(LineInfo li) const + { + auto pabi = package_abi(); + if (!pabi) + { + Checks::unreachable(li); + } + + return *pabi; } bool is_user_requested_install() const; Optional build_result; diff --git a/include/vcpkg/dependencies.h b/include/vcpkg/dependencies.h index c74f7b28aa..bc25093b93 100644 --- a/include/vcpkg/dependencies.h +++ b/include/vcpkg/dependencies.h @@ -56,7 +56,8 @@ namespace vcpkg const std::string& public_abi() const; bool has_package_abi() const; - Optional package_abi() const; + const std::string* package_abi() const; + const std::string& package_abi_or_exit(LineInfo li) const; const PreBuildInfo& pre_build_info(LineInfo li) const; Version version() const; std::string display_name() const; diff --git a/src/vcpkg/base/checks.cpp b/src/vcpkg/base/checks.cpp index 297e7dbcb4..6b8a4c2d71 100644 --- a/src/vcpkg/base/checks.cpp +++ b/src/vcpkg/base/checks.cpp @@ -95,6 +95,7 @@ namespace vcpkg exit_fail(line_info); } + VCPKG_SAL_ANNOTATION(_Post_satisfies_(_Old_(expression))) void Checks::check_exit(const LineInfo& line_info, bool expression) { if (!expression) diff --git a/src/vcpkg/binarycaching.cpp b/src/vcpkg/binarycaching.cpp index 3923cdee06..63cc210d87 100644 --- a/src/vcpkg/binarycaching.cpp +++ b/src/vcpkg/binarycaching.cpp @@ -426,7 +426,7 @@ namespace { for (size_t i = 0; i < actions.size(); ++i) { - const auto& abi_tag = actions[i]->package_abi().value_or_exit(VCPKG_LINE_INFO); + const auto& abi_tag = actions[i]->package_abi_or_exit(VCPKG_LINE_INFO); auto archive_path = m_dir / files_archive_subpath(abi_tag); if (m_fs.exists(archive_path, IgnoreErrors{})) { @@ -440,7 +440,7 @@ namespace for (size_t idx = 0; idx < actions.size(); ++idx) { const auto& action = *actions[idx]; - const auto& abi_tag = action.package_abi().value_or_exit(VCPKG_LINE_INFO); + const auto& abi_tag = action.package_abi_or_exit(VCPKG_LINE_INFO); bool any_available = false; if (m_fs.exists(m_dir / files_archive_subpath(abi_tag), IgnoreErrors{})) @@ -1008,7 +1008,7 @@ namespace for (size_t idx = 0; idx < actions.size(); ++idx) { auto&& action = *actions[idx]; - const auto& abi = action.package_abi().value_or_exit(VCPKG_LINE_INFO); + const auto& abi = action.package_abi_or_exit(VCPKG_LINE_INFO); auto tmp = make_temp_archive_path(m_buildtrees, action.spec, abi); auto res = m_tool->download_file(make_object_path(m_prefix, abi), tmp); if (auto cache_result = res.get()) @@ -1030,7 +1030,7 @@ namespace for (size_t idx = 0; idx < actions.size(); ++idx) { auto&& action = *actions[idx]; - const auto& abi = action.package_abi().value_or_exit(VCPKG_LINE_INFO); + const auto& abi = action.package_abi_or_exit(VCPKG_LINE_INFO); auto maybe_res = m_tool->stat(make_object_path(m_prefix, abi)); if (auto res = maybe_res.get()) { @@ -1166,7 +1166,7 @@ namespace for (size_t idx = 0; idx < actions.size(); ++idx) { auto&& action = *actions[idx]; - const auto& abi = action.package_abi().value_or_exit(VCPKG_LINE_INFO); + const auto& abi = action.package_abi_or_exit(VCPKG_LINE_INFO); abis.push_back(abi); abi_index_map[abi] = idx; } @@ -1225,7 +1225,7 @@ namespace for (size_t idx = 0; idx < actions.size(); ++idx) { auto&& action = *actions[idx]; - const auto& abi = action.package_abi().value_or_exit(VCPKG_LINE_INFO); + const auto& abi = action.package_abi_or_exit(VCPKG_LINE_INFO); cache_status[idx] = Util::contains(abis, abi) ? CacheAvailability::available : CacheAvailability::unavailable; } @@ -2406,7 +2406,7 @@ namespace vcpkg statuses.clear(); for (size_t i = 0; i < actions.size(); ++i) { - if (auto abi = actions[i].package_abi().get()) + if (auto abi = actions[i].package_abi()) { CacheStatus& status = m_status[*abi]; if (status.should_attempt_restore(provider.get())) @@ -2441,7 +2441,7 @@ namespace vcpkg bool ReadOnlyBinaryCache::is_restored(const InstallPlanAction& action) const { - if (auto abi = action.package_abi().get()) + if (auto abi = action.package_abi()) { auto it = m_status.find(*abi); if (it != m_status.end()) return it->second.is_restored(); @@ -2464,10 +2464,11 @@ namespace vcpkg std::vector ReadOnlyBinaryCache::precheck(View actions) { - std::vector statuses = Util::fmap(actions, [this](const auto& action) { - Checks::check_exit(VCPKG_LINE_INFO, action && action->package_abi()); - ASSUME(action); - return &m_status[*action->package_abi().get()]; + std::vector statuses = Util::fmap(actions, [this](const InstallPlanAction* action) { + Checks::check_exit(VCPKG_LINE_INFO, action); + auto pabi = action->package_abi(); + Checks::check_exit(VCPKG_LINE_INFO, pabi); + return &m_status[*pabi]; }); std::vector action_ptrs; @@ -2493,7 +2494,7 @@ namespace vcpkg for (size_t i = 0; i < action_ptrs.size(); ++i) { - auto&& this_status = m_status[*action_ptrs[i]->package_abi().get()]; + auto&& this_status = m_status[action_ptrs[i]->package_abi_or_exit(VCPKG_LINE_INFO)]; if (cache_result[i] == CacheAvailability::available) { this_status.mark_available(provider.get()); @@ -2793,7 +2794,7 @@ namespace vcpkg void BinaryCache::push_success(CleanPackages clean_packages, const InstallPlanAction& action) { - if (auto abi = action.package_abi().get()) + if (auto abi = action.package_abi()) { bool restored; auto it = m_status.find(*abi); @@ -3000,7 +3001,7 @@ namespace vcpkg } BinaryPackageReadInfo::BinaryPackageReadInfo(const InstallPlanAction& action) - : package_abi(action.package_abi().value_or_exit(VCPKG_LINE_INFO)) + : package_abi(action.package_abi_or_exit(VCPKG_LINE_INFO)) , spec(action.spec) , display_name(action.display_name()) , version(action.version()) diff --git a/src/vcpkg/commands.test-features.cpp b/src/vcpkg/commands.test-features.cpp index 771adfcff2..2e1289f3ff 100644 --- a/src/vcpkg/commands.test-features.cpp +++ b/src/vcpkg/commands.test-features.cpp @@ -786,8 +786,7 @@ namespace vcpkg if (auto iter = Util::find_if(install_plan.install_actions, [&known_failures](const auto& install_action) { return Util::Sets::contains( - known_failures, - install_action.package_abi().value_or_exit(VCPKG_LINE_INFO)); + known_failures, install_action.package_abi_or_exit(VCPKG_LINE_INFO)); }); iter != install_plan.install_actions.end()) { @@ -812,7 +811,7 @@ namespace vcpkg CacheAvailability::available) { msg::println(msgSkipTestingOfPortAlreadyInBinaryCache, - msg::sha = action->package_abi().value_or_exit(VCPKG_LINE_INFO)); + msg::sha = action->package_abi_or_exit(VCPKG_LINE_INFO)); handle_pass_feature_test_result(diagnostics, spec, ci_feature_baseline_file_name, baseline); continue; } @@ -861,7 +860,7 @@ namespace vcpkg [[fallthrough]]; case BuildResult::PostBuildChecksFailed: - known_failures.insert(result.get_abi().value_or_exit(VCPKG_LINE_INFO)); + known_failures.insert(result.package_abi_or_exit(VCPKG_LINE_INFO)); break; default: break; } @@ -896,7 +895,7 @@ namespace vcpkg case BuildResult::PostBuildChecksFailed: case BuildResult::FileConflicts: case BuildResult::CacheMissing: - if (auto abi = summary.results.back().get_abi().get()) + if (auto abi = summary.results.back().package_abi()) { known_failures.insert(*abi); } diff --git a/src/vcpkg/dependencies.cpp b/src/vcpkg/dependencies.cpp index 680ceb1fc9..4327d4a26e 100644 --- a/src/vcpkg/dependencies.cpp +++ b/src/vcpkg/dependencies.cpp @@ -554,11 +554,21 @@ namespace vcpkg const auto p = abi_info.get(); return p && !p->package_abi.empty(); } - Optional InstallPlanAction::package_abi() const + const std::string* InstallPlanAction::package_abi() const { const auto p = abi_info.get(); - if (!p || p->package_abi.empty()) return nullopt; - return p->package_abi; + if (p && !p->package_abi.empty()) return &p->package_abi; + return nullptr; + } + const std::string& InstallPlanAction::package_abi_or_exit(LineInfo li) const + { + auto pabi = package_abi(); + if (!pabi) + { + Checks::unreachable(li); + } + + return *pabi; } const PreBuildInfo& InstallPlanAction::pre_build_info(LineInfo li) const { diff --git a/src/vcpkg/spdx.cpp b/src/vcpkg/spdx.cpp index a5e6e54b27..06264a62fc 100644 --- a/src/vcpkg/spdx.cpp +++ b/src/vcpkg/spdx.cpp @@ -296,7 +296,7 @@ std::string vcpkg::create_spdx_sbom(const InstallPlanAction& action, const auto& scfl = action.source_control_file_and_location.value_or_exit(VCPKG_LINE_INFO); const auto& cpgh = *scfl.source_control_file->core_paragraph; StringView abi{SpdxNone}; - if (auto package_abi = action.package_abi().get()) + if (auto package_abi = action.package_abi()) { abi = *package_abi; } From 83286ec39af6fe68f3694fbd41f5c866482a3dd3 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Thu, 23 Oct 2025 17:30:53 -0700 Subject: [PATCH 06/20] fixup ci randomizer --- src/vcpkg/commands.ci.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index 31b6ff4f8b..8103597f0e 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -294,7 +294,7 @@ namespace return result; } - struct CiRandomizer : GraphRandomizer + struct CiRandomizer final : GraphRandomizer { virtual int random(int i) override { From ef282234c6c9d86c7965b809ebccdb960c11c3f5 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Thu, 23 Oct 2025 17:31:12 -0700 Subject: [PATCH 07/20] Simplify exclusionmap slightly, also record intentionally excluded specs. --- include/vcpkg/ci-baseline.h | 1 - src/vcpkg-test/ci-baseline.cpp | 4 ++-- src/vcpkg/ci-baseline.cpp | 13 ----------- src/vcpkg/commands.ci.cpp | 41 +++++++++++++++++++++------------- 4 files changed, 28 insertions(+), 31 deletions(-) diff --git a/include/vcpkg/ci-baseline.h b/include/vcpkg/ci-baseline.h index 0b664bc5c7..3b37f21d07 100644 --- a/include/vcpkg/ci-baseline.h +++ b/include/vcpkg/ci-baseline.h @@ -42,7 +42,6 @@ namespace vcpkg { std::vector triplets; - void insert(Triplet triplet); void insert(Triplet triplet, SortedVector&& exclusions); bool is_excluded(const PackageSpec& spec) const; }; diff --git a/src/vcpkg-test/ci-baseline.cpp b/src/vcpkg-test/ci-baseline.cpp index a849deacff..cfc76b8cbc 100644 --- a/src/vcpkg-test/ci-baseline.cpp +++ b/src/vcpkg-test/ci-baseline.cpp @@ -206,8 +206,8 @@ bill-made-up-another-skip:x64-linux=skip)"; // note no trailing newline SECTION ("Applies Skips and Fails") { ExclusionsMap exclusions; - exclusions.insert(x64_uwp); // example triplet - exclusions.insert(x64_linux); // example host triplet + exclusions.insert(x64_uwp, {}); // example triplet + exclusions.insert(x64_linux, {}); // example host triplet auto actual = parse_and_apply_ci_baseline(expected_from_example_input, exclusions, SkipFailures::No); const SortedVector expected_expected_failures{ PackageSpec{"aubio", x64_uwp}, diff --git a/src/vcpkg/ci-baseline.cpp b/src/vcpkg/ci-baseline.cpp index d1d6b26690..04721321bb 100644 --- a/src/vcpkg/ci-baseline.cpp +++ b/src/vcpkg/ci-baseline.cpp @@ -16,19 +16,6 @@ namespace vcpkg { } - void ExclusionsMap::insert(Triplet triplet) - { - for (auto& triplet_exclusions : triplets) - { - if (triplet_exclusions.triplet == triplet) - { - return; - } - } - - triplets.emplace_back(triplet); - } - void ExclusionsMap::insert(Triplet triplet, SortedVector&& exclusions) { for (auto& triplet_exclusions : triplets) diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index 8103597f0e..154d033c27 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -121,7 +121,7 @@ namespace std::unique_ptr compute_action_statuses( const ExclusionsMap& exclusions_map, const std::vector& precheck_results, - const std::unordered_set& known_failures, + const std::unordered_set& known_failure_abis, const ActionPlan& action_plan) { auto ret = std::make_unique(); @@ -142,7 +142,7 @@ namespace ret->known.emplace(p->spec, BuildResult::Excluded); will_fail.emplace(p->spec); } - else if (Util::Sets::contains(known_failures, p->public_abi())) + else if (Util::Sets::contains(known_failure_abis, p->public_abi())) { ret->action_state_string.emplace_back("will fail"); ret->known.emplace(p->spec, BuildResult::BuildFailed); @@ -263,15 +263,21 @@ namespace return has_error; } - std::vector calculate_ci_requested_specs(const ExclusionsMap& exclusions_map, - const Triplet& target_triplet, - PortFileProvider& provider) + struct CiSpecsResult + { + std::vector requested; + std::vector excluded; + }; + + CiSpecsResult calculate_ci_specs(const ExclusionsMap& exclusions_map, + const Triplet& target_triplet, + PortFileProvider& provider) { // Generate a spec for the default features for every package, except for those explicitly skipped. // While `reduce_action_plan` tries to remove skipped packages as expected failures, there // it is too late as we have already calculated an action plan with feature dependencies from // the skipped ports. - std::vector result; + CiSpecsResult result; const TripletExclusions* target_triplet_exclusions = nullptr; for (auto&& te : exclusions_map.triplets) { @@ -286,8 +292,13 @@ namespace { if (!target_triplet_exclusions || !target_triplet_exclusions->exclusions.contains(scfl->to_name())) { - result.emplace_back(PackageSpec{scfl->to_name(), target_triplet}, - InternalFeatureSet{FeatureNameCore.to_string(), FeatureNameDefault.to_string()}); + result.requested.emplace_back( + PackageSpec{scfl->to_name(), target_triplet}, + InternalFeatureSet{FeatureNameCore.to_string(), FeatureNameDefault.to_string()}); + } + else + { + result.excluded.emplace_back(PackageSpec{scfl->to_name(), target_triplet}); } } @@ -368,13 +379,13 @@ namespace vcpkg cidata = parse_and_apply_ci_baseline(lines, exclusions_map, skip_failures); } - std::unordered_set known_failures; + std::unordered_set known_failure_abis; auto it_known_failures = settings.find(SwitchKnownFailuresFrom); if (it_known_failures != settings.end()) { Path raw_path = it_known_failures->second; auto lines = paths.get_filesystem().read_lines(raw_path).value_or_exit(VCPKG_LINE_INFO); - known_failures.insert(lines.begin(), lines.end()); + known_failure_abis.insert(lines.begin(), lines.end()); } const auto is_dry_run = Util::Sets::contains(options.switches, SwitchDryRun); @@ -399,8 +410,8 @@ namespace vcpkg auto& var_provider = *var_provider_storage; const ElapsedTimer timer; - std::vector ci_requested_specs = - calculate_ci_requested_specs(exclusions_map, target_triplet, provider); + auto ci_specs = + calculate_ci_specs(exclusions_map, target_triplet, provider); Optional randomizer; if (Util::Sets::contains(options.switches, SwitchXRandomize)) @@ -412,7 +423,7 @@ namespace vcpkg CreateInstallPlanOptions create_install_plan_options( randomizer.get(), host_triplet, UnsupportedPortAction::Warn, UseHeadVersion::No, Editable::No); auto action_plan = compute_full_plan( - paths, provider, var_provider, ci_requested_specs, packages_dir_assigner, create_install_plan_options); + paths, provider, var_provider, ci_specs.requested, packages_dir_assigner, create_install_plan_options); BinaryCache binary_cache(fs); if (!binary_cache.install_providers(args, paths, out_sink)) { @@ -421,11 +432,11 @@ namespace vcpkg auto install_actions = Util::fmap(action_plan.install_actions, [](const InstallPlanAction& action) { return &action; }); const auto precheck_results = binary_cache.precheck(install_actions); - auto split_specs = compute_action_statuses(exclusions_map, precheck_results, known_failures, action_plan); + auto split_specs = compute_action_statuses(exclusions_map, precheck_results, known_failure_abis, action_plan); LocalizedString not_supported_regressions; { std::string msg; - for (const auto& spec : ci_requested_specs) + for (const auto& spec : ci_specs.requested) { if (!Util::Sets::contains(split_specs->abi_map, spec.package_spec)) { From 1177c7dcc4ba526d53ea610a28a66930a91fb699 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Thu, 23 Oct 2025 18:10:52 -0700 Subject: [PATCH 08/20] wip --- src/vcpkg/commands.ci.cpp | 86 ++++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 42 deletions(-) diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index 154d033c27..6e3f546014 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -50,7 +50,7 @@ namespace {SwitchXXUnitAll, msgCISwitchOptXUnitAll}, }; - struct UnknownCIPortsResults + struct CIPreBuildStatus { std::map known; std::map> features; @@ -118,60 +118,59 @@ namespace return action_plan; } - std::unique_ptr compute_action_statuses( - const ExclusionsMap& exclusions_map, - const std::vector& precheck_results, - const std::unordered_set& known_failure_abis, - const ActionPlan& action_plan) + CIPreBuildStatus compute_pre_build_statuses(const ExclusionsMap& exclusions_map, + const std::vector& precheck_results, + const std::unordered_set& known_failure_abis, + const ActionPlan& action_plan) { - auto ret = std::make_unique(); + CIPreBuildStatus ret; std::set will_fail; - ret->action_state_string.reserve(action_plan.install_actions.size()); + ret.action_state_string.reserve(action_plan.install_actions.size()); for (size_t action_idx = 0; action_idx < action_plan.install_actions.size(); ++action_idx) { auto&& action = action_plan.install_actions[action_idx]; auto p = &action; - ret->abi_map.emplace(action.spec, action.abi_info.value_or_exit(VCPKG_LINE_INFO).package_abi); - ret->features.emplace(action.spec, action.feature_list); + ret.abi_map.emplace(action.spec, action.abi_info.value_or_exit(VCPKG_LINE_INFO).package_abi); + ret.features.emplace(action.spec, action.feature_list); if (exclusions_map.is_excluded(p->spec)) { - ret->action_state_string.emplace_back("skip"); - ret->known.emplace(p->spec, BuildResult::Excluded); + ret.action_state_string.emplace_back("skip"); + ret.known.emplace(p->spec, BuildResult::Excluded); will_fail.emplace(p->spec); } else if (Util::Sets::contains(known_failure_abis, p->public_abi())) { - ret->action_state_string.emplace_back("will fail"); - ret->known.emplace(p->spec, BuildResult::BuildFailed); + ret.action_state_string.emplace_back("will fail"); + ret.known.emplace(p->spec, BuildResult::BuildFailed); will_fail.emplace(p->spec); } else if (Util::any_of(p->package_dependencies, [&](const PackageSpec& spec) { return Util::Sets::contains(will_fail, spec); })) { - ret->action_state_string.emplace_back("cascade"); - ret->known.emplace(p->spec, BuildResult::CascadedDueToMissingDependencies); + ret.action_state_string.emplace_back("cascade"); + ret.known.emplace(p->spec, BuildResult::CascadedDueToMissingDependencies); will_fail.emplace(p->spec); } else if (precheck_results[action_idx] == CacheAvailability::available) { - ret->action_state_string.emplace_back("pass"); - ret->known.emplace(p->spec, BuildResult::Succeeded); + ret.action_state_string.emplace_back("pass"); + ret.known.emplace(p->spec, BuildResult::Succeeded); } else { - ret->action_state_string.emplace_back("*"); + ret.action_state_string.emplace_back("*"); } } return ret; } // This algorithm reduces an action plan to only unknown actions and their dependencies - void reduce_action_plan(ActionPlan& action_plan, - const std::map& known, - View parent_hashes) + void prune_entirely_known_action_branches(ActionPlan& action_plan, + const std::map& known, + View parent_hashes) { std::set to_keep; for (auto it = action_plan.install_actions.rbegin(); it != action_plan.install_actions.rend(); ++it) @@ -244,6 +243,9 @@ namespace output.append(msg).append_raw('\n'); } } + + // FIXME this is the root of the problem: `results` should have *all* of the results, not need to be somehow + // combined with the pre-build known results for (auto&& r : known) { auto msg = @@ -270,8 +272,8 @@ namespace }; CiSpecsResult calculate_ci_specs(const ExclusionsMap& exclusions_map, - const Triplet& target_triplet, - PortFileProvider& provider) + const Triplet& target_triplet, + PortFileProvider& provider) { // Generate a spec for the default features for every package, except for those explicitly skipped. // While `reduce_action_plan` tries to remove skipped packages as expected failures, there @@ -410,8 +412,7 @@ namespace vcpkg auto& var_provider = *var_provider_storage; const ElapsedTimer timer; - auto ci_specs = - calculate_ci_specs(exclusions_map, target_triplet, provider); + auto ci_specs = calculate_ci_specs(exclusions_map, target_triplet, provider); Optional randomizer; if (Util::Sets::contains(options.switches, SwitchXRandomize)) @@ -432,18 +433,19 @@ namespace vcpkg auto install_actions = Util::fmap(action_plan.install_actions, [](const InstallPlanAction& action) { return &action; }); const auto precheck_results = binary_cache.precheck(install_actions); - auto split_specs = compute_action_statuses(exclusions_map, precheck_results, known_failure_abis, action_plan); + const auto pre_build_status = + compute_pre_build_statuses(exclusions_map, precheck_results, known_failure_abis, action_plan); LocalizedString not_supported_regressions; { std::string msg; for (const auto& spec : ci_specs.requested) { - if (!Util::Sets::contains(split_specs->abi_map, spec.package_spec)) + if (!Util::Sets::contains(pre_build_status.abi_map, spec.package_spec)) { bool supp = supported_for_triplet(var_provider, provider, spec.package_spec); - split_specs->known.emplace(spec.package_spec, - supp ? BuildResult::CascadedDueToMissingDependencies - : BuildResult::Excluded); + pre_build_status.known.emplace(spec.package_spec, + supp ? BuildResult::CascadedDueToMissingDependencies + : BuildResult::Excluded); if (cidata.expected_failures.contains(spec.package_spec)) { @@ -461,7 +463,7 @@ namespace vcpkg auto&& action = action_plan.install_actions[i]; msg += fmt::format("{:>40}: {:>8}: {}\n", action.spec, - split_specs->action_state_string[i], + pre_build_status.action_state_string[i], action.abi_info.value_or_exit(VCPKG_LINE_INFO).package_abi); } @@ -479,7 +481,7 @@ namespace vcpkg Json::Object obj; obj.insert(JsonIdName, Json::Value::string(action.spec.name())); obj.insert(JsonIdTriplet, Json::Value::string(action.spec.triplet().canonical_name())); - obj.insert(JsonIdState, Json::Value::string(split_specs->action_state_string[i])); + obj.insert(JsonIdState, Json::Value::string(pre_build_status.action_state_string[i])); obj.insert(JsonIdAbi, Json::Value::string(action.abi_info.value_or_exit(VCPKG_LINE_INFO).package_abi)); arr.push_back(std::move(obj)); } @@ -503,7 +505,7 @@ namespace vcpkg }); } - reduce_action_plan(action_plan, split_specs->known, parent_hashes); + prune_entirely_known_action_branches(action_plan, pre_build_status.known, parent_hashes); msg::println(msgElapsedTimeForChecks, msg::elapsed = timer.elapsed()); @@ -523,7 +525,7 @@ namespace vcpkg StatusParagraphs status_db = database_load_collapse(fs, paths.installed()); auto already_installed = adjust_action_plan_to_status_db(action_plan, status_db); Util::erase_if(already_installed, - [&](auto& spec) { return Util::Sets::contains(split_specs->known, spec); }); + [&](const PackageSpec& spec) { return Util::Sets::contains(pre_build_status.known, spec); }); if (!already_installed.empty()) { LocalizedString warning; @@ -542,7 +544,7 @@ namespace vcpkg msg::println(msgTotalInstallTime, msg::elapsed = summary.elapsed); for (auto&& result : summary.results) { - split_specs->known.erase(result.get_spec()); + pre_build_status.known.erase(result.get_spec()); } msg::print(LocalizedString::from_raw("\n") @@ -552,7 +554,7 @@ namespace vcpkg .append_raw('\n') .append(summary.format_results())); const bool any_regressions = print_regressions(summary.results, - split_specs->known, + pre_build_status.known, cidata, ci_baseline_file_name, not_supported_regressions, @@ -567,24 +569,24 @@ namespace vcpkg for (auto&& result : summary.results) { const auto& spec = result.get_spec(); - auto& port_features = split_specs->features.at(spec); + auto& port_features = pre_build_status.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); + spec, code, result.timing, result.start_time, pre_build_status.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, SwitchXXUnitAll)) { - for (auto&& port : split_specs->known) + for (auto&& port : pre_build_status.known) { const auto& spec = port.first; - auto& port_features = split_specs->features.at(spec); + auto& port_features = pre_build_status.features.at(spec); xunitTestResults.add_test_results(spec, port.second, ElapsedTime{}, std::chrono::system_clock::time_point{}, - split_specs->abi_map.at(spec), + pre_build_status.abi_map.at(spec), port_features); } } From f39e41433302827a7c248253674f35d0bef8e124 Mon Sep 17 00:00:00 2001 From: Billy O'Neal Date: Wed, 29 Oct 2025 17:36:27 -0700 Subject: [PATCH 09/20] Update src/vcpkg/commands.ci.cpp Co-authored-by: Robert Schumacher --- src/vcpkg/commands.ci.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index 6e3f546014..62d277e16c 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -276,7 +276,7 @@ namespace PortFileProvider& provider) { // Generate a spec for the default features for every package, except for those explicitly skipped. - // While `reduce_action_plan` tries to remove skipped packages as expected failures, there + // While `reduce_action_plan` removes skipped packages as expected failures, there // it is too late as we have already calculated an action plan with feature dependencies from // the skipped ports. CiSpecsResult result; From afe1b14237f1d92b62fe26c78c67f4c546398e0f Mon Sep 17 00:00:00 2001 From: Billy O'Neal Date: Wed, 29 Oct 2025 17:49:18 -0700 Subject: [PATCH 10/20] Apply suggestion from @ras0219-msft Co-authored-by: Robert Schumacher --- src/vcpkg/binarycaching.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/vcpkg/binarycaching.cpp b/src/vcpkg/binarycaching.cpp index 63cc210d87..dcd892d152 100644 --- a/src/vcpkg/binarycaching.cpp +++ b/src/vcpkg/binarycaching.cpp @@ -2466,9 +2466,7 @@ namespace vcpkg { std::vector statuses = Util::fmap(actions, [this](const InstallPlanAction* action) { Checks::check_exit(VCPKG_LINE_INFO, action); - auto pabi = action->package_abi(); - Checks::check_exit(VCPKG_LINE_INFO, pabi); - return &m_status[*pabi]; + return &m_status[action->package_abi_or_exit(VCPKG_LINE_INFO)]; }); std::vector action_ptrs; From babd75097f9f2f36715fdae24851b97e7052b9dc Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 29 Oct 2025 18:07:41 -0700 Subject: [PATCH 11/20] Another @ras0219-msft suggestion --- src/vcpkg/binarycaching.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/vcpkg/binarycaching.cpp b/src/vcpkg/binarycaching.cpp index dcd892d152..d70b3f00ee 100644 --- a/src/vcpkg/binarycaching.cpp +++ b/src/vcpkg/binarycaching.cpp @@ -2464,7 +2464,7 @@ namespace vcpkg std::vector ReadOnlyBinaryCache::precheck(View actions) { - std::vector statuses = Util::fmap(actions, [this](const InstallPlanAction* action) { + const std::vector statuses = Util::fmap(actions, [this](const InstallPlanAction* action) { Checks::check_exit(VCPKG_LINE_INFO, action); return &m_status[action->package_abi_or_exit(VCPKG_LINE_INFO)]; }); @@ -2492,14 +2492,13 @@ namespace vcpkg for (size_t i = 0; i < action_ptrs.size(); ++i) { - auto&& this_status = m_status[action_ptrs[i]->package_abi_or_exit(VCPKG_LINE_INFO)]; if (cache_result[i] == CacheAvailability::available) { - this_status.mark_available(provider.get()); + statuses[i]->mark_available(provider.get()); } else if (cache_result[i] == CacheAvailability::unavailable) { - this_status.mark_unavailable(provider.get()); + statuses[i]->mark_unavailable(provider.get()); } } } From aaa0ea7fb016f7a707bf431267f7e75a019e2b45 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Thu, 30 Oct 2025 19:07:15 -0700 Subject: [PATCH 12/20] wip --- src/vcpkg/commands.ci.cpp | 59 ++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index 62d277e16c..9787f57192 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -83,30 +83,10 @@ namespace ActionPlan compute_full_plan(const VcpkgPaths& paths, const PortFileProvider& provider, const CMakeVars::CMakeVarProvider& var_provider, - const std::vector& specs, + const std::vector& applicable_specs, PackagesDirAssigner& packages_dir_assigner, const CreateInstallPlanOptions& serialize_options) { - std::vector packages_with_qualified_deps; - for (auto&& spec : specs) - { - auto&& scfl = provider.get_control_file(spec.package_spec.name()).value_or_exit(VCPKG_LINE_INFO); - if (scfl.source_control_file->has_qualified_dependencies() || - !scfl.source_control_file->core_paragraph->supports_expression.is_empty()) - { - packages_with_qualified_deps.push_back(spec.package_spec); - } - } - - var_provider.load_dep_info_vars(packages_with_qualified_deps, serialize_options.host_triplet); - - const auto applicable_specs = Util::filter(specs, [&](auto& spec) -> bool { - PackagesDirAssigner this_packages_dir_not_used{""}; - return create_feature_install_plan( - provider, var_provider, {&spec, 1}, {}, this_packages_dir_not_used, serialize_options) - .unsupported_features.empty(); - }); - auto action_plan = create_feature_install_plan( provider, var_provider, applicable_specs, {}, packages_dir_assigner, serialize_options); var_provider.load_tag_vars(action_plan, serialize_options.host_triplet); @@ -268,12 +248,15 @@ namespace struct CiSpecsResult { std::vector requested; + std::vector applicable; std::vector excluded; }; CiSpecsResult calculate_ci_specs(const ExclusionsMap& exclusions_map, const Triplet& target_triplet, - PortFileProvider& provider) + PortFileProvider& provider, + const CMakeVars::CMakeVarProvider& var_provider, + const CreateInstallPlanOptions& serialize_options) { // Generate a spec for the default features for every package, except for those explicitly skipped. // While `reduce_action_plan` removes skipped packages as expected failures, there @@ -290,20 +273,38 @@ namespace } } + std::vector packages_with_qualified_deps; for (auto scfl : provider.load_all_control_files()) { + auto package_spec = PackageSpec{scfl->to_name(), target_triplet}; + if (scfl->source_control_file->has_qualified_dependencies() || + !scfl->source_control_file->core_paragraph->supports_expression.is_empty()) + { + packages_with_qualified_deps.push_back(package_spec); + } + if (!target_triplet_exclusions || !target_triplet_exclusions->exclusions.contains(scfl->to_name())) { result.requested.emplace_back( - PackageSpec{scfl->to_name(), target_triplet}, + std::move(package_spec), InternalFeatureSet{FeatureNameCore.to_string(), FeatureNameDefault.to_string()}); } else { - result.excluded.emplace_back(PackageSpec{scfl->to_name(), target_triplet}); + result.excluded.emplace_back(std::move(package_spec)); } } + var_provider.load_dep_info_vars(packages_with_qualified_deps, serialize_options.host_triplet); + + result.applicable = Util::filter(result.requested, [&](const FullPackageSpec& spec) -> bool { + PackagesDirAssigner this_packages_dir_not_used{""}; + return create_feature_install_plan( + provider, var_provider, {&spec, 1}, {}, this_packages_dir_not_used, serialize_options) + .unsupported_features.empty(); + }); + + return result; } @@ -412,19 +413,19 @@ namespace vcpkg auto& var_provider = *var_provider_storage; const ElapsedTimer timer; - auto ci_specs = calculate_ci_specs(exclusions_map, target_triplet, provider); Optional randomizer; if (Util::Sets::contains(options.switches, SwitchXRandomize)) { randomizer.emplace(); } - - PackagesDirAssigner packages_dir_assigner{paths.packages()}; CreateInstallPlanOptions create_install_plan_options( randomizer.get(), host_triplet, UnsupportedPortAction::Warn, UseHeadVersion::No, Editable::No); + auto ci_specs = calculate_ci_specs(exclusions_map, target_triplet, provider, var_provider, create_install_plan_options); + + PackagesDirAssigner packages_dir_assigner{paths.packages()}; auto action_plan = compute_full_plan( - paths, provider, var_provider, ci_specs.requested, packages_dir_assigner, create_install_plan_options); + paths, provider, var_provider, ci_specs.applicable, packages_dir_assigner, create_install_plan_options); BinaryCache binary_cache(fs); if (!binary_cache.install_providers(args, paths, out_sink)) { @@ -433,7 +434,7 @@ namespace vcpkg auto install_actions = Util::fmap(action_plan.install_actions, [](const InstallPlanAction& action) { return &action; }); const auto precheck_results = binary_cache.precheck(install_actions); - const auto pre_build_status = + auto pre_build_status = compute_pre_build_statuses(exclusions_map, precheck_results, known_failure_abis, action_plan); LocalizedString not_supported_regressions; { From 848b8286b1ed7f9cf04d0bfee202b7be05da690d Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Fri, 31 Oct 2025 17:51:48 -0700 Subject: [PATCH 13/20] wip --- include/vcpkg/base/json.h | 2 - include/vcpkg/base/message-data.inc.h | 5 + include/vcpkg/paragraphs.h | 1 + locales/messages.json | 1 + src/vcpkg-test/ci-baseline.cpp | 2 +- src/vcpkg/base/json.cpp | 12 - src/vcpkg/commands.ci.cpp | 320 ++++++++++-------- src/vcpkg/commands.z-generate-message-map.cpp | 6 +- src/vcpkg/xunitwriter.cpp | 13 +- 9 files changed, 206 insertions(+), 156 deletions(-) diff --git a/include/vcpkg/base/json.h b/include/vcpkg/base/json.h index 9b33d870ff..e7b741cd4f 100644 --- a/include/vcpkg/base/json.h +++ b/include/vcpkg/base/json.h @@ -1,6 +1,5 @@ #pragma once -#include #include #include @@ -336,7 +335,6 @@ namespace vcpkg::Json }; ExpectedL parse(StringView text, StringView origin); - ParsedJson parse_file(LineInfo li, const ReadOnlyFilesystem&, const Path&); ExpectedL parse_object(StringView text, StringView origin); std::string stringify(const Value&); diff --git a/include/vcpkg/base/message-data.inc.h b/include/vcpkg/base/message-data.inc.h index fb99423963..0febd27cab 100644 --- a/include/vcpkg/base/message-data.inc.h +++ b/include/vcpkg/base/message-data.inc.h @@ -186,6 +186,11 @@ DECLARE_MESSAGE(ARegistryPathMustStartWithDollar, "", "A registry path must start with `$` to mean the registry root; for example, `$/foo/bar`.") DECLARE_MESSAGE(ARelaxedVersionString, (), "", "a relaxed version string") +DECLARE_MESSAGE( + RequestedPortsNotInCIPlan, + (), + "", + "one or more ports requested to be installed were not present in the action plan. (Probably a vcpkg bug)") DECLARE_MESSAGE(ArtifactsBootstrapFailed, (), "", "vcpkg-artifacts is not installed and could not be bootstrapped.") DECLARE_MESSAGE(ArtifactsOptionIncompatibility, (msg::option), "", "--{option} has no effect on find artifact.") DECLARE_MESSAGE(ArtifactsOptionJson, diff --git a/include/vcpkg/paragraphs.h b/include/vcpkg/paragraphs.h index d738727ab4..0b15e1235e 100644 --- a/include/vcpkg/paragraphs.h +++ b/include/vcpkg/paragraphs.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include diff --git a/locales/messages.json b/locales/messages.json index 0200ce0e30..59584916ad 100644 --- a/locales/messages.json +++ b/locales/messages.json @@ -1409,6 +1409,7 @@ "_RemovePackageConflict.comment": "An example of {package_name} is zlib. An example of {spec} is zlib:x64-windows. An example of {triplet} is x64-windows.", "RemovingPackage": "Removing {action_index}/{count} {spec}", "_RemovingPackage.comment": "An example of {action_index} is 340. An example of {count} is 42. An example of {spec} is zlib:x64-windows.", + "RequestedPortsNotInCIPlan": "one or more ports requested to be installed were not present in the action plan. (Probably a vcpkg bug)", "ResponseFileCode": "@response_file", "_ResponseFileCode.comment": "Explains to the user that they can use response files on the command line, 'response_file' must have no spaces and be a legal file name.", "RestoredPackagesFromAWS": "Restored {count} package(s) from AWS in {elapsed}. Use --debug to see more details.", diff --git a/src/vcpkg-test/ci-baseline.cpp b/src/vcpkg-test/ci-baseline.cpp index cfc76b8cbc..01b8e94f68 100644 --- a/src/vcpkg-test/ci-baseline.cpp +++ b/src/vcpkg-test/ci-baseline.cpp @@ -206,7 +206,7 @@ bill-made-up-another-skip:x64-linux=skip)"; // note no trailing newline SECTION ("Applies Skips and Fails") { ExclusionsMap exclusions; - exclusions.insert(x64_uwp, {}); // example triplet + exclusions.insert(x64_uwp, {}); // example triplet exclusions.insert(x64_linux, {}); // example host triplet auto actual = parse_and_apply_ci_baseline(expected_from_example_input, exclusions, SkipFailures::No); const SortedVector expected_expected_failures{ diff --git a/src/vcpkg/base/json.cpp b/src/vcpkg/base/json.cpp index 45dd74a8a9..657ce30a5d 100644 --- a/src/vcpkg/base/json.cpp +++ b/src/vcpkg/base/json.cpp @@ -1168,18 +1168,6 @@ namespace vcpkg::Json return true; } - ParsedJson parse_file(vcpkg::LineInfo li, const ReadOnlyFilesystem& fs, const Path& json_file) - { - std::error_code ec; - auto disk_contents = fs.read_contents(json_file, ec); - if (ec) - { - Checks::msg_exit_with_error(li, format_filesystem_call_error(ec, "read_contents", {json_file})); - } - - return parse(disk_contents, json_file).value_or_exit(VCPKG_LINE_INFO); - } - ExpectedL parse(StringView json, StringView origin) { return Parser::parse(json, origin); } ExpectedL parse_object(StringView text, StringView origin) diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index 9787f57192..7968aa4f13 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -53,10 +53,22 @@ namespace struct CIPreBuildStatus { std::map known; + std::map report_lines; std::map> features; - std::map abi_map; - // action_state_string.size() will equal install_actions.size() - std::vector action_state_string; + Json::Array abis; + }; + + enum class ExcludeReason + { + Baseline, + Supports, + Cascade + }; + + struct CiSpecsResult + { + std::vector requested; + std::map excluded; }; bool supported_for_triplet(const CMakeVars::CMakeVarProvider& var_provider, @@ -68,16 +80,8 @@ namespace { return true; } - PlatformExpression::Context context = var_provider.get_dep_info_vars(spec).value_or_exit(VCPKG_LINE_INFO); - return supports_expression.evaluate(context); - } - bool supported_for_triplet(const CMakeVars::CMakeVarProvider& var_provider, - const PortFileProvider& provider, - PackageSpec spec) - { - auto&& scf = provider.get_control_file(spec.name()).value_or_exit(VCPKG_LINE_INFO).source_control_file; - return supported_for_triplet(var_provider, *scf, spec); + return supports_expression.evaluate(var_provider.get_dep_info_vars(spec).value_or_exit(VCPKG_LINE_INFO)); } ActionPlan compute_full_plan(const VcpkgPaths& paths, @@ -98,67 +102,107 @@ namespace return action_plan; } - CIPreBuildStatus compute_pre_build_statuses(const ExclusionsMap& exclusions_map, + CIPreBuildStatus compute_pre_build_statuses(const CiSpecsResult& ci_specs, const std::vector& precheck_results, const std::unordered_set& known_failure_abis, + const std::unordered_set& parent_hashes, const ActionPlan& action_plan) { - CIPreBuildStatus ret; + static constexpr StringLiteral STATE_ABI_FAIL = "fail"; + static constexpr StringLiteral STATE_UNSUPPORTED = "unsupported"; + static constexpr StringLiteral STATE_CACHED = "cached"; + static constexpr StringLiteral STATE_PARENT = "parent"; + static constexpr StringLiteral STATE_UNKNOWN = "*"; + static constexpr StringLiteral STATE_SKIP = "skip"; + static constexpr StringLiteral STATE_CASCADE = "cascade"; - std::set will_fail; + CIPreBuildStatus ret; + std::unordered_set missing_specs; + for (const FullPackageSpec& spec : ci_specs.requested) + { + missing_specs.insert(spec.package_spec); + } - ret.action_state_string.reserve(action_plan.install_actions.size()); for (size_t action_idx = 0; action_idx < action_plan.install_actions.size(); ++action_idx) { - auto&& action = action_plan.install_actions[action_idx]; - - auto p = &action; - ret.abi_map.emplace(action.spec, action.abi_info.value_or_exit(VCPKG_LINE_INFO).package_abi); + const auto& action = action_plan.install_actions[action_idx]; + missing_specs.erase(action.spec); // note action.spec won't be in missing_specs if it's a host dependency + const std::string& public_abi = action.public_abi(); ret.features.emplace(action.spec, action.feature_list); - if (exclusions_map.is_excluded(p->spec)) + + const StringLiteral* state; + if (Util::Sets::contains(known_failure_abis, public_abi)) { - ret.action_state_string.emplace_back("skip"); - ret.known.emplace(p->spec, BuildResult::Excluded); - will_fail.emplace(p->spec); + state = &STATE_ABI_FAIL; + ret.known.emplace(action.spec, BuildResult::BuildFailed); } - else if (Util::Sets::contains(known_failure_abis, p->public_abi())) + else if (precheck_results[action_idx] == CacheAvailability::available) { - ret.action_state_string.emplace_back("will fail"); - ret.known.emplace(p->spec, BuildResult::BuildFailed); - will_fail.emplace(p->spec); + state = &STATE_CACHED; + ret.known.emplace(action.spec, BuildResult::Succeeded); } - else if (Util::any_of(p->package_dependencies, - [&](const PackageSpec& spec) { return Util::Sets::contains(will_fail, spec); })) + else if (Util::Sets::contains(parent_hashes, public_abi)) { - ret.action_state_string.emplace_back("cascade"); - ret.known.emplace(p->spec, BuildResult::CascadedDueToMissingDependencies); - will_fail.emplace(p->spec); + state = &STATE_PARENT; + ret.known.emplace(action.spec, BuildResult::Succeeded); } - else if (precheck_results[action_idx] == CacheAvailability::available) + else { - ret.action_state_string.emplace_back("pass"); - ret.known.emplace(p->spec, BuildResult::Succeeded); + state = &STATE_UNKNOWN; } - else + + ret.report_lines.insert_or_assign(action.spec, + fmt::format("{:>40}: {:>6}: {}", action.spec, *state, public_abi)); + Json::Object obj; + obj.insert(JsonIdName, Json::Value::string(action.spec.name())); + obj.insert(JsonIdTriplet, Json::Value::string(action.spec.triplet().canonical_name())); + obj.insert(JsonIdState, Json::Value::string(*state)); + obj.insert(JsonIdAbi, public_abi); + ret.abis.push_back(std::move(obj)); + } + + if (!missing_specs.empty()) + { + auto warning_text = msg::format(msgRequestedPortsNotInCIPlan); + for (const PackageSpec& missing_spec : missing_specs) { - ret.action_state_string.emplace_back("*"); + warning_text.append_raw('\n'); + warning_text.append_raw(missing_spec.to_string()); } + + console_diagnostic_context.report(DiagnosticLine{DiagKind::Warning, std::move(warning_text)}); } + + for (const auto& exclusion : ci_specs.excluded) + { + const StringLiteral* state; + switch (exclusion.second) + { + case ExcludeReason::Baseline: + state = &STATE_SKIP; + break; // FIXME distingish between =skip and =fail in ci.baseline.txt? + case ExcludeReason::Supports: state = &STATE_UNSUPPORTED; break; + case ExcludeReason::Cascade: state = &STATE_CASCADE; break; + default: Checks::unreachable(VCPKG_LINE_INFO); + } + + ret.report_lines.insert_or_assign(exclusion.first, fmt::format("{:>40}: {}", exclusion.first, *state)); + } + return ret; } // This algorithm reduces an action plan to only unknown actions and their dependencies void prune_entirely_known_action_branches(ActionPlan& action_plan, const std::map& known, - View parent_hashes) + const std::unordered_set& parent_hashes) { std::set to_keep; for (auto it = action_plan.install_actions.rbegin(); it != action_plan.install_actions.rend(); ++it) { 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_parent == parent_hashes.end()) + if (!Util::Sets::contains(parent_hashes, abi)) { it->request_type = RequestType::USER_REQUESTED; if (it_known == known.end()) @@ -245,12 +289,21 @@ namespace return has_error; } - struct CiSpecsResult + std::vector calculate_packages_with_qualified_deps( + const std::vector& all_control_files, const Triplet& target_triplet) { - std::vector requested; - std::vector applicable; - std::vector excluded; - }; + std::vector ret; + for (auto scfl : all_control_files) + { + if (scfl->source_control_file->has_qualified_dependencies() || + !scfl->source_control_file->core_paragraph->supports_expression.is_empty()) + { + ret.emplace_back(scfl->to_name(), target_triplet); + } + } + + return ret; + } CiSpecsResult calculate_ci_specs(const ExclusionsMap& exclusions_map, const Triplet& target_triplet, @@ -273,37 +326,39 @@ namespace } } - std::vector packages_with_qualified_deps; - for (auto scfl : provider.load_all_control_files()) + auto all_control_files = provider.load_all_control_files(); + + // populate `var_provider` to evaluate supports expressions for all ports: + std::vector packages_with_qualified_deps = + calculate_packages_with_qualified_deps(all_control_files, target_triplet); + var_provider.load_dep_info_vars(packages_with_qualified_deps, serialize_options.host_triplet); + + for (auto scfl : all_control_files) { - auto package_spec = PackageSpec{scfl->to_name(), target_triplet}; - if (scfl->source_control_file->has_qualified_dependencies() || - !scfl->source_control_file->core_paragraph->supports_expression.is_empty()) + auto full_package_spec = + FullPackageSpec{PackageSpec{scfl->to_name(), target_triplet}, + InternalFeatureSet{FeatureNameCore.to_string(), FeatureNameDefault.to_string()}}; + if (target_triplet_exclusions && target_triplet_exclusions->exclusions.contains(scfl->to_name())) { - packages_with_qualified_deps.push_back(package_spec); + result.excluded.insert_or_assign(std::move(full_package_spec.package_spec), ExcludeReason::Baseline); + continue; } - if (!target_triplet_exclusions || !target_triplet_exclusions->exclusions.contains(scfl->to_name())) - { - result.requested.emplace_back( - std::move(package_spec), - InternalFeatureSet{FeatureNameCore.to_string(), FeatureNameDefault.to_string()}); - } - else + PackagesDirAssigner this_packages_dir_not_used{""}; + if (!create_feature_install_plan( + provider, var_provider, {&full_package_spec, 1}, {}, this_packages_dir_not_used, serialize_options) + .unsupported_features.empty()) { - result.excluded.emplace_back(std::move(package_spec)); + result.excluded.insert_or_assign( + std::move(full_package_spec.package_spec), + supported_for_triplet(var_provider, *scfl->source_control_file, full_package_spec.package_spec) + ? ExcludeReason::Supports + : ExcludeReason::Cascade); + continue; } - } - - var_provider.load_dep_info_vars(packages_with_qualified_deps, serialize_options.host_triplet); - - result.applicable = Util::filter(result.requested, [&](const FullPackageSpec& spec) -> bool { - PackagesDirAssigner this_packages_dir_not_used{""}; - return create_feature_install_plan( - provider, var_provider, {&spec, 1}, {}, this_packages_dir_not_used, serialize_options) - .unsupported_features.empty(); - }); + result.requested.emplace_back(std::move(full_package_spec)); + } return result; } @@ -388,7 +443,24 @@ namespace vcpkg { Path raw_path = it_known_failures->second; auto lines = paths.get_filesystem().read_lines(raw_path).value_or_exit(VCPKG_LINE_INFO); - known_failure_abis.insert(lines.begin(), lines.end()); + known_failure_abis.insert(std::make_move_iterator(lines.begin()), std::make_move_iterator(lines.end())); + } + + std::unordered_set parent_hashes; + auto it_parent_hashes = settings.find(SwitchParentHashes); + if (it_parent_hashes != settings.end()) + { + const Path parent_hashes_path = paths.original_cwd / it_parent_hashes->second; + auto parent_hashes_text = fs.try_read_contents(parent_hashes_path).value_or_exit(VCPKG_LINE_INFO); + const auto parsed_object = + Json::parse(parent_hashes_text.content, parent_hashes_text.origin).value_or_exit(VCPKG_LINE_INFO); + const auto& parent_hashes_array = parsed_object.value.array(VCPKG_LINE_INFO); + for (const Json::Value& array_value : parent_hashes_array) + { + auto abi = array_value.object(VCPKG_LINE_INFO).get(JsonIdAbi); + Checks::check_exit(VCPKG_LINE_INFO, abi); + parent_hashes.insert(abi->string(VCPKG_LINE_INFO).to_string()); + } } const auto is_dry_run = Util::Sets::contains(options.switches, SwitchDryRun); @@ -421,11 +493,12 @@ namespace vcpkg } CreateInstallPlanOptions create_install_plan_options( randomizer.get(), host_triplet, UnsupportedPortAction::Warn, UseHeadVersion::No, Editable::No); - auto ci_specs = calculate_ci_specs(exclusions_map, target_triplet, provider, var_provider, create_install_plan_options); + auto ci_specs = + calculate_ci_specs(exclusions_map, target_triplet, provider, var_provider, create_install_plan_options); PackagesDirAssigner packages_dir_assigner{paths.packages()}; auto action_plan = compute_full_plan( - paths, provider, var_provider, ci_specs.applicable, packages_dir_assigner, create_install_plan_options); + paths, provider, var_provider, ci_specs.requested, packages_dir_assigner, create_install_plan_options); BinaryCache binary_cache(fs); if (!binary_cache.install_providers(args, paths, out_sink)) { @@ -435,39 +508,44 @@ namespace vcpkg Util::fmap(action_plan.install_actions, [](const InstallPlanAction& action) { return &action; }); const auto precheck_results = binary_cache.precheck(install_actions); auto pre_build_status = - compute_pre_build_statuses(exclusions_map, precheck_results, known_failure_abis, action_plan); + compute_pre_build_statuses(ci_specs, precheck_results, known_failure_abis, parent_hashes, action_plan); LocalizedString not_supported_regressions; { - std::string msg; - for (const auto& spec : ci_specs.requested) - { - if (!Util::Sets::contains(pre_build_status.abi_map, spec.package_spec)) - { - bool supp = supported_for_triplet(var_provider, provider, spec.package_spec); - pre_build_status.known.emplace(spec.package_spec, - supp ? BuildResult::CascadedDueToMissingDependencies - : BuildResult::Excluded); + // std::string msg; + // for (const auto& spec : ci_specs.requested) + //{ + // if (!Util::Sets::contains(pre_build_status.abi_map, spec.package_spec)) + // { + // bool supp = supported_for_triplet(var_provider, provider, spec.package_spec); + // pre_build_status.known.emplace(spec.package_spec, + // supp ? BuildResult::CascadedDueToMissingDependencies + // : BuildResult::Excluded); + + // if (cidata.expected_failures.contains(spec.package_spec)) + // { + // not_supported_regressions // FIXME + // .append(supp ? msgCiBaselineUnexpectedFailCascade : msgCiBaselineUnexpectedFail, + // msg::spec = spec.package_spec, + // msg::triplet = spec.package_spec.triplet()) + // .append_raw('\n'); + // } + // msg += fmt::format("{:>40}: {:>8}\n", spec.package_spec, supp ? "cascade" : "skip"); + // } + //} + // for (size_t i = 0; i < action_plan.install_actions.size(); ++i) + //{ + // auto&& action = action_plan.install_actions[i]; + // msg += fmt::format("{:>40}: {:>8}: {}\n", + // action.spec, + // pre_build_status.action_state_string[i], + // action.abi_info.value_or_exit(VCPKG_LINE_INFO).package_abi); + //} - if (cidata.expected_failures.contains(spec.package_spec)) - { - not_supported_regressions - .append(supp ? msgCiBaselineUnexpectedFailCascade : msgCiBaselineUnexpectedFail, - msg::spec = spec.package_spec, - msg::triplet = spec.package_spec.triplet()) - .append_raw('\n'); - } - msg += fmt::format("{:>40}: {:>8}\n", spec.package_spec, supp ? "cascade" : "skip"); - } - } - for (size_t i = 0; i < action_plan.install_actions.size(); ++i) + std::string msg; + for (auto&& line : pre_build_status.report_lines) { - auto&& action = action_plan.install_actions[i]; - msg += fmt::format("{:>40}: {:>8}: {}\n", - action.spec, - pre_build_status.action_state_string[i], - action.abi_info.value_or_exit(VCPKG_LINE_INFO).package_abi); + msg += fmt::format("{}\n", line.second); } - msg::write_unlocalized_text(Color::none, msg); } @@ -475,35 +553,7 @@ namespace vcpkg if (it_output_hashes != settings.end()) { const Path output_hash_json = paths.original_cwd / it_output_hashes->second; - Json::Array arr; - for (size_t i = 0; i < action_plan.install_actions.size(); ++i) - { - auto&& action = action_plan.install_actions[i]; - Json::Object obj; - obj.insert(JsonIdName, Json::Value::string(action.spec.name())); - obj.insert(JsonIdTriplet, Json::Value::string(action.spec.triplet().canonical_name())); - obj.insert(JsonIdState, Json::Value::string(pre_build_status.action_state_string[i])); - obj.insert(JsonIdAbi, Json::Value::string(action.abi_info.value_or_exit(VCPKG_LINE_INFO).package_abi)); - arr.push_back(std::move(obj)); - } - fs.write_contents(output_hash_json, Json::stringify(arr), VCPKG_LINE_INFO); - } - - std::vector parent_hashes; - - auto it_parent_hashes = settings.find(SwitchParentHashes); - if (it_parent_hashes != settings.end()) - { - const Path parent_hashes_path = paths.original_cwd / it_parent_hashes->second; - auto parsed_json = Json::parse_file(VCPKG_LINE_INFO, fs, parent_hashes_path).value; - parent_hashes = Util::fmap(parsed_json.array(VCPKG_LINE_INFO), [](const auto& json_object) { - auto abi = json_object.object(VCPKG_LINE_INFO).get(JsonIdAbi); - Checks::check_exit(VCPKG_LINE_INFO, abi); -#ifdef _MSC_VER - _Analysis_assume_(abi); -#endif - return abi->string(VCPKG_LINE_INFO).to_string(); - }); + fs.write_contents(output_hash_json, Json::stringify(pre_build_status.abis), VCPKG_LINE_INFO); } prune_entirely_known_action_branches(action_plan, pre_build_status.known, parent_hashes); @@ -572,8 +622,12 @@ namespace vcpkg const auto& spec = result.get_spec(); auto& port_features = pre_build_status.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, pre_build_status.abi_map.at(spec), port_features); + xunitTestResults.add_test_results(spec, + code, + result.timing, + result.start_time, + result.package_abi_or_exit(VCPKG_LINE_INFO), + port_features); } // Adding results for ports that were not built because they have known states @@ -587,7 +641,7 @@ namespace vcpkg port.second, ElapsedTime{}, std::chrono::system_clock::time_point{}, - pre_build_status.abi_map.at(spec), + /* pre_build_status.abi_map.at(spec) FIXME */ "", port_features); } } diff --git a/src/vcpkg/commands.z-generate-message-map.cpp b/src/vcpkg/commands.z-generate-message-map.cpp index f7430781c7..ce80cfc80a 100644 --- a/src/vcpkg/commands.z-generate-message-map.cpp +++ b/src/vcpkg/commands.z-generate-message-map.cpp @@ -233,9 +233,9 @@ namespace vcpkg Path path_to_artifact_messages = parsed_args.command_arguments[1]; // parse file to get json obj - auto artifact_messages = Json::parse_file(VCPKG_LINE_INFO, fs, path_to_artifact_messages).value; - auto artifact_obj = artifact_messages.object(VCPKG_LINE_INFO); - + auto artifact_messages_content = fs.try_read_contents(path_to_artifact_messages).value_or_exit(VCPKG_LINE_INFO); + auto artifact_obj = Json::parse_object(artifact_messages_content.content, artifact_messages_content.origin) + .value_or_exit(VCPKG_LINE_INFO); for (auto&& it : artifact_obj) { obj.insert(it.first, it.second); diff --git a/src/vcpkg/xunitwriter.cpp b/src/vcpkg/xunitwriter.cpp index 246ff6fd28..07fc219dcb 100644 --- a/src/vcpkg/xunitwriter.cpp +++ b/src/vcpkg/xunitwriter.cpp @@ -25,12 +25,15 @@ namespace StringLiteral result_string = ""; switch (test.result) { - case BuildResult::PostBuildChecksFailed: - case BuildResult::FileConflicts: - case BuildResult::BuildFailed: result_string = "Fail"; break; - case BuildResult::Excluded: - case BuildResult::CascadedDueToMissingDependencies: result_string = "Skip"; break; case BuildResult::Succeeded: result_string = "Pass"; break; + case BuildResult::BuildFailed: + case BuildResult::PostBuildChecksFailed: + case BuildResult::FileConflicts: result_string = "Fail"; break; + case BuildResult::CascadedDueToMissingDependencies: + case BuildResult::Excluded: result_string = "Skip"; break; + case BuildResult::CacheMissing: + case BuildResult::Downloaded: + case BuildResult::Removed: default: Checks::unreachable(VCPKG_LINE_INFO); } From fa9941af387bf5622079359ed879d0c7bac8061d Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Mon, 3 Nov 2025 19:06:34 -0800 Subject: [PATCH 14/20] wip --- include/vcpkg/base/message-data.inc.h | 4 - include/vcpkg/ci-baseline.h | 3 +- include/vcpkg/commands.install.h | 5 +- include/vcpkg/xunitwriter.h | 21 ++++-- locales/messages.json | 2 - src/vcpkg-test/ci-baseline.cpp | 6 +- src/vcpkg-test/xunitwriter.cpp | 31 +++++--- src/vcpkg/ci-baseline.cpp | 10 +-- src/vcpkg/commands.ci.cpp | 103 +++++++++----------------- src/vcpkg/commands.install.cpp | 15 ++-- src/vcpkg/commands.test-features.cpp | 13 ++-- src/vcpkg/xunitwriter.cpp | 90 +++++++++++++--------- 12 files changed, 146 insertions(+), 157 deletions(-) diff --git a/include/vcpkg/base/message-data.inc.h b/include/vcpkg/base/message-data.inc.h index 0febd27cab..4b48b86c58 100644 --- a/include/vcpkg/base/message-data.inc.h +++ b/include/vcpkg/base/message-data.inc.h @@ -472,10 +472,6 @@ DECLARE_MESSAGE(CiBaselineDisallowedCascade, (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 3b37f21d07..77d809d3af 100644 --- a/include/vcpkg/ci-baseline.h +++ b/include/vcpkg/ci-baseline.h @@ -62,6 +62,5 @@ namespace vcpkg BuildResult result, const CiBaselineData& cidata, const std::string* cifile, - bool allow_unexpected_passing, - bool is_independent); + bool allow_unexpected_passing); } diff --git a/include/vcpkg/commands.install.h b/include/vcpkg/commands.install.h index 53abaddf9e..b3d2ce6c85 100644 --- a/include/vcpkg/commands.install.h +++ b/include/vcpkg/commands.install.h @@ -50,10 +50,7 @@ namespace vcpkg Optional build_result; vcpkg::ElapsedTime timing; std::chrono::system_clock::time_point start_time; - Optional get_install_plan_action() const - { - return m_install_action ? Optional(*m_install_action) : nullopt; - } + const InstallPlanAction* get_maybe_install_plan_action() const { return m_install_action; } private: const InstallPlanAction* m_install_action; diff --git a/include/vcpkg/xunitwriter.h b/include/vcpkg/xunitwriter.h index 6ed17acc38..64745438a4 100644 --- a/include/vcpkg/xunitwriter.h +++ b/include/vcpkg/xunitwriter.h @@ -13,6 +13,20 @@ namespace vcpkg { + struct CiBuiltResult + { + std::string package_abi; + InternalFeatureSet feature_list; + std::chrono::system_clock::time_point start_time; + ElapsedTime timing; + }; + + struct CiResult + { + BuildResult code; // FIXME describe why excluded + Optional build; + }; + struct XunitTest; // https://xunit.net/docs/format-xml-v2 @@ -22,12 +36,7 @@ namespace vcpkg // Out of line con/destructor avoids exposing XunitTest XunitWriter(); ~XunitWriter(); - void add_test_results(const PackageSpec& spec, - BuildResult build_result, - const ElapsedTime& elapsed_time, - const std::chrono::system_clock::time_point& start_time, - const std::string& abi_tag, - const std::vector& features); + void add_test_results(const PackageSpec& spec, const CiResult& result); std::string build_xml(Triplet controlling_triplet) const; diff --git a/locales/messages.json b/locales/messages.json index 59584916ad..1d73ccedfe 100644 --- a/locales/messages.json +++ b/locales/messages.json @@ -297,8 +297,6 @@ "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 01b8e94f68..d52cd28b38 100644 --- a/src/vcpkg-test/ci-baseline.cpp +++ b/src/vcpkg-test/ci-baseline.cpp @@ -333,7 +333,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, false); + return format_ci_result(s, BuildResult::Succeeded, cidata, &cifile, allow_unexpected_passing); }; CHECK(test({"pass", Test::X64_UWP}, true) == ""); CHECK(test({"pass", Test::X64_UWP}, false) == ""); @@ -347,7 +347,7 @@ TEST_CASE ("format_ci_result 1", "[ci-baseline]") SECTION ("BUILD_FAILED") { const auto test = [&](PackageSpec s) { - return format_ci_result(s, BuildResult::BuildFailed, cidata, &cifile, false, false); + return format_ci_result(s, BuildResult::BuildFailed, cidata, &cifile, false); }; CHECK(test({"pass", Test::X64_UWP}) == fmt::format(failmsg, "pass:x64-uwp")); CHECK(test({"fail", Test::X64_UWP}) == ""); @@ -357,7 +357,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::CascadedDueToMissingDependencies, cidata, &cifile, false, false); + return format_ci_result(s, BuildResult::CascadedDueToMissingDependencies, cidata, &cifile, false); }; CHECK(test({"pass", Test::X64_UWP}) == fmt::format(cascademsg, "pass:x64-uwp")); CHECK(test({"fail", Test::X64_UWP}) == ""); diff --git a/src/vcpkg-test/xunitwriter.cpp b/src/vcpkg-test/xunitwriter.cpp index 1a902ff1fb..464ed8cbf2 100644 --- a/src/vcpkg-test/xunitwriter.cpp +++ b/src/vcpkg-test/xunitwriter.cpp @@ -7,14 +7,13 @@ using namespace vcpkg; TEST_CASE ("Simple XunitWriter", "[xunitwriter]") { XunitWriter x; - time_t time = {0}; Triplet t = Triplet::from_canonical_name("triplet"); PackageSpec spec("name", t); - x.add_test_results(spec, BuildResult::BuildFailed, {}, std::chrono::system_clock::from_time_t(time), "", {}); + x.add_test_results(spec, CiResult{BuildResult::BuildFailed, nullopt}); CHECK(x.build_xml(t) == R"( - + @@ -29,28 +28,37 @@ TEST_CASE ("Simple XunitWriter", "[xunitwriter]") TEST_CASE ("XunitWriter Two", "[xunitwriter]") { XunitWriter x; - time_t time = {0}; + time_t january_first = {0}; + time_t january_second = {86400}; + auto january_first_st = std::chrono::system_clock::from_time_t(january_first); + auto january_second_st = std::chrono::system_clock::from_time_t(january_second); Triplet t = Triplet::from_canonical_name("triplet"); Triplet t2 = Triplet::from_canonical_name("triplet2"); Triplet t3 = Triplet::from_canonical_name("triplet3"); PackageSpec spec("name", t); PackageSpec spec2("name", t2); PackageSpec spec3("other", t2); - x.add_test_results(spec, BuildResult::Succeeded, {}, std::chrono::system_clock::from_time_t(time), "abihash", {}); x.add_test_results( - spec2, BuildResult::PostBuildChecksFailed, {}, std::chrono::system_clock::from_time_t(time), "", {}); + spec, + CiResult{BuildResult::Succeeded, + CiBuiltResult{"abihash", InternalFeatureSet{"a", "b", "core"}, january_first_st, ElapsedTime{}}}); + x.add_test_results(spec2, CiResult{BuildResult::PostBuildChecksFailed, nullopt}); x.add_test_results( - spec3, BuildResult::Succeeded, {}, std::chrono::system_clock::from_time_t(time), "", {"core", "feature"}); + spec3, + CiResult{ + BuildResult::Succeeded, + CiBuiltResult{"otherabihash", InternalFeatureSet{"core", "feature"}, january_second_st, ElapsedTime{}}}); CHECK(x.build_xml(t3) == R"( - + + - + @@ -58,11 +66,12 @@ TEST_CASE ("XunitWriter Two", "[xunitwriter]") - + - + + diff --git a/src/vcpkg/ci-baseline.cpp b/src/vcpkg/ci-baseline.cpp index 04721321bb..750bc14d4b 100644 --- a/src/vcpkg/ci-baseline.cpp +++ b/src/vcpkg/ci-baseline.cpp @@ -190,8 +190,7 @@ namespace vcpkg BuildResult result, const CiBaselineData& cidata, const std::string* cifile, - bool allow_unexpected_passing, - bool is_independent) + bool allow_unexpected_passing) { switch (result) { @@ -200,13 +199,6 @@ namespace vcpkg case BuildResult::FileConflicts: if (!cidata.expected_failures.contains(spec)) { - if (is_independent) - { - return msg::format(msgCiBaselineIndependentRegression, - msg::spec = spec, - msg::build_result = to_string_locale_invariant(result)); - } - if (cifile) { return msg::format(msgCiBaselineRegression, diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index 7968aa4f13..03cb8dbb74 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -54,7 +54,6 @@ namespace { std::map known; std::map report_lines; - std::map> features; Json::Array abis; }; @@ -128,8 +127,6 @@ namespace const auto& action = action_plan.install_actions[action_idx]; missing_specs.erase(action.spec); // note action.spec won't be in missing_specs if it's a host dependency const std::string& public_abi = action.public_abi(); - ret.features.emplace(action.spec, action.feature_list); - const StringLiteral* state; if (Util::Sets::contains(known_failure_abis, public_abi)) { @@ -178,9 +175,9 @@ namespace const StringLiteral* state; switch (exclusion.second) { - case ExcludeReason::Baseline: - state = &STATE_SKIP; - break; // FIXME distingish between =skip and =fail in ci.baseline.txt? + // it probably makes sense to distinguish between "--exclude", "=skip" and "=fail but --skip-failures" + // but we don't preserve that information right now, so all these cases report as "skip" + case ExcludeReason::Baseline: state = &STATE_SKIP; break; case ExcludeReason::Supports: state = &STATE_UNSUPPORTED; break; case ExcludeReason::Cascade: state = &STATE_CASCADE; break; default: Checks::unreachable(VCPKG_LINE_INFO); @@ -241,9 +238,8 @@ namespace : SortedVector(Strings::split(it_exclusions->second, ','))); } - bool print_regressions(const std::vector& results, - const std::map& known, - const CiBaselineData& cidata, + bool print_regressions(const std::map& ci_results, + const CiBaselineData& baseline_data, const std::string* ci_baseline_file_name, const LocalizedString& not_supported_regressions, bool allow_unexpected_passing) @@ -252,28 +248,10 @@ namespace LocalizedString output = msg::format(msgCiBaselineRegressionHeader); output.append_raw('\n'); output.append(not_supported_regressions); - 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()); - if (!msg.empty()) - { - has_error = true; - output.append(msg).append_raw('\n'); - } - } - - // FIXME this is the root of the problem: `results` should have *all* of the results, not need to be somehow - // combined with the pre-build known results - for (auto&& r : known) + for (auto&& ci_result : ci_results) { - auto msg = - format_ci_result(r.first, r.second, cidata, ci_baseline_file_name, allow_unexpected_passing, true); + auto msg = format_ci_result( + ci_result.first, ci_result.second.code, baseline_data, ci_baseline_file_name, allow_unexpected_passing); if (!msg.empty()) { has_error = true; @@ -417,7 +395,7 @@ namespace vcpkg auto baseline_iter = settings.find(SwitchCIBaseline); const std::string* ci_baseline_file_name = nullptr; const bool allow_unexpected_passing = Util::Sets::contains(options.switches, SwitchAllowUnexpectedPassing); - CiBaselineData cidata; + CiBaselineData baseline_data; if (baseline_iter == settings.end()) { if (allow_unexpected_passing) @@ -434,7 +412,7 @@ namespace vcpkg ParseMessages ci_parse_messages; const auto lines = parse_ci_baseline(ci_baseline_file_contents, *ci_baseline_file_name, ci_parse_messages); ci_parse_messages.exit_if_errors_or_warnings(); - cidata = parse_and_apply_ci_baseline(lines, exclusions_map, skip_failures); + baseline_data = parse_and_apply_ci_baseline(lines, exclusions_map, skip_failures); } std::unordered_set known_failure_abis; @@ -507,7 +485,7 @@ namespace vcpkg auto install_actions = Util::fmap(action_plan.install_actions, [](const InstallPlanAction& action) { return &action; }); const auto precheck_results = binary_cache.precheck(install_actions); - auto pre_build_status = + const auto pre_build_status = compute_pre_build_statuses(ci_specs, precheck_results, known_failure_abis, parent_hashes, action_plan); LocalizedString not_supported_regressions; { @@ -521,7 +499,7 @@ namespace vcpkg // supp ? BuildResult::CascadedDueToMissingDependencies // : BuildResult::Excluded); - // if (cidata.expected_failures.contains(spec.package_spec)) + // if (baseline_data.expected_failures.contains(spec.package_spec)) // { // not_supported_regressions // FIXME // .append(supp ? msgCiBaselineUnexpectedFailCascade : msgCiBaselineUnexpectedFail, @@ -593,9 +571,27 @@ namespace vcpkg auto summary = install_execute_plan( args, paths, host_triplet, build_options, action_plan, status_db, binary_cache, *build_logs_recorder); msg::println(msgTotalInstallTime, msg::elapsed = summary.elapsed); + + std::map ci_full_results; + for (auto&& pre_known_outcome : pre_build_status.known) + { + ci_full_results.insert_or_assign(pre_known_outcome.first, CiResult{pre_known_outcome.second, nullopt}); + } + + std::map ci_results; for (auto&& result : summary.results) { - pre_build_status.known.erase(result.get_spec()); + if (const auto* ipa = result.get_maybe_install_plan_action()) + { + // note that we assign over the 'guessed' known values from above + auto ci_result = CiResult{result.build_result.value_or_exit(VCPKG_LINE_INFO).code, + CiBuiltResult{ipa->package_abi_or_exit(VCPKG_LINE_INFO), + ipa->feature_list, + result.start_time, + result.timing}}; + ci_results.insert_or_assign(result.get_spec(), ci_result); + ci_full_results.insert_or_assign(result.get_spec(), std::move(ci_result)); + } } msg::print(LocalizedString::from_raw("\n") @@ -604,9 +600,8 @@ namespace vcpkg .append_raw(target_triplet) .append_raw('\n') .append(summary.format_results())); - const bool any_regressions = print_regressions(summary.results, - pre_build_status.known, - cidata, + const bool any_regressions = print_regressions(ci_full_results, + baseline_data, ci_baseline_file_name, not_supported_regressions, allow_unexpected_passing); @@ -615,35 +610,11 @@ namespace vcpkg 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 = pre_build_status.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, - result.package_abi_or_exit(VCPKG_LINE_INFO), - port_features); - } - - // Adding results for ports that were not built because they have known states - if (Util::Sets::contains(options.switches, SwitchXXUnitAll)) + const auto& xunit_results = + Util::Sets::contains(options.switches, SwitchXXUnitAll) ? ci_full_results : ci_results; + for (auto&& xunit_result : xunit_results) { - for (auto&& port : pre_build_status.known) - { - const auto& spec = port.first; - auto& port_features = pre_build_status.features.at(spec); - xunitTestResults.add_test_results(spec, - port.second, - ElapsedTime{}, - std::chrono::system_clock::time_point{}, - /* pre_build_status.abi_map.at(spec) FIXME */ "", - port_features); - } + xunitTestResults.add_test_results(xunit_result.first, xunit_result.second); } fs.write_contents(it_xunit->second, xunitTestResults.build_xml(target_triplet), VCPKG_LINE_INFO); diff --git a/src/vcpkg/commands.install.cpp b/src/vcpkg/commands.install.cpp index f607aac311..e09d1886ed 100644 --- a/src/vcpkg/commands.install.cpp +++ b/src/vcpkg/commands.install.cpp @@ -1484,12 +1484,15 @@ namespace vcpkg for (auto&& result : summary.results) { - xwriter.add_test_results(result.get_spec(), - result.build_result.value_or_exit(VCPKG_LINE_INFO).code, - result.timing, - result.start_time, - "", - {}); + if (const auto* ipa = result.get_maybe_install_plan_action()) + { + xwriter.add_test_results(result.get_spec(), + CiResult{result.build_result.value_or_exit(VCPKG_LINE_INFO).code, + CiBuiltResult{ipa->package_abi_or_exit(VCPKG_LINE_INFO), + ipa->feature_list, + result.start_time, + result.timing}}); + } } fs.write_contents(it_xunit->second, xwriter.build_xml(default_triplet), VCPKG_LINE_INFO); diff --git a/src/vcpkg/commands.test-features.cpp b/src/vcpkg/commands.test-features.cpp index 2e1289f3ff..f27a222b58 100644 --- a/src/vcpkg/commands.test-features.cpp +++ b/src/vcpkg/commands.test-features.cpp @@ -848,14 +848,11 @@ namespace vcpkg if (Path* logs_dir = maybe_logs_dir.get()) { auto issue_body_path = *logs_dir / FileIssueBodyMD; - fs.write_contents( - issue_body_path, - create_github_issue(args, - build_result, - paths, - result.get_install_plan_action().value_or_exit(VCPKG_LINE_INFO), - false), - VCPKG_LINE_INFO); + const auto* ipa = result.get_maybe_install_plan_action(); + Checks::check_exit(VCPKG_LINE_INFO, ipa); + fs.write_contents(issue_body_path, + create_github_issue(args, build_result, paths, *ipa, false), + VCPKG_LINE_INFO); } [[fallthrough]]; diff --git a/src/vcpkg/xunitwriter.cpp b/src/vcpkg/xunitwriter.cpp index 07fc219dcb..981d854fc1 100644 --- a/src/vcpkg/xunitwriter.cpp +++ b/src/vcpkg/xunitwriter.cpp @@ -11,11 +11,7 @@ struct vcpkg::XunitTest std::string name; std::string method; std::string owner; - BuildResult result; - ElapsedTime time; - std::chrono::system_clock::time_point start_time; - std::string abi_tag; - std::vector features; + CiResult ci_result; }; namespace @@ -23,7 +19,7 @@ namespace static void xml_test(XmlSerializer& xml, const XunitTest& test) { StringLiteral result_string = ""; - switch (test.result) + switch (test.ci_result.code) { case BuildResult::Succeeded: result_string = "Pass"; break; case BuildResult::BuildFailed: @@ -37,28 +33,32 @@ namespace default: Checks::unreachable(VCPKG_LINE_INFO); } + long long time_seconds = 0ll; + if (const auto* build = test.ci_result.build.get()) + { + time_seconds = build->timing.as().count(); + } + xml.start_complex_open_tag("test") .attr("name", test.name) .attr("method", test.method) - .attr("time", fmt::format("{}", test.time.as().count())) + .attr("time", fmt::format("{}", time_seconds)) .attr("result", result_string) .finish_complex_open_tag() .line_break(); + xml.open_tag("traits").line_break(); - if (!test.abi_tag.empty()) + if (const auto* build = test.ci_result.build.get()) { xml.start_complex_open_tag("trait") .attr("name", "abi_tag") - .attr("value", test.abi_tag) + .attr("value", build->package_abi) .finish_self_closing_complex_tag() .line_break(); - } - if (!test.features.empty()) - { xml.start_complex_open_tag("trait") .attr("name", "features") - .attr("value", Strings::join(", ", test.features)) + .attr("value", Strings::join(",", build->feature_list)) .finish_self_closing_complex_tag() .line_break(); } @@ -74,14 +74,17 @@ namespace { xml.open_tag("failure") .open_tag("message") - .cdata(to_string_locale_invariant(test.result)) + .cdata(to_string_locale_invariant(test.ci_result.code)) .close_tag("message") .close_tag("failure") .line_break(); } else if (result_string == "Skip") { - xml.open_tag("reason").cdata(to_string_locale_invariant(test.result)).close_tag("reason").line_break(); + xml.open_tag("reason") + .cdata(to_string_locale_invariant(test.ci_result.code)) + .close_tag("reason") + .line_break(); } else { @@ -95,22 +98,18 @@ namespace XunitWriter::XunitWriter() = default; XunitWriter::~XunitWriter() = default; -void XunitWriter::add_test_results(const PackageSpec& spec, - BuildResult build_result, - const ElapsedTime& elapsed_time, - const std::chrono::system_clock::time_point& start_time, - const std::string& abi_tag, - const std::vector& features) +void XunitWriter::add_test_results(const PackageSpec& spec, const CiResult& result) { - m_tests[spec.name()].push_back( - {spec.to_string(), - Strings::concat(spec.name(), '[', Strings::join(",", features), "]:", spec.triplet()), - spec.triplet().to_string(), - build_result, - elapsed_time, - start_time, - abi_tag, - features}); + const auto& name = spec.name(); + std::string method = name; + if (auto* built = result.build.get()) + { + fmt::format_to(std::back_inserter(method), "[{}]", Strings::join(",", built->feature_list)); + } + + fmt::format_to(std::back_inserter(method), ":{}", spec.triplet()); + + m_tests[spec.name()].push_back({spec.to_string(), std::move(method), spec.triplet().to_string(), result}); } std::string XunitWriter::build_xml(Triplet controlling_triplet) const @@ -126,17 +125,36 @@ std::string XunitWriter::build_xml(Triplet controlling_triplet) const ElapsedTime elapsed_sum{}; for (auto&& port_result : port_results) { - elapsed_sum += port_result.time; + if (const auto* build = port_result.ci_result.build.get()) + { + elapsed_sum += build->timing; + } } const auto elapsed_seconds = fmt::format("{}", elapsed_sum.as().count()); - auto earliest_start_time = - std::min_element(port_results.begin(), port_results.end(), [](const XunitTest& lhs, const XunitTest& rhs) { - return lhs.start_time < rhs.start_time; - })->start_time; + std::chrono::system_clock::time_point earliest_start_time = std::chrono::system_clock::time_point::max(); + for (auto&& port_result : port_results) + { + if (const auto* build = port_result.ci_result.build.get()) + { + if (build->start_time < earliest_start_time) + { + earliest_start_time = build->start_time; + } + } + } + + time_t as_time_t; + if (earliest_start_time == std::chrono::system_clock::time_point::max()) + { + as_time_t = 0; + } + else + { + as_time_t = std::chrono::system_clock::to_time_t(earliest_start_time); + } - const auto as_time_t = std::chrono::system_clock::to_time_t(earliest_start_time); const auto as_tm = to_utc_time(as_time_t).value_or_exit(VCPKG_LINE_INFO); char run_date_time[80]; strftime(run_date_time, sizeof(run_date_time), "%Y-%m-%d%H:%M:%S", &as_tm); From 5fa7e82373d87c8696b24d74901ee5a919cf585b Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 5 Nov 2025 01:34:03 -0800 Subject: [PATCH 15/20] WIP every report mechanism working except msgCiBaselineUnexpectedFailCascade Also want to add some more test cases. --- azure-pipelines/end-to-end-tests-dir/ci.ps1 | 24 +-- include/vcpkg/base/message-data.inc.h | 19 ++ include/vcpkg/base/messages.h | 3 +- include/vcpkg/commands.build.h | 4 + include/vcpkg/commands.install.h | 3 + include/vcpkg/fwd/build.h | 4 + include/vcpkg/xunitwriter.h | 5 +- locales/messages.json | 8 + src/vcpkg/base/messages.cpp | 3 +- src/vcpkg/ci-baseline.cpp | 38 +++- src/vcpkg/commands.build.cpp | 20 +++ src/vcpkg/commands.ci.cpp | 188 +++++++++----------- src/vcpkg/commands.install.cpp | 56 +++--- src/vcpkg/commands.test-features.cpp | 6 +- src/vcpkg/xunitwriter.cpp | 179 +++++++++++-------- 15 files changed, 333 insertions(+), 227 deletions(-) diff --git a/azure-pipelines/end-to-end-tests-dir/ci.ps1 b/azure-pipelines/end-to-end-tests-dir/ci.ps1 index 495376fdd5..7e59727f24 100644 --- a/azure-pipelines/end-to-end-tests-dir/ci.ps1 +++ b/azure-pipelines/end-to-end-tests-dir/ci.ps1 @@ -5,16 +5,16 @@ $Output = Run-VcpkgAndCaptureOutput ci --dry-run --triplet=$Triplet --x-builtin- Throw-IfNotFailed $ErrorOutput = Run-VcpkgAndCaptureStdErr ci --dry-run --triplet=$Triplet --x-builtin-ports-root="$PSScriptRoot/../e2e-ports/ci" --binarysource=clear --ci-baseline="$PSScriptRoot/../e2e-assets/ci/ci.baseline.txt" Throw-IfNotFailed -if (-not ($Output.Contains("dep-on-feature-not-sup:${Triplet}: cascade"))) { +if (-not ($Output.Contains("dep-on-feature-not-sup:${Triplet}: cascade"))) { throw 'dep-on-feature-not-sup must cascade because it depends on a features that is not supported' } -if (-not ($Output.Contains("not-sup-host-b:${Triplet}: skip"))) { +if (-not ($Output.Contains("not-sup-host-b:${Triplet}: unsupported"))) { throw 'not-sup-host-b must be skipped because it is not supported' } -if (-not ($Output.Contains("feature-not-sup:${Triplet}: *"))) { +if (-not ($Output.Contains("feature-not-sup:${Triplet}: *:"))) { throw 'feature-not-sup must be built because the port that causes this port to skip should not be installed' } -if (-not ($Output.Contains("feature-dep-missing:${Triplet}: *"))) { +if (-not ($Output.Contains("feature-dep-missing:${Triplet}: *:"))) { throw 'feature-dep-missing must be built because the broken feature is not selected.' } if ($Output.Split("*").Length -ne 4) { @@ -23,9 +23,10 @@ if ($Output.Split("*").Length -ne 4) { if (-not ($ErrorOutput.Contains("REGRESSION: not-sup-host-b:${Triplet} is marked as fail but not supported for ${Triplet}."))) { throw "feature-not-sup's baseline fail entry should result in a regression because the port is not supported" } -if (-not ($ErrorOutput.Contains("REGRESSION: dep-on-feature-not-sup:${Triplet} is marked as fail but one dependency is not supported for ${Triplet}."))) { - throw "feature-not-sup's baseline fail entry should result in a regression because the port is cascade for this triplet" -} +# FIXME +# if (-not ($ErrorOutput.Contains("REGRESSION: dep-on-feature-not-sup:${Triplet} is marked as fail but one dependency is not supported for ${Triplet}."))) { +# throw "feature-not-sup's baseline fail entry should result in a regression because the port is cascade for this triplet" +# } # pass means pass $Output = Run-VcpkgAndCaptureOutput ci --dry-run --triplet=$Triplet --x-builtin-ports-root="$PSScriptRoot/../e2e-ports/ci" --binarysource=clear --ci-baseline="$PSScriptRoot/../e2e-assets/ci/ci.baseline.txt" @@ -35,9 +36,10 @@ Throw-IfNotFailed if (-not ($ErrorOutput.Contains("REGRESSION: not-sup-host-b:${Triplet} is marked as pass but not supported for ${Triplet}."))) { throw "feature-not-sup's baseline pass entry should result in a regression because the port is not supported" } -if (-not ($ErrorOutput.Contains("REGRESSION: dep-on-feature-not-sup:${Triplet} is marked as pass but one dependency is not supported for ${Triplet}."))) { - throw "feature-not-sup's baseline pass entry should result in a regression because the port is cascade for this triplet" -} +# FIXME +# if (-not ($ErrorOutput.Contains("REGRESSION: dep-on-feature-not-sup:${Triplet} is marked as pass but one dependency is not supported for ${Triplet}."))) { +# throw "feature-not-sup's baseline pass entry should result in a regression because the port is cascade for this triplet" +# } # any invalid manifest must raise an error $Output = Run-VcpkgAndCaptureOutput ci --dry-run --triplet=$Triplet --x-builtin-ports-root="$PSScriptRoot/../e2e-ports/broken-manifests" --binarysource=clear --ci-baseline="$PSScriptRoot/../e2e-assets/ci/ci.baseline.txt" @@ -93,7 +95,7 @@ Remove-Problem-Matchers Refresh-TestRoot $Output = Run-VcpkgAndCaptureOutput ci @commonArgs --x-builtin-ports-root="$PSScriptRoot/../e2e-assets/ci-skipped-features" --binarysource=clear --ci-baseline="$PSScriptRoot/../e2e-assets/ci-skipped-features/baseline.txt" Throw-IfFailed -if (-not ($Output -match 'skipped-features:[^:]+: \*' -and $Output -match 'Building skipped-features:[^@]+@1.0.0...')) { +if (-not ($Output -match 'skipped-features:[^:]+: \*:' -and $Output -match 'Building skipped-features:[^@]+@1\.0\.0\.\.\.')) { throw 'did not attempt to build skipped-features' } diff --git a/include/vcpkg/base/message-data.inc.h b/include/vcpkg/base/message-data.inc.h index 7ff0bff2b6..b3e0af0a46 100644 --- a/include/vcpkg/base/message-data.inc.h +++ b/include/vcpkg/base/message-data.inc.h @@ -384,6 +384,11 @@ DECLARE_MESSAGE(BuildResultBuildFailed, (), "Printed after the name of an installed entity to indicate that it failed to build.", "BUILD_FAILED") +DECLARE_MESSAGE(BuildResultCached, + (), + "Printed after the name of an installed entity to indicate that it was not installed because it " + "already existed in a binary cache.", + "CACHED") DECLARE_MESSAGE( BuildResultCacheMissing, (), @@ -405,6 +410,15 @@ DECLARE_MESSAGE(BuildResultExcluded, "Printed after the name of an installed entity to indicate that the user explicitly " "requested it not be installed.", "EXCLUDED") +DECLARE_MESSAGE(BuildResultExcludedByDryRun, + (), + "Printed after the name of an entity that would be installed, but is not due to --dry-run.", + "EXCLUDED_BY_DRY_RUN") +DECLARE_MESSAGE(BuildResultExcludedByParent, + (), + "Printed after the name of an installed entity to indicate that it isn't tested due to an ABI hash in " + "--parent-hashes.", + "EXCLUDED_BY_PARENT") DECLARE_MESSAGE( BuildResultFileConflicts, (), @@ -432,6 +446,11 @@ DECLARE_MESSAGE(BuildResultSummaryLine, (msg::build_result, msg::count), "Displayed to show a count of results of a build_result in a summary.", "{build_result}: {count}") +DECLARE_MESSAGE( + BuildResultUnsupported, + (), + "Printed after the name of an installed entity to indicate that it was not included due to a \"supports\" clause.", + "UNSUPPORTED") DECLARE_MESSAGE(BuildTreesRootDir, (), "", "Buildtrees directory (experimental)") DECLARE_MESSAGE(BuildTroubleshootingMessage1, (), diff --git a/include/vcpkg/base/messages.h b/include/vcpkg/base/messages.h index d6dfd8d402..23b82e3fe6 100644 --- a/include/vcpkg/base/messages.h +++ b/include/vcpkg/base/messages.h @@ -75,7 +75,8 @@ namespace vcpkg { LocalizedString() = default; operator StringView() const noexcept; - const std::string& data() const noexcept; + const std::string& data() const& noexcept; + std::string&& data() && noexcept; const std::string& to_string() const noexcept; std::string extract_data(); diff --git a/include/vcpkg/commands.build.h b/include/vcpkg/commands.build.h index 13df235c78..ea90d31c0b 100644 --- a/include/vcpkg/commands.build.h +++ b/include/vcpkg/commands.build.h @@ -115,7 +115,11 @@ namespace vcpkg int file_conflicts = 0; int cascaded_due_to_missing_dependencies = 0; int excluded = 0; + int excluded_by_parent = 0; + int excluded_by_dry_run = 0; + int unsupported = 0; int cache_missing = 0; + int cached = 0; int downloaded = 0; int removed = 0; diff --git a/include/vcpkg/commands.install.h b/include/vcpkg/commands.install.h index b3d2ce6c85..2bbfb5ee52 100644 --- a/include/vcpkg/commands.install.h +++ b/include/vcpkg/commands.install.h @@ -52,6 +52,9 @@ namespace vcpkg std::chrono::system_clock::time_point start_time; const InstallPlanAction* get_maybe_install_plan_action() const { return m_install_action; } + std::string to_string() const; + void to_string(std::string& out_str) const; + private: const InstallPlanAction* m_install_action; PackageSpec m_spec; diff --git a/include/vcpkg/fwd/build.h b/include/vcpkg/fwd/build.h index 085b2fed9e..8c8ebd6637 100644 --- a/include/vcpkg/fwd/build.h +++ b/include/vcpkg/fwd/build.h @@ -10,7 +10,11 @@ namespace vcpkg FileConflicts, CascadedDueToMissingDependencies, Excluded, + ExcludedByParent, + ExcludedByDryRun, + Unsupported, CacheMissing, + Cached, Downloaded, Removed }; diff --git a/include/vcpkg/xunitwriter.h b/include/vcpkg/xunitwriter.h index 64745438a4..dc9329acc6 100644 --- a/include/vcpkg/xunitwriter.h +++ b/include/vcpkg/xunitwriter.h @@ -23,8 +23,11 @@ namespace vcpkg struct CiResult { - BuildResult code; // FIXME describe why excluded + BuildResult code; Optional build; + + std::string to_string() const; + void to_string(std::string& out_str) const; }; struct XunitTest; diff --git a/locales/messages.json b/locales/messages.json index 99d8f9c4d1..93ea8cf46d 100644 --- a/locales/messages.json +++ b/locales/messages.json @@ -228,12 +228,18 @@ "_BuildResultBuildFailed.comment": "Printed after the name of an installed entity to indicate that it failed to build.", "BuildResultCacheMissing": "CACHE_MISSING", "_BuildResultCacheMissing.comment": "Printed after the name of an installed entity to indicate that it was not present in the binary cache when the user has requested that things may only be installed from the cache rather than built.", + "BuildResultCached": "CACHED", + "_BuildResultCached.comment": "Printed after the name of an installed entity to indicate that it was not installed because it already existed in a binary cache.", "BuildResultCascadeDueToMissingDependencies": "CASCADED_DUE_TO_MISSING_DEPENDENCIES", "_BuildResultCascadeDueToMissingDependencies.comment": "Printed after the name of an installed entity to indicate that it could not attempt to be installed because one of its transitive dependencies failed to install.", "BuildResultDownloaded": "DOWNLOADED", "_BuildResultDownloaded.comment": "Printed after the name of an installed entity to indicate that it was successfully downloaded but no build or install was requested.", "BuildResultExcluded": "EXCLUDED", "_BuildResultExcluded.comment": "Printed after the name of an installed entity to indicate that the user explicitly requested it not be installed.", + "BuildResultExcludedByDryRun": "EXCLUDED_BY_DRY_RUN", + "_BuildResultExcludedByDryRun.comment": "Printed after the name of an entity that would be installed, but is not due to --dry-run.", + "BuildResultExcludedByParent": "EXCLUDED_BY_PARENT", + "_BuildResultExcludedByParent.comment": "Printed after the name of an installed entity to indicate that it isn't tested due to an ABI hash in --parent-hashes.", "BuildResultFileConflicts": "FILE_CONFLICTS", "_BuildResultFileConflicts.comment": "Printed after the name of an installed entity to indicate that it conflicts with something already installed", "BuildResultPostBuildChecksFailed": "POST_BUILD_CHECKS_FAILED", @@ -246,6 +252,8 @@ "_BuildResultSummaryHeader.comment": "Displayed before a list of a summary installation results. An example of {triplet} is x64-windows.", "BuildResultSummaryLine": "{build_result}: {count}", "_BuildResultSummaryLine.comment": "Displayed to show a count of results of a build_result in a summary. An example of {build_result} is One of the BuildResultXxx messages (such as BuildResultSucceeded/SUCCEEDED). An example of {count} is 42.", + "BuildResultUnsupported": "UNSUPPORTED", + "_BuildResultUnsupported.comment": "Printed after the name of an installed entity to indicate that it was not included due to a \"supports\" clause.", "BuildTreesRootDir": "Buildtrees directory (experimental)", "BuildTroubleshootingMessage1": "Please ensure you're using the latest port files with `git pull` and `vcpkg update`.\nThen check for known issues at:", "_BuildTroubleshootingMessage1.comment": "First part of build troubleshooting message, printed before the URI to look for existing bugs.", diff --git a/src/vcpkg/base/messages.cpp b/src/vcpkg/base/messages.cpp index f2643a0ada..68355a62ca 100644 --- a/src/vcpkg/base/messages.cpp +++ b/src/vcpkg/base/messages.cpp @@ -14,7 +14,8 @@ using namespace vcpkg; namespace vcpkg { LocalizedString::operator StringView() const noexcept { return m_data; } - const std::string& LocalizedString::data() const noexcept { return m_data; } + const std::string& LocalizedString::data() const& noexcept { return m_data; } + std::string&& LocalizedString::data() && noexcept { return std::move(m_data); } const std::string& LocalizedString::to_string() const noexcept { return m_data; } std::string LocalizedString::extract_data() { return std::exchange(m_data, std::string{}); } diff --git a/src/vcpkg/ci-baseline.cpp b/src/vcpkg/ci-baseline.cpp index 750bc14d4b..f88e8e0d5e 100644 --- a/src/vcpkg/ci-baseline.cpp +++ b/src/vcpkg/ci-baseline.cpp @@ -192,8 +192,16 @@ namespace vcpkg const std::string* cifile, bool allow_unexpected_passing) { + // FIXME how to report msgCiBaselineUnexpectedFailCascade? switch (result) { + case BuildResult::Succeeded: + case BuildResult::Cached: + if (!allow_unexpected_passing && cidata.expected_failures.contains(spec)) + { + return msg::format(msgCiBaselineUnexpectedPass, msg::spec = spec, msg::path = *cifile); + } + break; case BuildResult::BuildFailed: case BuildResult::PostBuildChecksFailed: case BuildResult::FileConflicts: @@ -212,18 +220,36 @@ namespace vcpkg msg::build_result = to_string_locale_invariant(result)); } break; - case BuildResult::Succeeded: - if (!allow_unexpected_passing && cidata.expected_failures.contains(spec)) + case BuildResult::CascadedDueToMissingDependencies: + if (cidata.required_success.contains(spec)) { - return msg::format(msgCiBaselineUnexpectedPass, msg::spec = spec, msg::path = *cifile); + return msg::format(msgCiBaselineDisallowedCascade, msg::spec = spec, msg::path = *cifile); } break; - case BuildResult::CascadedDueToMissingDependencies: + case BuildResult::Unsupported: + if (cidata.expected_failures.contains(spec)) + { + return msg::format(msgCiBaselineUnexpectedFail, msg::spec = spec, msg::triplet = spec.triplet()); + } + else if (cidata.required_success.contains(spec)) + { + return msg::format( + msgCiBaselineUnexpectedPassUnsupported, msg::spec = spec, msg::triplet = spec.triplet()); + } + break; + case BuildResult::Excluded: if (cidata.required_success.contains(spec)) { - return msg::format(msgCiBaselineDisallowedCascade, msg::spec = spec, msg::path = *cifile); + return msg::format( + msgCiBaselineUnexpectedPassCascade, msg::spec = spec, msg::triplet = spec.triplet()); } - default: break; + break; + case BuildResult::ExcludedByParent: + case BuildResult::ExcludedByDryRun: break; + case BuildResult::CacheMissing: + case BuildResult::Downloaded: + case BuildResult::Removed: + default: Checks::unreachable(VCPKG_LINE_INFO); break; } return {}; } diff --git a/src/vcpkg/commands.build.cpp b/src/vcpkg/commands.build.cpp index 25fa62e104..1967dcbef9 100644 --- a/src/vcpkg/commands.build.cpp +++ b/src/vcpkg/commands.build.cpp @@ -332,6 +332,10 @@ namespace vcpkg return 1; } case BuildResult::Excluded: + case BuildResult::ExcludedByParent: + case BuildResult::ExcludedByDryRun: + case BuildResult::Unsupported: + case BuildResult::Cached: default: Checks::unreachable(VCPKG_LINE_INFO); } } @@ -1687,7 +1691,11 @@ namespace vcpkg case BuildResult::FileConflicts: ++file_conflicts; return; case BuildResult::CascadedDueToMissingDependencies: ++cascaded_due_to_missing_dependencies; return; case BuildResult::Excluded: ++excluded; return; + case BuildResult::ExcludedByParent: ++excluded_by_parent; return; + case BuildResult::ExcludedByDryRun: ++excluded_by_dry_run; return; + case BuildResult::Unsupported: ++unsupported; return; case BuildResult::CacheMissing: ++cache_missing; return; + case BuildResult::Cached: ++cached; return; case BuildResult::Downloaded: ++downloaded; return; case BuildResult::Removed: ++removed; return; default: Checks::unreachable(VCPKG_LINE_INFO); @@ -1718,7 +1726,11 @@ namespace vcpkg append_build_result_summary_line( msgBuildResultCascadeDueToMissingDependencies, cascaded_due_to_missing_dependencies, str); append_build_result_summary_line(msgBuildResultExcluded, excluded, str); + append_build_result_summary_line(msgBuildResultExcludedByParent, excluded_by_parent, str); + append_build_result_summary_line(msgBuildResultExcludedByDryRun, excluded_by_dry_run, str); + append_build_result_summary_line(msgBuildResultUnsupported, unsupported, str); append_build_result_summary_line(msgBuildResultCacheMissing, cache_missing, str); + append_build_result_summary_line(msgBuildResultCached, cached, str); append_build_result_summary_line(msgBuildResultDownloaded, downloaded, str); append_build_result_summary_line(msgBuildResultRemoved, removed, str); return str; @@ -1734,7 +1746,11 @@ namespace vcpkg case BuildResult::FileConflicts: return "FILE_CONFLICTS"; case BuildResult::CascadedDueToMissingDependencies: return "CASCADED_DUE_TO_MISSING_DEPENDENCIES"; case BuildResult::Excluded: return "EXCLUDED"; + case BuildResult::ExcludedByParent: return "EXCLUDED_BY_PARENT"; + case BuildResult::ExcludedByDryRun: return "EXCLUDED_BY_DRY_RUN"; + case BuildResult::Unsupported: return "UNSUPPORTED"; case BuildResult::CacheMissing: return "CACHE_MISSING"; + case BuildResult::Cached: return "CACHED"; case BuildResult::Downloaded: return "DOWNLOADED"; case BuildResult::Removed: return "REMOVED"; default: Checks::unreachable(VCPKG_LINE_INFO); @@ -1752,7 +1768,11 @@ namespace vcpkg case BuildResult::CascadedDueToMissingDependencies: return msg::format(msgBuildResultCascadeDueToMissingDependencies); case BuildResult::Excluded: return msg::format(msgBuildResultExcluded); + case BuildResult::ExcludedByParent: return msg::format(msgBuildResultExcludedByParent); + case BuildResult::ExcludedByDryRun: return msg::format(msgBuildResultExcludedByDryRun); + case BuildResult::Unsupported: return msg::format(msgBuildResultUnsupported); case BuildResult::CacheMissing: return msg::format(msgBuildResultCacheMissing); + case BuildResult::Cached: return msg::format(msgBuildResultCached); case BuildResult::Downloaded: return msg::format(msgBuildResultDownloaded); case BuildResult::Removed: return msg::format(msgBuildResultRemoved); default: Checks::unreachable(VCPKG_LINE_INFO); diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index 5eafdcad7f..267a6c2b9d 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -128,28 +128,31 @@ namespace missing_specs.erase(action.spec); // note action.spec won't be in missing_specs if it's a host dependency const std::string& public_abi = action.public_abi(); const StringLiteral* state; + BuildResult known_result; if (Util::Sets::contains(known_failure_abis, public_abi)) { state = &STATE_ABI_FAIL; - ret.known.emplace(action.spec, BuildResult::BuildFailed); + known_result = BuildResult::BuildFailed; } else if (precheck_results[action_idx] == CacheAvailability::available) { state = &STATE_CACHED; - ret.known.emplace(action.spec, BuildResult::Succeeded); + known_result = BuildResult::Cached; } else if (Util::Sets::contains(parent_hashes, public_abi)) { state = &STATE_PARENT; - ret.known.emplace(action.spec, BuildResult::Succeeded); + known_result = BuildResult::ExcludedByParent; } else { state = &STATE_UNKNOWN; + known_result = BuildResult::ExcludedByDryRun; } ret.report_lines.insert_or_assign(action.spec, fmt::format("{:>40}: {:>6}: {}", action.spec, *state, public_abi)); + ret.known.emplace(action.spec, known_result); Json::Object obj; obj.insert(JsonIdName, Json::Value::string(action.spec.name())); obj.insert(JsonIdTriplet, Json::Value::string(action.spec.triplet().canonical_name())); @@ -173,36 +176,49 @@ namespace for (const auto& exclusion : ci_specs.excluded) { const StringLiteral* state; + BuildResult known_result; switch (exclusion.second) { // it probably makes sense to distinguish between "--exclude", "=skip" and "=fail but --skip-failures" // but we don't preserve that information right now, so all these cases report as "skip" - case ExcludeReason::Baseline: state = &STATE_SKIP; break; - case ExcludeReason::Supports: state = &STATE_UNSUPPORTED; break; - case ExcludeReason::Cascade: state = &STATE_CASCADE; break; + case ExcludeReason::Baseline: + state = &STATE_SKIP; + known_result = BuildResult::Excluded; + break; + case ExcludeReason::Supports: + state = &STATE_UNSUPPORTED; + known_result = BuildResult::Unsupported; + break; + case ExcludeReason::Cascade: + state = &STATE_CASCADE; + known_result = BuildResult::CascadedDueToMissingDependencies; + break; default: Checks::unreachable(VCPKG_LINE_INFO); } ret.report_lines.insert_or_assign(exclusion.first, fmt::format("{:>40}: {}", exclusion.first, *state)); + ret.known.emplace(exclusion.first, known_result); } return ret; } // This algorithm reduces an action plan to only unknown actions and their dependencies - void prune_entirely_known_action_branches(ActionPlan& action_plan, - const std::map& known, - const std::unordered_set& parent_hashes) + void prune_entirely_known_action_branches(ActionPlan& action_plan, const std::map& known) { std::set to_keep; for (auto it = action_plan.install_actions.rbegin(); it != action_plan.install_actions.rend(); ++it) { auto it_known = known.find(it->spec); - const auto& abi = it->abi_info.value_or_exit(VCPKG_LINE_INFO).package_abi; - if (!Util::Sets::contains(parent_hashes, abi)) + if (it_known == known.end()) + { + Checks::unreachable(VCPKG_LINE_INFO); + } + + if (it_known->second != BuildResult::ExcludedByParent) { it->request_type = RequestType::USER_REQUESTED; - if (it_known == known.end()) + if (it_known->second == BuildResult::ExcludedByDryRun) { to_keep.insert(it->spec); } @@ -210,7 +226,8 @@ namespace if (Util::Sets::contains(to_keep, it->spec)) { - if (it_known != known.end() && it_known->second == BuildResult::Excluded) + if (it_known != known.end() && + (it_known->second == BuildResult::Excluded || it_known->second == BuildResult::Unsupported)) { it->plan_type = InstallPlanType::EXCLUDED; } @@ -241,13 +258,11 @@ namespace bool print_regressions(const std::map& ci_results, const CiBaselineData& baseline_data, const std::string* ci_baseline_file_name, - const LocalizedString& not_supported_regressions, bool allow_unexpected_passing) { - bool has_error = !not_supported_regressions.empty(); + bool has_error = false; LocalizedString output = msg::format(msgCiBaselineRegressionHeader); output.append_raw('\n'); - output.append(not_supported_regressions); for (auto&& ci_result : ci_results) { auto msg = format_ci_result( @@ -330,8 +345,8 @@ namespace result.excluded.insert_or_assign( std::move(full_package_spec.package_spec), supported_for_triplet(var_provider, *scfl->source_control_file, full_package_spec.package_spec) - ? ExcludeReason::Supports - : ExcludeReason::Cascade); + ? ExcludeReason::Cascade + : ExcludeReason::Supports); continue; } @@ -487,46 +502,7 @@ namespace vcpkg const auto precheck_results = binary_cache.precheck(install_actions); const auto pre_build_status = compute_pre_build_statuses(ci_specs, precheck_results, known_failure_abis, parent_hashes, action_plan); - LocalizedString not_supported_regressions; { - // std::string msg; - // for (const auto& spec : ci_specs.requested) - //{ - // if (!Util::Sets::contains(pre_build_status.abi_map, spec.package_spec)) - // { - // bool supp = supported_for_triplet(var_provider, provider, spec.package_spec); - // pre_build_status.known.emplace(spec.package_spec, - // supp ? BuildResult::CascadedDueToMissingDependencies - // : BuildResult::Excluded); - - // if (baseline_data.expected_failures.contains(spec.package_spec)) - // { - // not_supported_regressions // FIXME - // .append(supp ? msgCiBaselineUnexpectedFailCascade : msgCiBaselineUnexpectedFail, - // msg::spec = spec.package_spec, - // msg::triplet = spec.package_spec.triplet()) - // .append_raw('\n'); - // } - // else if (cidata.required_success.contains(spec.package_spec)) - // { - // not_supported_regressions - // .append(supp ? msgCiBaselineUnexpectedPassCascade : msgCiBaselineUnexpectedPassUnsupported, - // msg::spec = spec.package_spec, - // msg::triplet = spec.package_spec.triplet()) - // .append_raw('\n'); - // } - // msg += fmt::format("{:>40}: {:>8}\n", spec.package_spec, supp ? "cascade" : "skip"); - // } - //} - // for (size_t i = 0; i < action_plan.install_actions.size(); ++i) - //{ - // auto&& action = action_plan.install_actions[i]; - // msg += fmt::format("{:>40}: {:>8}: {}\n", - // action.spec, - // pre_build_status.action_state_string[i], - // action.abi_info.value_or_exit(VCPKG_LINE_INFO).package_abi); - //} - std::string msg; for (auto&& line : pre_build_status.report_lines) { @@ -542,20 +518,19 @@ namespace vcpkg fs.write_contents(output_hash_json, Json::stringify(pre_build_status.abis), VCPKG_LINE_INFO); } - prune_entirely_known_action_branches(action_plan, pre_build_status.known, parent_hashes); + prune_entirely_known_action_branches(action_plan, pre_build_status.known); msg::println(msgElapsedTimeForChecks, msg::elapsed = timer.elapsed()); + std::map ci_results; + std::map ci_full_results; + for (auto&& pre_known_outcome : pre_build_status.known) + { + ci_full_results.insert_or_assign(pre_known_outcome.first, CiResult{pre_known_outcome.second, nullopt}); + } if (is_dry_run) { print_plan(action_plan); - if (!not_supported_regressions.empty()) - { - msg::write_unlocalized_text_to_stderr( - Color::error, - msg::format(msgCiBaselineRegressionHeader).append_raw('\n').append_raw(not_supported_regressions)); - Checks::exit_fail(VCPKG_LINE_INFO); - } } else { @@ -580,18 +555,11 @@ namespace vcpkg args, paths, host_triplet, build_options, action_plan, status_db, binary_cache, *build_logs_recorder); msg::println(msgTotalInstallTime, msg::elapsed = summary.elapsed); - std::map ci_full_results; - for (auto&& pre_known_outcome : pre_build_status.known) - { - ci_full_results.insert_or_assign(pre_known_outcome.first, CiResult{pre_known_outcome.second, nullopt}); - } - - std::map ci_results; for (auto&& result : summary.results) { if (const auto* ipa = result.get_maybe_install_plan_action()) { - // note that we assign over the 'guessed' known values from above + // note that we assign over the 'known' values from above auto ci_result = CiResult{result.build_result.value_or_exit(VCPKG_LINE_INFO).code, CiBuiltResult{ipa->package_abi_or_exit(VCPKG_LINE_INFO), ipa->feature_list, @@ -602,38 +570,58 @@ namespace vcpkg } } - msg::print(LocalizedString::from_raw("\n") - .append(msgTripletLabel) - .append_raw(' ') - .append_raw(target_triplet) - .append_raw('\n') - .append(summary.format_results())); - const bool any_regressions = print_regressions(ci_full_results, - baseline_data, - ci_baseline_file_name, - not_supported_regressions, - allow_unexpected_passing); - - auto it_xunit = settings.find(SwitchXXUnit); - if (it_xunit != settings.end()) - { - XunitWriter xunitTestResults; - const auto& xunit_results = - Util::Sets::contains(options.switches, SwitchXXUnitAll) ? ci_full_results : ci_results; - for (auto&& xunit_result : xunit_results) - { - xunitTestResults.add_test_results(xunit_result.first, xunit_result.second); - } + binary_cache.wait_for_async_complete_and_join(); + msg::println(); + } - fs.write_contents(it_xunit->second, xunitTestResults.build_xml(target_triplet), VCPKG_LINE_INFO); - } + std::map summary_counts; + auto summary_report = msg::format(msgTripletLabel).data(); + summary_report.push_back(' '); + target_triplet.to_string(summary_report); + summary_report.push_back('\n'); + for (auto&& ci_result : ci_full_results) + { + summary_counts[ci_result.first.triplet()].increment(ci_result.second.code); + + summary_report.append(2, ' '); + ci_result.first.to_string(summary_report); + summary_report.append(": "); + ci_result.second.to_string(summary_report); + summary_report.push_back('\n'); + } + + for (auto&& entry : summary_counts) + { + summary_report.push_back('\n'); + summary_report.append(entry.second.format(entry.first).data()); + } + + summary_report.push_back('\n'); + msg::println(); + msg::print(LocalizedString::from_raw(std::move(summary_report))); + + const bool any_regressions = + print_regressions(ci_full_results, baseline_data, ci_baseline_file_name, allow_unexpected_passing); - if (any_regressions) + auto it_xunit = settings.find(SwitchXXUnit); + if (it_xunit != settings.end()) + { + XunitWriter xunitTestResults; + const auto& xunit_results = + Util::Sets::contains(options.switches, SwitchXXUnitAll) ? ci_full_results : ci_results; + for (auto&& xunit_result : xunit_results) { - Checks::exit_fail(VCPKG_LINE_INFO); + xunitTestResults.add_test_results(xunit_result.first, xunit_result.second); } + + fs.write_contents(it_xunit->second, xunitTestResults.build_xml(target_triplet), VCPKG_LINE_INFO); } - binary_cache.wait_for_async_complete_and_join(); + + if (any_regressions) + { + Checks::exit_fail(VCPKG_LINE_INFO); + } + Checks::exit_success(VCPKG_LINE_INFO); } } // namespace vcpkg diff --git a/src/vcpkg/commands.install.cpp b/src/vcpkg/commands.install.cpp index e09d1886ed..f9c7f7ef35 100644 --- a/src/vcpkg/commands.install.cpp +++ b/src/vcpkg/commands.install.cpp @@ -476,39 +476,27 @@ namespace vcpkg Checks::unreachable(VCPKG_LINE_INFO); } - static LocalizedString format_result_row(const SpecSummary& result) - { - return LocalizedString() - .append_indent() - .append_raw(result.get_spec().to_string()) - .append_raw(": ") - .append(to_string(result.build_result.value_or_exit(VCPKG_LINE_INFO).code)) - .append_raw(": ") - .append_raw(result.timing.to_string()); - } - LocalizedString InstallSummary::format_results() const { - LocalizedString to_print; - to_print.append(msgResultsHeader).append_raw('\n'); - - for (const SpecSummary& result : this->results) - { - to_print.append(format_result_row(result)).append_raw('\n'); - } - to_print.append_raw('\n'); - - std::map summary; + std::map summary_counts; + std::string to_print = msg::format(msgResultsHeader).extract_data(); + to_print.push_back('\n'); for (const SpecSummary& r : this->results) { - summary[r.get_spec().triplet()].increment(r.build_result.value_or_exit(VCPKG_LINE_INFO).code); + summary_counts[r.get_spec().triplet()].increment(r.build_result.value_or_exit(VCPKG_LINE_INFO).code); + + to_print.append(2, ' '); + r.to_string(to_print); + to_print.push_back('\n'); } - for (auto&& entry : summary) + to_print.push_back('\n'); + for (auto&& entry : summary_counts) { - to_print.append(entry.second.format(entry.first)); + to_print.append(entry.second.format(entry.first).data()); } - return to_print; + + return LocalizedString::from_raw(std::move(to_print)); } void InstallSummary::print_failed() const @@ -520,7 +508,7 @@ namespace vcpkg { if (result.build_result.value_or_exit(VCPKG_LINE_INFO).code != BuildResult::Succeeded) { - output.append(format_result_row(result)).append_raw('\n'); + output.append_raw(" ").append_raw(result.to_string()).append_raw('\n'); } } output.append_raw('\n'); @@ -705,11 +693,15 @@ namespace vcpkg case BuildResult::Succeeded: case BuildResult::Removed: case BuildResult::Downloaded: - case BuildResult::Excluded: break; + case BuildResult::Excluded: + case BuildResult::ExcludedByParent: + case BuildResult::ExcludedByDryRun: + case BuildResult::Cached: break; case BuildResult::BuildFailed: case BuildResult::PostBuildChecksFailed: case BuildResult::FileConflicts: case BuildResult::CascadedDueToMissingDependencies: + case BuildResult::Unsupported: case BuildResult::CacheMissing: summary.failed = true; break; default: Checks::unreachable(VCPKG_LINE_INFO); } @@ -1565,6 +1557,16 @@ namespace vcpkg return m_install_action && m_install_action->request_type == RequestType::USER_REQUESTED; } + std::string SpecSummary::to_string() const { return adapt_to_string(*this); } + void SpecSummary::to_string(std::string& out_str) const + { + m_spec.to_string(out_str); + out_str.append(": "); + out_str.append(vcpkg::to_string(build_result.value_or_exit(VCPKG_LINE_INFO).code).data()); + out_str.append(": "); + timing.to_string(out_str); + } + void track_install_plan(const ActionPlan& plan) { Cache triplet_hashes; diff --git a/src/vcpkg/commands.test-features.cpp b/src/vcpkg/commands.test-features.cpp index f27a222b58..c5bffcf9f1 100644 --- a/src/vcpkg/commands.test-features.cpp +++ b/src/vcpkg/commands.test-features.cpp @@ -907,7 +907,11 @@ namespace vcpkg handle_fail_feature_test_result(diagnostics, spec, ci_feature_baseline_file_name, baseline); break; case BuildResult::Removed: - case BuildResult::Excluded: Checks::unreachable(VCPKG_LINE_INFO); + case BuildResult::Excluded: + case BuildResult::ExcludedByParent: + case BuildResult::ExcludedByDryRun: + case BuildResult::Cached: + default: Checks::unreachable(VCPKG_LINE_INFO); } msg::println(); diff --git a/src/vcpkg/xunitwriter.cpp b/src/vcpkg/xunitwriter.cpp index 981d854fc1..4e9610f85e 100644 --- a/src/vcpkg/xunitwriter.cpp +++ b/src/vcpkg/xunitwriter.cpp @@ -4,19 +4,21 @@ #include #include -using namespace vcpkg; - -struct vcpkg::XunitTest +namespace vcpkg { - std::string name; - std::string method; - std::string owner; - CiResult ci_result; -}; + struct XunitTest + { + std::string name; + std::string method; + std::string owner; + CiResult ci_result; + }; +} namespace { - static void xml_test(XmlSerializer& xml, const XunitTest& test) + using namespace vcpkg; + void xml_test(XmlSerializer& xml, const XunitTest& test) { StringLiteral result_string = ""; switch (test.ci_result.code) @@ -26,7 +28,13 @@ namespace case BuildResult::PostBuildChecksFailed: case BuildResult::FileConflicts: result_string = "Fail"; break; case BuildResult::CascadedDueToMissingDependencies: - case BuildResult::Excluded: result_string = "Skip"; break; + case BuildResult::Excluded: + case BuildResult::ExcludedByParent: + case BuildResult::ExcludedByDryRun: + case BuildResult::Unsupported: + case BuildResult::Cached: // should this be "Pass" instead? + result_string = "Skip"; + break; case BuildResult::CacheMissing: case BuildResult::Downloaded: case BuildResult::Removed: @@ -92,96 +100,109 @@ namespace } xml.close_tag("test").line_break(); } +} // unnamed namespace -} - -XunitWriter::XunitWriter() = default; -XunitWriter::~XunitWriter() = default; - -void XunitWriter::add_test_results(const PackageSpec& spec, const CiResult& result) +namespace vcpkg { - const auto& name = spec.name(); - std::string method = name; - if (auto* built = result.build.get()) + std::string CiResult::to_string() const { return adapt_to_string(*this); } + void CiResult::to_string(std::string& out_str) const { - fmt::format_to(std::back_inserter(method), "[{}]", Strings::join(",", built->feature_list)); + out_str.append(vcpkg::to_string(code).data()); + if (auto b = build.get()) + { + out_str.append(": "); + b->timing.to_string(out_str); + } } - fmt::format_to(std::back_inserter(method), ":{}", spec.triplet()); + XunitWriter::XunitWriter() = default; + XunitWriter::~XunitWriter() = default; - m_tests[spec.name()].push_back({spec.to_string(), std::move(method), spec.triplet().to_string(), result}); -} - -std::string XunitWriter::build_xml(Triplet controlling_triplet) const -{ - XmlSerializer xml; - xml.emit_declaration(); - xml.open_tag("assemblies").line_break(); - for (const auto& test_group : m_tests) + void XunitWriter::add_test_results(const PackageSpec& spec, const CiResult& result) { - const auto& port_name = test_group.first; - const auto& port_results = test_group.second; + const auto& name = spec.name(); + std::string method = name; + if (auto* built = result.build.get()) + { + fmt::format_to(std::back_inserter(method), "[{}]", Strings::join(",", built->feature_list)); + } + + fmt::format_to(std::back_inserter(method), ":{}", spec.triplet()); - ElapsedTime elapsed_sum{}; - for (auto&& port_result : port_results) + m_tests[spec.name()].push_back({spec.to_string(), std::move(method), spec.triplet().to_string(), result}); + } + + std::string XunitWriter::build_xml(Triplet controlling_triplet) const + { + XmlSerializer xml; + xml.emit_declaration(); + xml.open_tag("assemblies").line_break(); + for (const auto& test_group : m_tests) { - if (const auto* build = port_result.ci_result.build.get()) + const auto& port_name = test_group.first; + const auto& port_results = test_group.second; + + ElapsedTime elapsed_sum{}; + for (auto&& port_result : port_results) { - elapsed_sum += build->timing; + if (const auto* build = port_result.ci_result.build.get()) + { + elapsed_sum += build->timing; + } } - } - const auto elapsed_seconds = fmt::format("{}", elapsed_sum.as().count()); + const auto elapsed_seconds = fmt::format("{}", elapsed_sum.as().count()); - std::chrono::system_clock::time_point earliest_start_time = std::chrono::system_clock::time_point::max(); - for (auto&& port_result : port_results) - { - if (const auto* build = port_result.ci_result.build.get()) + std::chrono::system_clock::time_point earliest_start_time = std::chrono::system_clock::time_point::max(); + for (auto&& port_result : port_results) { - if (build->start_time < earliest_start_time) + if (const auto* build = port_result.ci_result.build.get()) { - earliest_start_time = build->start_time; + if (build->start_time < earliest_start_time) + { + earliest_start_time = build->start_time; + } } } - } - time_t as_time_t; - if (earliest_start_time == std::chrono::system_clock::time_point::max()) - { - as_time_t = 0; - } - else - { - as_time_t = std::chrono::system_clock::to_time_t(earliest_start_time); - } + time_t as_time_t; + if (earliest_start_time == std::chrono::system_clock::time_point::max()) + { + as_time_t = 0; + } + else + { + as_time_t = std::chrono::system_clock::to_time_t(earliest_start_time); + } - const auto as_tm = to_utc_time(as_time_t).value_or_exit(VCPKG_LINE_INFO); - char run_date_time[80]; - strftime(run_date_time, sizeof(run_date_time), "%Y-%m-%d%H:%M:%S", &as_tm); + const auto as_tm = to_utc_time(as_time_t).value_or_exit(VCPKG_LINE_INFO); + char run_date_time[80]; + strftime(run_date_time, sizeof(run_date_time), "%Y-%m-%d%H:%M:%S", &as_tm); - StringView run_date{run_date_time, 10}; - StringView run_time{run_date_time + 10, 8}; + StringView run_date{run_date_time, 10}; + StringView run_time{run_date_time + 10, 8}; - xml.start_complex_open_tag("assembly") - .attr("name", port_name) - .attr("run-date", run_date) - .attr("run-time", run_time) - .attr("time", elapsed_seconds) - .finish_complex_open_tag() - .line_break(); - xml.start_complex_open_tag("collection") - .attr("name", controlling_triplet) - .attr("time", elapsed_seconds) - .finish_complex_open_tag() - .line_break(); - for (const auto& port_result : port_results) - { - xml_test(xml, port_result); + xml.start_complex_open_tag("assembly") + .attr("name", port_name) + .attr("run-date", run_date) + .attr("run-time", run_time) + .attr("time", elapsed_seconds) + .finish_complex_open_tag() + .line_break(); + xml.start_complex_open_tag("collection") + .attr("name", controlling_triplet) + .attr("time", elapsed_seconds) + .finish_complex_open_tag() + .line_break(); + for (const auto& port_result : port_results) + { + xml_test(xml, port_result); + } + xml.close_tag("collection").line_break(); + xml.close_tag("assembly").line_break(); } - xml.close_tag("collection").line_break(); - xml.close_tag("assembly").line_break(); - } - xml.close_tag("assemblies").line_break(); - return std::move(xml.buf); + xml.close_tag("assemblies").line_break(); + return std::move(xml.buf); + } } From 52c149c9582c167b8f7a63cfb6108b387277c779 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 5 Nov 2025 16:01:40 -0800 Subject: [PATCH 16/20] Fix remaining test failures, remove "all results" from the report at the end. --- azure-pipelines/end-to-end-tests-dir/ci.ps1 | 14 ++++++-------- src/vcpkg/ci-baseline.cpp | 17 ++++++++--------- src/vcpkg/commands.ci.cpp | 19 +++++++++++-------- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/azure-pipelines/end-to-end-tests-dir/ci.ps1 b/azure-pipelines/end-to-end-tests-dir/ci.ps1 index 7e59727f24..fdd0c36fe6 100644 --- a/azure-pipelines/end-to-end-tests-dir/ci.ps1 +++ b/azure-pipelines/end-to-end-tests-dir/ci.ps1 @@ -23,10 +23,9 @@ if ($Output.Split("*").Length -ne 4) { if (-not ($ErrorOutput.Contains("REGRESSION: not-sup-host-b:${Triplet} is marked as fail but not supported for ${Triplet}."))) { throw "feature-not-sup's baseline fail entry should result in a regression because the port is not supported" } -# FIXME -# if (-not ($ErrorOutput.Contains("REGRESSION: dep-on-feature-not-sup:${Triplet} is marked as fail but one dependency is not supported for ${Triplet}."))) { -# throw "feature-not-sup's baseline fail entry should result in a regression because the port is cascade for this triplet" -# } +if (-not ($ErrorOutput.Contains("REGRESSION: dep-on-feature-not-sup:${Triplet} is marked as fail but one dependency is not supported for ${Triplet}."))) { + throw "feature-not-sup's baseline fail entry should result in a regression because the port is cascade for this triplet" +} # pass means pass $Output = Run-VcpkgAndCaptureOutput ci --dry-run --triplet=$Triplet --x-builtin-ports-root="$PSScriptRoot/../e2e-ports/ci" --binarysource=clear --ci-baseline="$PSScriptRoot/../e2e-assets/ci/ci.baseline.txt" @@ -36,10 +35,9 @@ Throw-IfNotFailed if (-not ($ErrorOutput.Contains("REGRESSION: not-sup-host-b:${Triplet} is marked as pass but not supported for ${Triplet}."))) { throw "feature-not-sup's baseline pass entry should result in a regression because the port is not supported" } -# FIXME -# if (-not ($ErrorOutput.Contains("REGRESSION: dep-on-feature-not-sup:${Triplet} is marked as pass but one dependency is not supported for ${Triplet}."))) { -# throw "feature-not-sup's baseline pass entry should result in a regression because the port is cascade for this triplet" -# } +if (-not ($ErrorOutput.Contains("REGRESSION: dep-on-feature-not-sup:${Triplet} is marked as pass but one dependency is not supported for ${Triplet}."))) { + throw "feature-not-sup's baseline pass entry should result in a regression because the port is cascade for this triplet" +} # any invalid manifest must raise an error $Output = Run-VcpkgAndCaptureOutput ci --dry-run --triplet=$Triplet --x-builtin-ports-root="$PSScriptRoot/../e2e-ports/broken-manifests" --binarysource=clear --ci-baseline="$PSScriptRoot/../e2e-assets/ci/ci.baseline.txt" diff --git a/src/vcpkg/ci-baseline.cpp b/src/vcpkg/ci-baseline.cpp index f88e8e0d5e..041b802889 100644 --- a/src/vcpkg/ci-baseline.cpp +++ b/src/vcpkg/ci-baseline.cpp @@ -192,7 +192,6 @@ namespace vcpkg const std::string* cifile, bool allow_unexpected_passing) { - // FIXME how to report msgCiBaselineUnexpectedFailCascade? switch (result) { case BuildResult::Succeeded: @@ -221,9 +220,15 @@ namespace vcpkg } break; case BuildResult::CascadedDueToMissingDependencies: - if (cidata.required_success.contains(spec)) + if (cidata.expected_failures.contains(spec)) + { + return msg::format( + msgCiBaselineUnexpectedFailCascade, msg::spec = spec, msg::triplet = spec.triplet()); + } + else if (cidata.required_success.contains(spec)) { - return msg::format(msgCiBaselineDisallowedCascade, msg::spec = spec, msg::path = *cifile); + return msg::format( + msgCiBaselineUnexpectedPassCascade, msg::spec = spec, msg::triplet = spec.triplet()); } break; case BuildResult::Unsupported: @@ -238,12 +243,6 @@ namespace vcpkg } break; case BuildResult::Excluded: - if (cidata.required_success.contains(spec)) - { - return msg::format( - msgCiBaselineUnexpectedPassCascade, msg::spec = spec, msg::triplet = spec.triplet()); - } - break; case BuildResult::ExcludedByParent: case BuildResult::ExcludedByDryRun: break; case BuildResult::CacheMissing: diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index 267a6c2b9d..f67008a190 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -521,7 +521,7 @@ namespace vcpkg prune_entirely_known_action_branches(action_plan, pre_build_status.known); msg::println(msgElapsedTimeForChecks, msg::elapsed = timer.elapsed()); - std::map ci_results; + std::map ci_plan_results; std::map ci_full_results; for (auto&& pre_known_outcome : pre_build_status.known) { @@ -565,7 +565,7 @@ namespace vcpkg ipa->feature_list, result.start_time, result.timing}}; - ci_results.insert_or_assign(result.get_spec(), ci_result); + ci_plan_results.insert_or_assign(result.get_spec(), ci_result); ci_full_results.insert_or_assign(result.get_spec(), std::move(ci_result)); } } @@ -579,10 +579,8 @@ namespace vcpkg summary_report.push_back(' '); target_triplet.to_string(summary_report); summary_report.push_back('\n'); - for (auto&& ci_result : ci_full_results) + for (auto&& ci_result : ci_plan_results) { - summary_counts[ci_result.first.triplet()].increment(ci_result.second.code); - summary_report.append(2, ' '); ci_result.first.to_string(summary_report); summary_report.append(": "); @@ -590,10 +588,15 @@ namespace vcpkg summary_report.push_back('\n'); } - for (auto&& entry : summary_counts) + for (auto&& ci_result : ci_full_results) + { + summary_counts[ci_result.first.triplet()].increment(ci_result.second.code); + } + + for (auto&& summary_count : summary_counts) { summary_report.push_back('\n'); - summary_report.append(entry.second.format(entry.first).data()); + summary_report.append(summary_count.second.format(summary_count.first).data()); } summary_report.push_back('\n'); @@ -608,7 +611,7 @@ namespace vcpkg { XunitWriter xunitTestResults; const auto& xunit_results = - Util::Sets::contains(options.switches, SwitchXXUnitAll) ? ci_full_results : ci_results; + Util::Sets::contains(options.switches, SwitchXXUnitAll) ? ci_full_results : ci_plan_results; for (auto&& xunit_result : xunit_results) { xunitTestResults.add_test_results(xunit_result.first, xunit_result.second); From f3befa55e3bbba151164f3b407b2fadbf45cb473 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 5 Nov 2025 19:37:12 -0800 Subject: [PATCH 17/20] Test fixes. Note that CiBaselineUnexpectedPassCascade is deleted and we consider "real" cascades equivalent to statically known cascades in reporting. --- azure-pipelines/end-to-end-tests-dir/ci.ps1 | 2 +- include/vcpkg/base/message-data.inc.h | 4 ---- locales/messages.json | 2 -- src/vcpkg-test/ci-baseline.cpp | 3 ++- src/vcpkg/ci-baseline.cpp | 9 +++++---- 5 files changed, 8 insertions(+), 12 deletions(-) diff --git a/azure-pipelines/end-to-end-tests-dir/ci.ps1 b/azure-pipelines/end-to-end-tests-dir/ci.ps1 index fdd0c36fe6..c3e1a6fd8b 100644 --- a/azure-pipelines/end-to-end-tests-dir/ci.ps1 +++ b/azure-pipelines/end-to-end-tests-dir/ci.ps1 @@ -35,7 +35,7 @@ Throw-IfNotFailed if (-not ($ErrorOutput.Contains("REGRESSION: not-sup-host-b:${Triplet} is marked as pass but not supported for ${Triplet}."))) { throw "feature-not-sup's baseline pass entry should result in a regression because the port is not supported" } -if (-not ($ErrorOutput.Contains("REGRESSION: dep-on-feature-not-sup:${Triplet} is marked as pass but one dependency is not supported for ${Triplet}."))) { +if (-not ($ErrorOutput.Contains("REGRESSION: dep-on-feature-not-sup:${Triplet} cascaded, but it is required to pass. ("))) { throw "feature-not-sup's baseline pass entry should result in a regression because the port is cascade for this triplet" } diff --git a/include/vcpkg/base/message-data.inc.h b/include/vcpkg/base/message-data.inc.h index b3e0af0a46..e45a85e5ac 100644 --- a/include/vcpkg/base/message-data.inc.h +++ b/include/vcpkg/base/message-data.inc.h @@ -515,10 +515,6 @@ DECLARE_MESSAGE(CiBaselineUnexpectedPass, (msg::spec, msg::path), "", "PASSING, REMOVE FROM FAIL LIST: {spec} ({path}).") -DECLARE_MESSAGE(CiBaselineUnexpectedPassCascade, - (msg::spec, msg::triplet), - "", - "REGRESSION: {spec} is marked as pass but one dependency is not supported for {triplet}.") DECLARE_MESSAGE(CiBaselineUnexpectedPassUnsupported, (msg::spec, msg::triplet), "", diff --git a/locales/messages.json b/locales/messages.json index 93ea8cf46d..a18dc99a11 100644 --- a/locales/messages.json +++ b/locales/messages.json @@ -317,8 +317,6 @@ "_CiBaselineUnexpectedFailCascade.comment": "An example of {spec} is zlib:x64-windows. An example of {triplet} is x64-windows.", "CiBaselineUnexpectedPass": "PASSING, REMOVE FROM FAIL LIST: {spec} ({path}).", "_CiBaselineUnexpectedPass.comment": "An example of {spec} is zlib:x64-windows. An example of {path} is /foo/bar.", - "CiBaselineUnexpectedPassCascade": "REGRESSION: {spec} is marked as pass but one dependency is not supported for {triplet}.", - "_CiBaselineUnexpectedPassCascade.comment": "An example of {spec} is zlib:x64-windows. An example of {triplet} is x64-windows.", "CiBaselineUnexpectedPassUnsupported": "REGRESSION: {spec} is marked as pass but not supported for {triplet}.", "_CiBaselineUnexpectedPassUnsupported.comment": "An example of {spec} is zlib:x64-windows. An example of {triplet} is x64-windows.", "ClearingContents": "Clearing contents of {path}", diff --git a/src/vcpkg-test/ci-baseline.cpp b/src/vcpkg-test/ci-baseline.cpp index d52cd28b38..f275769910 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]") return format_ci_result(s, BuildResult::CascadedDueToMissingDependencies, cidata, &cifile, false); }; CHECK(test({"pass", Test::X64_UWP}) == fmt::format(cascademsg, "pass:x64-uwp")); - CHECK(test({"fail", Test::X64_UWP}) == ""); + CHECK(test({"fail", Test::X64_UWP}) == + "REGRESSION: fail:x64-uwp is marked as fail but one dependency is not supported for x64-uwp."); CHECK(test({"neither", Test::X64_UWP}) == ""); } } diff --git a/src/vcpkg/ci-baseline.cpp b/src/vcpkg/ci-baseline.cpp index 041b802889..024c708fa0 100644 --- a/src/vcpkg/ci-baseline.cpp +++ b/src/vcpkg/ci-baseline.cpp @@ -225,10 +225,10 @@ namespace vcpkg return msg::format( msgCiBaselineUnexpectedFailCascade, msg::spec = spec, msg::triplet = spec.triplet()); } - else if (cidata.required_success.contains(spec)) + + if (cidata.required_success.contains(spec)) { - return msg::format( - msgCiBaselineUnexpectedPassCascade, msg::spec = spec, msg::triplet = spec.triplet()); + return msg::format(msgCiBaselineDisallowedCascade, msg::spec = spec, msg::path = *cifile); } break; case BuildResult::Unsupported: @@ -236,7 +236,8 @@ namespace vcpkg { return msg::format(msgCiBaselineUnexpectedFail, msg::spec = spec, msg::triplet = spec.triplet()); } - else if (cidata.required_success.contains(spec)) + + if (cidata.required_success.contains(spec)) { return msg::format( msgCiBaselineUnexpectedPassUnsupported, msg::spec = spec, msg::triplet = spec.triplet()); From 4ca3651e2cd9a593376014d4aaa993d1adea856c Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Mon, 10 Nov 2025 12:33:44 -0800 Subject: [PATCH 18/20] Use Throw-IfNonContains and add assertions for counts --- azure-pipelines/end-to-end-tests-dir/ci.ps1 | 86 +++++++++++---------- 1 file changed, 46 insertions(+), 40 deletions(-) diff --git a/azure-pipelines/end-to-end-tests-dir/ci.ps1 b/azure-pipelines/end-to-end-tests-dir/ci.ps1 index c3e1a6fd8b..010c0fc772 100644 --- a/azure-pipelines/end-to-end-tests-dir/ci.ps1 +++ b/azure-pipelines/end-to-end-tests-dir/ci.ps1 @@ -5,42 +5,42 @@ $Output = Run-VcpkgAndCaptureOutput ci --dry-run --triplet=$Triplet --x-builtin- Throw-IfNotFailed $ErrorOutput = Run-VcpkgAndCaptureStdErr ci --dry-run --triplet=$Triplet --x-builtin-ports-root="$PSScriptRoot/../e2e-ports/ci" --binarysource=clear --ci-baseline="$PSScriptRoot/../e2e-assets/ci/ci.baseline.txt" Throw-IfNotFailed -if (-not ($Output.Contains("dep-on-feature-not-sup:${Triplet}: cascade"))) { - throw 'dep-on-feature-not-sup must cascade because it depends on a features that is not supported' -} -if (-not ($Output.Contains("not-sup-host-b:${Triplet}: unsupported"))) { - throw 'not-sup-host-b must be skipped because it is not supported' -} -if (-not ($Output.Contains("feature-not-sup:${Triplet}: *:"))) { - throw 'feature-not-sup must be built because the port that causes this port to skip should not be installed' -} -if (-not ($Output.Contains("feature-dep-missing:${Triplet}: *:"))) { - throw 'feature-dep-missing must be built because the broken feature is not selected.' -} +# dep-on-feature-not-sup must cascade because it depends on a features that is not supported +Throw-IfNonContains -Actual $Output -Expected "dep-on-feature-not-sup:${Triplet}: cascade" +# not-sup-host-b must be skipped because it is not supported +Throw-IfNonContains -Actual $Output -Expected "not-sup-host-b:${Triplet}: unsupported" +# feature-not-sup must be built because the port that causes this port to skip should not be installed +Throw-IfNonContains -Actual $Output -Expected "feature-not-sup:${Triplet}: *:" +# feature-dep-missing must be built because the broken feature is not selected. +Throw-IfNonContains -Actual $Output -Expected "feature-dep-missing:${Triplet}: *:" if ($Output.Split("*").Length -ne 4) { throw 'base-port should not be installed for the host' } -if (-not ($ErrorOutput.Contains("REGRESSION: not-sup-host-b:${Triplet} is marked as fail but not supported for ${Triplet}."))) { - throw "feature-not-sup's baseline fail entry should result in a regression because the port is not supported" -} -if (-not ($ErrorOutput.Contains("REGRESSION: dep-on-feature-not-sup:${Triplet} is marked as fail but one dependency is not supported for ${Triplet}."))) { - throw "feature-not-sup's baseline fail entry should result in a regression because the port is cascade for this triplet" -} +Throw-IfNonContains -Actual $Output -Expected @" +SUMMARY FOR $Triplet + CASCADED_DUE_TO_MISSING_DEPENDENCIES: 1 + EXCLUDED_BY_DRY_RUN: 3 + UNSUPPORTED: 1 +"@ +# feature-not-sup's baseline fail entry should result in a regression because the port is not supported +Throw-IfNonContains -Actual $ErrorOutput -Expected "REGRESSION: not-sup-host-b:${Triplet} is marked as fail but not supported for ${Triplet}." +# feature-not-sup's baseline fail entry should result in a regression because the port is cascade for this triplet +Throw-IfNonContains -Actual $ErrorOutput -Expected "REGRESSION: dep-on-feature-not-sup:${Triplet} is marked as fail but one dependency is not supported for ${Triplet}." # pass means pass -$Output = Run-VcpkgAndCaptureOutput ci --dry-run --triplet=$Triplet --x-builtin-ports-root="$PSScriptRoot/../e2e-ports/ci" --binarysource=clear --ci-baseline="$PSScriptRoot/../e2e-assets/ci/ci.baseline.txt" +Run-Vcpkg ci --dry-run --triplet=$Triplet --x-builtin-ports-root="$PSScriptRoot/../e2e-ports/ci" --binarysource=clear --ci-baseline="$PSScriptRoot/../e2e-assets/ci/ci.baseline.txt" Throw-IfNotFailed $ErrorOutput = Run-VcpkgAndCaptureStdErr ci --dry-run --triplet=$Triplet --x-builtin-ports-root="$PSScriptRoot/../e2e-ports/ci" --binarysource=clear --ci-baseline="$PSScriptRoot/../e2e-assets/ci/ci.pass.baseline.txt" Throw-IfNotFailed -if (-not ($ErrorOutput.Contains("REGRESSION: not-sup-host-b:${Triplet} is marked as pass but not supported for ${Triplet}."))) { - throw "feature-not-sup's baseline pass entry should result in a regression because the port is not supported" -} -if (-not ($ErrorOutput.Contains("REGRESSION: dep-on-feature-not-sup:${Triplet} cascaded, but it is required to pass. ("))) { - throw "feature-not-sup's baseline pass entry should result in a regression because the port is cascade for this triplet" -} +# feature-not-sup's baseline pass entry should result in a regression because the port is not supported +Throw-IfNonContains -Actual $ErrorOutput -Expected "REGRESSION: not-sup-host-b:${Triplet} is marked as pass but not supported for ${Triplet}." +# feature-not-sup's baseline pass entry should result in a regression because the port is cascade for this triplet +Throw-IfNonContains -Actual $ErrorOutput -Expected "REGRESSION: dep-on-feature-not-sup:${Triplet} cascaded, but it is required to pass. (" # any invalid manifest must raise an error -$Output = Run-VcpkgAndCaptureOutput ci --dry-run --triplet=$Triplet --x-builtin-ports-root="$PSScriptRoot/../e2e-ports/broken-manifests" --binarysource=clear --ci-baseline="$PSScriptRoot/../e2e-assets/ci/ci.baseline.txt" +Remove-Problem-Matchers +Run-Vcpkg ci --dry-run --triplet=$Triplet --x-builtin-ports-root="$PSScriptRoot/../e2e-ports/broken-manifests" --binarysource=clear --ci-baseline="$PSScriptRoot/../e2e-assets/ci/ci.baseline.txt" +Restore-Problem-Matchers Throw-IfNotFailed # test malformed individual overlay port manifest @@ -48,26 +48,24 @@ Remove-Problem-Matchers $Output = Run-VcpkgAndCaptureOutput ci --dry-run --triplet=$Triplet --x-builtin-ports-root="$PSScriptRoot/../e2e-ports/ci" --binarysource=clear --ci-baseline="$PSScriptRoot/../e2e-assets/ci/ci.baseline.txt" --overlay-ports="$PSScriptRoot/../e2e-ports/broken-manifests/malformed" Restore-Problem-Matchers Throw-IfNotFailed -if (-not ($Output.Contains("vcpkg.json:3:17: error: Trailing comma"))) { - throw 'malformed port manifest must raise a parsing error' -} +# malformed port manifest must raise a parsing error +Throw-IfNonContains -Actual $Output -Expected "vcpkg.json:3:17: error: Trailing comma" # test malformed overlay port manifests Remove-Problem-Matchers $Output = Run-VcpkgAndCaptureOutput ci --dry-run --triplet=$Triplet --x-builtin-ports-root="$PSScriptRoot/../e2e-ports/ci" --binarysource=clear --ci-baseline="$PSScriptRoot/../e2e-assets/ci/ci.baseline.txt" --overlay-ports="$PSScriptRoot/../e2e-ports/broken-manifests" Restore-Problem-Matchers Throw-IfNotFailed -if (-not ($Output.Contains("vcpkg.json:3:17: error: Trailing comma"))) { - throw 'malformed overlay port manifest must raise a parsing error' -} +# malformed overlay manifest must raise a parsing error +Throw-IfNonContains -Actual $Output -Expected "vcpkg.json:3:17: error: Trailing comma" # test that file conflicts are detected Remove-Problem-Matchers $emptyDir = "$TestingRoot/empty" New-Item -ItemType Directory -Path $emptyDir -Force | Out-Null $Output = Run-VcpkgAndCaptureOutput ci @commonArgs --x-builtin-ports-root="$emptyDir" --binarysource=clear --overlay-ports="$PSScriptRoot/../e2e-ports/duplicate-file-a" --overlay-ports="$PSScriptRoot/../e2e-ports/duplicate-file-b" -Throw-IfNotFailed Restore-Problem-Matchers +Throw-IfNotFailed # test effect of parent hashes Remove-Item -Recurse -Force $installRoot -ErrorAction SilentlyContinue @@ -84,18 +82,26 @@ Remove-Item -Recurse -Force $installRoot -ErrorAction SilentlyContinue New-Item -ItemType Directory -Path $installRoot -Force | Out-Null $Output = Run-VcpkgAndCaptureOutput ci @commonArgs --x-builtin-ports-root="$PSScriptRoot/../e2e-ports/ci" --binarysource="clear;files,$ArchiveRoot" --parent-hashes="$TestingRoot/parent-hashes.json" Throw-IfFailed -if ($Output.Contains("base-port:${Triplet}: SUCCEEDED:")) { - throw 'base-port must not be rebuilt again' -} +# base-port must not be rebuilt again +Throw-IfNonContains -Actual $Output -Expected "base-port:${Triplet}: parent: " +Throw-IfNonContains -Actual $Output -Expected @" +SUMMARY FOR $Triplet + CASCADED_DUE_TO_MISSING_DEPENDENCIES: 1 + EXCLUDED_BY_PARENT: 3 + UNSUPPORTED: 1 +"@ # test that features included only by skipped ports are not included -Remove-Problem-Matchers Refresh-TestRoot +Remove-Problem-Matchers $Output = Run-VcpkgAndCaptureOutput ci @commonArgs --x-builtin-ports-root="$PSScriptRoot/../e2e-assets/ci-skipped-features" --binarysource=clear --ci-baseline="$PSScriptRoot/../e2e-assets/ci-skipped-features/baseline.txt" +Restore-Problem-Matchers Throw-IfFailed if (-not ($Output -match 'skipped-features:[^:]+: \*:' -and $Output -match 'Building skipped-features:[^@]+@1\.0\.0\.\.\.')) { throw 'did not attempt to build skipped-features' } - -Restore-Problem-Matchers - +Throw-IfNonContains -Actual $Output -Expected @" +SUMMARY FOR $Triplet + SUCCEEDED: 1 + EXCLUDED: 1 +"@ From 788d96dbaf6b7c4d62901d7ee4f18e95a1c99de7 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Mon, 10 Nov 2025 13:04:46 -0800 Subject: [PATCH 19/20] Add xunit test. --- azure-pipelines/end-to-end-tests-dir/ci.ps1 | 36 +++++++++++++++++++- azure-pipelines/end-to-end-tests-prelude.ps1 | 32 ++++++++--------- 2 files changed, 51 insertions(+), 17 deletions(-) diff --git a/azure-pipelines/end-to-end-tests-dir/ci.ps1 b/azure-pipelines/end-to-end-tests-dir/ci.ps1 index b7e74d90b8..a6703b0dc6 100644 --- a/azure-pipelines/end-to-end-tests-dir/ci.ps1 +++ b/azure-pipelines/end-to-end-tests-dir/ci.ps1 @@ -94,9 +94,10 @@ SUMMARY FOR $Triplet "@ # test that features included only by skipped ports are not included +$xunitFile = Join-Path $TestingRoot 'xunit.xml' Refresh-TestRoot Remove-Problem-Matchers -$Output = Run-VcpkgAndCaptureOutput ci @commonArgs --x-builtin-ports-root="$PSScriptRoot/../e2e-assets/ci-skipped-features" --binarysource=clear --ci-baseline="$PSScriptRoot/../e2e-assets/ci-skipped-features/baseline.txt" +$Output = Run-VcpkgAndCaptureOutput ci @commonArgs --x-builtin-ports-root="$PSScriptRoot/../e2e-assets/ci-skipped-features" --binarysource=clear --ci-baseline="$PSScriptRoot/../e2e-assets/ci-skipped-features/baseline.txt" --x-xunit-all --x-xunit="$xunitFile" Restore-Problem-Matchers Throw-IfFailed if (-not ($Output -match 'skipped-features:[^:]+: \*:' -and $Output -match 'Building skipped-features:[^@]+@1\.0\.0\.\.\.')) { @@ -107,3 +108,36 @@ SUMMARY FOR $Triplet SUCCEEDED: 1 EXCLUDED: 1 "@ + +$xunitContent = Get-Content $xunitFile -Raw +$expected = @" +<\?xml version="1\.0" encoding="utf-8"\?> + + + + + + + + + + + + + + + + + + + + + + + +"@ + +if (-not ($xunitContent -match $expected)) { + Write-Diff -Actual $xunitContent -Expected $expected + throw 'xUnit output did not match expected output' +} diff --git a/azure-pipelines/end-to-end-tests-prelude.ps1 b/azure-pipelines/end-to-end-tests-prelude.ps1 index e227ecad57..569eadec34 100644 --- a/azure-pipelines/end-to-end-tests-prelude.ps1 +++ b/azure-pipelines/end-to-end-tests-prelude.ps1 @@ -234,16 +234,25 @@ function Set-EmptyTestPort { git -C $PortsRoot status } +function Write-Diff { + Param( + [string]$Actual, + [string]$Expected + ) + + Set-Content -Value $Expected -LiteralPath "$TestingRoot/expected.txt" + Set-Content -Value $Actual -LiteralPath "$TestingRoot/actual.txt" + git diff --no-index -- "$TestingRoot/expected.txt" "$TestingRoot/actual.txt" + Write-Stack +} + function Throw-IfNonEqual { Param( [string]$Actual, [string]$Expected ) if ($Actual -ne $Expected) { - Set-Content -Value $Expected -LiteralPath "$TestingRoot/expected.txt" - Set-Content -Value $Actual -LiteralPath "$TestingRoot/actual.txt" - git diff --no-index -- "$TestingRoot/expected.txt" "$TestingRoot/actual.txt" - Write-Stack + Write-Diff -Actual $Actual -Expected $Expected throw "Expected '$Expected' but got '$Actual'" } } @@ -261,10 +270,7 @@ function Throw-IfNonEndsWith { } if ($actualSuffix -ne $Expected) { - Set-Content -Value $Expected -LiteralPath "$TestingRoot/expected.txt" - Set-Content -Value $Actual -LiteralPath "$TestingRoot/actual.txt" - git diff --no-index -- "$TestingRoot/expected.txt" "$TestingRoot/actual.txt" - Write-Stack + Write-Diff -Actual $Actual -Expected $Expected throw "Expected '$Expected' but got '$actualSuffix'" } } @@ -275,10 +281,7 @@ function Throw-IfContains { [string]$Expected ) if ($Actual.Contains($Expected)) { - Set-Content -Value $Expected -LiteralPath "$TestingRoot/expected.txt" - Set-Content -Value $Actual -LiteralPath "$TestingRoot/actual.txt" - git diff --no-index -- "$TestingRoot/expected.txt" "$TestingRoot/actual.txt" - Write-Stack + Write-Diff -Actual $Actual -Expected $Expected throw "Expected '$Expected' to not be in '$Actual'" } } @@ -289,10 +292,7 @@ function Throw-IfNonContains { [string]$Expected ) if (-not ($Actual.Contains($Expected))) { - Set-Content -Value $Expected -LiteralPath "$TestingRoot/expected.txt" - Set-Content -Value $Actual -LiteralPath "$TestingRoot/actual.txt" - git diff --no-index -- "$TestingRoot/expected.txt" "$TestingRoot/actual.txt" - Write-Stack + Write-Diff -Actual $Actual -Expected $Expected throw "Expected '$Expected' to be in '$Actual'" } } From 29ca4023622bd0230a6d13addddbdfefa7916a63 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 12 Nov 2025 14:43:01 -0800 Subject: [PATCH 20/20] Code review feedback from @ras0219-msft --- include/vcpkg/commands.build.h | 2 +- src/vcpkg/commands.build.cpp | 2 +- src/vcpkg/commands.ci.cpp | 72 +++++++++++++++++----------------- src/vcpkg/xunitwriter.cpp | 4 +- 4 files changed, 39 insertions(+), 41 deletions(-) diff --git a/include/vcpkg/commands.build.h b/include/vcpkg/commands.build.h index 8ab7b9d2ea..22afd9aec4 100644 --- a/include/vcpkg/commands.build.h +++ b/include/vcpkg/commands.build.h @@ -123,7 +123,7 @@ namespace vcpkg int downloaded = 0; int removed = 0; - void increment(const BuildResult build_result); + void increment(BuildResult build_result); LocalizedString format(const Triplet& triplet) const; }; diff --git a/src/vcpkg/commands.build.cpp b/src/vcpkg/commands.build.cpp index f34138f6b8..c7f7b2099b 100644 --- a/src/vcpkg/commands.build.cpp +++ b/src/vcpkg/commands.build.cpp @@ -1699,7 +1699,7 @@ namespace vcpkg return result; } - void BuildResultCounts::increment(const BuildResult build_result) + void BuildResultCounts::increment(BuildResult build_result) { switch (build_result) { diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index f67008a190..8c94c375bf 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -226,8 +226,7 @@ namespace if (Util::Sets::contains(to_keep, it->spec)) { - if (it_known != known.end() && - (it_known->second == BuildResult::Excluded || it_known->second == BuildResult::Unsupported)) + if (it_known->second == BuildResult::Excluded || it_known->second == BuildResult::Unsupported) { it->plan_type = InstallPlanType::EXCLUDED; } @@ -282,7 +281,7 @@ namespace return has_error; } - std::vector calculate_packages_with_qualified_deps( + std::vector calculate_packages_with_qualifiers( const std::vector& all_control_files, const Triplet& target_triplet) { std::vector ret; @@ -309,21 +308,16 @@ namespace // it is too late as we have already calculated an action plan with feature dependencies from // the skipped ports. CiSpecsResult result; - const TripletExclusions* target_triplet_exclusions = nullptr; - for (auto&& te : exclusions_map.triplets) - { - if (te.triplet == target_triplet) - { - target_triplet_exclusions = &te; - break; - } - } - + auto it = Util::find_if(exclusions_map.triplets, [&](const TripletExclusions& exclusions) { + return exclusions.triplet == target_triplet; + }); + const SortedVector* const target_triplet_exclusions = + it == exclusions_map.triplets.end() ? nullptr : &it->exclusions; auto all_control_files = provider.load_all_control_files(); // populate `var_provider` to evaluate supports expressions for all ports: std::vector packages_with_qualified_deps = - calculate_packages_with_qualified_deps(all_control_files, target_triplet); + calculate_packages_with_qualifiers(all_control_files, target_triplet); var_provider.load_dep_info_vars(packages_with_qualified_deps, serialize_options.host_triplet); for (auto scfl : all_control_files) @@ -331,7 +325,7 @@ namespace auto full_package_spec = FullPackageSpec{PackageSpec{scfl->to_name(), target_triplet}, InternalFeatureSet{FeatureNameCore.to_string(), FeatureNameDefault.to_string()}}; - if (target_triplet_exclusions && target_triplet_exclusions->exclusions.contains(scfl->to_name())) + if (target_triplet_exclusions && target_triplet_exclusions->contains(scfl->to_name())) { result.excluded.insert_or_assign(std::move(full_package_spec.package_spec), ExcludeReason::Baseline); continue; @@ -367,6 +361,30 @@ namespace std::random_device e; }; + + std::unordered_set parse_parent_hashes( + const std::map>& settings, const VcpkgPaths& paths) + { + std::unordered_set parent_hashes; + const auto& fs = paths.get_filesystem(); + auto it_parent_hashes = settings.find(SwitchParentHashes); + if (it_parent_hashes != settings.end()) + { + const Path parent_hashes_path = paths.original_cwd / it_parent_hashes->second; + auto parent_hashes_text = fs.try_read_contents(parent_hashes_path).value_or_exit(VCPKG_LINE_INFO); + const auto parsed_object = + Json::parse(parent_hashes_text.content, parent_hashes_text.origin).value_or_exit(VCPKG_LINE_INFO); + const auto& parent_hashes_array = parsed_object.value.array(VCPKG_LINE_INFO); + for (const Json::Value& array_value : parent_hashes_array) + { + auto abi = array_value.object(VCPKG_LINE_INFO).get(JsonIdAbi); + Checks::check_exit(VCPKG_LINE_INFO, abi); + parent_hashes.insert(abi->string(VCPKG_LINE_INFO).to_string()); + } + } + + return parent_hashes; + } } // unnamed namespace namespace vcpkg @@ -439,25 +457,8 @@ namespace vcpkg known_failure_abis.insert(std::make_move_iterator(lines.begin()), std::make_move_iterator(lines.end())); } - std::unordered_set parent_hashes; - auto it_parent_hashes = settings.find(SwitchParentHashes); - if (it_parent_hashes != settings.end()) - { - const Path parent_hashes_path = paths.original_cwd / it_parent_hashes->second; - auto parent_hashes_text = fs.try_read_contents(parent_hashes_path).value_or_exit(VCPKG_LINE_INFO); - const auto parsed_object = - Json::parse(parent_hashes_text.content, parent_hashes_text.origin).value_or_exit(VCPKG_LINE_INFO); - const auto& parent_hashes_array = parsed_object.value.array(VCPKG_LINE_INFO); - for (const Json::Value& array_value : parent_hashes_array) - { - auto abi = array_value.object(VCPKG_LINE_INFO).get(JsonIdAbi); - Checks::check_exit(VCPKG_LINE_INFO, abi); - parent_hashes.insert(abi->string(VCPKG_LINE_INFO).to_string()); - } - } - + const std::unordered_set parent_hashes = parse_parent_hashes(settings, paths); const auto is_dry_run = Util::Sets::contains(options.switches, SwitchDryRun); - const IBuildLogsRecorder* build_logs_recorder = &null_build_logs_recorder; Optional build_logs_recorder_storage; { @@ -569,11 +570,10 @@ namespace vcpkg ci_full_results.insert_or_assign(result.get_spec(), std::move(ci_result)); } } - - binary_cache.wait_for_async_complete_and_join(); - msg::println(); } + binary_cache.wait_for_async_complete_and_join(); + msg::println(); std::map summary_counts; auto summary_report = msg::format(msgTripletLabel).data(); summary_report.push_back(' '); diff --git a/src/vcpkg/xunitwriter.cpp b/src/vcpkg/xunitwriter.cpp index 4e9610f85e..adaa308be6 100644 --- a/src/vcpkg/xunitwriter.cpp +++ b/src/vcpkg/xunitwriter.cpp @@ -32,9 +32,7 @@ namespace case BuildResult::ExcludedByParent: case BuildResult::ExcludedByDryRun: case BuildResult::Unsupported: - case BuildResult::Cached: // should this be "Pass" instead? - result_string = "Skip"; - break; + case BuildResult::Cached: result_string = "Skip"; break; case BuildResult::CacheMissing: case BuildResult::Downloaded: case BuildResult::Removed: