Skip to content

Test and and fix detection of ci regressions of independent ports #1830

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Test and and fix detection of ci regressions of independent ports #1830

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b4c7736
Test regressions of independent ports
dg0yt Nov 4, 2025
0d09b2c
Fix detection of regressions of independent ports
dg0yt Nov 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions azure-pipelines/e2e-ports/ci-independent-regression/v1/regression/portfile.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
set(VCPKG_POLICY_EMPTY_PACKAGE enabled)
12 changes: 12 additions & 0 deletions azure-pipelines/e2e-ports/ci-independent-regression/v1/regression/vcpkg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"name": "regression",
"version": "1",
"dependencies": [
{
"name": "feature-fails",
"features": [
"fail"
]
}
]
}
1 change: 1 addition & 0 deletions azure-pipelines/e2e-ports/ci-independent-regression/v2/regression/portfile.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
set(VCPKG_POLICY_EMPTY_PACKAGE enabled)
12 changes: 12 additions & 0 deletions azure-pipelines/e2e-ports/ci-independent-regression/v2/regression/vcpkg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"name": "regression",
"version": "2",
"dependencies": [
{
"name": "feature-fails",
"features": [
"fail"
]
}
]
}
4 changes: 4 additions & 0 deletions azure-pipelines/e2e-ports/ci/feature-fails/portfile.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
set(VCPKG_POLICY_EMPTY_PACKAGE enabled)
if("fail" IN_LIST FEATURES)
message(FATAL_ERROR "Failing, triggered by feature 'fail'.")
endif()
9 changes: 9 additions & 0 deletions azure-pipelines/e2e-ports/ci/feature-fails/vcpkg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "feature-fails",
"version": "1",
"features": {
"fail": {
"description": "fails to build"
}
}
}
39 changes: 37 additions & 2 deletions azure-pipelines/end-to-end-tests-dir/ci.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ if (-not ($Output.Contains("feature-not-sup:${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) {
throw 'base-port should not be installed for the host'
if ($Output.Split("*: ").Length -ne 5) {
throw 'Exactly 4 ports should be installed'
}
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"
Expand Down Expand Up @@ -80,10 +80,45 @@ Throw-IfFailed
if (-not ($Output.Contains("base-port:${Triplet}: SUCCEEDED:"))) {
throw 'base-port build must succeed'
}
if (-not ($Output.Contains("feature-fails:${Triplet}: SUCCEEDED:"))) {
throw 'feature-fails[core] build must succeed'
}
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'
}

# With parent-hashes, test detection of regressions of independent ports.
Remove-Item -Recurse -Force $installRoot -ErrorAction SilentlyContinue
New-Item -ItemType Directory -Path $installRoot -Force | Out-Null
Remove-Item -Recurse -Force $ArchiveRoot -ErrorAction SilentlyContinue
New-Item -ItemType Directory -Path $ArchiveRoot -Force | Out-Null
# A dry run in order to determine all parent hashes without failing, regression@1
$Output = Run-VcpkgAndCaptureOutput ci --dry-run @commonArgs --x-builtin-ports-root="$PSScriptRoot/../e2e-ports/ci" --binarysource="clear;files,$ArchiveRoot" --output-hashes="$TestingRoot/parent-hashes.json" --overlay-ports="$PSScriptRoot/../e2e-ports/ci-independent-regression/v1"
Throw-IfFailed
Remove-Item -Recurse -Force $installRoot -ErrorAction SilentlyContinue
New-Item -ItemType Directory -Path $installRoot -Force | Out-Null
# Non-dry run with port regression@2
$Output = Run-VcpkgAndCaptureOutput ci @commonArgs --x-builtin-ports-root="$PSScriptRoot/../e2e-ports/ci" --binarysource="clear;files,$ArchiveRoot" --parent-hashes="$TestingRoot/parent-hashes.json" --overlay-ports="$PSScriptRoot/../e2e-ports/ci-independent-regression/v2"
Throw-IfNotFailed
if ($Output.Contains("base-port:${Triplet}: SUCCEEDED:")) {
throw 'base-port must not be rebuilt again'
}
if (-not ($Output.Contains("feature-fails:${Triplet}: BUILD_FAILED:"))) {
throw 'feature-fails[fail] build must fail'
}
if (-not ($Output.Contains("regression:${Triplet}: CASCADED_DUE_TO_MISSING_DEPENDENCIES:"))) {
throw 'regression build must cascade'
}
# Non-dry run with port regression@2
$Output = Run-VcpkgAndCaptureStdErr ci @commonArgs --x-builtin-ports-root="$PSScriptRoot/../e2e-ports/ci" --binarysource="clear;files,$ArchiveRoot" --parent-hashes="$TestingRoot/parent-hashes.json" --overlay-ports="$PSScriptRoot/../e2e-ports/ci-independent-regression/v2"
Throw-IfNotFailed
if (-not ($Output.Contains("REGRESSION: Independent feature-fails:${Triplet} failed with BUILD_FAILED."))) {
throw 'feature-fails[fail] build failure must be reported as independent regression'
}
if ($Output.Contains("REGRESSION: Independent regression:${Triplet} failed with BUILD_FAILED.")) {
throw 'regression (port) build failure must not be reported as independent regression'
}
6 changes: 6 additions & 0 deletions src/vcpkg/commands.ci.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ namespace
const std::map<PackageSpec, BuildResult>& known,
View<std::string> parent_hashes)
{
// With parent hashes, ports are merely "auto selected" unless the abi hash changed.
auto const default_request_type = parent_hashes.empty() ? RequestType::USER_REQUESTED : RequestType::AUTO_SELECTED;
Copy link
Member

@BillyONeal BillyONeal Nov 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Contributor Author

@dg0yt dg0yt Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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")?

std::set<PackageSpec> to_keep;
for (auto it = action_plan.install_actions.rbegin(); it != action_plan.install_actions.rend(); ++it)
{
Expand All @@ -187,6 +189,10 @@ namespace
to_keep.insert(it->spec);
}
}
else
{
it->request_type = default_request_type;
}

if (Util::Sets::contains(to_keep, it->spec))
{
Expand Down
Loading