From c8da40f8bdb243f1ad85eaa0076e0f699e044877 Mon Sep 17 00:00:00 2001 From: Robert Schumacher Date: Fri, 3 Dec 2021 10:45:59 +0000 Subject: [PATCH 01/14] Implement depend-defaults --- include/vcpkg/dependencies.h | 9 +- include/vcpkg/packagespec.h | 10 ++ include/vcpkg/sourceparagraph.h | 1 + src/vcpkg-test/dependencies.cpp | 275 ++++++++++++++++++++++---------- src/vcpkg-test/manifests.cpp | 31 ++-- src/vcpkg-test/plan.cpp | 62 ++++++- src/vcpkg/dependencies.cpp | 53 +++--- src/vcpkg/install.cpp | 20 +-- src/vcpkg/packagespec.cpp | 23 +++ src/vcpkg/sourceparagraph.cpp | 13 +- 10 files changed, 367 insertions(+), 130 deletions(-) diff --git a/include/vcpkg/dependencies.h b/include/vcpkg/dependencies.h index 70b2e91774..2fcf3aa0f5 100644 --- a/include/vcpkg/dependencies.h +++ b/include/vcpkg/dependencies.h @@ -203,6 +203,12 @@ namespace vcpkg::Dependencies std::vector features, CMakeVars::CMakeVarProvider& var_provider); + enum class DependDefaults + { + NO, + YES, + }; + ExpectedS create_versioned_install_plan(const PortFileProvider::IVersionedPortfileProvider& vprovider, const PortFileProvider::IBaselineProvider& bprovider, const PortFileProvider::IOverlayProvider& oprovider, @@ -211,7 +217,8 @@ namespace vcpkg::Dependencies const std::vector& overrides, const PackageSpec& toplevel, Triplet host_triplet, - UnsupportedPortAction unsupported_port_action); + UnsupportedPortAction unsupported_port_action, + DependDefaults depend_defaults); void print_plan(const ActionPlan& action_plan, const bool is_recursive = true, const Path& builtin_ports_dir = {}); } diff --git a/include/vcpkg/packagespec.h b/include/vcpkg/packagespec.h index 8929e2cedd..bd2b14e338 100644 --- a/include/vcpkg/packagespec.h +++ b/include/vcpkg/packagespec.h @@ -112,6 +112,14 @@ namespace vcpkg std::vector to_feature_specs(const std::vector& default_features, const std::vector& all_features) const; + void to_feature_specs(std::vector& out, + const std::vector& default_features, + const std::vector& all_features) const; + + /// Splats out individual FeatureSpec's into out. If "core" is missing, adds "core". + /// @param with_defaults also add "default" if "core" is missing + void expand_to(std::vector& out, bool with_defaults) const; + static ExpectedS from_string(const std::string& spec_as_string, Triplet default_triplet); bool operator==(const FullPackageSpec& o) const @@ -157,6 +165,8 @@ namespace vcpkg Json::Object extra_info; + FullPackageSpec to_full_spec(Triplet target, Triplet host) const; + friend bool operator==(const Dependency& lhs, const Dependency& rhs); friend bool operator!=(const Dependency& lhs, const Dependency& rhs) { return !(lhs == rhs); } }; diff --git a/include/vcpkg/sourceparagraph.h b/include/vcpkg/sourceparagraph.h index 8eb41dbf8b..18d4df389f 100644 --- a/include/vcpkg/sourceparagraph.h +++ b/include/vcpkg/sourceparagraph.h @@ -62,6 +62,7 @@ namespace vcpkg Versions::Scheme version_scheme = Versions::Scheme::String; std::string version; int port_version = 0; + bool depend_defaults = true; std::vector description; std::vector summary; std::vector maintainers; diff --git a/src/vcpkg-test/dependencies.cpp b/src/vcpkg-test/dependencies.cpp index e36e9b1cb3..7785897c3d 100644 --- a/src/vcpkg-test/dependencies.cpp +++ b/src/vcpkg-test/dependencies.cpp @@ -85,6 +85,7 @@ struct MockVersionedPortfileProvider : PortFileProvider::IVersionedPortfileProvi } }; +using Dependencies::DependDefaults; using Versions::Constraint; using Versions::Scheme; @@ -99,16 +100,16 @@ T unwrap(ExpectedS e) return std::move(*e.get()); } -static void check_name_and_version(const Dependencies::InstallPlanAction& ipa, - StringLiteral name, - Versions::Version v, - std::initializer_list features = {}) +static void check_name_and_features(const Dependencies::InstallPlanAction& ipa, + StringLiteral name, + std::initializer_list features) { CHECK(ipa.spec.name() == name); CHECK(ipa.source_control_file_and_location.has_value()); - CHECK(ipa.feature_list.size() == features.size() + 1); { INFO("ipa.feature_list = [" << Strings::join(", ", ipa.feature_list) << "]"); + INFO("features = [" << Strings::join(", ", features) << "]"); + CHECK(ipa.feature_list.size() == features.size() + 1); for (auto&& f : features) { INFO("f = \"" << f.c_str() << "\""); @@ -116,6 +117,14 @@ static void check_name_and_version(const Dependencies::InstallPlanAction& ipa, } CHECK(Util::find(ipa.feature_list, "core") != ipa.feature_list.end()); } +} + +static void check_name_and_version(const Dependencies::InstallPlanAction& ipa, + StringLiteral name, + Versions::Version v, + std::initializer_list features = {}) +{ + check_name_and_features(ipa, name, features); if (auto scfl = ipa.source_control_file_and_location.get()) { CHECK(scfl->source_control_file->core_paragraph->version == v.text()); @@ -201,6 +210,8 @@ struct MockOverlayProvider : PortFileProvider::IOverlayProvider return it->second; } + SourceControlFileAndLocation& emplace(const std::string& name) { return emplace(name, {"1", 0}); } + virtual void load_all_control_files(std::map&) const override { Checks::unreachable(VCPKG_LINE_INFO); @@ -228,30 +239,51 @@ static ExpectedS create_versioned_install_plan( overrides, toplevel, Test::ARM_UWP, - Dependencies::UnsupportedPortAction::Error); + Dependencies::UnsupportedPortAction::Error, + DependDefaults::YES); } -namespace vcpkg::Dependencies +static ExpectedS create_versioned_install_plan( + const PortFileProvider::IVersionedPortfileProvider& provider, + const PortFileProvider::IBaselineProvider& bprovider, + const PortFileProvider::IOverlayProvider& oprovider, + const CMakeVars::CMakeVarProvider& var_provider, + const std::vector& deps, + const std::vector& overrides, + const PackageSpec& toplevel) { - static ExpectedS create_versioned_install_plan( - const PortFileProvider::IVersionedPortfileProvider& provider, - const PortFileProvider::IBaselineProvider& bprovider, - const PortFileProvider::IOverlayProvider& oprovider, - const CMakeVars::CMakeVarProvider& var_provider, - const std::vector& deps, - const std::vector& overrides, - const PackageSpec& toplevel) - { - return vcpkg::Dependencies::create_versioned_install_plan(provider, - bprovider, - oprovider, - var_provider, - deps, - overrides, - toplevel, - Test::ARM_UWP, - Dependencies::UnsupportedPortAction::Error); - } + return vcpkg::Dependencies::create_versioned_install_plan(provider, + bprovider, + oprovider, + var_provider, + deps, + overrides, + toplevel, + Test::ARM_UWP, + Dependencies::UnsupportedPortAction::Error, + Dependencies::DependDefaults::YES); +} + +static ExpectedS create_versioned_install_plan( + const PortFileProvider::IOverlayProvider& oprovider, + const std::vector& deps, + const PackageSpec& toplevel, + DependDefaults depend_defaults) +{ + MockVersionedPortfileProvider vp; + MockCMakeVarProvider var_provider; + MockBaselineProvider bp; + + return vcpkg::Dependencies::create_versioned_install_plan(vp, + bp, + oprovider, + var_provider, + deps, + {}, + toplevel, + Test::ARM_UWP, + Dependencies::UnsupportedPortAction::Error, + depend_defaults); } TEST_CASE ("basic version install single", "[versionplan]") @@ -1487,6 +1519,83 @@ TEST_CASE ("version install default features", "[versionplan]") check_name_and_version(install_plan.install_actions[0], "a", {"1", 0}, {"x"}); } +TEST_CASE ("version install depend-defaults", "[versionplan]") +{ + MockOverlayProvider op; + auto& a_scf = op.emplace("a"); + a_scf.source_control_file->core_paragraph->dependencies.push_back(Dependency{"b"}); + auto& b_scf = op.emplace("b"); + b_scf.source_control_file->core_paragraph->default_features.emplace_back("0"); + b_scf.source_control_file->feature_paragraphs.emplace_back(make_fpgh("0")); + auto& c_scf = op.emplace("c"); + auto& c0 = c_scf.source_control_file->feature_paragraphs.emplace_back(make_fpgh("0")); + c0->dependencies.push_back(Dependency{"b"}); + + auto& d_scf = op.emplace("d"); + d_scf.source_control_file->core_paragraph->dependencies.push_back(Dependency{"b", {"core"}}); + + auto& e_scf = op.emplace("e"); + e_scf.source_control_file->core_paragraph->dependencies.push_back(Dependency{"b"}); + e_scf.source_control_file->core_paragraph->depend_defaults = false; + + SECTION ("toplevel depend-defaults true") + { + auto install_plan = + unwrap(create_versioned_install_plan(op, {Dependency{"b"}}, toplevel_spec(), DependDefaults::YES)); + REQUIRE(install_plan.size() == 1); + check_name_and_features(install_plan.install_actions[0], "b", {"0"}); + } + + SECTION ("toplevel depend-defaults false") + { + auto install_plan = + unwrap(create_versioned_install_plan(op, {Dependency{"b"}}, toplevel_spec(), DependDefaults::NO)); + REQUIRE(install_plan.size() == 1); + check_name_and_features(install_plan.install_actions[0], "b", {}); + + // a -> b[default], so depend-defaults should not suppress it + install_plan = + unwrap(create_versioned_install_plan(op, {Dependency{"a"}}, toplevel_spec(), DependDefaults::NO)); + REQUIRE(install_plan.size() == 2); + check_name_and_features(install_plan.install_actions[0], "b", {"0"}); + } + + SECTION ("toplevel depend-defaults true (transitive)") + { + auto install_plan = + unwrap(create_versioned_install_plan(op, {Dependency{"d"}}, toplevel_spec(), DependDefaults::YES)); + REQUIRE(install_plan.size() == 2); + check_name_and_features(install_plan.install_actions[0], "b", {"0"}); + } + + SECTION ("toplevel depend-defaults false (transitive)") + { + auto install_plan = + unwrap(create_versioned_install_plan(op, {Dependency{"d"}}, toplevel_spec(), DependDefaults::NO)); + REQUIRE(install_plan.size() == 2); + check_name_and_features(install_plan.install_actions[0], "b", {}); + } + + SECTION ("transitive depend-defaults false") + { + SECTION ("toplevel false") + { + auto install_plan = + unwrap(create_versioned_install_plan(op, {Dependency{"e"}}, toplevel_spec(), DependDefaults::NO)); + REQUIRE(install_plan.size() == 2); + check_name_and_features(install_plan.install_actions[0], "b", {}); + } + + SECTION ("toplevel core") + { + auto install_plan = unwrap(create_versioned_install_plan( + op, {Dependency{"e"}, Dependency{"b", {"core"}}}, toplevel_spec(), DependDefaults::YES)); + REQUIRE(install_plan.size() == 2); + check_name_and_features(install_plan.install_actions[0], "b", {}); + } + } +} + TEST_CASE ("version dont install default features", "[versionplan]") { MockVersionedPortfileProvider vp; @@ -1932,8 +2041,8 @@ TEST_CASE ("version overlay ports", "[versionplan]") { const MockBaselineProvider empty_bp; - auto install_plan = unwrap(Dependencies::create_versioned_install_plan( - vp, empty_bp, oprovider, var_provider, {{"a"}}, {}, toplevel_spec())); + auto install_plan = + unwrap(create_versioned_install_plan(vp, empty_bp, oprovider, var_provider, {{"a"}}, {}, toplevel_spec())); REQUIRE(install_plan.size() == 1); check_name_and_version(install_plan.install_actions[0], "a", {"overlay", 0}); @@ -1941,8 +2050,8 @@ TEST_CASE ("version overlay ports", "[versionplan]") SECTION ("transitive") { - auto install_plan = unwrap( - Dependencies::create_versioned_install_plan(vp, bp, oprovider, var_provider, {{"b"}}, {}, toplevel_spec())); + auto install_plan = + unwrap(create_versioned_install_plan(vp, bp, oprovider, var_provider, {{"b"}}, {}, toplevel_spec())); REQUIRE(install_plan.size() == 2); check_name_and_version(install_plan.install_actions[0], "a", {"overlay", 0}); @@ -1951,8 +2060,8 @@ TEST_CASE ("version overlay ports", "[versionplan]") SECTION ("transitive constraint") { - auto install_plan = unwrap( - Dependencies::create_versioned_install_plan(vp, bp, oprovider, var_provider, {{"c"}}, {}, toplevel_spec())); + auto install_plan = + unwrap(create_versioned_install_plan(vp, bp, oprovider, var_provider, {{"c"}}, {}, toplevel_spec())); REQUIRE(install_plan.size() == 2); check_name_and_version(install_plan.install_actions[0], "a", {"overlay", 0}); @@ -1961,59 +2070,59 @@ TEST_CASE ("version overlay ports", "[versionplan]") SECTION ("none") { - auto install_plan = unwrap( - Dependencies::create_versioned_install_plan(vp, bp, oprovider, var_provider, {{"a"}}, {}, toplevel_spec())); + auto install_plan = + unwrap(create_versioned_install_plan(vp, bp, oprovider, var_provider, {{"a"}}, {}, toplevel_spec())); REQUIRE(install_plan.size() == 1); check_name_and_version(install_plan.install_actions[0], "a", {"overlay", 0}); } SECTION ("constraint") { - auto install_plan = unwrap(Dependencies::create_versioned_install_plan( - vp, - bp, - oprovider, - var_provider, - { - Dependency{"a", {}, {}, {Constraint::Type::Minimum, "1", 1}}, - }, - {}, - toplevel_spec())); + auto install_plan = + unwrap(create_versioned_install_plan(vp, + bp, + oprovider, + var_provider, + { + Dependency{"a", {}, {}, {Constraint::Type::Minimum, "1", 1}}, + }, + {}, + toplevel_spec())); REQUIRE(install_plan.size() == 1); check_name_and_version(install_plan.install_actions[0], "a", {"overlay", 0}); } SECTION ("constraint+override") { - auto install_plan = unwrap(Dependencies::create_versioned_install_plan( - vp, - bp, - oprovider, - var_provider, - { - Dependency{"a", {}, {}, {Constraint::Type::Minimum, "1", 1}}, - }, - { - DependencyOverride{"a", "2", 0}, - }, - toplevel_spec())); + auto install_plan = + unwrap(create_versioned_install_plan(vp, + bp, + oprovider, + var_provider, + { + Dependency{"a", {}, {}, {Constraint::Type::Minimum, "1", 1}}, + }, + { + DependencyOverride{"a", "2", 0}, + }, + toplevel_spec())); REQUIRE(install_plan.size() == 1); check_name_and_version(install_plan.install_actions[0], "a", {"overlay", 0}); } SECTION ("override") { - auto install_plan = unwrap(Dependencies::create_versioned_install_plan(vp, - bp, - oprovider, - var_provider, - { - Dependency{"a"}, - }, - { - DependencyOverride{"a", "2", 0}, - }, - toplevel_spec())); + auto install_plan = unwrap(create_versioned_install_plan(vp, + bp, + oprovider, + var_provider, + { + Dependency{"a"}, + }, + { + DependencyOverride{"a", "2", 0}, + }, + toplevel_spec())); REQUIRE(install_plan.size() == 1); check_name_and_version(install_plan.install_actions[0], "a", {"overlay", 0}); @@ -2042,12 +2151,12 @@ TEST_CASE ("respect supports expression", "[versionplan]") { // override from non supported to supported version MockOverlayProvider oprovider; - install_plan = Dependencies::create_versioned_install_plan( + install_plan = create_versioned_install_plan( vp, bp, oprovider, var_provider, {Dependency{"a"}}, {DependencyOverride{"a", "1", 1}}, toplevel_spec()); CHECK(install_plan.has_value()); // override from supported to non supported version bp.v["a"] = {"1", 1}; - install_plan = Dependencies::create_versioned_install_plan( + install_plan = create_versioned_install_plan( vp, bp, oprovider, var_provider, {Dependency{"a"}}, {DependencyOverride{"a", "1", 0}}, toplevel_spec()); CHECK_FALSE(install_plan.has_value()); } @@ -2080,23 +2189,23 @@ TEST_CASE ("respect supports expressions of features", "[versionplan]") { // override from non supported to supported version MockOverlayProvider oprovider; - install_plan = Dependencies::create_versioned_install_plan(vp, - bp, - oprovider, - var_provider, - {Dependency{"a", {"x"}}}, - {DependencyOverride{"a", "1", 1}}, - toplevel_spec()); + install_plan = create_versioned_install_plan(vp, + bp, + oprovider, + var_provider, + {Dependency{"a", {"x"}}}, + {DependencyOverride{"a", "1", 1}}, + toplevel_spec()); CHECK(install_plan.has_value()); // override from supported to non supported version bp.v["a"] = {"1", 1}; - install_plan = Dependencies::create_versioned_install_plan(vp, - bp, - oprovider, - var_provider, - {Dependency{"a", {"x"}}}, - {DependencyOverride{"a", "1", 0}}, - toplevel_spec()); + install_plan = create_versioned_install_plan(vp, + bp, + oprovider, + var_provider, + {Dependency{"a", {"x"}}}, + {DependencyOverride{"a", "1", 0}}, + toplevel_spec()); CHECK_FALSE(install_plan.has_value()); } } diff --git a/src/vcpkg-test/manifests.cpp b/src/vcpkg-test/manifests.cpp index e5da304be6..1ee3c7d17e 100644 --- a/src/vcpkg-test/manifests.cpp +++ b/src/vcpkg-test/manifests.cpp @@ -59,6 +59,7 @@ TEST_CASE ("manifest construct minimum", "[manifests]") REQUIRE(pgh.core_paragraph->name == "zlib"); REQUIRE(pgh.core_paragraph->version == "1.2.8"); + REQUIRE(pgh.core_paragraph->depend_defaults == true); REQUIRE(pgh.core_paragraph->maintainers.empty()); REQUIRE(pgh.core_paragraph->contacts.is_empty()); REQUIRE(pgh.core_paragraph->summary.empty()); @@ -716,31 +717,32 @@ TEST_CASE ("manifest construct maximum", "[manifests]") "summary": "d", "description": "d", "builtin-baseline": "123", + "depend-defaults": false, "dependencies": ["bd"], "default-features": ["df"], "features": { "iroh" : { "$comment": "hello", - "description": "zuko's uncle", - "dependencies": [ - "firebending", - { - "name": "order.white-lotus", - "features": [ "the-ancient-ways" ], - "platform": "!(windows & arm)" + "description": "zuko's uncle", + "dependencies": [ + "firebending", + { + "name": "order.white-lotus", + "features": [ "the-ancient-ways" ], + "platform": "!(windows & arm)" }, { "$extra": [], "$my": [], "name": "tea" - } - ] - }, - "zuko": { - "description": ["son of the fire lord", "firebending 師父"], - "supports": "!(windows & arm)" - } + } + ] + }, + "zuko": { + "description": ["son of the fire lord", "firebending 師父"], + "supports": "!(windows & arm)" } + } })json"; auto object = parse_json_object(raw); auto res = SourceControlFile::parse_manifest_object("", object); @@ -766,6 +768,7 @@ TEST_CASE ("manifest construct maximum", "[manifests]") REQUIRE(contact_a_aa->string() == "aa"); REQUIRE(pgh.core_paragraph->summary.size() == 1); REQUIRE(pgh.core_paragraph->summary[0] == "d"); + REQUIRE(pgh.core_paragraph->depend_defaults == false); REQUIRE(pgh.core_paragraph->description.size() == 1); REQUIRE(pgh.core_paragraph->description[0] == "d"); REQUIRE(pgh.core_paragraph->dependencies.size() == 1); diff --git a/src/vcpkg-test/plan.cpp b/src/vcpkg-test/plan.cpp index b9fd772a42..a4e006c15c 100644 --- a/src/vcpkg-test/plan.cpp +++ b/src/vcpkg-test/plan.cpp @@ -791,8 +791,8 @@ TEST_CASE ("install with default features", "[plan]") FullPackageSpec{b_spec, {"core"}}, }; - auto install_plan = Dependencies::create_feature_install_plan( - map_port, var_provider, full_package_specs, StatusParagraphs(std::move(status_db))); + auto install_plan = + Dependencies::create_feature_install_plan(map_port, var_provider, full_package_specs, status_db); // Install "a" and indicate that "b" should not install default features REQUIRE(install_plan.size() == 3); @@ -801,6 +801,64 @@ TEST_CASE ("install with default features", "[plan]") features_check(install_plan.install_actions.at(1), "a", {"0", "core"}); } +TEST_CASE ("install with depend-defaults false", "[plan]") +{ + StatusParagraphs status_db; + + PackageSpecMap spec_map; + auto a_spec = spec_map.emplace("a", "b"); + auto b_spec = spec_map.emplace("b", "", {{"0", ""}}, {"0"}); + auto c_spec = spec_map.emplace("c", "", {{"0", "b"}}, {"0"}); + + PortFileProvider::MapPortFileProvider map_port{spec_map.map}; + MockCMakeVarProvider var_provider; + + std::vector full_package_specs{ + FullPackageSpec{a_spec}, + FullPackageSpec{b_spec, {"core"}}, + }; + + std::vector full_package_specs2{ + FullPackageSpec{c_spec}, + FullPackageSpec{b_spec, {"core"}}, + }; + + // Install "a" and then "b" _should_ install default features + SECTION ("depend-defaults true from core") + { + auto install_plan = Dependencies::create_feature_install_plan(map_port, var_provider, full_package_specs, {}); + REQUIRE(install_plan.size() == 2); + features_check(install_plan.install_actions.at(0), "b", {"0", "core"}); + features_check(install_plan.install_actions.at(1), "a", {"core"}); + } + + SECTION ("depend-defaults true from feature") + { + auto install_plan = Dependencies::create_feature_install_plan(map_port, var_provider, full_package_specs2, {}); + REQUIRE(install_plan.size() == 2); + features_check(install_plan.install_actions.at(0), "b", {"0", "core"}); + features_check(install_plan.install_actions.at(1), "c", {"0", "core"}); + } + + // now, disable the implicit default dependency from `a` and `c[0]` + SECTION ("depend-defaults false from core") + { + spec_map.map["a"].source_control_file->core_paragraph->depend_defaults = false; + auto install_plan = Dependencies::create_feature_install_plan(map_port, var_provider, full_package_specs, {}); + REQUIRE(install_plan.size() == 2); + features_check(install_plan.install_actions.at(0), "b", {"core"}); + features_check(install_plan.install_actions.at(1), "a", {"core"}); + } + SECTION ("depend-defaults false from feature") + { + spec_map.map["c"].source_control_file->core_paragraph->depend_defaults = false; + auto install_plan = Dependencies::create_feature_install_plan(map_port, var_provider, full_package_specs2, {}); + REQUIRE(install_plan.size() == 2); + features_check(install_plan.install_actions.at(0), "b", {"core"}); + features_check(install_plan.install_actions.at(1), "c", {"0", "core"}); + } +} + TEST_CASE ("upgrade with default features 1", "[plan]") { std::vector> pghs; diff --git a/src/vcpkg/dependencies.cpp b/src/vcpkg/dependencies.cpp index 909ef6f528..87bb170ce9 100644 --- a/src/vcpkg/dependencies.cpp +++ b/src/vcpkg/dependencies.cpp @@ -93,13 +93,15 @@ namespace vcpkg::Dependencies std::vector& out_new_dependencies, Triplet host_triplet) { + const auto& scfl = get_scfl_or_exit(); + ClusterInstallInfo& info = m_install_info.value_or_exit(VCPKG_LINE_INFO); if (feature == "default") { if (!info.defaults_requested) { info.defaults_requested = true; - for (auto&& f : get_scfl_or_exit().source_control_file->core_paragraph->default_features) + for (auto&& f : scfl.source_control_file->core_paragraph->default_features) out_new_dependencies.emplace_back(m_spec, f); } return; @@ -112,7 +114,7 @@ namespace vcpkg::Dependencies } auto maybe_vars = var_provider.get_dep_info_vars(m_spec); Optional&> maybe_qualified_deps = - get_scfl_or_exit().source_control_file->find_dependencies_for_feature(feature); + scfl.source_control_file->find_dependencies_for_feature(feature); if (!maybe_qualified_deps.has_value()) { Checks::exit_with_message( @@ -120,6 +122,8 @@ namespace vcpkg::Dependencies } const std::vector* qualified_deps = &maybe_qualified_deps.value_or_exit(VCPKG_LINE_INFO); + const auto depend_defaults = scfl.source_control_file->core_paragraph->depend_defaults; + std::vector dep_list; if (auto vars = maybe_vars.get()) { @@ -128,7 +132,7 @@ namespace vcpkg::Dependencies for (auto&& fspec : fullspec_list) { - Util::Vectors::append(&dep_list, fspec.to_feature_specs({"default"}, {"default"})); + fspec.expand_to(dep_list, depend_defaults); } Util::sort_unique_erase(dep_list); @@ -141,10 +145,7 @@ namespace vcpkg::Dependencies { if (dep.platform.is_empty()) { - auto t = dep.host ? host_triplet : m_spec.triplet(); - Util::Vectors::append(&dep_list, - FullPackageSpec({dep.name, t}, dep.features) - .to_feature_specs({"default"}, {"default"})); + dep.to_full_spec(m_spec.triplet(), host_triplet).expand_to(dep_list, depend_defaults); } else { @@ -1270,12 +1271,14 @@ namespace vcpkg::Dependencies const IBaselineProvider& base_provider, const PortFileProvider::IOverlayProvider& oprovider, const CMakeVars::CMakeVarProvider& var_provider, - Triplet host_triplet) + Triplet host_triplet, + DependDefaults depend_defaults) : 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_depend_defaults(depend_defaults == DependDefaults::YES) { } @@ -1292,6 +1295,7 @@ namespace vcpkg::Dependencies const PortFileProvider::IOverlayProvider& m_o_provider; const CMakeVars::CMakeVarProvider& m_var_provider; const Triplet m_host_triplet; + const bool m_depend_defaults; struct DepSpec { @@ -1337,7 +1341,7 @@ namespace vcpkg::Dependencies Optional> semver; Optional> date; std::set requested_features; - bool default_features = true; + bool default_features; bool user_requested = false; VersionSchemeInfo* get_node(const Versions::Version& ver); @@ -1349,7 +1353,7 @@ namespace vcpkg::Dependencies // replaces the current entry for the scheme VersionSchemeInfo& emplace_node(Versions::Scheme scheme, const Versions::Version& ver); - PackageNode() = default; + explicit PackageNode(bool default_features) : default_features(default_features) { } PackageNode(const PackageNode&) = delete; PackageNode(PackageNode&&) = default; PackageNode& operator=(const PackageNode&) = delete; @@ -1389,6 +1393,7 @@ namespace vcpkg::Dependencies // the following functions will add stuff recursively void require_dependency(std::pair& ref, const Dependency& dep, + bool depend_defaults, const std::string& origin); void require_port_version(std::pair& graph_entry, const Versions::Version& ver, @@ -1577,7 +1582,10 @@ namespace vcpkg::Dependencies } else { - require_dependency(dep_node, dep, ref.first.name()); + require_dependency(dep_node, + dep, + vsi.scfl->source_control_file->core_paragraph->depend_defaults, + ref.first.name()); } p.first->second.emplace_back(dep_spec, "core"); @@ -1590,6 +1598,7 @@ namespace vcpkg::Dependencies void VersionedPackageGraph::require_dependency(std::pair& ref, const Dependency& dep, + bool depend_defaults, const std::string& origin) { auto maybe_overlay = m_o_provider.get_control_file(ref.first.name()); @@ -1624,7 +1633,7 @@ namespace vcpkg::Dependencies require_port_feature(ref, f, origin); } - if (Util::find(dep.features, StringView{"core"}) == dep.features.end()) + if (depend_defaults && Util::find(dep.features, StringView{"core"}) == dep.features.end()) { require_port_defaults(ref, origin); } @@ -1742,7 +1751,12 @@ namespace vcpkg::Dependencies std::pair& VersionedPackageGraph::emplace_package( const PackageSpec& spec) { - return *m_graph.emplace(spec, PackageNode{}).first; + auto it = m_graph.find(spec); + if (it == m_graph.end()) + { + return *m_graph.emplace(spec, m_depend_defaults).first; + } + return *it; } Optional VersionedPackageGraph::dep_to_version(const std::string& name, @@ -1961,7 +1975,7 @@ namespace vcpkg::Dependencies const Versions::Version& new_ver, const PackageSpec& origin, View features) -> Optional { - auto&& node = m_graph[spec]; + auto&& node = emplace_package(spec).second; auto overlay = m_o_provider.get_control_file(spec.name()); auto over_it = m_overrides.find(spec.name()); @@ -2125,8 +2139,10 @@ namespace vcpkg::Dependencies auto& back = stack.back(); if (back.deps.empty()) { - emitted[back.ipa.spec] = m_graph[back.ipa.spec].get_node( - to_version(*back.ipa.source_control_file_and_location.get()->source_control_file)); + emitted[back.ipa.spec] = + emplace_package(back.ipa.spec) + .second.get_node( + to_version(*back.ipa.source_control_file_and_location.get()->source_control_file)); ret.install_actions.push_back(std::move(back.ipa)); stack.pop_back(); } @@ -2153,9 +2169,10 @@ namespace vcpkg::Dependencies const std::vector& overrides, const PackageSpec& toplevel, Triplet host_triplet, - UnsupportedPortAction unsupported_port_action) + UnsupportedPortAction unsupported_port_action, + DependDefaults depend_defaults) { - VersionedPackageGraph vpg(provider, bprovider, oprovider, var_provider, host_triplet); + VersionedPackageGraph vpg(provider, bprovider, oprovider, var_provider, host_triplet, depend_defaults); for (auto&& o : overrides) vpg.add_override(o.name, {o.version, o.port_version}); vpg.add_roots(deps, toplevel); diff --git a/src/vcpkg/install.cpp b/src/vcpkg/install.cpp index d6a70ff5b9..cb781124c7 100644 --- a/src/vcpkg/install.cpp +++ b/src/vcpkg/install.cpp @@ -1008,15 +1008,17 @@ namespace vcpkg::Install } auto oprovider = PortFileProvider::make_overlay_provider(paths, extended_overlay_ports); PackageSpec toplevel{manifest_scf.core_paragraph->name, default_triplet}; - auto install_plan = Dependencies::create_versioned_install_plan(*verprovider, - *baseprovider, - *oprovider, - var_provider, - dependencies, - manifest_scf.core_paragraph->overrides, - toplevel, - host_triplet, - unsupported_port_action) + auto install_plan = Dependencies::create_versioned_install_plan( + *verprovider, + *baseprovider, + *oprovider, + var_provider, + dependencies, + manifest_scf.core_paragraph->overrides, + toplevel, + host_triplet, + unsupported_port_action, + Util::Enum::to_enum(manifest_scf.core_paragraph->depend_defaults)) .value_or_exit(VCPKG_LINE_INFO); for (const auto& warning : install_plan.warnings) { diff --git a/src/vcpkg/packagespec.cpp b/src/vcpkg/packagespec.cpp index 14692d8f2a..80b32f9c78 100644 --- a/src/vcpkg/packagespec.cpp +++ b/src/vcpkg/packagespec.cpp @@ -59,6 +59,24 @@ namespace vcpkg return feature_specs; } + void FullPackageSpec::expand_to(std::vector& out, bool with_defaults) const + { + bool core = false; + for (auto&& feature : features) + { + out.emplace_back(package_spec, feature); + core = core || feature == "core"; + } + if (!core) + { + out.emplace_back(package_spec, "core"); + if (with_defaults) + { + out.emplace_back(package_spec, "default"); + } + } + } + ExpectedS FullPackageSpec::from_string(const std::string& spec_as_string, Triplet default_triplet) { return parse_qualified_specifier(spec_as_string) @@ -267,6 +285,11 @@ namespace vcpkg } bool operator!=(const DependencyConstraint& lhs, const DependencyConstraint& rhs); + FullPackageSpec Dependency::to_full_spec(Triplet target, Triplet host_triplet) const + { + return FullPackageSpec({name, host ? host_triplet : target}, features); + } + bool operator==(const Dependency& lhs, const Dependency& rhs) { if (lhs.name != rhs.name) return false; diff --git a/src/vcpkg/sourceparagraph.cpp b/src/vcpkg/sourceparagraph.cpp index 9ce20de376..5634670b67 100644 --- a/src/vcpkg/sourceparagraph.cpp +++ b/src/vcpkg/sourceparagraph.cpp @@ -872,12 +872,13 @@ namespace vcpkg constexpr static StringLiteral DEV_DEPENDENCIES = "dev-dependencies"; constexpr static StringLiteral FEATURES = "features"; constexpr static StringLiteral DEFAULT_FEATURES = "default-features"; + constexpr static StringLiteral DEPEND_DEFAULTS = "depend-defaults"; constexpr static StringLiteral SUPPORTS = "supports"; constexpr static StringLiteral OVERRIDES = "overrides"; constexpr static StringLiteral BUILTIN_BASELINE = "builtin-baseline"; constexpr static StringLiteral VCPKG_CONFIGURATION = "vcpkg-configuration"; - virtual Span valid_fields() const override + virtual View valid_fields() const override { static const StringView u[] = { NAME, @@ -889,6 +890,7 @@ namespace vcpkg DOCUMENTATION, LICENSE, DEPENDENCIES, + DEPEND_DEFAULTS, DEV_DEPENDENCIES, FEATURES, DEFAULT_FEATURES, @@ -925,6 +927,7 @@ namespace vcpkg spgh->version_scheme = schemed_version.scheme; spgh->port_version = schemed_version.versiont.port_version(); + r.optional_object_field(obj, DEPEND_DEFAULTS, spgh->depend_defaults, Json::BooleanDeserializer::instance); r.optional_object_field(obj, MAINTAINERS, spgh->maintainers, Json::ParagraphDeserializer::instance); r.optional_object_field(obj, CONTACTS, spgh->contacts, ContactsDeserializer::instance); r.optional_object_field(obj, SUMMARY, spgh->summary, Json::ParagraphDeserializer::instance); @@ -1304,8 +1307,7 @@ namespace vcpkg { if (dep.platform.evaluate(cmake_vars)) { - Triplet t = dep.host ? host : target; - ret.emplace_back(FullPackageSpec({dep.name, t}, dep.features)); + ret.emplace_back(dep.to_full_spec(target, host)); } } return ret; @@ -1461,6 +1463,11 @@ namespace vcpkg Json::Value::string(scf.core_paragraph->builtin_baseline.value_or_exit(VCPKG_LINE_INFO))); } + if (!scf.core_paragraph->depend_defaults || debug) + { + obj.insert(ManifestDeserializer::DEPEND_DEFAULTS, Json::Value::boolean(false)); + } + if (!scf.core_paragraph->dependencies.empty() || debug) { auto& deps = obj.insert(ManifestDeserializer::DEPENDENCIES, Json::Array()); From 02a21330ccfdfd58969940db90db284f14b65e18 Mon Sep 17 00:00:00 2001 From: Robert Schumacher Date: Thu, 17 Feb 2022 23:30:07 +0000 Subject: [PATCH 02/14] Fixup after merge --- include/vcpkg/base/jsonreader.h | 18 ++++++++++++ include/vcpkg/packagespec.h | 1 + src/vcpkg-test/dependencies.cpp | 2 -- src/vcpkg-test/plan.cpp | 19 ++++++++++-- src/vcpkg/dependencies.cpp | 16 ++++++---- src/vcpkg/packagespec.cpp | 52 ++++----------------------------- src/vcpkg/sourceparagraph.cpp | 7 +---- 7 files changed, 53 insertions(+), 62 deletions(-) diff --git a/include/vcpkg/base/jsonreader.h b/include/vcpkg/base/jsonreader.h index 7e285108cd..f503202699 100644 --- a/include/vcpkg/base/jsonreader.h +++ b/include/vcpkg/base/jsonreader.h @@ -160,6 +160,24 @@ namespace vcpkg::Json } } + // returns whether key \in obj + template + bool optional_object_field(const Object& obj, + StringView key, + Optional& place, + IDeserializer& visitor) + { + if (auto value = obj.get(key)) + { + visit_in_key(*value, key, place.emplace(), visitor); + return true; + } + else + { + return false; + } + } + template Optional visit(const Value& value, IDeserializer& visitor) { diff --git a/include/vcpkg/packagespec.h b/include/vcpkg/packagespec.h index a9b9ee6cbf..9ac80629a9 100644 --- a/include/vcpkg/packagespec.h +++ b/include/vcpkg/packagespec.h @@ -153,6 +153,7 @@ namespace vcpkg PlatformExpression::Expr platform; DependencyConstraint constraint; bool host = false; + Optional default_features; Json::Object extra_info; diff --git a/src/vcpkg-test/dependencies.cpp b/src/vcpkg-test/dependencies.cpp index bfff2bab52..ae73c2361c 100644 --- a/src/vcpkg-test/dependencies.cpp +++ b/src/vcpkg-test/dependencies.cpp @@ -93,8 +93,6 @@ struct MockVersionedPortfileProvider : PortFileProvider::IVersionedPortfileProvi }; using Dependencies::DependDefaults; -using Versions::Constraint; -using Versions::Scheme; template T unwrap(ExpectedS e) diff --git a/src/vcpkg-test/plan.cpp b/src/vcpkg-test/plan.cpp index 108e3ccae9..fc255c8273 100644 --- a/src/vcpkg-test/plan.cpp +++ b/src/vcpkg-test/plan.cpp @@ -764,20 +764,27 @@ TEST_CASE ("install with depend-defaults false", "[plan]") auto a_spec = spec_map.emplace("a", "b"); auto b_spec = spec_map.emplace("b", "", {{"0", ""}}, {"0"}); auto c_spec = spec_map.emplace("c", "", {{"0", "b"}}, {"0"}); + auto d_spec = spec_map.emplace("d", "b"); + spec_map.map["d"].source_control_file->core_paragraph->depend_defaults = false; + spec_map.map["d"].source_control_file->core_paragraph->dependencies[0].default_features = true; PortFileProvider::MapPortFileProvider map_port{spec_map.map}; MockCMakeVarProvider var_provider; std::vector full_package_specs{ - FullPackageSpec{a_spec}, + FullPackageSpec{a_spec, {"core"}}, FullPackageSpec{b_spec, {"core"}}, }; std::vector full_package_specs2{ - FullPackageSpec{c_spec}, + FullPackageSpec{c_spec, {"default", "core"}}, FullPackageSpec{b_spec, {"core"}}, }; + std::vector full_package_specs_d{ + FullPackageSpec{d_spec, {"core"}}, + }; + // Install "a" and then "b" _should_ install default features SECTION ("depend-defaults true from core") { @@ -795,6 +802,14 @@ TEST_CASE ("install with depend-defaults false", "[plan]") features_check(install_plan.install_actions.at(1), "c", {"0", "core"}); } + SECTION ("depend-defaults false overridden from dependency") + { + auto install_plan = Dependencies::create_feature_install_plan(map_port, var_provider, full_package_specs_d, {}); + REQUIRE(install_plan.size() == 2); + features_check(install_plan.install_actions.at(0), "b", {"0", "core"}); + features_check(install_plan.install_actions.at(1), "d", {"core"}); + } + // now, disable the implicit default dependency from `a` and `c[0]` SECTION ("depend-defaults false from core") { diff --git a/src/vcpkg/dependencies.cpp b/src/vcpkg/dependencies.cpp index fe0159c6ce..50c7497fc9 100644 --- a/src/vcpkg/dependencies.cpp +++ b/src/vcpkg/dependencies.cpp @@ -129,12 +129,16 @@ namespace vcpkg::Dependencies if (auto vars = maybe_vars.get()) { // Qualified dependency resolution is available - auto fullspec_list = filter_dependencies( - *qualified_deps, m_spec.triplet(), host_triplet, *vars, ImplicitDefault::YES); + auto fullspec_list = + filter_dependencies(*qualified_deps, + m_spec.triplet(), + host_triplet, + *vars, + depend_defaults ? ImplicitDefault::YES : ImplicitDefault::NO); for (auto&& fspec : fullspec_list) { - fspec.expand_fspecs_to(dep_list, depend_defaults); + fspec.expand_fspecs_to(dep_list); } Util::sort_unique_erase(dep_list); @@ -147,8 +151,10 @@ namespace vcpkg::Dependencies { if (dep.platform.is_empty()) { - dep.to_full_spec(m_spec.triplet(), host_triplet, ImplicitDefault::YES) - .expand_fspecs_to(dep_list, depend_defaults); + dep.to_full_spec(m_spec.triplet(), + host_triplet, + depend_defaults ? ImplicitDefault::YES : ImplicitDefault::NO) + .expand_fspecs_to(dep_list); } else { diff --git a/src/vcpkg/packagespec.cpp b/src/vcpkg/packagespec.cpp index aa202919d6..c6eb994bd2 100644 --- a/src/vcpkg/packagespec.cpp +++ b/src/vcpkg/packagespec.cpp @@ -26,44 +26,6 @@ namespace vcpkg { out.emplace_back(package_spec, feature); } - - return feature_specs; - } - - void FullPackageSpec::expand_to(std::vector& out, bool with_defaults) const - { - bool core = false; - for (auto&& feature : features) - { - out.emplace_back(package_spec, feature); - core = core || feature == "core"; - } - if (!core) - { - out.emplace_back(package_spec, "core"); - if (with_defaults) - { - out.emplace_back(package_spec, "default"); - } - } - } - - ExpectedS FullPackageSpec::from_string(const std::string& spec_as_string, Triplet default_triplet) - { - return parse_qualified_specifier(spec_as_string) - .then([&](ParsedQualifiedSpecifier&& p) -> ExpectedS { - if (p.platform) - return "Error: platform specifier not allowed in this context: " + spec_as_string + "\n"; - auto triplet = p.triplet ? Triplet::from_canonical_name(std::move(*p.triplet.get())) : default_triplet; - return FullPackageSpec({p.name, triplet}, p.features.value_or({})); - }); - } - - std::vector PackageSpec::to_package_specs(const std::vector& ports, Triplet triplet) - { - return Util::fmap(ports, [&](const std::string& spec_as_string) -> PackageSpec { - return {spec_as_string, triplet}; - }); } const std::string& PackageSpec::name() const { return this->m_name; } @@ -86,7 +48,7 @@ namespace vcpkg "Error: Platform qualifier is not allowed in this context"); DECLARE_AND_REGISTER_MESSAGE(IllegalFeatures, (), "", "Error: List of features is not allowed in this contect"); - static InternalFeatureSet normalize_feature_list(View fs, ImplicitDefault id) + static InternalFeatureSet normalize_feature_list(View fs, bool implicit_default) { InternalFeatureSet ret; bool core = false; @@ -102,7 +64,7 @@ namespace vcpkg if (!core) { ret.emplace_back("core"); - if (id == ImplicitDefault::YES) + if (implicit_default) { ret.emplace_back("default"); } @@ -119,7 +81,7 @@ namespace vcpkg const Triplet t = triplet ? Triplet::from_canonical_name(*triplet.get()) : default_triplet; const View fs = !features.get() ? View{} : *features.get(); - return FullPackageSpec{{name, t}, normalize_feature_list(fs, id)}; + return FullPackageSpec{{name, t}, normalize_feature_list(fs, id == ImplicitDefault::YES)}; } ExpectedS ParsedQualifiedSpecifier::to_package_spec(Triplet default_triplet) const @@ -324,12 +286,8 @@ namespace vcpkg FullPackageSpec Dependency::to_full_spec(Triplet target, Triplet host_triplet, ImplicitDefault id) const { - return FullPackageSpec{{name, host ? host_triplet : target}, normalize_feature_list(features, id)}; - } - - FullPackageSpec Dependency::to_full_spec(Triplet target, Triplet host_triplet) const - { - return FullPackageSpec({name, host ? host_triplet : target}, features); + return FullPackageSpec{{name, host ? host_triplet : target}, + normalize_feature_list(features, default_features.value_or(id == ImplicitDefault::YES))}; } bool operator==(const Dependency& lhs, const Dependency& rhs) diff --git a/src/vcpkg/sourceparagraph.cpp b/src/vcpkg/sourceparagraph.cpp index 199779ae51..5edc00266a 100644 --- a/src/vcpkg/sourceparagraph.cpp +++ b/src/vcpkg/sourceparagraph.cpp @@ -561,12 +561,7 @@ namespace vcpkg r.required_object_field(type_name(), obj, NAME, dep.name, Json::PackageNameDeserializer::instance); r.optional_object_field(obj, FEATURES, dep.features, arr_id_d); - bool default_features = true; - r.optional_object_field(obj, DEFAULT_FEATURES, default_features, Json::BooleanDeserializer::instance); - if (!default_features) - { - dep.features.push_back("core"); - } + r.optional_object_field(obj, DEFAULT_FEATURES, dep.default_features, Json::BooleanDeserializer::instance); r.optional_object_field(obj, HOST, dep.host, Json::BooleanDeserializer::instance); r.optional_object_field(obj, PLATFORM, dep.platform, PlatformExprDeserializer::instance); From 4542303d49c3f516ebcb435979d2ec331fa9d9af Mon Sep 17 00:00:00 2001 From: Robert Schumacher Date: Fri, 18 Feb 2022 00:50:51 +0000 Subject: [PATCH 03/14] Fix serialization error --- src/vcpkg/sourceparagraph.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/vcpkg/sourceparagraph.cpp b/src/vcpkg/sourceparagraph.cpp index 5edc00266a..79a9012da1 100644 --- a/src/vcpkg/sourceparagraph.cpp +++ b/src/vcpkg/sourceparagraph.cpp @@ -1520,7 +1520,7 @@ namespace vcpkg static bool is_dependency_trivial(const Dependency& dep) { return dep.features.empty() && dep.platform.is_empty() && dep.extra_info.is_empty() && - dep.constraint.type == VersionConstraintKind::None && !dep.host; + dep.constraint.type == VersionConstraintKind::None && !dep.host && !dep.default_features.has_value(); } static Json::Object serialize_manifest_impl(const SourceControlFile& scf, bool debug) @@ -1584,11 +1584,16 @@ namespace vcpkg auto features_copy = dep.features; auto core_it = std::find(features_copy.begin(), features_copy.end(), "core"); + auto default_features = dep.default_features; if (core_it != features_copy.end()) { - dep_obj.insert(DependencyDeserializer::DEFAULT_FEATURES, Json::Value::boolean(false)); + default_features = false; features_copy.erase(core_it); } + if (auto b = default_features.get()) + { + dep_obj.insert(DependencyDeserializer::DEFAULT_FEATURES, Json::Value::boolean(*b)); + } serialize_optional_array(dep_obj, DependencyDeserializer::FEATURES, features_copy); serialize_optional_string(dep_obj, DependencyDeserializer::PLATFORM, to_string(dep.platform)); From e337036b4d299413968e6660066527a9b0a64052 Mon Sep 17 00:00:00 2001 From: Robert Schumacher Date: Fri, 18 Feb 2022 18:22:16 +0000 Subject: [PATCH 04/14] Fix e2e test break --- include/vcpkg/packagespec.h | 3 +++ src/vcpkg-test/dependencies.cpp | 38 +++++++++++++++++++++++++++++---- src/vcpkg/sourceparagraph.cpp | 24 +++++++++++++++++++++ 3 files changed, 61 insertions(+), 4 deletions(-) diff --git a/include/vcpkg/packagespec.h b/include/vcpkg/packagespec.h index 9ac80629a9..f5942ec0e4 100644 --- a/include/vcpkg/packagespec.h +++ b/include/vcpkg/packagespec.h @@ -149,6 +149,9 @@ namespace vcpkg struct Dependency { std::string name; + + // When created through parsing a manifest, `features` will be pre-normalized (containing 'core' and 'default' + // as appropriate) std::vector features; PlatformExpression::Expr platform; DependencyConstraint constraint; diff --git a/src/vcpkg-test/dependencies.cpp b/src/vcpkg-test/dependencies.cpp index ae73c2361c..05dedbaca0 100644 --- a/src/vcpkg-test/dependencies.cpp +++ b/src/vcpkg-test/dependencies.cpp @@ -1548,11 +1548,41 @@ TEST_CASE ("version install default features", "[versionplan]") MockBaselineProvider bp; bp.v["a"] = {"1", 0}; - auto install_plan = - unwrap(create_versioned_install_plan(vp, bp, var_provider, {Dependency{"a"}}, {}, toplevel_spec())); + SECTION ("toplevel requires defaults") + { + auto install_plan = + unwrap(create_versioned_install_plan(vp, bp, var_provider, {Dependency{"a"}}, {}, toplevel_spec())); - REQUIRE(install_plan.size() == 1); - check_name_and_version(install_plan.install_actions[0], "a", {"1", 0}, {"x"}); + REQUIRE(install_plan.size() == 1); + check_name_and_version(install_plan.install_actions[0], "a", {"1", 0}, {"x"}); + } + + SECTION ("default-features false") + { + auto install_plan = unwrap(create_versioned_install_plan( + vp, bp, var_provider, {Dependency{"a", {"core"}, {}, {}, false, false}}, {}, toplevel_spec())); + + REQUIRE(install_plan.size() == 1); + check_name_and_version(install_plan.install_actions[0], "a", {"1", 0}, {}); + } + + SECTION ("default-features true") + { + auto install_plan = unwrap(create_versioned_install_plan( + vp, bp, var_provider, {Dependency{"a", {"core", "default"}, {}, {}, false, false}}, {}, toplevel_spec())); + + REQUIRE(install_plan.size() == 1); + check_name_and_version(install_plan.install_actions[0], "a", {"1", 0}, {"x"}); + } + + SECTION ("default-features true 2") + { + auto install_plan = unwrap(create_versioned_install_plan( + vp, bp, var_provider, {Dependency{"a", {"core", "default"}, {}, {}, false, true}}, {}, toplevel_spec())); + + REQUIRE(install_plan.size() == 1); + check_name_and_version(install_plan.install_actions[0], "a", {"1", 0}, {"x"}); + } } TEST_CASE ("version install depend-defaults", "[versionplan]") diff --git a/src/vcpkg/sourceparagraph.cpp b/src/vcpkg/sourceparagraph.cpp index 79a9012da1..bd3d6ece73 100644 --- a/src/vcpkg/sourceparagraph.cpp +++ b/src/vcpkg/sourceparagraph.cpp @@ -1178,6 +1178,30 @@ namespace vcpkg Checks::exit_with_message(VCPKG_LINE_INFO, maybe_error->error); } + // This ugly fixup is needed because Dependency is serving two roles: one as a parse tree (context + // dependent) and another as a context-free description of a package dependency. This fixup applies the + // context from the parent SourceControlFile onto all child Dependencies. + auto fixup_dependencies = [&spgh](Span deps) { + for (auto&& dep : deps) + { + auto it = Util::find(dep.features, "core"); + if (it == dep.features.end()) + { + dep.features.push_back("core"); + it = Util::find(dep.features, "default"); + if (dep.default_features.value_or(spgh->depend_defaults) && it == dep.features.end()) + { + dep.features.push_back("default"); + } + } + } + }; + fixup_dependencies(spgh->dependencies); + for (auto&& fpgh : control_file->feature_paragraphs) + { + fixup_dependencies(fpgh->dependencies); + } + return std::move(control_file); // gcc-7 bug workaround redundant move } From 8fea981bfc65a24e0047d0e0d09aba9966bfc39f Mon Sep 17 00:00:00 2001 From: Leander Schulten Date: Wed, 11 May 2022 11:38:30 +0200 Subject: [PATCH 05/14] Fix tests --- src/vcpkg-test/manifests.cpp | 2 +- src/vcpkg/sourceparagraph.cpp | 26 ++++++++++++++++---------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/vcpkg-test/manifests.cpp b/src/vcpkg-test/manifests.cpp index 560bc6ed55..00a4fb1178 100644 --- a/src/vcpkg-test/manifests.cpp +++ b/src/vcpkg-test/manifests.cpp @@ -785,7 +785,7 @@ TEST_CASE ("manifest construct maximum", "[manifests]") REQUIRE(pgh.feature_paragraphs[0]->dependencies[0].name == "firebending"); REQUIRE(pgh.feature_paragraphs[0]->dependencies[1].name == "order.white-lotus"); - REQUIRE(pgh.feature_paragraphs[0]->dependencies[1].features.size() == 1); + REQUIRE(pgh.feature_paragraphs[0]->dependencies[1].features.size() == 2); // "core" and "default" REQUIRE(pgh.feature_paragraphs[0]->dependencies[1].features[0] == "the-ancient-ways"); REQUIRE_FALSE(pgh.feature_paragraphs[0]->dependencies[1].platform.evaluate( {{"VCPKG_CMAKE_SYSTEM_NAME", ""}, {"VCPKG_TARGET_ARCHITECTURE", "arm"}})); diff --git a/src/vcpkg/sourceparagraph.cpp b/src/vcpkg/sourceparagraph.cpp index 1c7eddc697..c7bba4aff9 100644 --- a/src/vcpkg/sourceparagraph.cpp +++ b/src/vcpkg/sourceparagraph.cpp @@ -1544,7 +1544,18 @@ namespace vcpkg static bool is_dependency_trivial(const Dependency& dep) { - return dep.features.empty() && dep.platform.is_empty() && dep.extra_info.is_empty() && + if (Util::Vectors::contains(dep.features, "default")) + { + if (dep.features.size() > 2) + { + return false; + } + } + else if (dep.features.size() > 1) + { + return false; + } + return dep.platform.is_empty() && dep.extra_info.is_empty() && dep.constraint.type == VersionConstraintKind::None && !dep.host && !dep.default_features.has_value(); } @@ -1607,19 +1618,14 @@ namespace vcpkg dep_obj.insert(DependencyDeserializer::NAME, dep.name); if (dep.host) dep_obj.insert(DependencyDeserializer::HOST, Json::Value::boolean(true)); - auto features_copy = dep.features; - auto core_it = std::find(features_copy.begin(), features_copy.end(), "core"); - auto default_features = dep.default_features; - if (core_it != features_copy.end()) - { - default_features = false; - features_copy.erase(core_it); - } - if (auto b = default_features.get()) + if (auto b = dep.default_features.get()) { dep_obj.insert(DependencyDeserializer::DEFAULT_FEATURES, Json::Value::boolean(*b)); } + auto features_copy = dep.features; + Util::erase_remove(features_copy, "core"); + Util::erase_remove(features_copy, "default"); serialize_optional_array(dep_obj, DependencyDeserializer::FEATURES, features_copy); serialize_optional_string(dep_obj, DependencyDeserializer::PLATFORM, to_string(dep.platform)); if (dep.constraint.type == VersionConstraintKind::Minimum) From 4de6b9199d82a2a95938b67f767aca8dba5ef2a8 Mon Sep 17 00:00:00 2001 From: Leander Schulten Date: Thu, 12 May 2022 11:07:40 +0200 Subject: [PATCH 06/14] Format --- src/vcpkg/dependencies.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/vcpkg/dependencies.cpp b/src/vcpkg/dependencies.cpp index 5e2fb9830f..90adaf48c0 100644 --- a/src/vcpkg/dependencies.cpp +++ b/src/vcpkg/dependencies.cpp @@ -163,7 +163,10 @@ namespace vcpkg::Dependencies { if (dep.platform.is_empty()) { - auto fullspec = dep.to_full_spec(m_spec.triplet(), host_triplet, depend_defaults ? ImplicitDefault::YES : ImplicitDefault::NO); + auto fullspec = + dep.to_full_spec(m_spec.triplet(), + host_triplet, + depend_defaults ? ImplicitDefault::YES : ImplicitDefault::NO); fullspec.expand_fspecs_to(dep_list); if (auto opt = dep.constraint.try_get_minimum_version()) { From 5d786d3c22099d66f75109911eabc96b744702f5 Mon Sep 17 00:00:00 2001 From: Leander Schulten Date: Sun, 13 Nov 2022 13:51:05 +0100 Subject: [PATCH 07/14] Fix null pointer access --- src/vcpkg/install.cpp | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/vcpkg/install.cpp b/src/vcpkg/install.cpp index 60d415fc22..d3215485ec 100644 --- a/src/vcpkg/install.cpp +++ b/src/vcpkg/install.cpp @@ -1078,20 +1078,21 @@ namespace vcpkg extended_overlay_ports.emplace_back(paths.builtin_ports_directory().native()); } + DependDefaults depend_defaults = + Util::Enum::to_enum(manifest_scf->core_paragraph->depend_defaults); auto oprovider = make_manifest_provider(paths, extended_overlay_ports, manifest->path, std::move(manifest_scf)); PackageSpec toplevel{manifest_core.name, default_triplet}; - auto install_plan = create_versioned_install_plan( - *verprovider, - *baseprovider, - *oprovider, - var_provider, - dependencies, - manifest_core.overrides, - toplevel, - host_triplet, - unsupported_port_action, - Util::Enum::to_enum(manifest_scf->core_paragraph->depend_defaults)) + auto install_plan = create_versioned_install_plan(*verprovider, + *baseprovider, + *oprovider, + var_provider, + dependencies, + manifest_core.overrides, + toplevel, + host_triplet, + unsupported_port_action, + depend_defaults) .value_or_exit(VCPKG_LINE_INFO); for (const auto& warning : install_plan.warnings) From 17f4abde0ac5e19d9844a4fb617cb4b11e4006e3 Mon Sep 17 00:00:00 2001 From: Leander Schulten Date: Mon, 14 Nov 2022 17:38:36 +0100 Subject: [PATCH 08/14] Ignore default feature when checking if it is installed --- src/vcpkg/build.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/vcpkg/build.cpp b/src/vcpkg/build.cpp index 96c0568959..9290ce7101 100644 --- a/src/vcpkg/build.cpp +++ b/src/vcpkg/build.cpp @@ -1279,6 +1279,7 @@ namespace vcpkg { for (const FeatureSpec& fspec : kv.second) { + if (fspec.feature() == "default") continue; if (!status_db.is_installed(fspec) && !(fspec.port() == name && fspec.triplet() == spec.triplet())) { missing_fspecs.emplace_back(fspec); From 63c4074e363686841e6d4c0902722032296e583f Mon Sep 17 00:00:00 2001 From: Leander Schulten Date: Thu, 3 Aug 2023 18:48:05 +0200 Subject: [PATCH 09/14] Fix build after Merge with main --- include/vcpkg/base/jsonreader.h | 18 ------- include/vcpkg/dependencies.h | 6 --- src/vcpkg-test/dependencies.cpp | 85 +++++++++++++++------------------ src/vcpkg/commands.install.cpp | 9 ++-- src/vcpkg/dependencies.cpp | 34 +++++++------ src/vcpkg/packagespec.cpp | 2 +- src/vcpkg/sourceparagraph.cpp | 13 ++--- 7 files changed, 69 insertions(+), 98 deletions(-) diff --git a/include/vcpkg/base/jsonreader.h b/include/vcpkg/base/jsonreader.h index 5d50b82b78..24fda8d19e 100644 --- a/include/vcpkg/base/jsonreader.h +++ b/include/vcpkg/base/jsonreader.h @@ -163,24 +163,6 @@ namespace vcpkg::Json } } - // returns whether key \in obj - template - bool optional_object_field(const Object& obj, - StringView key, - Optional& place, - IDeserializer& visitor) - { - if (auto value = obj.get(key)) - { - visit_in_key(*value, key, place.emplace(), visitor); - return true; - } - else - { - return false; - } - } - template Optional visit(const Value& value, const IDeserializer& visitor) { diff --git a/include/vcpkg/dependencies.h b/include/vcpkg/dependencies.h index 89b968e7c7..853e0e7f8a 100644 --- a/include/vcpkg/dependencies.h +++ b/include/vcpkg/dependencies.h @@ -195,12 +195,6 @@ namespace vcpkg const StatusParagraphs& status_db, const CreateInstallPlanOptions& options); - enum class DependDefaults - { - NO, - YES, - }; - ExpectedL create_versioned_install_plan(const IVersionedPortfileProvider& vprovider, const IBaselineProvider& bprovider, const IOverlayProvider& oprovider, diff --git a/src/vcpkg-test/dependencies.cpp b/src/vcpkg-test/dependencies.cpp index 7bafe5c975..a7ad903767 100644 --- a/src/vcpkg-test/dependencies.cpp +++ b/src/vcpkg-test/dependencies.cpp @@ -242,7 +242,7 @@ static ExpectedL create_versioned_install_plan(const IVersionedPortf deps, overrides, toplevel, - {Test::ARM_UWP, "pkgs", UnsupportedPortAction::Error, DependDefaults::YES}); + {Test::ARM_UWP, "pkgs", UnsupportedPortAction::Error, ImplicitDefault::YES}); } static ExpectedL create_versioned_install_plan(const IVersionedPortfileProvider& provider, @@ -253,35 +253,35 @@ static ExpectedL create_versioned_install_plan(const IVersionedPortf const std::vector& overrides, const PackageSpec& toplevel) { - return vcpkg::create_versioned_install_plan(provider, - bprovider, - oprovider, - var_provider, - deps, - overrides, - toplevel, - Test::ARM_UWP, - UnsupportedPortAction::Error, - DependDefaults::YES); + return vcpkg::create_versioned_install_plan( + provider, + bprovider, + oprovider, + var_provider, + deps, + overrides, + toplevel, + {Test::ARM_UWP, "pkgs", UnsupportedPortAction::Error, ImplicitDefault::YES}); } -static ExpectedS create_versioned_install_plan(const IOverlayProvider& oprovider, - const std::vector& deps, - const PackageSpec& toplevel, - DependDefaults depend_defaults) +static ExpectedL create_versioned_install_plan(const IOverlayProvider& oprovider, + const std::vector& deps, + const PackageSpec& toplevel, + ImplicitDefault implicit_default) { MockVersionedPortfileProvider vp; MockCMakeVarProvider var_provider; MockBaselineProvider bp; - return create_versioned_install_plan(vp, - bp, - oprovider, - var_provider, - deps, - {}, - toplevel, - {Test::ARM_UWP, "pkgs", UnsupportedPortAction::Error, depend_defaults}); + return vcpkg::create_versioned_install_plan( + vp, + bp, + oprovider, + var_provider, + deps, + {}, + toplevel, + {Test::ARM_UWP, "pkgs", UnsupportedPortAction::Error, implicit_default}); } TEST_CASE ("basic version install single", "[versionplan]") @@ -1660,9 +1660,8 @@ TEST_CASE ("version install default features", "[versionplan]") SECTION ("toplevel requires defaults") { - WITH_EXPECTED(install_plan, - create_versioned_install_plan(vp, bp, var_provider, {Dependency{"a"}}, {}, toplevel_spec())); + create_versioned_install_plan(vp, bp, var_provider, {Dependency{"a"}}, {}, toplevel_spec())); REQUIRE(install_plan.size() == 1); check_name_and_version(install_plan.install_actions[0], "a", {"1", 0}, {"x"}); @@ -1671,8 +1670,7 @@ TEST_CASE ("version install default features", "[versionplan]") SECTION ("default-features false") { auto install_plan = - create_versioned_install_plan( - vp, bp, var_provider, {Dependency{"a", {"core"}, {}, {}, false, false}}, {}, toplevel_spec()) + create_versioned_install_plan(vp, bp, var_provider, {CoreDependency{"a"}}, {}, toplevel_spec()) .value_or_exit(VCPKG_LINE_INFO); REQUIRE(install_plan.size() == 1); @@ -1681,10 +1679,8 @@ TEST_CASE ("version install default features", "[versionplan]") SECTION ("default-features true") { - auto install_plan = - create_versioned_install_plan( - vp, bp, var_provider, {Dependency{"a", {"core", "default"}, {}, {}, false, false}}, {}, toplevel_spec()) - .value_or_exit(VCPKG_LINE_INFO); + auto install_plan = create_versioned_install_plan(vp, bp, var_provider, {Dependency{"a"}}, {}, toplevel_spec()) + .value_or_exit(VCPKG_LINE_INFO); REQUIRE(install_plan.size() == 1); check_name_and_version(install_plan.install_actions[0], "a", {"1", 0}, {"x"}); @@ -1692,10 +1688,8 @@ TEST_CASE ("version install default features", "[versionplan]") SECTION ("default-features true 2") { - auto install_plan = - create_versioned_install_plan( - vp, bp, var_provider, {Dependency{"a", {"core", "default"}, {}, {}, false, true}}, {}, toplevel_spec()) - .value_or_exit(VCPKG_LINE_INFO); + auto install_plan = create_versioned_install_plan(vp, bp, var_provider, {Dependency{"a"}}, {}, toplevel_spec()) + .value_or_exit(VCPKG_LINE_INFO); REQUIRE(install_plan.size() == 1); check_name_and_version(install_plan.install_actions[0], "a", {"1", 0}, {"x"}); @@ -1715,7 +1709,7 @@ TEST_CASE ("version install depend-defaults", "[versionplan]") c0->dependencies.push_back(Dependency{"b"}); auto& d_scf = op.emplace("d"); - d_scf.source_control_file->core_paragraph->dependencies.push_back(Dependency{"b", {"core"}}); + d_scf.source_control_file->core_paragraph->dependencies.push_back(CoreDependency{"b"}); auto& e_scf = op.emplace("e"); e_scf.source_control_file->core_paragraph->dependencies.push_back(Dependency{"b"}); @@ -1723,7 +1717,7 @@ TEST_CASE ("version install depend-defaults", "[versionplan]") SECTION ("toplevel depend-defaults true") { - auto install_plan = create_versioned_install_plan(op, {Dependency{"b"}}, toplevel_spec(), DependDefaults::YES) + auto install_plan = create_versioned_install_plan(op, {Dependency{"b"}}, toplevel_spec(), ImplicitDefault::YES) .value_or_exit(VCPKG_LINE_INFO); REQUIRE(install_plan.size() == 1); check_name_and_features(install_plan.install_actions[0], "b", {"0"}); @@ -1731,13 +1725,13 @@ TEST_CASE ("version install depend-defaults", "[versionplan]") SECTION ("toplevel depend-defaults false") { - auto install_plan = create_versioned_install_plan(op, {Dependency{"b"}}, toplevel_spec(), DependDefaults::NO) + auto install_plan = create_versioned_install_plan(op, {Dependency{"b"}}, toplevel_spec(), ImplicitDefault::NO) .value_or_exit(VCPKG_LINE_INFO); REQUIRE(install_plan.size() == 1); check_name_and_features(install_plan.install_actions[0], "b", {}); // a -> b[default], so depend-defaults should not suppress it - install_plan = create_versioned_install_plan(op, {Dependency{"a"}}, toplevel_spec(), DependDefaults::NO) + install_plan = create_versioned_install_plan(op, {Dependency{"a"}}, toplevel_spec(), ImplicitDefault::NO) .value_or_exit(VCPKG_LINE_INFO); REQUIRE(install_plan.size() == 2); check_name_and_features(install_plan.install_actions[0], "b", {"0"}); @@ -1745,7 +1739,7 @@ TEST_CASE ("version install depend-defaults", "[versionplan]") SECTION ("toplevel depend-defaults true (transitive)") { - auto install_plan = create_versioned_install_plan(op, {Dependency{"d"}}, toplevel_spec(), DependDefaults::YES) + auto install_plan = create_versioned_install_plan(op, {Dependency{"d"}}, toplevel_spec(), ImplicitDefault::YES) .value_or_exit(VCPKG_LINE_INFO); REQUIRE(install_plan.size() == 2); check_name_and_features(install_plan.install_actions[0], "b", {"0"}); @@ -1753,7 +1747,7 @@ TEST_CASE ("version install depend-defaults", "[versionplan]") SECTION ("toplevel depend-defaults false (transitive)") { - auto install_plan = create_versioned_install_plan(op, {Dependency{"d"}}, toplevel_spec(), DependDefaults::NO) + auto install_plan = create_versioned_install_plan(op, {Dependency{"d"}}, toplevel_spec(), ImplicitDefault::NO) .value_or_exit(VCPKG_LINE_INFO); REQUIRE(install_plan.size() == 2); check_name_and_features(install_plan.install_actions[0], "b", {}); @@ -1764,7 +1758,7 @@ TEST_CASE ("version install depend-defaults", "[versionplan]") SECTION ("toplevel false") { auto install_plan = - create_versioned_install_plan(op, {Dependency{"e"}}, toplevel_spec(), DependDefaults::NO) + create_versioned_install_plan(op, {Dependency{"e"}}, toplevel_spec(), ImplicitDefault::NO) .value_or_exit(VCPKG_LINE_INFO); REQUIRE(install_plan.size() == 2); check_name_and_features(install_plan.install_actions[0], "b", {}); @@ -1772,10 +1766,9 @@ TEST_CASE ("version install depend-defaults", "[versionplan]") SECTION ("toplevel core") { - auto install_plan = - create_versioned_install_plan( - op, {Dependency{"e"}, Dependency{"b", {"core"}}}, toplevel_spec(), DependDefaults::YES) - .value_or_exit(VCPKG_LINE_INFO); + auto install_plan = create_versioned_install_plan( + op, {Dependency{"e"}, CoreDependency{"b"}}, toplevel_spec(), ImplicitDefault::YES) + .value_or_exit(VCPKG_LINE_INFO); REQUIRE(install_plan.size() == 2); check_name_and_features(install_plan.install_actions[0], "b", {}); } diff --git a/src/vcpkg/commands.install.cpp b/src/vcpkg/commands.install.cpp index 2a5725ef02..cf3a81a4a8 100644 --- a/src/vcpkg/commands.install.cpp +++ b/src/vcpkg/commands.install.cpp @@ -1076,7 +1076,7 @@ namespace vcpkg auto var_provider_storage = CMakeVars::make_triplet_cmake_var_provider(paths); auto& var_provider = *var_provider_storage; - const CreateInstallPlanOptions create_options{host_triplet, paths.packages(), unsupported_port_action}; + CreateInstallPlanOptions create_options{host_triplet, paths.packages(), unsupported_port_action}; if (auto manifest = paths.get_manifest().get()) { @@ -1181,9 +1181,8 @@ namespace vcpkg { extended_overlay_ports.emplace_back(paths.builtin_ports_directory().native()); } - - DependDefaults depend_defaults = - Util::Enum::to_enum(manifest_scf->core_paragraph->depend_defaults); + create_options.implicit_default = + (manifest_scf->core_paragraph->depend_defaults ? ImplicitDefault::YES : ImplicitDefault::NO); auto oprovider = make_manifest_provider( fs, paths.original_cwd, extended_overlay_ports, manifest->path, std::move(manifest_scf)); auto install_plan = create_versioned_install_plan(*verprovider, @@ -1193,7 +1192,7 @@ namespace vcpkg dependencies, manifest_core.overrides, toplevel, - create_options) // TODO Add default + create_options) .value_or_exit(VCPKG_LINE_INFO); install_plan.print_unsupported_warnings(); diff --git a/src/vcpkg/dependencies.cpp b/src/vcpkg/dependencies.cpp index d23fea3b19..764a3b67c2 100644 --- a/src/vcpkg/dependencies.cpp +++ b/src/vcpkg/dependencies.cpp @@ -179,8 +179,6 @@ namespace vcpkg } const std::vector* qualified_deps = &maybe_qualified_deps.value_or_exit(VCPKG_LINE_INFO); - const auto depend_defaults = scfl.source_control_file->core_paragraph->depend_defaults; - std::vector dep_list; if (auto vars = maybe_vars.get()) { @@ -1336,14 +1334,14 @@ namespace vcpkg const IOverlayProvider& oprovider, const CMakeVars::CMakeVarProvider& var_provider, Triplet host_triplet, - DependDefaults depend_defaults, + ImplicitDefault implicit_default, 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_depend_defaults(depend_defaults == DependDefaults::YES) + , m_implicit_default(implicit_default) , m_packages_dir(packages_dir) { } @@ -1361,7 +1359,7 @@ namespace vcpkg const IOverlayProvider& m_o_provider; const CMakeVars::CMakeVarProvider& m_var_provider; const Triplet m_host_triplet; - const bool m_depend_defaults; + const ImplicitDefault m_implicit_default; const Path m_packages_dir; struct DepSpec @@ -1373,6 +1371,10 @@ namespace vcpkg struct PackageNodeData { + explicit PackageNodeData(ImplicitDefault implicit_default) + : default_features(Util::Enum::to_bool(implicit_default)) + { + } // set of all scfls that have been considered std::set considered; @@ -1388,7 +1390,7 @@ namespace vcpkg // The set of features that have been requested across all constraints std::set requested_features; - bool default_features = false; + bool default_features; }; using PackageNode = std::pair; @@ -1413,7 +1415,7 @@ namespace vcpkg // Add an initial requirement for a package. // Returns a reference to the node to place additional constraints - Optional require_package(const PackageSpec& spec, bool depend_defaults, const std::string& origin); + Optional require_package(const PackageSpec& spec, const std::string& origin); void require_scfl(PackageNode& ref, const SourceControlFileAndLocation* scfl, const std::string& origin); @@ -1597,13 +1599,12 @@ namespace vcpkg } Optional VersionedPackageGraph::require_package(const PackageSpec& spec, - bool depends_defaults, const std::string& origin) { auto it = m_graph.find(spec); if (it != m_graph.end()) { - it->second.origins.insert(depends_defaults, origin); + it->second.origins.insert(origin); return *it; } @@ -1615,7 +1616,7 @@ namespace vcpkg const auto maybe_overlay = m_o_provider.get_control_file(spec.name()); if (auto p_overlay = maybe_overlay.get()) { - it = m_graph.emplace(spec, PackageNodeData{}).first; + it = m_graph.emplace(spec, PackageNodeData{m_implicit_default}).first; it->second.overlay_or_override = true; it->second.scfl = p_overlay; } @@ -1627,7 +1628,7 @@ namespace vcpkg auto maybe_scfl = m_ver_provider.get_control_file({spec.name(), over_it->second}); if (auto p_scfl = maybe_scfl.get()) { - it = m_graph.emplace(spec, PackageNodeData{}).first; + it = m_graph.emplace(spec, PackageNodeData{m_implicit_default}).first; it->second.overlay_or_override = true; it->second.scfl = p_scfl; } @@ -1645,7 +1646,7 @@ namespace vcpkg }); if (auto p_scfl = maybe_scfl.get()) { - it = m_graph.emplace(spec, PackageNodeData{}).first; + it = m_graph.emplace(spec, PackageNodeData{m_implicit_default}).first; it->second.baseline = p_scfl->schemed_version(); it->second.scfl = p_scfl; } @@ -2008,8 +2009,13 @@ namespace vcpkg const PackageSpec& toplevel, const CreateInstallPlanOptions& options) { - VersionedPackageGraph vpg( - provider, bprovider, oprovider, var_provider, options.host_triplet, options.packages_dir); + VersionedPackageGraph vpg(provider, + bprovider, + oprovider, + var_provider, + options.host_triplet, + options.implicit_default, + options.packages_dir); for (auto&& o : overrides) { vpg.add_override(o.name, {o.version, o.port_version}); diff --git a/src/vcpkg/packagespec.cpp b/src/vcpkg/packagespec.cpp index 7be0e2aed8..e0358f9b4a 100644 --- a/src/vcpkg/packagespec.cpp +++ b/src/vcpkg/packagespec.cpp @@ -61,7 +61,7 @@ namespace vcpkg if (!core) { ret.emplace_back("core"); - if (implicit_default) + if (id == ImplicitDefault::YES) { ret.emplace_back("default"); } diff --git a/src/vcpkg/sourceparagraph.cpp b/src/vcpkg/sourceparagraph.cpp index be09c7d661..5a922f681d 100644 --- a/src/vcpkg/sourceparagraph.cpp +++ b/src/vcpkg/sourceparagraph.cpp @@ -1123,7 +1123,7 @@ namespace vcpkg constexpr static StringLiteral BUILTIN_BASELINE = "builtin-baseline"; constexpr static StringLiteral VCPKG_CONFIGURATION = "vcpkg-configuration"; - virtual View valid_fields() const override + virtual Span valid_fields() const override { static constexpr StringView u[] = { NAME, @@ -1162,8 +1162,6 @@ namespace vcpkg } } - static Json::StringDeserializer url_deserializer{"a url"}; - r.optional_object_field(obj, DEPEND_DEFAULTS, spgh.depend_defaults, Json::BooleanDeserializer::instance); r.optional_object_field(obj, MAINTAINERS, spgh.maintainers, Json::ParagraphDeserializer::instance); r.optional_object_field(obj, CONTACTS, spgh.contacts, ContactsDeserializer::instance); @@ -1632,10 +1630,10 @@ namespace vcpkg return ret; } - static bool is_dependency_trivial(const Dependency& dep) + static bool is_dependency_trivial(const Dependency& dep, bool depend_defaults) { - return dep.features.empty() && dep.default_features && dep.platform.is_empty() && dep.extra_info.is_empty() && - dep.constraint.type == VersionConstraintKind::None && !dep.host; // TODO not right + return dep.features.empty() && dep.default_features == depend_defaults && dep.platform.is_empty() && + dep.extra_info.is_empty() && dep.constraint.type == VersionConstraintKind::None && !dep.host; } Json::Object serialize_manifest(const SourceControlFile& scf) @@ -1689,7 +1687,7 @@ namespace vcpkg } }; auto serialize_dependency = [&](Json::Array& arr, const Dependency& dep) { - if (is_dependency_trivial(dep)) + if (is_dependency_trivial(dep, scf.core_paragraph->depend_defaults)) { arr.push_back(Json::Value::string(dep.name)); } @@ -1706,7 +1704,6 @@ namespace vcpkg if (!dep.default_features) { - // TODO depends default dep_obj.insert(DependencyDeserializer::DEFAULT_FEATURES, Json::Value::boolean(false)); } serialize_dependency_features(dep_obj, DependencyDeserializer::FEATURES, dep.features); From d40fcb092aaf4f2fab7ef61dc8274d6672000810 Mon Sep 17 00:00:00 2001 From: Leander Schulten Date: Sat, 5 Aug 2023 13:54:53 +0200 Subject: [PATCH 10/14] Fix tests --- src/vcpkg-test/dependencies.cpp | 77 +++++++++------------------------ src/vcpkg-test/manifests.cpp | 64 ++++++++++++++++++++++++++- src/vcpkg-test/plan.cpp | 40 +++++++---------- src/vcpkg/dependencies.cpp | 18 ++++---- src/vcpkg/sourceparagraph.cpp | 41 +++++++++--------- 5 files changed, 127 insertions(+), 113 deletions(-) diff --git a/src/vcpkg-test/dependencies.cpp b/src/vcpkg-test/dependencies.cpp index a7ad903767..ac080e85ac 100644 --- a/src/vcpkg-test/dependencies.cpp +++ b/src/vcpkg-test/dependencies.cpp @@ -105,6 +105,7 @@ static void check_name_and_features(const InstallPlanAction& ipa, std::initializer_list features) { CHECK(ipa.spec.name() == name); + INFO("spec.name: " << name); CHECK(ipa.source_control_file_and_location.has_value()); { INFO("ipa.feature_list = [" << Strings::join(", ", ipa.feature_list) << "]"); @@ -1698,80 +1699,44 @@ TEST_CASE ("version install default features", "[versionplan]") TEST_CASE ("version install depend-defaults", "[versionplan]") { + // Dependencies: + // b has default feature '0' + // a -> b[core] + // c -> b[default] + MockOverlayProvider op; auto& a_scf = op.emplace("a"); - a_scf.source_control_file->core_paragraph->dependencies.push_back(Dependency{"b"}); + a_scf.source_control_file->core_paragraph->dependencies.push_back(CoreDependency{"b"}); auto& b_scf = op.emplace("b"); b_scf.source_control_file->core_paragraph->default_features.emplace_back("0"); b_scf.source_control_file->feature_paragraphs.emplace_back(make_fpgh("0")); - auto& c_scf = op.emplace("c"); - auto& c0 = c_scf.source_control_file->feature_paragraphs.emplace_back(make_fpgh("0")); - c0->dependencies.push_back(Dependency{"b"}); - - auto& d_scf = op.emplace("d"); - d_scf.source_control_file->core_paragraph->dependencies.push_back(CoreDependency{"b"}); - - auto& e_scf = op.emplace("e"); - e_scf.source_control_file->core_paragraph->dependencies.push_back(Dependency{"b"}); - e_scf.source_control_file->core_paragraph->depend_defaults = false; - - SECTION ("toplevel depend-defaults true") - { - auto install_plan = create_versioned_install_plan(op, {Dependency{"b"}}, toplevel_spec(), ImplicitDefault::YES) - .value_or_exit(VCPKG_LINE_INFO); - REQUIRE(install_plan.size() == 1); - check_name_and_features(install_plan.install_actions[0], "b", {"0"}); - } + auto& d_scf = op.emplace("c"); + d_scf.source_control_file->core_paragraph->dependencies.push_back(Dependency{"b"}); - SECTION ("toplevel depend-defaults false") + SECTION ("toplevel ImplicitDefault::YES") { - auto install_plan = create_versioned_install_plan(op, {Dependency{"b"}}, toplevel_spec(), ImplicitDefault::NO) + auto install_plan = create_versioned_install_plan(op, {Dependency{"a"}}, toplevel_spec(), ImplicitDefault::YES) .value_or_exit(VCPKG_LINE_INFO); - REQUIRE(install_plan.size() == 1); - check_name_and_features(install_plan.install_actions[0], "b", {}); - - // a -> b[default], so depend-defaults should not suppress it - install_plan = create_versioned_install_plan(op, {Dependency{"a"}}, toplevel_spec(), ImplicitDefault::NO) - .value_or_exit(VCPKG_LINE_INFO); REQUIRE(install_plan.size() == 2); check_name_and_features(install_plan.install_actions[0], "b", {"0"}); + check_name_and_features(install_plan.install_actions[1], "a", {}); } - SECTION ("toplevel depend-defaults true (transitive)") + SECTION ("toplevel ImplicitDefault::NO") { - auto install_plan = create_versioned_install_plan(op, {Dependency{"d"}}, toplevel_spec(), ImplicitDefault::YES) + auto install_plan = create_versioned_install_plan(op, {Dependency{"a"}}, toplevel_spec(), ImplicitDefault::NO) .value_or_exit(VCPKG_LINE_INFO); REQUIRE(install_plan.size() == 2); - check_name_and_features(install_plan.install_actions[0], "b", {"0"}); - } - SECTION ("toplevel depend-defaults false (transitive)") - { - auto install_plan = create_versioned_install_plan(op, {Dependency{"d"}}, toplevel_spec(), ImplicitDefault::NO) - .value_or_exit(VCPKG_LINE_INFO); - REQUIRE(install_plan.size() == 2); check_name_and_features(install_plan.install_actions[0], "b", {}); - } - - SECTION ("transitive depend-defaults false") - { - SECTION ("toplevel false") - { - auto install_plan = - create_versioned_install_plan(op, {Dependency{"e"}}, toplevel_spec(), ImplicitDefault::NO) - .value_or_exit(VCPKG_LINE_INFO); - REQUIRE(install_plan.size() == 2); - check_name_and_features(install_plan.install_actions[0], "b", {}); - } + check_name_and_features(install_plan.install_actions[1], "a", {}); - SECTION ("toplevel core") - { - auto install_plan = create_versioned_install_plan( - op, {Dependency{"e"}, CoreDependency{"b"}}, toplevel_spec(), ImplicitDefault::YES) - .value_or_exit(VCPKG_LINE_INFO); - REQUIRE(install_plan.size() == 2); - check_name_and_features(install_plan.install_actions[0], "b", {}); - } + // c -> b[default], so depend-defaults should not suppress it + install_plan = create_versioned_install_plan(op, {Dependency{"c"}}, toplevel_spec(), ImplicitDefault::NO) + .value_or_exit(VCPKG_LINE_INFO); + REQUIRE(install_plan.size() == 2); + check_name_and_features(install_plan.install_actions[0], "b", {"0"}); + check_name_and_features(install_plan.install_actions[1], "c", {}); } } diff --git a/src/vcpkg-test/manifests.cpp b/src/vcpkg-test/manifests.cpp index ab0b31e9cf..e0c46c07c5 100644 --- a/src/vcpkg-test/manifests.cpp +++ b/src/vcpkg-test/manifests.cpp @@ -838,7 +838,6 @@ TEST_CASE ("manifest construct maximum", "[manifests]") "summary": "d", "description": "d", "builtin-baseline": "123", - "depend-defaults": false, "dependencies": ["bd"], "default-features": [ "df", @@ -908,7 +907,6 @@ TEST_CASE ("manifest construct maximum", "[manifests]") REQUIRE(contact_a_aa->string(VCPKG_LINE_INFO) == "aa"); REQUIRE(pgh.core_paragraph->summary.size() == 1); REQUIRE(pgh.core_paragraph->summary[0] == "d"); - REQUIRE(pgh.core_paragraph->depend_defaults == false); REQUIRE(pgh.core_paragraph->description.size() == 1); REQUIRE(pgh.core_paragraph->description[0] == "d"); REQUIRE(pgh.core_paragraph->dependencies.size() == 1); @@ -967,6 +965,68 @@ TEST_CASE ("manifest construct maximum", "[manifests]") check_json_eq_ordered(serialize_manifest(pgh), object); } +TEST_CASE ("SourceParagraph manifest depend-default", "[manifests]") +{ + auto json = parse_json_object(R"json({ + "name": "zlib", + "version-string": "1.2.8", + "depend-defaults": false, + "dependencies": [ + "y", + {"name": "z", "host": true}, + {"name": "z2", "host": true, "default-features": true} + ], + "features": { + "t": { + "description": "", + "dependencies": ["z"] + } + } + })json"); + SECTION ("depend-defaults: false") + { + auto m_pgh = test_parse_port_manifest(json); + REQUIRE(m_pgh.has_value()); + auto& pgh = **m_pgh.get(); + REQUIRE(pgh.core_paragraph->depend_defaults == false); + REQUIRE(pgh.core_paragraph->dependencies.size() == 3); + REQUIRE(pgh.core_paragraph->dependencies[0].name == "y"); + REQUIRE(pgh.core_paragraph->dependencies[0].default_features == false); + REQUIRE(pgh.core_paragraph->dependencies[1].name == "z"); + REQUIRE(pgh.core_paragraph->dependencies[1].default_features == false); + REQUIRE(pgh.core_paragraph->dependencies[2].name == "z2"); + REQUIRE(pgh.core_paragraph->dependencies[2].default_features == true); + + REQUIRE(pgh.feature_paragraphs.size() == 1); + REQUIRE(pgh.feature_paragraphs[0]->dependencies[0].name == "z"); + REQUIRE(pgh.feature_paragraphs[0]->dependencies[0].default_features == false); + check_json_eq_ordered(serialize_manifest(pgh), json); + } + SECTION ("depend-defaults: true") + { + json.remove("depend-defaults"); + json["dependencies"].array(VCPKG_LINE_INFO)[2].object(VCPKG_LINE_INFO)["default-features"] = + Json::Value::boolean(false); + auto m_pgh = test_parse_port_manifest(json); + REQUIRE(m_pgh.has_value()); + auto& pgh = **m_pgh.get(); + + REQUIRE(pgh.core_paragraph->depend_defaults == true); + REQUIRE(pgh.core_paragraph->dependencies.size() == 3); + REQUIRE(pgh.core_paragraph->dependencies[0].name == "y"); + REQUIRE(pgh.core_paragraph->dependencies[0].default_features == true); + REQUIRE(pgh.core_paragraph->dependencies[1].name == "z"); + REQUIRE(pgh.core_paragraph->dependencies[1].default_features == true); + REQUIRE(pgh.core_paragraph->dependencies[2].name == "z2"); + REQUIRE(pgh.core_paragraph->dependencies[2].default_features == false); + + REQUIRE(pgh.feature_paragraphs.size() == 1); + REQUIRE(pgh.feature_paragraphs[0]->dependencies[0].name == "z"); + REQUIRE(pgh.feature_paragraphs[0]->dependencies[0].default_features == true); + check_json_eq_ordered(serialize_manifest(pgh), json); + } +} + TEST_CASE ("SourceParagraph manifest two dependencies", "[manifests]") { auto m_pgh = test_parse_port_manifest(R"json({ diff --git a/src/vcpkg-test/plan.cpp b/src/vcpkg-test/plan.cpp index bb5f02593f..9e03ee0fa3 100644 --- a/src/vcpkg-test/plan.cpp +++ b/src/vcpkg-test/plan.cpp @@ -61,9 +61,11 @@ static void remove_plan_check(RemovePlanAction& plan, std::string pkg_name, Trip static ActionPlan create_feature_install_plan(const PortFileProvider& port_provider, const CMakeVars::CMakeVarProvider& var_provider, View specs, - const StatusParagraphs& status_db) + const StatusParagraphs& status_db, + ImplicitDefault implicit_defaults = ImplicitDefault::YES) { - const CreateInstallPlanOptions create_options{Test::X64_ANDROID, "pkg"}; + const CreateInstallPlanOptions create_options{ + Test::X64_ANDROID, "pkg", UnsupportedPortAction::Error, implicit_defaults}; return create_feature_install_plan(port_provider, var_provider, specs, status_db, create_options); } @@ -818,7 +820,7 @@ TEST_CASE ("install with default features", "[plan]") features_check(install_plan.install_actions.at(1), "a", {"0", "core"}); } -TEST_CASE ("install with depend-defaults false", "[plan]") +TEST_CASE ("install with ImplicitDefault::NO", "[plan]") { StatusParagraphs status_db; @@ -826,9 +828,6 @@ TEST_CASE ("install with depend-defaults false", "[plan]") auto a_spec = spec_map.emplace("a", "b"); auto b_spec = spec_map.emplace("b", "", {{"0", ""}}, {"0"}); auto c_spec = spec_map.emplace("c", "", {{"0", "b"}}, {"0"}); - auto d_spec = spec_map.emplace("d", "b"); - spec_map.map["d"].source_control_file->core_paragraph->depend_defaults = false; - spec_map.map["d"].source_control_file->core_paragraph->dependencies[0].default_features = true; MapPortFileProvider map_port{spec_map.map}; MockCMakeVarProvider var_provider; @@ -843,14 +842,11 @@ TEST_CASE ("install with depend-defaults false", "[plan]") FullPackageSpec{b_spec, {"core"}}, }; - std::vector full_package_specs_d{ - FullPackageSpec{d_spec, {"core"}}, - }; - // Install "a" and then "b" _should_ install default features SECTION ("depend-defaults true from core") { - auto install_plan = create_feature_install_plan(map_port, var_provider, full_package_specs, {}); + auto install_plan = + create_feature_install_plan(map_port, var_provider, full_package_specs, {}, ImplicitDefault::NO); REQUIRE(install_plan.size() == 2); features_check(install_plan.install_actions.at(0), "b", {"0", "core"}); features_check(install_plan.install_actions.at(1), "a", {"core"}); @@ -858,25 +854,20 @@ TEST_CASE ("install with depend-defaults false", "[plan]") SECTION ("depend-defaults true from feature") { - auto install_plan = create_feature_install_plan(map_port, var_provider, full_package_specs2, {}); + auto install_plan = + create_feature_install_plan(map_port, var_provider, full_package_specs2, {}, ImplicitDefault::NO); REQUIRE(install_plan.size() == 2); features_check(install_plan.install_actions.at(0), "b", {"0", "core"}); features_check(install_plan.install_actions.at(1), "c", {"0", "core"}); } - SECTION ("depend-defaults false overridden from dependency") - { - auto install_plan = create_feature_install_plan(map_port, var_provider, full_package_specs_d, {}); - REQUIRE(install_plan.size() == 2); - features_check(install_plan.install_actions.at(0), "b", {"0", "core"}); - features_check(install_plan.install_actions.at(1), "d", {"core"}); - } - - // now, disable the implicit default dependency from `a` and `c[0]` + // now, disable the default dependency from `a` and `c[0]` + spec_map.map["a"].source_control_file->core_paragraph->dependencies.at(0).default_features = false; + spec_map.map["c"].source_control_file->feature_paragraphs.at(0)->dependencies.at(0).default_features = false; SECTION ("depend-defaults false from core") { - spec_map.map["a"].source_control_file->core_paragraph->depend_defaults = false; - auto install_plan = create_feature_install_plan(map_port, var_provider, full_package_specs, {}); + auto install_plan = + create_feature_install_plan(map_port, var_provider, full_package_specs, {}, ImplicitDefault::NO); REQUIRE(install_plan.size() == 2); features_check(install_plan.install_actions.at(0), "b", {"core"}); features_check(install_plan.install_actions.at(1), "a", {"core"}); @@ -884,7 +875,8 @@ TEST_CASE ("install with depend-defaults false", "[plan]") SECTION ("depend-defaults false from feature") { spec_map.map["c"].source_control_file->core_paragraph->depend_defaults = false; - auto install_plan = create_feature_install_plan(map_port, var_provider, full_package_specs2, {}); + auto install_plan = + create_feature_install_plan(map_port, var_provider, full_package_specs2, {}, ImplicitDefault::NO); REQUIRE(install_plan.size() == 2); features_check(install_plan.install_actions.at(0), "b", {"core"}); features_check(install_plan.install_actions.at(1), "c", {"0", "core"}); diff --git a/src/vcpkg/dependencies.cpp b/src/vcpkg/dependencies.cpp index 764a3b67c2..54d1e039f0 100644 --- a/src/vcpkg/dependencies.cpp +++ b/src/vcpkg/dependencies.cpp @@ -1371,10 +1371,6 @@ namespace vcpkg struct PackageNodeData { - explicit PackageNodeData(ImplicitDefault implicit_default) - : default_features(Util::Enum::to_bool(implicit_default)) - { - } // set of all scfls that have been considered std::set considered; @@ -1390,7 +1386,7 @@ namespace vcpkg // The set of features that have been requested across all constraints std::set requested_features; - bool default_features; + bool default_features = false; }; using PackageNode = std::pair; @@ -1616,7 +1612,7 @@ namespace vcpkg const auto maybe_overlay = m_o_provider.get_control_file(spec.name()); if (auto p_overlay = maybe_overlay.get()) { - it = m_graph.emplace(spec, PackageNodeData{m_implicit_default}).first; + it = m_graph.emplace(spec, PackageNodeData{}).first; it->second.overlay_or_override = true; it->second.scfl = p_overlay; } @@ -1628,7 +1624,7 @@ namespace vcpkg auto maybe_scfl = m_ver_provider.get_control_file({spec.name(), over_it->second}); if (auto p_scfl = maybe_scfl.get()) { - it = m_graph.emplace(spec, PackageNodeData{m_implicit_default}).first; + it = m_graph.emplace(spec, PackageNodeData{}).first; it->second.overlay_or_override = true; it->second.scfl = p_scfl; } @@ -1646,7 +1642,7 @@ namespace vcpkg }); if (auto p_scfl = maybe_scfl.get()) { - it = m_graph.emplace(spec, PackageNodeData{m_implicit_default}).first; + it = m_graph.emplace(spec, PackageNodeData{}).first; it->second.baseline = p_scfl->schemed_version(); it->second.scfl = p_scfl; } @@ -1660,8 +1656,10 @@ namespace vcpkg } // Implicit defaults are disabled if spec has been mentioned at top-level. - // Note that if top-level doesn't also mark that reference as `[core]`, defaults will be re-engaged. - it->second.default_features = !Util::Maps::contains(m_user_requested, spec); + // Note that if top-level doesn't also mark that reference as `[core]` and ImplicitDefault::YES, defaults + // will be re-engaged. + it->second.default_features = + (m_implicit_default == ImplicitDefault::YES) && !Util::Maps::contains(m_user_requested, spec); it->second.requested_features.insert("core"); require_scfl(*it, it->second.scfl, origin); diff --git a/src/vcpkg/sourceparagraph.cpp b/src/vcpkg/sourceparagraph.cpp index 5a922f681d..3a622f0954 100644 --- a/src/vcpkg/sourceparagraph.cpp +++ b/src/vcpkg/sourceparagraph.cpp @@ -549,6 +549,8 @@ namespace vcpkg struct DependencyDeserializer final : Json::IDeserializer { + explicit DependencyDeserializer(bool depends_defaults) : depends_defaults(depends_defaults) { } + bool depends_defaults; virtual LocalizedString type_name() const override { return msg::format(msgADependency); } constexpr static StringLiteral NAME = "name"; @@ -580,6 +582,7 @@ namespace vcpkg } Dependency dep; + dep.default_features = depends_defaults; dep.name = sv.to_string(); return dep; } @@ -587,6 +590,7 @@ namespace vcpkg virtual Optional visit_object(Json::Reader& r, const Json::Object& obj) const override { Dependency dep; + dep.default_features = depends_defaults; for (const auto& el : obj) { @@ -628,25 +632,20 @@ namespace vcpkg return dep; } - - static DependencyDeserializer instance; }; - DependencyDeserializer DependencyDeserializer::instance; struct DependencyArrayDeserializer final : Json::IDeserializer> { + explicit DependencyArrayDeserializer(bool depends_defaults) : depends_defaults(depends_defaults) { } + bool depends_defaults; virtual LocalizedString type_name() const override { return msg::format(msgAnArrayOfDependencies); } virtual Optional> visit_array(Json::Reader& r, const Json::Array& arr) const override { - return r.array_elements(arr, DependencyDeserializer::instance); + return r.array_elements(arr, DependencyDeserializer{depends_defaults}); } - - static const DependencyArrayDeserializer instance; }; - const DependencyArrayDeserializer DependencyArrayDeserializer::instance; - constexpr StringLiteral DependencyDeserializer::NAME; constexpr StringLiteral DependencyDeserializer::HOST; constexpr StringLiteral DependencyDeserializer::FEATURES; @@ -978,6 +977,8 @@ namespace vcpkg // `ArrayFeatureDeserializer` is used for the former, `FeatureDeserializer` is used for the latter. struct FeatureDeserializer final : Json::IDeserializer> { + explicit FeatureDeserializer(bool depends_defaults) : depends_defaults(depends_defaults) { } + bool depends_defaults; virtual LocalizedString type_name() const override { return msg::format(msgAFeature); } constexpr static StringLiteral NAME = "name"; @@ -1006,7 +1007,8 @@ namespace vcpkg r.required_object_field( type_name(), obj, DESCRIPTION, feature->description, Json::ParagraphDeserializer::instance); - r.optional_object_field(obj, DEPENDENCIES, feature->dependencies, DependencyArrayDeserializer::instance); + r.optional_object_field( + obj, DEPENDENCIES, feature->dependencies, DependencyArrayDeserializer{depends_defaults}); r.optional_object_field(obj, SUPPORTS, feature->supports_expression, PlatformExprDeserializer::instance); std::string license; if (r.optional_object_field(obj, LICENSE, license, LicenseExpressionDeserializer::instance)) @@ -1016,11 +1018,8 @@ namespace vcpkg return std::move(feature); // gcc-7 bug workaround redundant move } - - static const FeatureDeserializer instance; }; - const FeatureDeserializer FeatureDeserializer::instance; constexpr StringLiteral FeatureDeserializer::NAME; constexpr StringLiteral FeatureDeserializer::DESCRIPTION; constexpr StringLiteral FeatureDeserializer::DEPENDENCIES; @@ -1034,6 +1033,8 @@ namespace vcpkg struct FeaturesFieldDeserializer final : Json::IDeserializer { + explicit FeaturesFieldDeserializer(bool depends_defaults) : depends_defaults(depends_defaults) { } + bool depends_defaults; virtual LocalizedString type_name() const override { return msg::format(msgASetOfFeatures); } virtual Span valid_fields() const override { return {}; } @@ -1056,7 +1057,7 @@ namespace vcpkg continue; } std::unique_ptr v; - r.visit_in_key(pr.second, pr.first, v, FeatureDeserializer::instance); + r.visit_in_key(pr.second, pr.first, v, FeatureDeserializer{depends_defaults}); if (v) { v->name = pr.first.to_string(); @@ -1066,12 +1067,8 @@ namespace vcpkg return std::move(res); // gcc-7 bug workaround redundant move } - - static const FeaturesFieldDeserializer instance; }; - const FeaturesFieldDeserializer FeaturesFieldDeserializer::instance; - struct ContactsDeserializer final : Json::IDeserializer { virtual LocalizedString type_name() const override { return msg::format(msgADictionaryOfContacts); } @@ -1176,7 +1173,8 @@ namespace vcpkg spgh.license = {std::move(license)}; } - r.optional_object_field(obj, DEPENDENCIES, spgh.dependencies, DependencyArrayDeserializer::instance); + r.optional_object_field( + obj, DEPENDENCIES, spgh.dependencies, DependencyArrayDeserializer{spgh.depend_defaults}); r.optional_object_field(obj, OVERRIDES, spgh.overrides, DependencyOverrideArrayDeserializer::instance); std::string baseline; @@ -1190,7 +1188,7 @@ namespace vcpkg obj, DEFAULT_FEATURES, spgh.default_features, DependencyFeatureArrayDeserializer::instance); FeaturesObject features_tmp; - r.optional_object_field(obj, FEATURES, features_tmp, FeaturesFieldDeserializer::instance); + r.optional_object_field(obj, FEATURES, features_tmp, FeaturesFieldDeserializer{spgh.depend_defaults}); control_file->feature_paragraphs = std::move(features_tmp.feature_paragraphs); control_file->extra_features_info = std::move(features_tmp.extra_features_info); @@ -1702,9 +1700,10 @@ namespace vcpkg dep_obj.insert(DependencyDeserializer::NAME, dep.name); if (dep.host) dep_obj.insert(DependencyDeserializer::HOST, Json::Value::boolean(true)); - if (!dep.default_features) + if (dep.default_features != scf.core_paragraph->depend_defaults) { - dep_obj.insert(DependencyDeserializer::DEFAULT_FEATURES, Json::Value::boolean(false)); + dep_obj.insert(DependencyDeserializer::DEFAULT_FEATURES, + Json::Value::boolean(dep.default_features)); } serialize_dependency_features(dep_obj, DependencyDeserializer::FEATURES, dep.features); serialize_optional_string(dep_obj, DependencyDeserializer::PLATFORM, to_string(dep.platform)); From 5114b658295ca005d6de264a8474b45447415d77 Mon Sep 17 00:00:00 2001 From: Leander Schulten Date: Sat, 5 Aug 2023 16:03:02 +0200 Subject: [PATCH 11/14] Remove unnecessary change --- src/vcpkg/commands.build.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/vcpkg/commands.build.cpp b/src/vcpkg/commands.build.cpp index b22127714e..4f5f3aee04 100644 --- a/src/vcpkg/commands.build.cpp +++ b/src/vcpkg/commands.build.cpp @@ -1329,7 +1329,6 @@ namespace vcpkg { for (const FeatureSpec& fspec : kv.second) { - if (fspec.feature() == "default") continue; if (!status_db.is_installed(fspec) && !(fspec.port() == name && fspec.triplet() == spec.triplet())) { missing_fspecs.emplace_back(fspec); From fe3b6126efc9a1df3d3fd5e26d2699f4977282f6 Mon Sep 17 00:00:00 2001 From: Leander Schulten Date: Sat, 5 Aug 2023 17:35:15 +0200 Subject: [PATCH 12/14] Adapt schema. --- docs/vcpkg.schema.json | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/vcpkg.schema.json b/docs/vcpkg.schema.json index 0d32d81484..92312bc730 100644 --- a/docs/vcpkg.schema.json +++ b/docs/vcpkg.schema.json @@ -80,6 +80,11 @@ } } }, + "depend-defaults": { + "description": "The default value for the default-features field of a dependency", + "type": "boolean", + "default": true + }, "default-features": { "description": "Features enabled by default with the package.", "type": "array", From 008018423f8f39e40765cbb05031de441af0e4ed Mon Sep 17 00:00:00 2001 From: Leander Schulten Date: Sun, 12 Nov 2023 03:26:20 +0100 Subject: [PATCH 13/14] Fix merge bug --- src/vcpkg-test/dependencies.cpp | 2 +- src/vcpkg/sourceparagraph.cpp | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/vcpkg-test/dependencies.cpp b/src/vcpkg-test/dependencies.cpp index da092c4000..97451159e3 100644 --- a/src/vcpkg-test/dependencies.cpp +++ b/src/vcpkg-test/dependencies.cpp @@ -1687,7 +1687,7 @@ TEST_CASE ("version install depend-defaults", "[versionplan]") auto& a_scf = op.emplace("a"); a_scf.source_control_file->core_paragraph->dependencies.push_back(CoreDependency{"b"}); auto& b_scf = op.emplace("b"); - b_scf.source_control_file->core_paragraph->default_features.emplace_back("0"); + b_scf.source_control_file->core_paragraph->default_features.push_back({"0"}); b_scf.source_control_file->feature_paragraphs.emplace_back(make_fpgh("0")); auto& d_scf = op.emplace("c"); d_scf.source_control_file->core_paragraph->dependencies.push_back(Dependency{"b"}); diff --git a/src/vcpkg/sourceparagraph.cpp b/src/vcpkg/sourceparagraph.cpp index 87ef722b01..0beddf7064 100644 --- a/src/vcpkg/sourceparagraph.cpp +++ b/src/vcpkg/sourceparagraph.cpp @@ -666,8 +666,11 @@ namespace vcpkg virtual Optional visit_string(Json::Reader& r, StringView sv) const override { - return Json::PackageNameDeserializer::instance.visit_string(r, sv).map( - [](std::string&& name) { return Dependency{std::move(name)}; }); + return Json::PackageNameDeserializer::instance.visit_string(r, sv).map([this](std::string&& name) { + Dependency dep{std::move(name)}; + dep.default_features = depends_defaults; + return dep; + }); } virtual Optional visit_object(Json::Reader& r, const Json::Object& obj) const override From 9befbe2e9d87630630772a24e75980cc5789b569 Mon Sep 17 00:00:00 2001 From: Leander Schulten Date: Wed, 17 Apr 2024 19:07:14 +0200 Subject: [PATCH 14/14] More merge conflicts --- src/vcpkg-test/dependencies.cpp | 14 +++++++------- src/vcpkg-test/plan.cpp | 10 +++++----- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/vcpkg-test/dependencies.cpp b/src/vcpkg-test/dependencies.cpp index eb3cb71efc..6d108eb8ee 100644 --- a/src/vcpkg-test/dependencies.cpp +++ b/src/vcpkg-test/dependencies.cpp @@ -227,7 +227,7 @@ static ExpectedL create_versioned_install_plan(const IVersionedPortf deps, overrides, toplevel, - {Test::ARM_UWP, "pkgs", UnsupportedPortAction::Error, ImplicitDefault::YES}); + {Test::ARM_UWP, "pkgs", UnsupportedPortAction::Error, ImplicitDefault::Yes}); } static ExpectedL create_versioned_install_plan(const IVersionedPortfileProvider& provider, @@ -246,7 +246,7 @@ static ExpectedL create_versioned_install_plan(const IVersionedPortf deps, overrides, toplevel, - {Test::ARM_UWP, "pkgs", UnsupportedPortAction::Error, ImplicitDefault::YES}); + {Test::ARM_UWP, "pkgs", UnsupportedPortAction::Error, ImplicitDefault::Yes}); } static ExpectedL create_versioned_install_plan(const IOverlayProvider& oprovider, @@ -1699,18 +1699,18 @@ TEST_CASE ("version install depend-defaults", "[versionplan]") auto& d_scf = op.emplace("c"); d_scf.source_control_file->core_paragraph->dependencies.push_back(Dependency{"b"}); - SECTION ("toplevel ImplicitDefault::YES") + SECTION ("toplevel ImplicitDefault::Yes") { - auto install_plan = create_versioned_install_plan(op, {Dependency{"a"}}, toplevel_spec(), ImplicitDefault::YES) + auto install_plan = create_versioned_install_plan(op, {Dependency{"a"}}, toplevel_spec(), ImplicitDefault::Yes) .value_or_exit(VCPKG_LINE_INFO); REQUIRE(install_plan.size() == 2); check_name_and_features(install_plan.install_actions[0], "b", {"0"}); check_name_and_features(install_plan.install_actions[1], "a", {}); } - SECTION ("toplevel ImplicitDefault::NO") + SECTION ("toplevel ImplicitDefault::No") { - auto install_plan = create_versioned_install_plan(op, {Dependency{"a"}}, toplevel_spec(), ImplicitDefault::NO) + auto install_plan = create_versioned_install_plan(op, {Dependency{"a"}}, toplevel_spec(), ImplicitDefault::No) .value_or_exit(VCPKG_LINE_INFO); REQUIRE(install_plan.size() == 2); @@ -1718,7 +1718,7 @@ TEST_CASE ("version install depend-defaults", "[versionplan]") check_name_and_features(install_plan.install_actions[1], "a", {}); // c -> b[default], so depend-defaults should not suppress it - install_plan = create_versioned_install_plan(op, {Dependency{"c"}}, toplevel_spec(), ImplicitDefault::NO) + install_plan = create_versioned_install_plan(op, {Dependency{"c"}}, toplevel_spec(), ImplicitDefault::No) .value_or_exit(VCPKG_LINE_INFO); REQUIRE(install_plan.size() == 2); check_name_and_features(install_plan.install_actions[0], "b", {"0"}); diff --git a/src/vcpkg-test/plan.cpp b/src/vcpkg-test/plan.cpp index 80a243f00f..3308e4d083 100644 --- a/src/vcpkg-test/plan.cpp +++ b/src/vcpkg-test/plan.cpp @@ -61,7 +61,7 @@ static ActionPlan create_feature_install_plan(const PortFileProvider& port_provi const CMakeVars::CMakeVarProvider& var_provider, View specs, const StatusParagraphs& status_db, - ImplicitDefault implicit_defaults = ImplicitDefault::YES) + ImplicitDefault implicit_defaults = ImplicitDefault::Yes) { const CreateInstallPlanOptions create_options{ Test::X64_ANDROID, "pkg", UnsupportedPortAction::Error, implicit_defaults}; @@ -845,7 +845,7 @@ TEST_CASE ("install with ImplicitDefault::NO", "[plan]") SECTION ("depend-defaults true from core") { auto install_plan = - create_feature_install_plan(map_port, var_provider, full_package_specs, {}, ImplicitDefault::NO); + create_feature_install_plan(map_port, var_provider, full_package_specs, {}, ImplicitDefault::No); REQUIRE(install_plan.size() == 2); features_check(install_plan.install_actions.at(0), "b", {"0", "core"}); features_check(install_plan.install_actions.at(1), "a", {"core"}); @@ -854,7 +854,7 @@ TEST_CASE ("install with ImplicitDefault::NO", "[plan]") SECTION ("depend-defaults true from feature") { auto install_plan = - create_feature_install_plan(map_port, var_provider, full_package_specs2, {}, ImplicitDefault::NO); + create_feature_install_plan(map_port, var_provider, full_package_specs2, {}, ImplicitDefault::No); REQUIRE(install_plan.size() == 2); features_check(install_plan.install_actions.at(0), "b", {"0", "core"}); features_check(install_plan.install_actions.at(1), "c", {"0", "core"}); @@ -866,7 +866,7 @@ TEST_CASE ("install with ImplicitDefault::NO", "[plan]") SECTION ("depend-defaults false from core") { auto install_plan = - create_feature_install_plan(map_port, var_provider, full_package_specs, {}, ImplicitDefault::NO); + create_feature_install_plan(map_port, var_provider, full_package_specs, {}, ImplicitDefault::No); REQUIRE(install_plan.size() == 2); features_check(install_plan.install_actions.at(0), "b", {"core"}); features_check(install_plan.install_actions.at(1), "a", {"core"}); @@ -875,7 +875,7 @@ TEST_CASE ("install with ImplicitDefault::NO", "[plan]") { spec_map.map["c"].source_control_file->core_paragraph->depend_defaults = false; auto install_plan = - create_feature_install_plan(map_port, var_provider, full_package_specs2, {}, ImplicitDefault::NO); + create_feature_install_plan(map_port, var_provider, full_package_specs2, {}, ImplicitDefault::No); REQUIRE(install_plan.size() == 2); features_check(install_plan.install_actions.at(0), "b", {"core"}); features_check(install_plan.install_actions.at(1), "c", {"0", "core"});