Skip to content

Commit

Permalink
Embed package_dir in InstallPlanAction (#1040)
Browse files Browse the repository at this point in the history 
* Embed package_dir in InstallPlanAction

* Fix build

* Fix test

* Fix tests

* Address PR comments
  • Loading branch information
@ras0219-msft
ras0219-msft committed May 1, 2023
1 parent 3beed78 commit 57808ea
Show file tree
Hide file tree
Showing 16 changed files with 151 additions and 80 deletions.
15 changes: 15 additions & 0 deletions include/vcpkg-test/util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,21 @@ namespace vcpkg
{
return os << "LL" << std::quoted(value.data());
}

inline std::ostream& operator<<(std::ostream& os, const Path& value) { return os << value.native(); }

template<class T>
inline auto operator<<(std::ostream& os, const Optional<T>& value) -> decltype(os << *(value.get()))
{
if (auto v = value.get())
{
return os << *v;
}
else
{
return os << "nullopt";
}
}
}

namespace vcpkg::Test
Expand Down
2 changes: 1 addition & 1 deletion include/vcpkg/binarycaching.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ namespace vcpkg
RestoreResult try_restore(const InstallPlanAction& action);

/// Called upon a successful build of `action` to store those contents in the binary cache.
void push_success(const InstallPlanAction& action, Path package_dir);
void push_success(const InstallPlanAction& action);

/// Gives the IBinaryProvider an opportunity to batch any downloading or server communication for
/// executing `actions`.
Expand Down
18 changes: 10 additions & 8 deletions include/vcpkg/dependencies.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include <vcpkg/commands.build.h>
#include <vcpkg/packagespec.h>
#include <vcpkg/statusparagraph.h>

#include <functional>
#include <map>
Expand Down Expand Up @@ -87,7 +88,9 @@ namespace vcpkg
std::vector<LocalizedString> build_failure_messages;
Triplet host_triplet;

// only valid with source_control_file_and_location
Optional<AbiInfo> abi_info;
Optional<Path> package_dir;
};

struct NotInstalledAction : BasicAction
Expand Down Expand Up @@ -144,15 +147,15 @@ namespace vcpkg

struct CreateInstallPlanOptions
{
CreateInstallPlanOptions(GraphRandomizer* r, Triplet t) : randomizer(r), host_triplet(t) { }
CreateInstallPlanOptions(Triplet t, UnsupportedPortAction action = UnsupportedPortAction::Error)
: host_triplet(t), unsupported_port_action(action)
CreateInstallPlanOptions(Triplet t, const Path& p, UnsupportedPortAction action = UnsupportedPortAction::Error)
: host_triplet(t), packages_dir(p), unsupported_port_action(action)
{
}

GraphRandomizer* randomizer = nullptr;
Triplet host_triplet;
UnsupportedPortAction unsupported_port_action = UnsupportedPortAction::Warn;
Path packages_dir;
UnsupportedPortAction unsupported_port_action;
};

struct RemovePlan
Expand All @@ -177,13 +180,13 @@ namespace vcpkg
const CMakeVars::CMakeVarProvider& var_provider,
View<FullPackageSpec> specs,
const StatusParagraphs& status_db,
const CreateInstallPlanOptions& options = {Triplet{}});
const CreateInstallPlanOptions& options);

ActionPlan create_upgrade_plan(const PortFileProvider& provider,
const CMakeVars::CMakeVarProvider& var_provider,
const std::vector<PackageSpec>& specs,
const StatusParagraphs& status_db,
const CreateInstallPlanOptions& options = {Triplet{}});
const CreateInstallPlanOptions& options);

ExpectedL<ActionPlan> create_versioned_install_plan(const IVersionedPortfileProvider& vprovider,
const IBaselineProvider& bprovider,
Expand All @@ -192,8 +195,7 @@ namespace vcpkg
const std::vector<Dependency>& deps,
const std::vector<DependencyOverride>& overrides,
const PackageSpec& toplevel,
Triplet host_triplet,
UnsupportedPortAction unsupported_port_action);
const CreateInstallPlanOptions& options);

struct FormattedPlan
{
Expand Down
3 changes: 2 additions & 1 deletion src/vcpkg-test/binarycaching.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -393,9 +393,10 @@ Version: 1.5
std::map<std::string, std::vector<FeatureSpec>>{},
std::vector<LocalizedString>{});
InstallPlanAction& ipa_without_abi = install_plan.back();
ipa_without_abi.package_dir = "pkgs/someheadpackage";

// test that the binary cache does the right thing. See also CHECKs etc. in KnowNothingBinaryProvider
uut.push_success(ipa_without_abi, {}); // should have no effects
uut.push_success(ipa_without_abi); // should have no effects
CHECK(uut.try_restore(ipa_without_abi) == RestoreResult::unavailable);
uut.prefetch(install_plan); // should have no effects
}
Expand Down
6 changes: 2 additions & 4 deletions src/vcpkg-test/dependencies.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,7 @@ static ExpectedL<ActionPlan> create_versioned_install_plan(const IVersionedPortf
deps,
overrides,
toplevel,
Test::ARM_UWP,
UnsupportedPortAction::Error);
{Test::ARM_UWP, "pkgs", UnsupportedPortAction::Error});
}

static ExpectedL<ActionPlan> create_versioned_install_plan(const IVersionedPortfileProvider& provider,
Expand All @@ -266,8 +265,7 @@ static ExpectedL<ActionPlan> create_versioned_install_plan(const IVersionedPortf
deps,
overrides,
toplevel,
Test::ARM_UWP,
UnsupportedPortAction::Error);
{Test::ARM_UWP, "pkgs", UnsupportedPortAction::Error});
}

TEST_CASE ("basic version install single", "[versionplan]")
Expand Down
53 changes: 38 additions & 15 deletions src/vcpkg-test/plan.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,34 @@ static void remove_plan_check(RemovePlanAction& plan, std::string pkg_name, Trip
REQUIRE(pkg_name == plan.spec.name());
}

static ActionPlan create_feature_install_plan(const PortFileProvider& port_provider,
const CMakeVars::CMakeVarProvider& var_provider,
View<FullPackageSpec> specs,
const StatusParagraphs& status_db)
{
const CreateInstallPlanOptions create_options{Test::X64_ANDROID, "pkg"};
return create_feature_install_plan(port_provider, var_provider, specs, status_db, create_options);
}

static ActionPlan create_feature_install_plan(const PortFileProvider& port_provider,
const CMakeVars::CMakeVarProvider& var_provider,
View<FullPackageSpec> specs,
const StatusParagraphs& status_db,
Triplet host_triplet)
{
const CreateInstallPlanOptions create_options{host_triplet, "pkg"};
return create_feature_install_plan(port_provider, var_provider, specs, status_db, create_options);
}

static ActionPlan create_upgrade_plan(const PortFileProvider& provider,
const CMakeVars::CMakeVarProvider& var_provider,
const std::vector<PackageSpec>& specs,
const StatusParagraphs& status_db)
{
const CreateInstallPlanOptions create_options{Test::X64_ANDROID, "pkg"};
return create_upgrade_plan(provider, var_provider, specs, status_db, create_options);
}

TEST_CASE ("basic install scheme", "[plan]")
{
std::vector<std::unique_ptr<StatusParagraph>> status_paragraphs;
Expand Down Expand Up @@ -1028,16 +1056,16 @@ TEST_CASE ("self-referencing scheme", "[plan]")

SECTION ("basic")
{
auto install_plan = create_feature_install_plan(
map_port, var_provider, Test::parse_test_fspecs("a"), {}, {{}, Test::X64_WINDOWS});
auto install_plan =
create_feature_install_plan(map_port, var_provider, Test::parse_test_fspecs("a"), {}, Test::X64_WINDOWS);

REQUIRE(install_plan.size() == 1);
REQUIRE(install_plan.install_actions.at(0).spec == spec_a);
}
SECTION ("qualified")
{
auto install_plan = create_feature_install_plan(
map_port, var_provider, Test::parse_test_fspecs("b"), {}, {{}, Test::X64_WINDOWS});
auto install_plan =
create_feature_install_plan(map_port, var_provider, Test::parse_test_fspecs("b"), {}, Test::X64_WINDOWS);

REQUIRE(install_plan.size() == 1);
REQUIRE(install_plan.install_actions.at(0).spec == spec_b);
Expand All @@ -1062,7 +1090,7 @@ TEST_CASE ("basic tool port scheme", "[plan]")
var_provider,
Test::parse_test_fspecs("a"),
StatusParagraphs(std::move(status_paragraphs)),
{{}, Test::X64_WINDOWS});
Test::X64_WINDOWS);

REQUIRE(install_plan.size() == 3);
REQUIRE(install_plan.install_actions.at(0).spec.name() == "c");
Expand Down Expand Up @@ -1091,8 +1119,7 @@ TEST_CASE ("basic existing tool port scheme", "[plan]")

MapPortFileProvider map_port(spec_map.map);

auto install_plan =
create_feature_install_plan(map_port, var_provider, fspecs_a, status_db, {{}, Test::X64_WINDOWS});
auto install_plan = create_feature_install_plan(map_port, var_provider, fspecs_a, status_db, Test::X64_WINDOWS);

REQUIRE(install_plan.size() == 1);
REQUIRE(install_plan.install_actions.at(0).spec == spec_a);
Expand All @@ -1107,16 +1134,14 @@ TEST_CASE ("basic existing tool port scheme", "[plan]")

MapPortFileProvider map_port(spec_map.map);

auto install_plan =
create_feature_install_plan(map_port, var_provider, fspecs_a, status_db, {{}, Test::X64_WINDOWS});
auto install_plan = create_feature_install_plan(map_port, var_provider, fspecs_a, status_db, Test::X64_WINDOWS);

REQUIRE(install_plan.size() == 2);
REQUIRE(install_plan.install_actions.at(0).spec.name() == "a");
REQUIRE(install_plan.install_actions.at(0).spec.triplet() == Test::X64_WINDOWS);
REQUIRE(install_plan.install_actions.at(1).spec == spec_a);

install_plan =
create_feature_install_plan(map_port, var_provider, fspecs_a, status_db, {{}, Test::X86_WINDOWS});
install_plan = create_feature_install_plan(map_port, var_provider, fspecs_a, status_db, Test::X86_WINDOWS);

REQUIRE(install_plan.size() == 1);
REQUIRE(install_plan.install_actions.at(0).spec == spec_a);
Expand All @@ -1132,8 +1157,7 @@ TEST_CASE ("basic existing tool port scheme", "[plan]")

MapPortFileProvider map_port(spec_map.map);

auto install_plan =
create_feature_install_plan(map_port, var_provider, fspecs_a, status_db, {{}, Test::ARM_UWP});
auto install_plan = create_feature_install_plan(map_port, var_provider, fspecs_a, status_db, Test::ARM_UWP);

REQUIRE(install_plan.size() == 2);
REQUIRE(install_plan.install_actions.at(0).spec.name() == "b");
Expand All @@ -1152,8 +1176,7 @@ TEST_CASE ("basic existing tool port scheme", "[plan]")

MapPortFileProvider map_port(spec_map.map);

auto install_plan =
create_feature_install_plan(map_port, var_provider, fspecs_a, status_db, {{}, Test::X64_WINDOWS});
auto install_plan = create_feature_install_plan(map_port, var_provider, fspecs_a, status_db, Test::X64_WINDOWS);

REQUIRE(install_plan.size() == 1);
REQUIRE(install_plan.install_actions.at(0).spec == spec_a);
Expand Down
15 changes: 10 additions & 5 deletions src/vcpkg-test/versionplan.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,19 @@ TEST_CASE ("qualified dependency", "[dependencies]")

MapPortFileProvider map_port{spec_map.map};
MockCMakeVarProvider var_provider;
var_provider.dep_info_vars[{"a", Triplet::from_canonical_name("x64-linux")}].emplace("VCPKG_CMAKE_SYSTEM_NAME",
"Linux");
var_provider.dep_info_vars[{"a", Test::X64_LINUX}].emplace("VCPKG_CMAKE_SYSTEM_NAME", "Linux");

auto plan = vcpkg::create_feature_install_plan(map_port, var_provider, Test::parse_test_fspecs("a"), {});
const CreateInstallPlanOptions create_options{Test::X64_ANDROID, "pkg"};

auto plan =
vcpkg::create_feature_install_plan(map_port, var_provider, Test::parse_test_fspecs("a"), {}, create_options);
REQUIRE(plan.install_actions.size() == 2);
REQUIRE(plan.install_actions.at(0).feature_list == std::vector<std::string>{"core"});
REQUIRE(plan.install_actions[0].package_dir == "pkg" VCPKG_PREFERRED_SEPARATOR "b_x86-windows");

auto plan2 = vcpkg::create_feature_install_plan(map_port, var_provider, Test::parse_test_fspecs("a:x64-linux"), {});
auto plan2 = vcpkg::create_feature_install_plan(
map_port, var_provider, Test::parse_test_fspecs("a:x64-linux"), {}, create_options);
REQUIRE(plan2.install_actions.size() == 2);
REQUIRE(plan2.install_actions.at(0).feature_list == std::vector<std::string>{"b1", "core"});
REQUIRE(plan2.install_actions[0].feature_list == std::vector<std::string>{"b1", "core"});
REQUIRE(plan2.install_actions[0].package_dir == "pkg" VCPKG_PREFERRED_SEPARATOR "b_x64-linux");
}
Loading

0 comments on commit 57808ea

Please sign in to comment.