Skip to content

Commit

Permalink
Allow overrides to select ports that aren't in the baseline. (#1109)
Browse files Browse the repository at this point in the history
* Add an e2e test for removed-from-baseline ports.

* Rename patterns.ps1 to e2e-registry.ps1, to reflect that there are
  tests for more than just patterns now.
* Add removed.json.in and a call to it.

See also #1108 which
fixes all the e2e infrastructure to consistently use -s.

* Actually fix the bug: anything accepting both an ActionPlan and a PortFileProvider is suspect. In this case it was:

```
        void load_tag_vars(const ActionPlan& action_plan,
                           const PortFileProvider& port_provider,
                           Triplet host_triplet) const;
```

Fix that by just pushing the ActionPlan part down to the underlying virtual load_tag_vars.
  • Loading branch information
BillyONeal authored Jun 23, 2023
1 parent f19f3d9 commit d89474b
Show file tree
Hide file tree
Showing 32 changed files with 94 additions and 90 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"name": "foo",
"version": "1.0.0",
"description": "e2e test port"
}
{
"name": "foo",
"version": "1.0.0",
"description": "e2e test port"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
set(VCPKG_POLICY_EMPTY_PACKAGE enabled)
4 changes: 4 additions & 0 deletions azure-pipelines/e2e-registry/removed-ports/removed/vcpkg.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "removed",
"version": "1.0.0"
}
9 changes: 9 additions & 0 deletions azure-pipelines/e2e-registry/versions/r-/removed.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"versions": [
{
"git-tree": "9b82c31964570870d27a5bb634f5b84e13f8b90a",
"version": "1.0.0",
"port-version": 0
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"vcpkg-configuration": {
"default-registry": {
"kind": "git",
"baseline": "$E2ERegistryBaseline",
"repository": "$E2ERegistryPath"
}
},
"dependencies": [
"removed"
],
"overrides": [
{
"name": "removed",
"version": "1.0.0"
}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,18 @@
try
{
Copy-Item -Recurse -LiteralPath @(
"$PSScriptRoot/../e2e_projects/registries-package-patterns",
"$PSScriptRoot/../e2e_registry"
"$PSScriptRoot/../e2e_projects/e2e-registry-templates",
"$PSScriptRoot/../e2e-registry"
) $WorkingRoot

$manifestRoot = "$WorkingRoot/registries-package-patterns"
$e2eRegistryPath = "$WorkingRoot/e2e_registry".Replace('\', '\\')
$manifestRoot = "$WorkingRoot/e2e-registry-templates"
$e2eRegistryPath = "$WorkingRoot/e2e-registry".Replace('\', '\\')
Push-Location $e2eRegistryPath
### <Initialize registry>
# Creates a git registry to run the e2e tests on
try
{
Write-Host "Initializing test registry"
if (Test-Path "$e2eRegistryPath/.git")
{
Remove-Item -Recurse -Force "$e2eRegistryPath/.git"
}


$gitConfig = @(
'-c', 'user.name=Nobody',
'-c', '[email protected]',
Expand All @@ -42,6 +36,7 @@ try
}
### </Initialize Registry>

# Testing registries' package selection patterns
function Update-VcpkgJson {
param($PreReplacementName)
$content = Get-Content -LiteralPath "$manifestRoot/$PreReplacementName"
Expand All @@ -55,40 +50,54 @@ try
# [patterns] No patterns (no default)
Write-Host "[patterns] No patterns (no default)"
Update-VcpkgJson 'no-patterns.json.in'
Run-Vcpkg -EndToEndTestSilent @commonArgs install | Out-Null
Run-Vcpkg @commonArgs install
Throw-IfFailed
Refresh-TestRoot

# [patterns] Patterns only (no default)
Write-Host "[patterns] Patterns only (no default)"
Update-VcpkgJson 'only-patterns.json.in'
Run-Vcpkg -EndToEndTestSilent @commonArgs install | Out-Null
Run-Vcpkg @commonArgs install
Throw-IfFailed
Refresh-TestRoot

# [patterns] Patterns with default
Write-Host "[patterns] Patterns with default"
Update-VcpkgJson 'with-default.json.in'
Run-Vcpkg -EndToEndTestSilent @commonArgs install | Out-Null
Run-Vcpkg @commonArgs install
Throw-IfFailed
Refresh-TestRoot

# [patterns] Repeated patterns
Write-Host "[patterns] Repeated patterns"
Update-VcpkgJson 'with-redeclaration.json.in'
$out = Run-VcpkgAndCaptureOutput -EndToEndTestSilent @commonArgs install
$out = Run-VcpkgAndCaptureOutput @commonArgs install
Throw-IfFailed
if ($out -notmatch "redeclarations will be ignored")
{
$out
throw "Expected warning about redeclaration"
throw 'Expected warning about redeclaration'
}

Refresh-TestRoot

# Testing that overrides can select ports that are removed from the baseline
Write-Host "[removed] Removed from baseline"
Update-VcpkgJson 'removed.json.in'
$out = Run-VcpkgAndCaptureOutput @commonArgs install
Throw-IfFailed
if ($out -match 'error: the baseline does not contain an entry for port removed' -Or
$out -notmatch 'The following packages will be built and installed:\s+removed:[^ ]+ -> 1.0.0 -- [^ ]+git-trees[\\/]9b82c31964570870d27a5bb634f5b84e13f8b90a'
)
{
throw 'Baseline removed port could not be selected with overrides'
}

Refresh-TestRoot
}
finally
{
Remove-Item -Recurse -Force -LiteralPath @(
"$WorkingRoot/registries-package-patterns",
"$WorkingRoot/e2e_registry"
"$WorkingRoot/e2e-registry-templates",
"$WorkingRoot/e2e-registry"
) -ErrorAction SilentlyContinue
}
12 changes: 3 additions & 9 deletions azure-pipelines/end-to-end-tests-prelude.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,6 @@ function Write-Trace ([string]$text) {

function Run-VcpkgAndCaptureOutput {
Param(
[Parameter(Mandatory = $false)]
[Switch]$EndToEndTestSilent,

[Parameter(Mandatory = $false)]
[Switch]$ForceExe,

Expand All @@ -112,24 +109,21 @@ function Run-VcpkgAndCaptureOutput {
}

$Script:CurrentTest = "$thisVcpkg $($testArgs -join ' ')"
if (!$EndToEndTestSilent) { Write-Host -ForegroundColor red $Script:CurrentTest }
Write-Host -ForegroundColor red $Script:CurrentTest
$result = (& "$thisVcpkg" @testArgs) | Out-String
if (!$EndToEndTestSilent) { Write-Host -ForegroundColor Gray $result }
Write-Host -ForegroundColor Gray $result
$result
}

function Run-Vcpkg {
Param(
[Parameter(Mandatory = $false)]
[Switch]$EndToEndTestSilent,

[Parameter(Mandatory = $false)]
[Switch]$ForceExe,

[Parameter(ValueFromRemainingArguments)]
[string[]]$TestArgs
)
Run-VcpkgAndCaptureOutput -EndToEndTestSilent:$EndToEndTestSilent -ForceExe:$ForceExe @TestArgs | Out-Null
Run-VcpkgAndCaptureOutput -ForceExe:$ForceExe @TestArgs | Out-Null
}

Refresh-TestRoot
10 changes: 4 additions & 6 deletions include/vcpkg-test/mockcmakevarprovider.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#pragma once

#include <vcpkg/cmakevars.h>
#include <vcpkg/dependencies.h>

namespace vcpkg::Test
{
Expand All @@ -18,13 +19,10 @@ namespace vcpkg::Test
dep_info_vars.emplace(spec, SMap{});
}

void load_tag_vars(Span<const FullPackageSpec> specs,
const PortFileProvider& port_provider,
Triplet host_triplet) const override
void load_tag_vars(const ActionPlan& action_plan, Triplet host_triplet) const override
{
for (auto&& spec : specs)
tag_vars.emplace(spec.package_spec, SMap{});
(void)(port_provider);
for (auto&& install_action : action_plan.install_actions)
tag_vars.emplace(install_action.spec, SMap{});
(void)(host_triplet);
}

Expand Down
8 changes: 1 addition & 7 deletions include/vcpkg/cmakevars.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,7 @@ namespace vcpkg::CMakeVars

virtual void load_dep_info_vars(Span<const PackageSpec> specs, Triplet host_triplet) const = 0;

virtual void load_tag_vars(Span<const FullPackageSpec> specs,
const PortFileProvider& port_provider,
Triplet host_triplet) const = 0;

void load_tag_vars(const ActionPlan& action_plan,
const PortFileProvider& port_provider,
Triplet host_triplet) const;
virtual void load_tag_vars(const ActionPlan& action_plan, Triplet host_triplet) const = 0;
};

std::unique_ptr<CMakeVarProvider> make_triplet_cmake_var_provider(const VcpkgPaths& paths);
Expand Down
1 change: 0 additions & 1 deletion include/vcpkg/commands.set-installed.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ namespace vcpkg::Commands::SetInstalled

void perform_and_exit_ex(const VcpkgCmdArguments& args,
const VcpkgPaths& paths,
const PathsPortFileProvider& provider,
const CMakeVars::CMakeVarProvider& cmake_vars,
ActionPlan action_plan,
DryRun dry_run,
Expand Down
50 changes: 16 additions & 34 deletions src/vcpkg/cmakevars.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,6 @@
using namespace vcpkg;
namespace vcpkg::CMakeVars
{
void CMakeVarProvider::load_tag_vars(const ActionPlan& action_plan,
const PortFileProvider& port_provider,
Triplet host_triplet) const
{
std::vector<FullPackageSpec> install_package_specs;
install_package_specs.reserve(action_plan.install_actions.size());
for (auto&& action : action_plan.install_actions)
{
install_package_specs.emplace_back(action.spec, action.feature_list);
}

load_tag_vars(install_package_specs, port_provider, host_triplet);
}

const std::unordered_map<std::string, std::string>& CMakeVarProvider::get_or_load_dep_info_vars(
const PackageSpec& spec, Triplet host_triplet) const
{
Expand All @@ -53,9 +39,7 @@ namespace vcpkg::CMakeVars

void load_dep_info_vars(View<PackageSpec> specs, Triplet host_triplet) const override;

void load_tag_vars(View<FullPackageSpec> specs,
const PortFileProvider& port_provider,
Triplet host_triplet) const override;
void load_tag_vars(const ActionPlan& action_plan, Triplet host_triplet) const override;

Optional<const std::unordered_map<std::string, std::string>&> get_generic_triplet_vars(
Triplet triplet) const override;
Expand All @@ -68,7 +52,7 @@ namespace vcpkg::CMakeVars

public:
Path create_tag_extraction_file(
const View<std::pair<const FullPackageSpec*, std::string>> spec_abi_settings) const;
const View<std::pair<FullPackageSpec, std::string>> spec_abi_settings) const;

Path create_dep_info_extraction_file(const View<PackageSpec> specs) const;

Expand Down Expand Up @@ -121,7 +105,7 @@ endmacro()
}

Path TripletCMakeVarProvider::create_tag_extraction_file(
const View<std::pair<const FullPackageSpec*, std::string>> spec_abi_settings) const
const View<std::pair<FullPackageSpec, std::string>> spec_abi_settings) const
{
const Filesystem& fs = paths.get_filesystem();
static int tag_extract_id = 0;
Expand All @@ -130,7 +114,7 @@ endmacro()
int emitted_triplet_id = 0;
for (const auto& spec_abi_setting : spec_abi_settings)
{
emitted_triplets[spec_abi_setting.first->package_spec.triplet()] = emitted_triplet_id++;
emitted_triplets[spec_abi_setting.first.package_spec.triplet()] = emitted_triplet_id++;
}
std::string extraction_file = create_extraction_file_prelude(paths, emitted_triplets);

Expand Down Expand Up @@ -177,7 +161,7 @@ endfunction()

for (const auto& spec_abi_setting : spec_abi_settings)
{
const FullPackageSpec& spec = *spec_abi_setting.first;
const FullPackageSpec& spec = spec_abi_setting.first;

std::string featurelist;
for (auto&& f : spec.features)
Expand Down Expand Up @@ -336,9 +320,8 @@ endfunction()
{
std::vector<std::vector<std::pair<std::string, std::string>>> vars(1);
// Hack: PackageSpecs should never have .name==""
FullPackageSpec full_spec({"", triplet}, {});
const auto file_path = create_tag_extraction_file(std::array<std::pair<const FullPackageSpec*, std::string>, 1>{
std::pair<const FullPackageSpec*, std::string>{&full_spec, ""}});
const auto file_path = create_tag_extraction_file(std::array<std::pair<FullPackageSpec, std::string>, 1>{
std::pair<FullPackageSpec, std::string>{FullPackageSpec{{"", triplet}, {}}, ""}});
launch_and_split(file_path, vars);
paths.get_filesystem().remove(file_path, VCPKG_LINE_INFO);

Expand Down Expand Up @@ -375,19 +358,18 @@ endfunction()
}
}

void TripletCMakeVarProvider::load_tag_vars(View<FullPackageSpec> specs,
const PortFileProvider& port_provider,
Triplet host_triplet) const
void TripletCMakeVarProvider::load_tag_vars(const ActionPlan& action_plan, Triplet host_triplet) const
{
if (specs.size() == 0) return;
std::vector<std::pair<const FullPackageSpec*, std::string>> spec_abi_settings;
spec_abi_settings.reserve(specs.size());
if (action_plan.install_actions.empty()) return;
std::vector<std::pair<FullPackageSpec, std::string>> spec_abi_settings;
spec_abi_settings.reserve(action_plan.install_actions.size());

for (const FullPackageSpec& spec : specs)
for (const auto& install_action : action_plan.install_actions)
{
auto& scfl = port_provider.get_control_file(spec.package_spec.name()).value_or_exit(VCPKG_LINE_INFO);
auto& scfl = install_action.source_control_file_and_location.value_or_exit(VCPKG_LINE_INFO);
const auto override_path = scfl.source_location / "vcpkg-abi-settings.cmake";
spec_abi_settings.emplace_back(&spec, override_path.generic_u8string());
spec_abi_settings.emplace_back(FullPackageSpec{install_action.spec, install_action.feature_list},
override_path.generic_u8string());
}

std::vector<std::vector<std::pair<std::string, std::string>>> vars(spec_abi_settings.size());
Expand All @@ -398,7 +380,7 @@ endfunction()
auto var_list_itr = vars.begin();
for (const auto& spec_abi_setting : spec_abi_settings)
{
const FullPackageSpec& spec = *spec_abi_setting.first;
const FullPackageSpec& spec = spec_abi_setting.first;
PlatformExpression::Context ctxt{std::make_move_iterator(var_list_itr->begin()),
std::make_move_iterator(var_list_itr->end())};
++var_list_itr;
Expand Down
2 changes: 1 addition & 1 deletion src/vcpkg/commands.build.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ namespace vcpkg::Build
auto action_plan = create_feature_install_plan(
provider, var_provider, {&full_spec, 1}, status_db, {host_triplet, paths.packages()});

var_provider.load_tag_vars(action_plan, provider, host_triplet);
var_provider.load_tag_vars(action_plan, host_triplet);

compute_all_abis(paths, action_plan, var_provider, status_db);

Expand Down
2 changes: 1 addition & 1 deletion src/vcpkg/commands.check-support.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ namespace vcpkg::Commands
{
auto action_plan = create_feature_install_plan(provider, *cmake_vars, {&user_spec, 1}, {}, create_options);

cmake_vars->load_tag_vars(action_plan, provider, host_triplet);
cmake_vars->load_tag_vars(action_plan, host_triplet);

Port user_port;
user_port.port_name = user_spec.package_spec.name();
Expand Down
2 changes: 1 addition & 1 deletion src/vcpkg/commands.ci.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ namespace vcpkg::Commands::CI
});

auto action_plan = create_feature_install_plan(provider, var_provider, applicable_specs, {}, serialize_options);
var_provider.load_tag_vars(action_plan, provider, serialize_options.host_triplet);
var_provider.load_tag_vars(action_plan, serialize_options.host_triplet);

Checks::check_exit(VCPKG_LINE_INFO, action_plan.already_installed.empty());
Checks::check_exit(VCPKG_LINE_INFO, action_plan.remove_actions.empty());
Expand Down
4 changes: 1 addition & 3 deletions src/vcpkg/commands.install.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1198,10 +1198,8 @@ namespace vcpkg
Util::erase_remove_if(install_plan.install_actions,
[&toplevel](auto&& action) { return action.spec == toplevel; });

PathsPortFileProvider provider(fs, *registry_set, std::move(oprovider));
Commands::SetInstalled::perform_and_exit_ex(args,
paths,
provider,
var_provider,
std::move(install_plan),
dry_run ? Commands::SetInstalled::DryRun::Yes
Expand Down Expand Up @@ -1246,7 +1244,7 @@ namespace vcpkg
}
}

var_provider.load_tag_vars(action_plan, provider, host_triplet);
var_provider.load_tag_vars(action_plan, host_triplet);

// install plan will be empty if it is already installed - need to change this at status paragraph part
if (action_plan.empty())
Expand Down
Loading

0 comments on commit d89474b

Please sign in to comment.