Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history 
Co-authored-by: nicole mazzuca <[email protected]>
  • Loading branch information
@autoantwort @strega-nil-ms
autoantwort and strega-nil-ms committed Sep 14, 2021
1 parent 30b8e32 commit a2d563f
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 40 deletions.
10 changes: 5 additions & 5 deletions include/vcpkg/dependencies.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ namespace vcpkg

namespace vcpkg::Dependencies
{
enum class SupportExpressionAction : bool
enum class UnsupportedPortAction : bool
{
Warn,
Error,
Expand Down Expand Up @@ -166,14 +166,14 @@ namespace vcpkg::Dependencies
struct CreateInstallPlanOptions
{
CreateInstallPlanOptions(Graphs::Randomizer* r, Triplet t) : randomizer(r), host_triplet(t) { }
CreateInstallPlanOptions(Triplet t, SupportExpressionAction action = SupportExpressionAction::Error)
: host_triplet(t), support_expression_action(action)
CreateInstallPlanOptions(Triplet t, UnsupportedPortAction action = UnsupportedPortAction::Error)
: host_triplet(t), unsupported_port_action(action)
{
}

Graphs::Randomizer* randomizer = nullptr;
Triplet host_triplet;
SupportExpressionAction support_expression_action;
UnsupportedPortAction unsupported_port_action;
};

std::vector<RemovePlanAction> create_remove_plan(const std::vector<PackageSpec>& specs,
Expand Down Expand Up @@ -213,7 +213,7 @@ namespace vcpkg::Dependencies
const std::vector<DependencyOverride>& overrides,
const PackageSpec& toplevel,
Triplet host_triplet,
SupportExpressionAction support_expression_action);
UnsupportedPortAction unsupported_port_action);

void print_plan(const ActionPlan& action_plan, const bool is_recursive = true, const Path& builtin_ports_dir = {});
}
4 changes: 2 additions & 2 deletions src/vcpkg-test/dependencies.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ static ExpectedS<Dependencies::ActionPlan> create_versioned_install_plan(
overrides,
toplevel,
Test::ARM_UWP,
Dependencies::SupportExpressionAction::Error);
Dependencies::UnsupportedPortAction::Error);
}

namespace vcpkg::Dependencies
Expand All @@ -250,7 +250,7 @@ namespace vcpkg::Dependencies
overrides,
toplevel,
Test::ARM_UWP,
Dependencies::SupportExpressionAction::Error);
Dependencies::UnsupportedPortAction::Error);
}
}

Expand Down
15 changes: 7 additions & 8 deletions src/vcpkg/commands.upgrade.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,12 @@ namespace vcpkg::Commands::Upgrade

static constexpr StringLiteral OPTION_NO_DRY_RUN = "no-dry-run";
static constexpr StringLiteral OPTION_KEEP_GOING = "keep-going";
static constexpr StringLiteral OPTION_ONLY_WARN_UNSUPPORTED = "only-warn-unsupported";
static constexpr StringLiteral OPTION_ALLOW_UNSUPPORTED_PORT = "allow-unsupported";

static constexpr std::array<CommandSwitch, 3> INSTALL_SWITCHES = {{
{OPTION_NO_DRY_RUN, "Actually upgrade"},
{OPTION_KEEP_GOING, "Continue installing packages on failure"},
{OPTION_ONLY_WARN_UNSUPPORTED,
"Only issue a warning if a port or feature is unsupported regarding to support expressions"},
{OPTION_ALLOW_UNSUPPORTED_PORT, "Instead of erroring on an unsupported port, continue with a warning."},
}};

const CommandStructure COMMAND_STRUCTURE = {
Expand All @@ -55,9 +54,9 @@ namespace vcpkg::Commands::Upgrade

const bool no_dry_run = Util::Sets::contains(options.switches, OPTION_NO_DRY_RUN);
const KeepGoing keep_going = to_keep_going(Util::Sets::contains(options.switches, OPTION_KEEP_GOING));
const auto support_expression_action = Util::Sets::contains(options.switches, OPTION_ONLY_WARN_UNSUPPORTED)
? Dependencies::SupportExpressionAction::Warn
: Dependencies::SupportExpressionAction::Error;
const auto unsupported_port_action = Util::Sets::contains(options.switches, OPTION_ALLOW_UNSUPPORTED_PORT)
? Dependencies::UnsupportedPortAction::Warn
: Dependencies::UnsupportedPortAction::Error;

BinaryCache binary_cache{args};
StatusParagraphs status_db = database_load_check(paths);
Expand Down Expand Up @@ -94,7 +93,7 @@ namespace vcpkg::Commands::Upgrade
var_provider,
Util::fmap(outdated_packages, [](const Update::OutdatedPackage& package) { return package.spec; }),
status_db,
{host_triplet, support_expression_action});
{host_triplet, unsupported_port_action});
}
else
{
Expand Down Expand Up @@ -172,7 +171,7 @@ namespace vcpkg::Commands::Upgrade
if (to_upgrade.empty()) Checks::exit_success(VCPKG_LINE_INFO);

action_plan = Dependencies::create_upgrade_plan(
provider, var_provider, to_upgrade, status_db, {host_triplet, support_expression_action});
provider, var_provider, to_upgrade, status_db, {host_triplet, unsupported_port_action});
}

Checks::check_exit(VCPKG_LINE_INFO, !action_plan.empty());
Expand Down
30 changes: 15 additions & 15 deletions src/vcpkg/dependencies.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,8 @@ namespace vcpkg::Dependencies
Triplet host_triplet);
~PackageGraph();

void install(Span<const FeatureSpec> specs, SupportExpressionAction support_expression_action);
void upgrade(Span<const PackageSpec> specs, SupportExpressionAction support_expression_action);
void install(Span<const FeatureSpec> specs, UnsupportedPortAction unsupported_port_action);
void upgrade(Span<const PackageSpec> specs, UnsupportedPortAction unsupported_port_action);
void mark_user_requested(const PackageSpec& spec);

ActionPlan serialize(Graphs::Randomizer* randomizer) const;
Expand Down Expand Up @@ -761,7 +761,7 @@ namespace vcpkg::Dependencies
{
pgraph.mark_user_requested(spec.spec());
}
pgraph.install(feature_specs, options.support_expression_action);
pgraph.install(feature_specs, options.unsupported_port_action);

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

Expand Down Expand Up @@ -794,7 +794,7 @@ namespace vcpkg::Dependencies
}

/// The list of specs to install should already have default features expanded
void PackageGraph::install(Span<const FeatureSpec> specs, SupportExpressionAction support_expression_action)
void PackageGraph::install(Span<const FeatureSpec> specs, UnsupportedPortAction unsupported_port_action)
{
// We batch resolving qualified dependencies, because it's an invocation of CMake which
// takes ~150ms per call.
Expand Down Expand Up @@ -882,7 +882,7 @@ namespace vcpkg::Dependencies
spec.name(),
spec.feature(),
to_string(*supports_expression));
if (support_expression_action == SupportExpressionAction::Error)
if (unsupported_port_action == UnsupportedPortAction::Error)
{
Checks::exit_with_message(VCPKG_LINE_INFO, "Error: " + msg);
}
Expand Down Expand Up @@ -952,7 +952,7 @@ namespace vcpkg::Dependencies
}
}

void PackageGraph::upgrade(Span<const PackageSpec> specs, SupportExpressionAction support_expression_action)
void PackageGraph::upgrade(Span<const PackageSpec> specs, UnsupportedPortAction unsupported_port_action)
{
std::vector<FeatureSpec> reinstall_reqs;

Expand All @@ -961,7 +961,7 @@ namespace vcpkg::Dependencies

Util::sort_unique_erase(reinstall_reqs);

install(reinstall_reqs, support_expression_action);
install(reinstall_reqs, unsupported_port_action);
}

ActionPlan create_upgrade_plan(const PortFileProvider::PortFileProvider& port_provider,
Expand All @@ -972,7 +972,7 @@ namespace vcpkg::Dependencies
{
PackageGraph pgraph(port_provider, var_provider, status_db, options.host_triplet);

pgraph.upgrade(specs, options.support_expression_action);
pgraph.upgrade(specs, options.unsupported_port_action);

return pgraph.serialize(options.randomizer);
}
Expand Down Expand Up @@ -1281,7 +1281,7 @@ namespace vcpkg::Dependencies
void add_roots(View<Dependency> dep, const PackageSpec& toplevel);

ExpectedS<ActionPlan> finalize_extract_plan(const PackageSpec& toplevel,
SupportExpressionAction support_expression_action);
UnsupportedPortAction unsupported_port_action);

private:
const IVersionedPortfileProvider& m_ver_provider;
Expand Down Expand Up @@ -1898,7 +1898,7 @@ namespace vcpkg::Dependencies
// serializing out the final execution graph and performing all final validations (such as all required
// features being selected and present)
ExpectedS<ActionPlan> VersionedPackageGraph::finalize_extract_plan(
const PackageSpec& toplevel, SupportExpressionAction support_expression_action)
const PackageSpec& toplevel, UnsupportedPortAction unsupported_port_action)
{
if (m_errors.size() > 0)
{
Expand All @@ -1916,7 +1916,7 @@ namespace vcpkg::Dependencies
};
std::vector<Frame> stack;

auto push = [&emitted, this, &stack, support_expression_action, &ret](
auto push = [&emitted, this, &stack, unsupported_port_action, &ret](
const PackageSpec& spec,
const Versions::Version& new_ver,
const PackageSpec& origin,
Expand Down Expand Up @@ -1962,7 +1962,7 @@ namespace vcpkg::Dependencies
{
const auto msg = Strings::concat(
spec, "@", new_ver, " is only supported on '", to_string(supports_expr), "'\n");
if (support_expression_action == SupportExpressionAction::Error)
if (unsupported_port_action == UnsupportedPortAction::Error)
{
return "Error: " + msg;
}
Expand Down Expand Up @@ -1993,7 +1993,7 @@ namespace vcpkg::Dependencies
" is only supported on '",
to_string(supports_expr),
"'\n");
if (support_expression_action == SupportExpressionAction::Error)
if (unsupported_port_action == UnsupportedPortAction::Error)
{
return "Error: " + msg;
}
Expand Down Expand Up @@ -2123,12 +2123,12 @@ namespace vcpkg::Dependencies
const std::vector<DependencyOverride>& overrides,
const PackageSpec& toplevel,
Triplet host_triplet,
SupportExpressionAction support_expression_action)
UnsupportedPortAction unsupported_port_action)
{
VersionedPackageGraph vpg(provider, bprovider, oprovider, var_provider, host_triplet);
for (auto&& o : overrides)
vpg.add_override(o.name, {o.version, o.port_version});
vpg.add_roots(deps, toplevel);
return vpg.finalize_extract_plan(toplevel, support_expression_action);
return vpg.finalize_extract_plan(toplevel, unsupported_port_action);
}
}
18 changes: 8 additions & 10 deletions src/vcpkg/install.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,7 @@ namespace vcpkg::Install
static constexpr StringLiteral OPTION_MANIFEST_NO_DEFAULT_FEATURES = "x-no-default-features";
static constexpr StringLiteral OPTION_MANIFEST_FEATURE = "x-feature";
static constexpr StringLiteral OPTION_PROHIBIT_BACKCOMPAT_FEATURES = "x-prohibit-backcompat-features";
static constexpr StringLiteral OPTION_ONLY_WARN_UNSUPPORTED = "only-warn-unsupported";
static constexpr StringLiteral OPTION_ALLOW_UNSUPPORTED_PORT = "allow-unsupported";

static constexpr std::array<CommandSwitch, 15> INSTALL_SWITCHES = {{
{OPTION_DRY_RUN, "Do not actually build or install"},
Expand All @@ -543,8 +543,7 @@ namespace vcpkg::Install
{OPTION_CLEAN_DOWNLOADS_AFTER_BUILD, "Clean downloads after building each package"},
{OPTION_PROHIBIT_BACKCOMPAT_FEATURES,
"(experimental) Fail install if a package attempts to use a deprecated feature"},
{OPTION_ONLY_WARN_UNSUPPORTED,
"Only issue a warning if a port or feature is unsupported regarding to support expressions"},
{OPTION_ALLOW_UNSUPPORTED_PORT, "Instead of erroring on an unsupported port, continue with a warning."},
}};
static constexpr std::array<CommandSwitch, 15> MANIFEST_INSTALL_SWITCHES = {{
{OPTION_DRY_RUN, "Do not actually build or install"},
Expand All @@ -561,8 +560,7 @@ namespace vcpkg::Install
{OPTION_CLEAN_PACKAGES_AFTER_BUILD, "Clean packages after building each package"},
{OPTION_CLEAN_DOWNLOADS_AFTER_BUILD, "Clean downloads after building each package"},
{OPTION_MANIFEST_NO_DEFAULT_FEATURES, "Don't install the default features from the manifest."},
{OPTION_ONLY_WARN_UNSUPPORTED,
"Only issue a warning if a port or feature is unsupported regarding to support expressions"},
{OPTION_ALLOW_UNSUPPORTED_PORT, "Instead of erroring on an unsupported port, continue with a warning."},
}};

static constexpr std::array<CommandSetting, 2> INSTALL_SETTINGS = {{
Expand Down Expand Up @@ -801,9 +799,9 @@ namespace vcpkg::Install
to_keep_going(Util::Sets::contains(options.switches, OPTION_KEEP_GOING) || only_downloads);
const bool prohibit_backcompat_features =
Util::Sets::contains(options.switches, (OPTION_PROHIBIT_BACKCOMPAT_FEATURES));
const auto support_expression_action = Util::Sets::contains(options.switches, OPTION_ONLY_WARN_UNSUPPORTED)
? Dependencies::SupportExpressionAction::Warn
: Dependencies::SupportExpressionAction::Error;
const auto unsupported_port_action = Util::Sets::contains(options.switches, OPTION_ALLOW_UNSUPPORTED_PORT)
? Dependencies::UnsupportedPortAction::Warn
: Dependencies::UnsupportedPortAction::Error;

BinaryCache binary_cache;
if (!only_downloads)
Expand Down Expand Up @@ -954,7 +952,7 @@ namespace vcpkg::Install
manifest_scf.core_paragraph->overrides,
toplevel,
host_triplet,
support_expression_action)
unsupported_port_action)
.value_or_exit(VCPKG_LINE_INFO);
for (const auto& warning : install_plan.warnings)
{
Expand Down Expand Up @@ -1002,7 +1000,7 @@ namespace vcpkg::Install

// Note: action_plan will hold raw pointers to SourceControlFileLocations from this map
auto action_plan = Dependencies::create_feature_install_plan(
provider, var_provider, specs, status_db, {host_triplet, support_expression_action});
provider, var_provider, specs, status_db, {host_triplet, unsupported_port_action});

for (const auto& warning : action_plan.warnings)
{
Expand Down

0 comments on commit a2d563f

Please sign in to comment.