Test and and fix detection of ci regressions of independent ports

[dg0yt]

Restore

"REGRESSION: Independent feature[fail] failed with BUILD_FAILED."

as originally contributed in #794.

[BillyONeal]

This reduces "independent" to "the ABI hash changed." Which is going to be true for cases with dependent feature changes as originally discussed in #794 but is also going to be true for every port that is edited in a PR. I think this change merely replaces the existing regression output with saying they are all independent. It's true that you have a test counterexample but the test misses that all PR builds for us build with --skip-failures meaning the:

if ($Output.Contains("REGRESSION: Independent regression:${Triplet} failed with BUILD_FAILED.")) {
    throw 'regression (port) build failure must not be reported as independent regression'
}

block is testing something that we never do?

[BillyONeal]

I'm also really not a fan of trying to overload RequestType like this because that controls a bunch of other settings, like whether default features are added:

vcpkg-tool/src/vcpkg/dependencies.cpp

Lines 269 to 272 in d5c3552

	else if (request_type != RequestType::USER_REQUESTED)
	{
	out_reinstall_requirements.emplace_back(m_spec, FeatureNameDefault);
	m_install_info.get()->reduced_defaults = true;

as well as --head/--editable handling.

(I know that once we have an ActionPlan we are out of dependencies.cpp but that this is already being used to track other things strongly suggests that it is not the right tool to track what you're trying to track here. If all we care about is "is the abi hash in parent_hashes" let's ask that question directly at reporting time rather than trying to smuggle it through this setting)

[dg0yt]

I'm also really not a fan of trying to overload RequestType like this because that controls a bunch of other settings, like whether default features are added.

With parent hashes, we do know that ports are installed for different reasons. Tracking the reason in RequestType seems a natural fit, not smuggling. AUTO_SELECTED seems to perfectly explain why an independent port is part of the installation plan.

I see that it might make sense to use the information already in the construction of the installation plan (i.e. "once, early").

as well as --head/--editable handling.

Not relevant for ci command.

If all we care about is "is the abi hash in parent_hashes" let's ask that question directly at reporting time rather than trying to smuggle it through this setting.

So you suggest to process the parent hashes a second time in the build result reporting (i.e "late")?

[dg0yt]

This reduces "independent" to "the ABI hash changed."

Actually this maps "independent" to "the ABI hash DID NOT change."

Dependencies become visible by propagation of ABI hash changes. The cone of destruction is formed by the destruction of original ABI hashes.

It's true that you have a test counterexample but the test misses that all PR builds for us build with --skip-failures

--skip-failures skips ports marked =fail in ci.baseline.txt. But we are not dealing with ports which are known to fail. We are dealing with dependencies which fail unexpectedly, in particular because the specific configuration (features, installation order, triplet) wasn't tested. The goal is to point out that a particular unexpected failure is not in the PR's cone of destruction but in dependencies needed by the cone of the destruction.

[BillyONeal]

Actually this maps "independent" to "the ABI hash DID NOT change."

Please forgive my inversion.

Dependencies become visible by propagation of ABI hash changes. The cone of destruction is the destruction of original ABI hashes.

But the ABI hashes of the dependencies change in those cases too.

From #794

vcpkg "parent" state:
Port A.
Port B.
Port C depends on A and on B.
CI successful.

After a vcpkg PR modifying C.
Port A.
Port B.
Port C depends on A and on B[non-default-feature].
CI fails on B[non-default-feature] which wasn't built before:

B[non-default-feature] has a different ABI hash than B[default] even though it isn't in the cone of destruction. We can't distinguish between B ("I didn't even edit B!") and C by looking at parent hashes here. And we do want the "if expected, add =fail" language for C.

[BillyONeal]

I suppose given my inversion what you want is to not emit the "add =fail" for A in that example because it isn't edited. But I still don't think that addresses "but I didn't even edit B"

[dg0yt]

Okay, I get the point. The configuration changes for the independent port(s) take them out of the overlap of (parent vs. current) ABI hash information, coloring the relevant ports as "not independent". This kills this approach.

FTR for feature testing, vcpkg CI already collects the list of changed ports. This information could be an alternative starting point to color the cone of destruction. Not sure if it is not worth continuing. These regression do occur, but it is not very often now.

[BillyONeal]

FTR for feature testing, vcpkg CI already collects the list of changed ports. This information could be an alternative starting point to color the cone of destruction.

Yes, now that we have the "--for-merge-with" tech that would be way better for something like this.

What do you think of this?