Skip to content

Commit

Permalink
Ensure package_dir is always filled in. (#1103)
Browse files Browse the repository at this point in the history 
Resolves microsoft/vcpkg#31908

In #1040 , create_feature_install_plan and create_versioned_install_plan were fixed, but not create_upgrade_plan.

Make this bug structurally impossible by making the FooAction ctors that fill in m_scfl also fill in package_dir.
  • Loading branch information
@BillyONeal
BillyONeal authored Jun 22, 2023
1 parent add000a commit f19f3d9
Show file tree
Hide file tree
Showing 8 changed files with 107 additions and 46 deletions.
8 changes: 4 additions & 4 deletions azure-pipelines/end-to-end-tests-dir/bundles.ps1
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ foreach ($k in $b.keys) {

Run-Vcpkg install vcpkg-hello-world-1 --dry-run @commonArgs `
--x-buildtrees-root=$buildtreesRoot `
--x-builtin-ports-root=$deploy/ports `
--x-builtin-ports-root=$deployment/ports `
--x-install-root=$installRoot `
--x-packages-root=$packagesRoot
Throw-IfNotFailed
Expand All @@ -164,7 +164,7 @@ $CurrentTest = "Testing bundle.usegitregistry"
Run-Vcpkg install --dry-run @commonArgs `
--x-manifest-root=$manifestdir `
--x-buildtrees-root=$buildtreesRoot `
--x-builtin-ports-root=$deploy/ports `
--x-builtin-ports-root=$deployment/ports `
--x-install-root=$installRoot `
--x-packages-root=$packagesRoot
Throw-IfNotFailed
Expand All @@ -184,15 +184,15 @@ New-Item -ItemType Directory -Force $manifestdir2 | Out-Null
Run-Vcpkg install @commonArgs `
--x-manifest-root=$manifestdir2 `
--x-buildtrees-root=$buildtreesRoot `
--x-builtin-ports-root=$deploy/ports `
--x-builtin-ports-root=$deployment/ports `
--x-install-root=$installRoot `
--x-packages-root=$packagesRoot
Throw-IfFailed

Run-Vcpkg search vcpkg-hello-world-1 @commonArgs `
--x-manifest-root=$manifestdir2 `
--x-buildtrees-root=$buildtreesRoot `
--x-builtin-ports-root=$deploy/ports `
--x-builtin-ports-root=$deployment/ports `
--x-install-root=$installRoot `
--x-packages-root=$packagesRoot
Throw-IfFailed
Expand Down
48 changes: 48 additions & 0 deletions azure-pipelines/end-to-end-tests-dir/upgrade.ps1
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
. "$PSScriptRoot/../end-to-end-tests-prelude.ps1"

git clone $VcpkgRoot "$TestingRoot/temp-repo" --local
try
{
$env:VCPKG_ROOT = "$TestingRoot/temp-repo"
git -C "$TestingRoot/temp-repo" switch -d e1934f4a2a0c58bb75099d89ed980832379907fa # vcpkg-cmake @ 2022-12-22
$output = Run-VcpkgAndCaptureOutput install vcpkg-cmake
Throw-IfFailed
if (-Not ($output -match 'vcpkg-cmake:[^ ]+ -> 2022-12-22'))
{
throw 'Unexpected vcpkg-cmake install'
}

git -C "$TestingRoot/temp-repo" switch -d f6a5d4e8eb7476b8d7fc12a56dff300c1c986131 # vcpkg-cmake @ 2023-05-04
$output = Run-VcpkgAndCaptureOutput upgrade
Throw-IfNotFailed
if (-Not ($output -match 'If you are sure you want to rebuild the above packages, run this command with the --no-dry-run option.'))
{
throw "Upgrade didn't handle dry-run correctly"
}

if (-Not ($output -match '\* vcpkg-cmake:[^ ]+ -> 2023-05-04'))
{
throw "Upgrade didn't choose expected version"
}

$output = Run-VcpkgAndCaptureOutput upgrade --no-dry-run
Throw-IfFailed
if (-Not ($output -match '\* vcpkg-cmake:[^ ]+ -> 2023-05-04'))
{
throw "Upgrade didn't choose expected version"
}

if (-Not ($output -match 'vcpkg-cmake:[^:]+: REMOVED:'))
{
throw "Upgrade didn't remove"
}

if (-Not ($output -match 'vcpkg-cmake:[^:]+: SUCCEEDED:'))
{
throw "Upgrade didn't install"
}
}
finally
{
$env:VCPKG_ROOT = $VcpkgRoot
}
1 change: 1 addition & 0 deletions include/vcpkg/dependencies.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ namespace vcpkg

InstallPlanAction(const PackageSpec& spec,
const SourceControlFileAndLocation& scfl,
const Path& packages_dir,
const RequestType& request_type,
Triplet host_triplet,
std::map<std::string, std::vector<FeatureSpec>>&& dependencies,
Expand Down
4 changes: 4 additions & 0 deletions src/vcpkg-test/binarycaching.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ Build-Depends: bzip

InstallPlanAction ipa(PackageSpec{"zlib2", Test::X64_WINDOWS},
scfl,
"test_packages_root",
RequestType::USER_REQUESTED,
Test::ARM_UWP,
{{"a", {}}, {"b", {}}},
Expand Down Expand Up @@ -347,6 +348,7 @@ Version: 1.5
std::vector<InstallPlanAction> install_plan;
install_plan.emplace_back(PackageSpec{"someheadpackage", Test::X64_WINDOWS},
scfl,
"test_packages_root",
RequestType::USER_REQUESTED,
Test::ARM_UWP,
std::map<std::string, std::vector<FeatureSpec>>{},
Expand Down Expand Up @@ -422,6 +424,7 @@ Description: a spiffy compression library wrapper
SourceControlFileAndLocation scfl{std::move(*maybe_scf.get()), Path()};
plan.install_actions.emplace_back(PackageSpec("zlib", Test::X64_ANDROID),
scfl,
"test_packages_root",
RequestType::USER_REQUESTED,
Test::ARM64_WINDOWS,
std::map<std::string, std::vector<FeatureSpec>>{},
Expand All @@ -448,6 +451,7 @@ Description: a spiffy compression library wrapper
SourceControlFileAndLocation scfl2{std::move(*maybe_scf2.get()), Path()};
plan.install_actions.emplace_back(PackageSpec("zlib2", Test::X64_ANDROID),
scfl2,
"test_packages_root",
RequestType::USER_REQUESTED,
Test::ARM64_WINDOWS,
std::map<std::string, std::vector<FeatureSpec>>{},
Expand Down
15 changes: 10 additions & 5 deletions src/vcpkg-test/dependencies.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2278,11 +2278,16 @@ TEST_CASE ("formatting plan 1", "[dependencies]")
const RemovePlanAction remove_b({"b", Test::X64_OSX}, RequestType::USER_REQUESTED);
const RemovePlanAction remove_a({"a", Test::X64_OSX}, RequestType::USER_REQUESTED);
const RemovePlanAction remove_c({"c", Test::X64_OSX}, RequestType::AUTO_SELECTED);
InstallPlanAction install_a({"a", Test::X64_OSX}, scfl_a, RequestType::AUTO_SELECTED, Test::X64_ANDROID, {}, {});

const Path pr = "packages_root";
InstallPlanAction install_a(
{"a", Test::X64_OSX}, scfl_a, pr, RequestType::AUTO_SELECTED, Test::X64_ANDROID, {}, {});
InstallPlanAction install_b(
{"b", Test::X64_OSX}, scfl_b, RequestType::AUTO_SELECTED, Test::X64_ANDROID, {{"1", {}}}, {});
InstallPlanAction install_c({"c", Test::X64_OSX}, scfl_c, RequestType::USER_REQUESTED, Test::X64_ANDROID, {}, {});
InstallPlanAction install_f({"f", Test::X64_OSX}, scfl_f, RequestType::USER_REQUESTED, Test::X64_ANDROID, {}, {});
{"b", Test::X64_OSX}, scfl_b, pr, RequestType::AUTO_SELECTED, Test::X64_ANDROID, {{"1", {}}}, {});
InstallPlanAction install_c(
{"c", Test::X64_OSX}, scfl_c, pr, RequestType::USER_REQUESTED, Test::X64_ANDROID, {}, {});
InstallPlanAction install_f(
{"f", Test::X64_OSX}, scfl_f, pr, RequestType::USER_REQUESTED, Test::X64_ANDROID, {}, {});
install_f.plan_type = InstallPlanType::EXCLUDED;

InstallPlanAction already_installed_d(
Expand Down Expand Up @@ -2374,7 +2379,7 @@ TEST_CASE ("dependency graph API snapshot")
MockVersionedPortfileProvider vp;
auto& scfl_a = vp.emplace("a", {"1", 0});
InstallPlanAction install_a(
{"a", Test::X64_WINDOWS}, scfl_a, RequestType::AUTO_SELECTED, Test::X64_ANDROID, {}, {});
{"a", Test::X64_WINDOWS}, scfl_a, "packages_root", RequestType::AUTO_SELECTED, Test::X64_ANDROID, {}, {});

ActionPlan plan;
plan.install_actions.push_back(std::move(install_a));
Expand Down
6 changes: 3 additions & 3 deletions src/vcpkg-test/spdx.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ TEST_CASE ("spdx maximum serialization", "[spdx]")
cpgh.raw_version = "1.0";
cpgh.version_scheme = VersionScheme::Relaxed;

InstallPlanAction ipa(spec, scfl, RequestType::USER_REQUESTED, Test::X86_WINDOWS, {}, {});
InstallPlanAction ipa(spec, scfl, "test_packages_root", RequestType::USER_REQUESTED, Test::X86_WINDOWS, {}, {});
auto& abi = *(ipa.abi_info = AbiInfo{}).get();
abi.package_abi = "ABIHASH";

Expand Down Expand Up @@ -175,7 +175,7 @@ TEST_CASE ("spdx minimum serialization", "[spdx]")
cpgh.raw_version = "1.0";
cpgh.version_scheme = VersionScheme::String;

InstallPlanAction ipa(spec, scfl, RequestType::USER_REQUESTED, Test::X86_WINDOWS, {}, {});
InstallPlanAction ipa(spec, scfl, "test_packages_root", RequestType::USER_REQUESTED, Test::X86_WINDOWS, {}, {});
auto& abi = *(ipa.abi_info = AbiInfo{}).get();
abi.package_abi = "deadbeef";

Expand Down Expand Up @@ -303,7 +303,7 @@ TEST_CASE ("spdx concat resources", "[spdx]")
cpgh.raw_version = "1.0";
cpgh.version_scheme = VersionScheme::String;

InstallPlanAction ipa(spec, scfl, RequestType::USER_REQUESTED, Test::X86_WINDOWS, {}, {});
InstallPlanAction ipa(spec, scfl, "test_packages_root", RequestType::USER_REQUESTED, Test::X86_WINDOWS, {}, {});
auto& abi = *(ipa.abi_info = AbiInfo{}).get();
abi.package_abi = "deadbeef";

Expand Down
65 changes: 34 additions & 31 deletions src/vcpkg/dependencies.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,8 @@ namespace vcpkg
PackageGraph(const PortFileProvider& provider,
const CMakeVars::CMakeVarProvider& var_provider,
const StatusParagraphs& status_db,
Triplet host_triplet);
Triplet host_triplet,
const Path& packages_dir);
~PackageGraph() = default;

void install(Span<const FeatureSpec> specs, UnsupportedPortAction unsupported_port_action);
Expand All @@ -306,6 +307,7 @@ namespace vcpkg
const CMakeVars::CMakeVarProvider& m_var_provider;

std::unique_ptr<ClusterGraph> m_graph;
Path m_packages_dir;
std::map<FeatureSpec, PlatformExpression::Expr> m_unsupported_features;
};

Expand Down Expand Up @@ -457,6 +459,7 @@ namespace vcpkg

InstallPlanAction::InstallPlanAction(const PackageSpec& spec,
const SourceControlFileAndLocation& scfl,
const Path& packages_dir,
const RequestType& request_type,
Triplet host_triplet,
std::map<std::string, std::vector<FeatureSpec>>&& dependencies,
Expand All @@ -469,6 +472,7 @@ namespace vcpkg
, feature_dependencies(std::move(dependencies))
, build_failure_messages(std::move(build_failure_messages))
, host_triplet(host_triplet)
, package_dir(packages_dir / spec.dir())
{
}

Expand Down Expand Up @@ -707,7 +711,7 @@ namespace vcpkg
const StatusParagraphs& status_db,
const CreateInstallPlanOptions& options)
{
PackageGraph pgraph(port_provider, var_provider, status_db, options.host_triplet);
PackageGraph pgraph(port_provider, var_provider, status_db, options.host_triplet, options.packages_dir);

std::vector<FeatureSpec> feature_specs;
for (const FullPackageSpec& spec : specs)
Expand All @@ -719,17 +723,7 @@ namespace vcpkg

pgraph.install(feature_specs, options.unsupported_port_action);

auto res = pgraph.serialize(options.randomizer);

for (auto&& action : res.install_actions)
{
if (action.source_control_file_and_location.has_value())
{
action.package_dir.emplace(options.packages_dir / action.spec.dir());
}
}

return res;
return pgraph.serialize(options.randomizer);
}

void PackageGraph::mark_for_reinstall(const PackageSpec& first_remove_spec,
Expand Down Expand Up @@ -927,7 +921,7 @@ namespace vcpkg
const StatusParagraphs& status_db,
const CreateInstallPlanOptions& options)
{
PackageGraph pgraph(port_provider, var_provider, status_db, options.host_triplet);
PackageGraph pgraph(port_provider, var_provider, status_db, options.host_triplet, options.packages_dir);

pgraph.upgrade(specs, options.unsupported_port_action);

Expand Down Expand Up @@ -1044,21 +1038,33 @@ namespace vcpkg
fspecs.insert(fspec);
continue;
}

auto&& dep_clust = m_graph->get(fspec.spec());
const auto& default_features = [&] {
if (dep_clust.m_install_info.has_value())
{
return dep_clust.get_scfl_or_exit()
.source_control_file->core_paragraph->default_features;
if (auto p = dep_clust.m_installed.get()) return p->ipv.core->package.default_features;
}

if (auto p = dep_clust.m_installed.get())
{
return p->ipv.core->package.default_features;
}

Checks::unreachable(VCPKG_LINE_INFO);
}();

for (auto&& default_feature : default_features)
{
fspecs.emplace(fspec.spec(), default_feature);
}
}
computed_edges[kv.first].assign(fspecs.begin(), fspecs.end());
}
plan.install_actions.emplace_back(p_cluster->m_spec,
p_cluster->get_scfl_or_exit(),
m_packages_dir,
p_cluster->request_type,
m_graph->m_host_triplet,
std::move(computed_edges),
Expand Down Expand Up @@ -1113,8 +1119,11 @@ namespace vcpkg
PackageGraph::PackageGraph(const PortFileProvider& port_provider,
const CMakeVars::CMakeVarProvider& var_provider,
const StatusParagraphs& status_db,
Triplet host_triplet)
: m_var_provider(var_provider), m_graph(create_feature_install_graph(port_provider, status_db, host_triplet))
Triplet host_triplet,
const Path& packages_dir)
: m_var_provider(var_provider)
, m_graph(create_feature_install_graph(port_provider, status_db, host_triplet))
, m_packages_dir(packages_dir)
{
}

Expand Down Expand Up @@ -1272,12 +1281,14 @@ namespace vcpkg
const IBaselineProvider& base_provider,
const IOverlayProvider& oprovider,
const CMakeVars::CMakeVarProvider& var_provider,
Triplet host_triplet)
Triplet host_triplet,
const Path& packages_dir)
: m_ver_provider(ver_provider)
, m_base_provider(base_provider)
, m_o_provider(oprovider)
, m_var_provider(var_provider)
, m_host_triplet(host_triplet)
, m_packages_dir(packages_dir)
{
}

Expand All @@ -1294,6 +1305,7 @@ namespace vcpkg
const IOverlayProvider& m_o_provider;
const CMakeVars::CMakeVarProvider& m_var_provider;
const Triplet m_host_triplet;
const Path m_packages_dir;

struct DepSpec
{
Expand Down Expand Up @@ -1784,6 +1796,7 @@ namespace vcpkg
: RequestType::AUTO_SELECTED;
InstallPlanAction ipa(dep.spec,
*node.second.scfl,
m_packages_dir,
request,
m_host_triplet,
compute_feature_dependencies(node, deps),
Expand Down Expand Up @@ -1901,24 +1914,14 @@ namespace vcpkg
const PackageSpec& toplevel,
const CreateInstallPlanOptions& options)
{
VersionedPackageGraph vpg(provider, bprovider, oprovider, var_provider, options.host_triplet);
VersionedPackageGraph vpg(
provider, bprovider, oprovider, var_provider, options.host_triplet, options.packages_dir);
for (auto&& o : overrides)
{
vpg.add_override(o.name, {o.version, o.port_version});
}

vpg.solve_with_roots(deps, toplevel);
auto ret = vpg.finalize_extract_plan(toplevel, options.unsupported_port_action);
if (auto plan = ret.get())
{
for (auto&& action : plan->install_actions)
{
if (action.source_control_file_and_location.has_value())
{
action.package_dir.emplace(options.packages_dir / action.spec.dir());
}
}
}
return ret;
return vpg.finalize_extract_plan(toplevel, options.unsupported_port_action);
}
}
6 changes: 3 additions & 3 deletions src/vcpkg/postbuildlint.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -353,11 +353,11 @@ namespace vcpkg

static LintStatus check_for_copyright_file(const ReadOnlyFilesystem& fs,
const PackageSpec& spec,
const Path& package_dir,
const VcpkgPaths& paths,
MessageSink& msg_sink)
{
const auto packages_dir = paths.package_dir(spec);
const auto copyright_file = packages_dir / "share" / spec.name() / "copyright";
const auto copyright_file = package_dir / "share" / spec.name() / "copyright";

switch (fs.status(copyright_file, IgnoreErrors{}))
{
Expand Down Expand Up @@ -1364,7 +1364,7 @@ namespace vcpkg
error_count += check_folder_debug_lib_cmake(fs, package_dir, spec, msg_sink);
error_count += check_for_dlls_in_lib_dir(fs, package_dir, msg_sink);
error_count += check_for_dlls_in_lib_dir(fs, package_dir / "debug", msg_sink);
error_count += check_for_copyright_file(fs, spec, paths, msg_sink);
error_count += check_for_copyright_file(fs, spec, package_dir, paths, msg_sink);
error_count += check_for_exes(fs, package_dir, msg_sink);
error_count += check_for_exes(fs, package_dir / "debug", msg_sink);
error_count += check_for_usage_forgot_install(fs, port_dir, package_dir, spec, msg_sink);
Expand Down

0 comments on commit f19f3d9

Please sign in to comment.