From 10e3295b0202a4f9e373a9a60fb9e221b813adf1 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Mon, 19 Jun 2023 18:50:38 -0700 Subject: [PATCH 1/5] Ensure package_dir is always filled in. Resolves https://github.com/microsoft/vcpkg/issues/31908 In https://github.com/microsoft/vcpkg-tool/pull/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. --- include/vcpkg/commands.set-installed.h | 4 +- include/vcpkg/dependencies.h | 8 ++- src/vcpkg-test/binarycaching.cpp | 4 ++ src/vcpkg-test/dependencies.cpp | 18 +++--- src/vcpkg-test/plan.cpp | 16 +++--- src/vcpkg-test/spdx.cpp | 6 +- src/vcpkg/commands.ci.cpp | 3 +- src/vcpkg/commands.install.cpp | 2 +- src/vcpkg/commands.remove.cpp | 4 +- src/vcpkg/commands.set-installed.cpp | 8 ++- src/vcpkg/dependencies.cpp | 78 ++++++++++++++------------ src/vcpkg/postbuildlint.cpp | 6 +- 12 files changed, 90 insertions(+), 67 deletions(-) diff --git a/include/vcpkg/commands.set-installed.h b/include/vcpkg/commands.set-installed.h index 9cce5142f7..ce8006f86b 100644 --- a/include/vcpkg/commands.set-installed.h +++ b/include/vcpkg/commands.set-installed.h @@ -29,7 +29,9 @@ namespace vcpkg::Commands::SetInstalled * @param status_db The status db of the installed folder * @returns A set of PackageSpec's that are already installed */ - std::set adjust_action_plan_to_status_db(ActionPlan& action_plan, const StatusParagraphs& status_db); + std::set adjust_action_plan_to_status_db(ActionPlan& action_plan, + const StatusParagraphs& status_db, + const Path& packages_dir); void perform_and_exit_ex(const VcpkgCmdArguments& args, const VcpkgPaths& paths, diff --git a/include/vcpkg/dependencies.h b/include/vcpkg/dependencies.h index a273e9dc9f..0abe4d7e39 100644 --- a/include/vcpkg/dependencies.h +++ b/include/vcpkg/dependencies.h @@ -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>&& dependencies, @@ -100,9 +101,10 @@ namespace vcpkg struct RemovePlanAction : BasicAction { - RemovePlanAction(const PackageSpec& spec, RequestType rt); + RemovePlanAction(const PackageSpec& spec, RequestType rt, const Path& package_dir); RequestType request_type; + Path package_dir; }; struct ActionPlan @@ -167,7 +169,9 @@ namespace vcpkg std::vector remove; }; - RemovePlan create_remove_plan(const std::vector& specs, const StatusParagraphs& status_db); + RemovePlan create_remove_plan(const std::vector& specs, + const StatusParagraphs& status_db, + const Path& packages_dir); std::vector create_export_plan(const std::vector& specs, const StatusParagraphs& status_db); diff --git a/src/vcpkg-test/binarycaching.cpp b/src/vcpkg-test/binarycaching.cpp index b74d930c5b..db6483dee6 100644 --- a/src/vcpkg-test/binarycaching.cpp +++ b/src/vcpkg-test/binarycaching.cpp @@ -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", {}}}, @@ -347,6 +348,7 @@ Version: 1.5 std::vector install_plan; install_plan.emplace_back(PackageSpec{"someheadpackage", Test::X64_WINDOWS}, scfl, + "test_packages_root", RequestType::USER_REQUESTED, Test::ARM_UWP, std::map>{}, @@ -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>{}, @@ -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>{}, diff --git a/src/vcpkg-test/dependencies.cpp b/src/vcpkg-test/dependencies.cpp index b468be1ab4..81e8697fd9 100644 --- a/src/vcpkg-test/dependencies.cpp +++ b/src/vcpkg-test/dependencies.cpp @@ -2271,14 +2271,18 @@ TEST_CASE ("formatting plan 1", "[dependencies]") auto& scfl_c = vp.emplace("c", {"1", 0}); auto& scfl_f = vp.emplace("f", {"1", 0}); - 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"; + const RemovePlanAction remove_b({"b", Test::X64_OSX}, RequestType::USER_REQUESTED, pr); + const RemovePlanAction remove_a({"a", Test::X64_OSX}, RequestType::USER_REQUESTED, pr); + const RemovePlanAction remove_c({"c", Test::X64_OSX}, RequestType::AUTO_SELECTED, pr); + 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( diff --git a/src/vcpkg-test/plan.cpp b/src/vcpkg-test/plan.cpp index 35ae4c036b..6b64a988cf 100644 --- a/src/vcpkg-test/plan.cpp +++ b/src/vcpkg-test/plan.cpp @@ -942,7 +942,7 @@ TEST_CASE ("basic remove scheme", "[plan]") pghs.push_back(make_status_pgh("a")); StatusParagraphs status_db(std::move(pghs)); - auto plan = create_remove_plan({{"a", Test::X86_WINDOWS}}, status_db); + auto plan = create_remove_plan({{"a", Test::X86_WINDOWS}}, status_db, "test_packages_root"); auto& remove_plan = plan.remove; REQUIRE(remove_plan.size() == 1); REQUIRE(remove_plan.at(0).spec.name() == "a"); @@ -955,7 +955,7 @@ TEST_CASE ("recurse remove scheme", "[plan]") pghs.push_back(make_status_pgh("b", "a")); StatusParagraphs status_db(std::move(pghs)); - auto plan = create_remove_plan({{"a", Test::X86_WINDOWS}}, status_db); + auto plan = create_remove_plan({{"a", Test::X86_WINDOWS}}, status_db, "test_packages_root"); auto& remove_plan = plan.remove; REQUIRE(remove_plan.size() == 2); @@ -971,7 +971,7 @@ TEST_CASE ("features depend remove scheme", "[plan]") pghs.push_back(make_status_feature_pgh("b", "0", "a")); StatusParagraphs status_db(std::move(pghs)); - auto plan = create_remove_plan({{"a", Test::X86_WINDOWS}}, status_db); + auto plan = create_remove_plan({{"a", Test::X86_WINDOWS}}, status_db, "test_packages_root"); auto& remove_plan = plan.remove; REQUIRE(remove_plan.size() == 2); @@ -988,7 +988,7 @@ TEST_CASE ("features depend remove scheme once removed", "[plan]") pghs.push_back(make_status_feature_pgh("opencv", "vtk", "vtk")); StatusParagraphs status_db(std::move(pghs)); - auto plan = create_remove_plan({{"expat", Test::X86_WINDOWS}}, status_db); + auto plan = create_remove_plan({{"expat", Test::X86_WINDOWS}}, status_db, "test_packages_root"); auto& remove_plan = plan.remove; REQUIRE(remove_plan.size() == 3); @@ -1006,7 +1006,7 @@ TEST_CASE ("features depend remove scheme once removed x64", "[plan]") pghs.push_back(make_status_feature_pgh("opencv", "vtk", "vtk", "x64")); StatusParagraphs status_db(std::move(pghs)); - auto plan = create_remove_plan({{"expat", Triplet::from_canonical_name("x64")}}, status_db); + auto plan = create_remove_plan({{"expat", Triplet::from_canonical_name("x64")}}, status_db, "test_packages_root"); auto& remove_plan = plan.remove; REQUIRE(remove_plan.size() == 3); @@ -1022,7 +1022,7 @@ TEST_CASE ("features depend core remove scheme", "[plan]") pghs.push_back(make_status_pgh("cpr", "curl[core]", "", "x64")); StatusParagraphs status_db(std::move(pghs)); - auto plan = create_remove_plan({{"curl", Triplet::from_canonical_name("x64")}}, status_db); + auto plan = create_remove_plan({{"curl", Triplet::from_canonical_name("x64")}}, status_db, "test_packages_root"); auto& remove_plan = plan.remove; REQUIRE(remove_plan.size() == 2); @@ -1038,7 +1038,7 @@ TEST_CASE ("features depend core remove scheme 2", "[plan]") pghs.push_back(make_status_feature_pgh("curl", "b", "curl[a]", "x64")); StatusParagraphs status_db(std::move(pghs)); - auto plan = create_remove_plan({{"curl", Triplet::from_canonical_name("x64")}}, status_db); + auto plan = create_remove_plan({{"curl", Triplet::from_canonical_name("x64")}}, status_db, "test_packages_root"); auto& remove_plan = plan.remove; REQUIRE(remove_plan.size() == 1); @@ -1189,7 +1189,7 @@ TEST_CASE ("remove tool port scheme", "[plan]") pghs.push_back(make_status_pgh("a")); StatusParagraphs status_db(std::move(pghs)); - auto plan = create_remove_plan({{"a", Test::X86_WINDOWS}}, status_db); + auto plan = create_remove_plan({{"a", Test::X86_WINDOWS}}, status_db, "test_packages_root"); auto& remove_plan = plan.remove; REQUIRE(remove_plan.size() == 1); diff --git a/src/vcpkg-test/spdx.cpp b/src/vcpkg-test/spdx.cpp index c23a466c4c..6f790f8453 100644 --- a/src/vcpkg-test/spdx.cpp +++ b/src/vcpkg-test/spdx.cpp @@ -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"; @@ -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"; @@ -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"; diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index a50aac133d..cdcdaa257c 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -507,7 +507,8 @@ namespace vcpkg::Commands::CI else { StatusParagraphs status_db = database_load_check(paths.get_filesystem(), paths.installed()); - auto already_installed = SetInstalled::adjust_action_plan_to_status_db(action_plan, status_db); + auto already_installed = + SetInstalled::adjust_action_plan_to_status_db(action_plan, status_db, paths.packages()); Util::erase_if(already_installed, [&](auto& spec) { return Util::Sets::contains(split_specs->known, spec); }); if (!already_installed.empty()) diff --git a/src/vcpkg/commands.install.cpp b/src/vcpkg/commands.install.cpp index f27aecb60c..a6c135da9a 100644 --- a/src/vcpkg/commands.install.cpp +++ b/src/vcpkg/commands.install.cpp @@ -539,7 +539,7 @@ namespace vcpkg auto& fs = paths.get_filesystem(); for (auto&& action : action_plan.remove_actions) { - fs.remove_all(paths.package_dir(action.spec), VCPKG_LINE_INFO); + fs.remove_all(action.package_dir, VCPKG_LINE_INFO); } for (auto&& action : action_plan.install_actions) diff --git a/src/vcpkg/commands.remove.cpp b/src/vcpkg/commands.remove.cpp index 82ced08097..ea5c17c789 100644 --- a/src/vcpkg/commands.remove.cpp +++ b/src/vcpkg/commands.remove.cpp @@ -232,7 +232,7 @@ namespace vcpkg::Remove const bool is_recursive = Util::Sets::contains(options.switches, OPTION_RECURSE); const bool dry_run = Util::Sets::contains(options.switches, OPTION_DRY_RUN); - const auto plan = create_remove_plan(specs, status_db); + const auto plan = create_remove_plan(specs, status_db, paths.packages()); if (plan.empty()) { @@ -302,7 +302,7 @@ namespace vcpkg::Remove remove_package(fs, paths.installed(), action.spec, status_db); if (purge == Purge::YES) { - fs.remove_all(paths.package_dir(action.spec), VCPKG_LINE_INFO); + fs.remove_all(action.package_dir, VCPKG_LINE_INFO); } } diff --git a/src/vcpkg/commands.set-installed.cpp b/src/vcpkg/commands.set-installed.cpp index d17c5c4617..fd180ce048 100644 --- a/src/vcpkg/commands.set-installed.cpp +++ b/src/vcpkg/commands.set-installed.cpp @@ -42,7 +42,9 @@ namespace vcpkg::Commands::SetInstalled nullptr, }; - std::set adjust_action_plan_to_status_db(ActionPlan& action_plan, const StatusParagraphs& status_db) + std::set adjust_action_plan_to_status_db(ActionPlan& action_plan, + const StatusParagraphs& status_db, + const Path& packages_dir) { std::set all_abis; for (const auto& action : action_plan.install_actions) @@ -67,7 +69,7 @@ namespace vcpkg::Commands::SetInstalled specs_installed.emplace(status_pgh->package.spec); } } - action_plan.remove_actions = create_remove_plan(specs_to_remove, status_db).remove; + action_plan.remove_actions = create_remove_plan(specs_to_remove, status_db, packages_dir).remove; for (const auto& action : action_plan.remove_actions) { @@ -111,7 +113,7 @@ namespace vcpkg::Commands::SetInstalled // currently (or once) installed specifications auto status_db = database_load_check(fs, paths.installed()); - adjust_action_plan_to_status_db(action_plan, status_db); + adjust_action_plan_to_status_db(action_plan, status_db, paths.packages()); print_plan(action_plan, true, paths.builtin_ports_directory()); diff --git a/src/vcpkg/dependencies.cpp b/src/vcpkg/dependencies.cpp index 45ce1cdf31..94ff3bbe96 100644 --- a/src/vcpkg/dependencies.cpp +++ b/src/vcpkg/dependencies.cpp @@ -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 specs, UnsupportedPortAction unsupported_port_action); @@ -306,6 +307,7 @@ namespace vcpkg const CMakeVars::CMakeVarProvider& m_var_provider; std::unique_ptr m_graph; + Path m_packages_dir; std::map m_unsupported_features; }; @@ -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>&& dependencies, @@ -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()) { } @@ -532,8 +536,8 @@ namespace vcpkg NotInstalledAction::NotInstalledAction(const PackageSpec& spec) : BasicAction{spec} { } - RemovePlanAction::RemovePlanAction(const PackageSpec& spec, RequestType request_type) - : BasicAction{spec}, request_type(request_type) + RemovePlanAction::RemovePlanAction(const PackageSpec& spec, RequestType request_type, const Path& packages_dir) + : BasicAction{spec}, request_type(request_type), package_dir(packages_dir / spec.dir()) { } @@ -608,7 +612,9 @@ namespace vcpkg return Util::find_if(remove, non_user_requested) != remove.end(); } - RemovePlan create_remove_plan(const std::vector& specs, const StatusParagraphs& status_db) + RemovePlan create_remove_plan(const std::vector& specs, + const StatusParagraphs& status_db, + const Path& packages_dir) { struct RemoveAdjacencyProvider final : AdjacencyProvider { @@ -644,7 +650,8 @@ namespace vcpkg // installed plan.remove.emplace_back(step, Util::Sets::contains(requested, step) ? RequestType::USER_REQUESTED - : RequestType::AUTO_SELECTED); + : RequestType::AUTO_SELECTED, + packages_dir); } else { @@ -707,7 +714,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 feature_specs; for (const FullPackageSpec& spec : specs) @@ -719,17 +726,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, @@ -927,7 +924,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); @@ -1001,7 +998,7 @@ namespace vcpkg for (auto&& p_cluster : remove_toposort) { - plan.remove_actions.emplace_back(p_cluster->m_spec, p_cluster->request_type); + plan.remove_actions.emplace_back(p_cluster->m_spec, p_cluster->request_type, m_packages_dir); } for (auto&& p_cluster : insert_toposort) @@ -1044,21 +1041,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), @@ -1113,8 +1122,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) { } @@ -1272,12 +1284,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) { } @@ -1294,6 +1308,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 { @@ -1784,6 +1799,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), @@ -1901,24 +1917,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); } } diff --git a/src/vcpkg/postbuildlint.cpp b/src/vcpkg/postbuildlint.cpp index 8c7c3d4924..57e2251bbb 100644 --- a/src/vcpkg/postbuildlint.cpp +++ b/src/vcpkg/postbuildlint.cpp @@ -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{})) { @@ -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); From 011c51278dca3bdecb6ceda9e0f32c5d61ee9cf9 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Tue, 20 Jun 2023 00:02:13 -0700 Subject: [PATCH 2/5] Build fix --- src/vcpkg-test/dependencies.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vcpkg-test/dependencies.cpp b/src/vcpkg-test/dependencies.cpp index 173e1b1114..65ef5f8f19 100644 --- a/src/vcpkg-test/dependencies.cpp +++ b/src/vcpkg-test/dependencies.cpp @@ -2378,7 +2378,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)); From 0be75b33d61954421995a85220d1676cf823b687 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Tue, 20 Jun 2023 14:53:37 -0700 Subject: [PATCH 3/5] Fix misspelled $deployment in bundles.ps1 test, damaged in https://github.com/microsoft/vcpkg-tool/pull/931 --- azure-pipelines/end-to-end-tests-dir/bundles.ps1 | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/azure-pipelines/end-to-end-tests-dir/bundles.ps1 b/azure-pipelines/end-to-end-tests-dir/bundles.ps1 index 71c0cf31c7..13ee72d4e7 100644 --- a/azure-pipelines/end-to-end-tests-dir/bundles.ps1 +++ b/azure-pipelines/end-to-end-tests-dir/bundles.ps1 @@ -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 @@ -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 @@ -184,7 +184,7 @@ 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 @@ -192,7 +192,7 @@ 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 From dc0b15ddcd4cbe39a96dde6964f506808174fb21 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Tue, 20 Jun 2023 20:30:01 -0700 Subject: [PATCH 4/5] Add upgrade end to end smoke test. --- .../end-to-end-tests-dir/upgrade.ps1 | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 azure-pipelines/end-to-end-tests-dir/upgrade.ps1 diff --git a/azure-pipelines/end-to-end-tests-dir/upgrade.ps1 b/azure-pipelines/end-to-end-tests-dir/upgrade.ps1 new file mode 100644 index 0000000000..dacd0be1d3 --- /dev/null +++ b/azure-pipelines/end-to-end-tests-dir/upgrade.ps1 @@ -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 +} From 116fafe226d3ec8d975de90bee400a7fa5bab599 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 21 Jun 2023 15:39:53 -0700 Subject: [PATCH 5/5] Revert adding package_dir to RemovePlanAction. --- include/vcpkg/commands.set-installed.h | 4 +--- include/vcpkg/dependencies.h | 7 ++----- src/vcpkg-test/dependencies.cpp | 7 ++++--- src/vcpkg-test/plan.cpp | 16 ++++++++-------- src/vcpkg/commands.ci.cpp | 3 +-- src/vcpkg/commands.install.cpp | 2 +- src/vcpkg/commands.remove.cpp | 4 ++-- src/vcpkg/commands.set-installed.cpp | 8 +++----- src/vcpkg/dependencies.cpp | 13 +++++-------- 9 files changed, 27 insertions(+), 37 deletions(-) diff --git a/include/vcpkg/commands.set-installed.h b/include/vcpkg/commands.set-installed.h index 92cf1cb489..43e0fe74c7 100644 --- a/include/vcpkg/commands.set-installed.h +++ b/include/vcpkg/commands.set-installed.h @@ -30,9 +30,7 @@ namespace vcpkg::Commands::SetInstalled * @param status_db The status db of the installed folder * @returns A set of PackageSpec's that are already installed */ - std::set adjust_action_plan_to_status_db(ActionPlan& action_plan, - const StatusParagraphs& status_db, - const Path& packages_dir); + std::set adjust_action_plan_to_status_db(ActionPlan& action_plan, const StatusParagraphs& status_db); void perform_and_exit_ex(const VcpkgCmdArguments& args, const VcpkgPaths& paths, diff --git a/include/vcpkg/dependencies.h b/include/vcpkg/dependencies.h index 0abe4d7e39..7682a42de1 100644 --- a/include/vcpkg/dependencies.h +++ b/include/vcpkg/dependencies.h @@ -101,10 +101,9 @@ namespace vcpkg struct RemovePlanAction : BasicAction { - RemovePlanAction(const PackageSpec& spec, RequestType rt, const Path& package_dir); + RemovePlanAction(const PackageSpec& spec, RequestType rt); RequestType request_type; - Path package_dir; }; struct ActionPlan @@ -169,9 +168,7 @@ namespace vcpkg std::vector remove; }; - RemovePlan create_remove_plan(const std::vector& specs, - const StatusParagraphs& status_db, - const Path& packages_dir); + RemovePlan create_remove_plan(const std::vector& specs, const StatusParagraphs& status_db); std::vector create_export_plan(const std::vector& specs, const StatusParagraphs& status_db); diff --git a/src/vcpkg-test/dependencies.cpp b/src/vcpkg-test/dependencies.cpp index 65ef5f8f19..93aa2d63a6 100644 --- a/src/vcpkg-test/dependencies.cpp +++ b/src/vcpkg-test/dependencies.cpp @@ -2275,10 +2275,11 @@ TEST_CASE ("formatting plan 1", "[dependencies]") auto& scfl_c = vp.emplace("c", {"1", 0}); auto& scfl_f = vp.emplace("f", {"1", 0}); + 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); + const Path pr = "packages_root"; - const RemovePlanAction remove_b({"b", Test::X64_OSX}, RequestType::USER_REQUESTED, pr); - const RemovePlanAction remove_a({"a", Test::X64_OSX}, RequestType::USER_REQUESTED, pr); - const RemovePlanAction remove_c({"c", Test::X64_OSX}, RequestType::AUTO_SELECTED, pr); InstallPlanAction install_a( {"a", Test::X64_OSX}, scfl_a, pr, RequestType::AUTO_SELECTED, Test::X64_ANDROID, {}, {}); InstallPlanAction install_b( diff --git a/src/vcpkg-test/plan.cpp b/src/vcpkg-test/plan.cpp index 6b64a988cf..35ae4c036b 100644 --- a/src/vcpkg-test/plan.cpp +++ b/src/vcpkg-test/plan.cpp @@ -942,7 +942,7 @@ TEST_CASE ("basic remove scheme", "[plan]") pghs.push_back(make_status_pgh("a")); StatusParagraphs status_db(std::move(pghs)); - auto plan = create_remove_plan({{"a", Test::X86_WINDOWS}}, status_db, "test_packages_root"); + auto plan = create_remove_plan({{"a", Test::X86_WINDOWS}}, status_db); auto& remove_plan = plan.remove; REQUIRE(remove_plan.size() == 1); REQUIRE(remove_plan.at(0).spec.name() == "a"); @@ -955,7 +955,7 @@ TEST_CASE ("recurse remove scheme", "[plan]") pghs.push_back(make_status_pgh("b", "a")); StatusParagraphs status_db(std::move(pghs)); - auto plan = create_remove_plan({{"a", Test::X86_WINDOWS}}, status_db, "test_packages_root"); + auto plan = create_remove_plan({{"a", Test::X86_WINDOWS}}, status_db); auto& remove_plan = plan.remove; REQUIRE(remove_plan.size() == 2); @@ -971,7 +971,7 @@ TEST_CASE ("features depend remove scheme", "[plan]") pghs.push_back(make_status_feature_pgh("b", "0", "a")); StatusParagraphs status_db(std::move(pghs)); - auto plan = create_remove_plan({{"a", Test::X86_WINDOWS}}, status_db, "test_packages_root"); + auto plan = create_remove_plan({{"a", Test::X86_WINDOWS}}, status_db); auto& remove_plan = plan.remove; REQUIRE(remove_plan.size() == 2); @@ -988,7 +988,7 @@ TEST_CASE ("features depend remove scheme once removed", "[plan]") pghs.push_back(make_status_feature_pgh("opencv", "vtk", "vtk")); StatusParagraphs status_db(std::move(pghs)); - auto plan = create_remove_plan({{"expat", Test::X86_WINDOWS}}, status_db, "test_packages_root"); + auto plan = create_remove_plan({{"expat", Test::X86_WINDOWS}}, status_db); auto& remove_plan = plan.remove; REQUIRE(remove_plan.size() == 3); @@ -1006,7 +1006,7 @@ TEST_CASE ("features depend remove scheme once removed x64", "[plan]") pghs.push_back(make_status_feature_pgh("opencv", "vtk", "vtk", "x64")); StatusParagraphs status_db(std::move(pghs)); - auto plan = create_remove_plan({{"expat", Triplet::from_canonical_name("x64")}}, status_db, "test_packages_root"); + auto plan = create_remove_plan({{"expat", Triplet::from_canonical_name("x64")}}, status_db); auto& remove_plan = plan.remove; REQUIRE(remove_plan.size() == 3); @@ -1022,7 +1022,7 @@ TEST_CASE ("features depend core remove scheme", "[plan]") pghs.push_back(make_status_pgh("cpr", "curl[core]", "", "x64")); StatusParagraphs status_db(std::move(pghs)); - auto plan = create_remove_plan({{"curl", Triplet::from_canonical_name("x64")}}, status_db, "test_packages_root"); + auto plan = create_remove_plan({{"curl", Triplet::from_canonical_name("x64")}}, status_db); auto& remove_plan = plan.remove; REQUIRE(remove_plan.size() == 2); @@ -1038,7 +1038,7 @@ TEST_CASE ("features depend core remove scheme 2", "[plan]") pghs.push_back(make_status_feature_pgh("curl", "b", "curl[a]", "x64")); StatusParagraphs status_db(std::move(pghs)); - auto plan = create_remove_plan({{"curl", Triplet::from_canonical_name("x64")}}, status_db, "test_packages_root"); + auto plan = create_remove_plan({{"curl", Triplet::from_canonical_name("x64")}}, status_db); auto& remove_plan = plan.remove; REQUIRE(remove_plan.size() == 1); @@ -1189,7 +1189,7 @@ TEST_CASE ("remove tool port scheme", "[plan]") pghs.push_back(make_status_pgh("a")); StatusParagraphs status_db(std::move(pghs)); - auto plan = create_remove_plan({{"a", Test::X86_WINDOWS}}, status_db, "test_packages_root"); + auto plan = create_remove_plan({{"a", Test::X86_WINDOWS}}, status_db); auto& remove_plan = plan.remove; REQUIRE(remove_plan.size() == 1); diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index cdcdaa257c..a50aac133d 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -507,8 +507,7 @@ namespace vcpkg::Commands::CI else { StatusParagraphs status_db = database_load_check(paths.get_filesystem(), paths.installed()); - auto already_installed = - SetInstalled::adjust_action_plan_to_status_db(action_plan, status_db, paths.packages()); + auto already_installed = SetInstalled::adjust_action_plan_to_status_db(action_plan, status_db); Util::erase_if(already_installed, [&](auto& spec) { return Util::Sets::contains(split_specs->known, spec); }); if (!already_installed.empty()) diff --git a/src/vcpkg/commands.install.cpp b/src/vcpkg/commands.install.cpp index a6c135da9a..f27aecb60c 100644 --- a/src/vcpkg/commands.install.cpp +++ b/src/vcpkg/commands.install.cpp @@ -539,7 +539,7 @@ namespace vcpkg auto& fs = paths.get_filesystem(); for (auto&& action : action_plan.remove_actions) { - fs.remove_all(action.package_dir, VCPKG_LINE_INFO); + fs.remove_all(paths.package_dir(action.spec), VCPKG_LINE_INFO); } for (auto&& action : action_plan.install_actions) diff --git a/src/vcpkg/commands.remove.cpp b/src/vcpkg/commands.remove.cpp index ea5c17c789..82ced08097 100644 --- a/src/vcpkg/commands.remove.cpp +++ b/src/vcpkg/commands.remove.cpp @@ -232,7 +232,7 @@ namespace vcpkg::Remove const bool is_recursive = Util::Sets::contains(options.switches, OPTION_RECURSE); const bool dry_run = Util::Sets::contains(options.switches, OPTION_DRY_RUN); - const auto plan = create_remove_plan(specs, status_db, paths.packages()); + const auto plan = create_remove_plan(specs, status_db); if (plan.empty()) { @@ -302,7 +302,7 @@ namespace vcpkg::Remove remove_package(fs, paths.installed(), action.spec, status_db); if (purge == Purge::YES) { - fs.remove_all(action.package_dir, VCPKG_LINE_INFO); + fs.remove_all(paths.package_dir(action.spec), VCPKG_LINE_INFO); } } diff --git a/src/vcpkg/commands.set-installed.cpp b/src/vcpkg/commands.set-installed.cpp index d9d8441a43..a546bff5fc 100644 --- a/src/vcpkg/commands.set-installed.cpp +++ b/src/vcpkg/commands.set-installed.cpp @@ -119,9 +119,7 @@ namespace vcpkg::Commands::SetInstalled return nullopt; } - std::set adjust_action_plan_to_status_db(ActionPlan& action_plan, - const StatusParagraphs& status_db, - const Path& packages_dir) + std::set adjust_action_plan_to_status_db(ActionPlan& action_plan, const StatusParagraphs& status_db) { std::set all_abis; for (const auto& action : action_plan.install_actions) @@ -146,7 +144,7 @@ namespace vcpkg::Commands::SetInstalled specs_installed.emplace(status_pgh->package.spec); } } - action_plan.remove_actions = create_remove_plan(specs_to_remove, status_db, packages_dir).remove; + action_plan.remove_actions = create_remove_plan(specs_to_remove, status_db).remove; for (const auto& action : action_plan.remove_actions) { @@ -201,7 +199,7 @@ namespace vcpkg::Commands::SetInstalled // currently (or once) installed specifications auto status_db = database_load_check(fs, paths.installed()); - adjust_action_plan_to_status_db(action_plan, status_db, paths.packages()); + adjust_action_plan_to_status_db(action_plan, status_db); print_plan(action_plan, true, paths.builtin_ports_directory()); diff --git a/src/vcpkg/dependencies.cpp b/src/vcpkg/dependencies.cpp index 94ff3bbe96..e759bb5390 100644 --- a/src/vcpkg/dependencies.cpp +++ b/src/vcpkg/dependencies.cpp @@ -536,8 +536,8 @@ namespace vcpkg NotInstalledAction::NotInstalledAction(const PackageSpec& spec) : BasicAction{spec} { } - RemovePlanAction::RemovePlanAction(const PackageSpec& spec, RequestType request_type, const Path& packages_dir) - : BasicAction{spec}, request_type(request_type), package_dir(packages_dir / spec.dir()) + RemovePlanAction::RemovePlanAction(const PackageSpec& spec, RequestType request_type) + : BasicAction{spec}, request_type(request_type) { } @@ -612,9 +612,7 @@ namespace vcpkg return Util::find_if(remove, non_user_requested) != remove.end(); } - RemovePlan create_remove_plan(const std::vector& specs, - const StatusParagraphs& status_db, - const Path& packages_dir) + RemovePlan create_remove_plan(const std::vector& specs, const StatusParagraphs& status_db) { struct RemoveAdjacencyProvider final : AdjacencyProvider { @@ -650,8 +648,7 @@ namespace vcpkg // installed plan.remove.emplace_back(step, Util::Sets::contains(requested, step) ? RequestType::USER_REQUESTED - : RequestType::AUTO_SELECTED, - packages_dir); + : RequestType::AUTO_SELECTED); } else { @@ -998,7 +995,7 @@ namespace vcpkg for (auto&& p_cluster : remove_toposort) { - plan.remove_actions.emplace_back(p_cluster->m_spec, p_cluster->request_type, m_packages_dir); + plan.remove_actions.emplace_back(p_cluster->m_spec, p_cluster->request_type); } for (auto&& p_cluster : insert_toposort)