Ci: Detect not needed entries in ci.baseline.txt

[autoantwort]

Fixes

https://discord.com/channels/400588936151433218/687365466422902841/1121509279992651857

[autoantwort]

Test run can be found here: https://dev.azure.com/vcpkg/public/_build/results?buildId=91185

[BillyONeal]

Oh lol this isn't what I meant with my statement but this is an improvement :)

[autoantwort]

Ah did you mean that we should try to build the port even if the supports expressions states that a configuration is unsupported?

[BillyONeal]

Ah did you mean that we should try to build the port even if the supports expressions states that a configuration is unsupported?

Yeah that's what I meant. But like I said this is still good. 👍

[BillyONeal]

Upon further reflection, I don't think this behavior is correct. I think the correct behavior is "if something is marked fail in ci.baseline.txt and also excluded by a supports expression, emit an error", but this is adding "... and was excluded in the build" which seems inappropriate.

For example, it should be possible to emit that message from cascaded due to missing dependencies entries too.

I think you want something like: BillyONeal@0b635aa

diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp
index 9d3db453..90bc6e1f 100644
--- a/src/vcpkg/commands.ci.cpp
+++ b/src/vcpkg/commands.ci.cpp
@@ -196,7 +196,8 @@ namespace vcpkg::Commands::CI
         ExclusionPredicate is_excluded,
         const CMakeVars::CMakeVarProvider& var_provider,
         const std::vector<CacheAvailability>& precheck_results,
-        const ActionPlan& action_plan)
+        const ActionPlan& action_plan,
+        const View<CiBaselineLine> baseline_lines)
     {
         auto ret = std::make_unique<UnknownCIPortsResults>();
 
@@ -225,6 +226,13 @@ namespace vcpkg::Commands::CI
                 ret->action_state_string.emplace_back("n/a");
                 ret->known.emplace(p->spec, BuildResult::EXCLUDED);
                 will_fail.emplace(p->spec);
+                if (Util::any_of(baseline_lines, [&](const CiBaselineLine& baseline_line) {
+                        return baseline_line.port_name == action.spec.name() &&
+                               baseline_line.triplet == action.spec.triplet();
+                    }))
+                {
+                    // record error?
+                }
             }
             else if (Util::any_of(p->package_dependencies,
                                   [&](const PackageSpec& spec) { return Util::Sets::contains(will_fail, spec); }))
@@ -358,6 +366,7 @@ namespace vcpkg::Commands::CI
         auto baseline_iter = settings.find(OPTION_CI_BASELINE);
         const bool allow_unexpected_passing = Util::Sets::contains(options.switches, OPTION_ALLOW_UNEXPECTED_PASSING);
         CiBaselineData cidata;
+        std::vector<CiBaselineLine> baseline_lines;
         if (baseline_iter == settings.end())
         {
             if (allow_unexpected_passing)
@@ -373,9 +382,9 @@ namespace vcpkg::Commands::CI
             const auto ci_baseline_file_contents =
                 paths.get_filesystem().read_contents(ci_baseline_file_name, VCPKG_LINE_INFO);
             ParseMessages ci_parse_messages;
-            const auto lines = parse_ci_baseline(ci_baseline_file_contents, ci_baseline_file_name, ci_parse_messages);
+            baseline_lines = parse_ci_baseline(ci_baseline_file_contents, ci_baseline_file_name, ci_parse_messages);
             ci_parse_messages.exit_if_errors_or_warnings(ci_baseline_file_name);
-            cidata = parse_and_apply_ci_baseline(lines, exclusions_map, skip_failures);
+            cidata = parse_and_apply_ci_baseline(baseline_lines, exclusions_map, skip_failures);
         }
 
         const auto is_dry_run = Util::Sets::contains(options.switches, OPTION_DRY_RUN);
@@ -434,8 +443,8 @@ namespace vcpkg::Commands::CI
         auto action_plan = compute_full_plan(paths, provider, var_provider, all_default_full_specs, serialize_options);
         auto binary_cache = BinaryCache::make(args, paths, stdout_sink).value_or_exit(VCPKG_LINE_INFO);
         const auto precheck_results = binary_cache.precheck(action_plan.install_actions);
-        auto split_specs =
-            compute_action_statuses(ExclusionPredicate{&exclusions_map}, var_provider, precheck_results, action_plan);
+        auto split_specs = compute_action_statuses(
+            ExclusionPredicate{&exclusions_map}, var_provider, precheck_results, action_plan, baseline_lines);
 
         {
             std::string msg;

?

[autoantwort]

"... and was excluded in the build" which seems inappropriate.

But ports that are unsupported gets excluded in the build. So the ports that are unsupported on a triplet are a subset of ports that are excluded from the build.

[autoantwort]

I think your proposed code results in the same detected errors + also detects wrong skip and pass entries.

[BillyONeal]

But ports that are unsupported gets excluded in the build. So the ports that are unsupported on a triplet are a subset of ports that are excluded from the build.

I think 'cascaded due to missing dependencies' still happens here?

I just don't see why we need to do any builds at all to answer these questions.

I think your proposed code results in the same detected errors + also detects wrong skip and pass entries.

Oops I forgot about those.

[autoantwort]

This was never hit since unsupported ports are excluded from the action plan beforehand:

vcpkg-tool/src/vcpkg/commands.ci.cpp

Lines 180 to 184 in 6040912

	const auto applicable_specs = Util::filter(specs, [&](auto& spec) -> bool {
	return create_feature_install_plan(provider, var_provider, {&spec, 1}, {}, serialize_options)
	.unsupported_features.empty();
	});

[BillyONeal]

Thanks for removing the build dependency and sorry the review took so long :)