From 9b151e6b7efb66591123a2a75aa541de0c9c0681 Mon Sep 17 00:00:00 2001 From: Billy O'Neal Date: Mon, 23 Oct 2023 14:21:10 -0700 Subject: [PATCH] Use Version in DependencyOverride rather than reinventing it. (#1238) --- include/vcpkg/sourceparagraph.h | 5 +-- include/vcpkg/versions.h | 1 + src/vcpkg-test/dependencies.cpp | 80 +++++++++++++++++---------------- src/vcpkg-test/manifests.cpp | 55 ++++++++++++++++------- src/vcpkg/dependencies.cpp | 2 +- src/vcpkg/sourceparagraph.cpp | 27 +++-------- src/vcpkg/versions.cpp | 2 + 7 files changed, 93 insertions(+), 79 deletions(-) diff --git a/include/vcpkg/sourceparagraph.h b/include/vcpkg/sourceparagraph.h index 0bd760cc1f..3fc06a42d3 100644 --- a/include/vcpkg/sourceparagraph.h +++ b/include/vcpkg/sourceparagraph.h @@ -70,9 +70,8 @@ namespace vcpkg struct DependencyOverride { std::string name; - std::string version; - int port_version = 0; - VersionScheme version_scheme = VersionScheme::String; + Version version; + VersionScheme scheme; Json::Object extra_info; diff --git a/include/vcpkg/versions.h b/include/vcpkg/versions.h index c86c0e8ac2..0f9257fb97 100644 --- a/include/vcpkg/versions.h +++ b/include/vcpkg/versions.h @@ -73,6 +73,7 @@ namespace vcpkg Version version; friend bool operator==(const SchemedVersion& lhs, const SchemedVersion& rhs); + friend bool operator!=(const SchemedVersion& lhs, const SchemedVersion& rhs); }; StringLiteral to_string_literal(VersionScheme scheme); diff --git a/src/vcpkg-test/dependencies.cpp b/src/vcpkg-test/dependencies.cpp index 359e4233ae..01a36d6d46 100644 --- a/src/vcpkg-test/dependencies.cpp +++ b/src/vcpkg-test/dependencies.cpp @@ -1554,15 +1554,13 @@ TEST_CASE ("version install overrides", "[versionplan]") bp.v["b"] = {"2", 0}; bp.v["c"] = {"2", 0}; + DependencyOverride bdo{"b", Version{"1", 0}, VersionScheme::String}; + DependencyOverride cdo{"c", Version{"1", 0}, VersionScheme::String}; SECTION ("string") { - auto install_plan = create_versioned_install_plan(vp, - bp, - var_provider, - {Dependency{"c"}}, - {DependencyOverride{"b", "1"}, DependencyOverride{"c", "1"}}, - toplevel_spec()) - .value_or_exit(VCPKG_LINE_INFO); + auto install_plan = + create_versioned_install_plan(vp, bp, var_provider, {Dependency{"c"}}, {bdo, cdo}, toplevel_spec()) + .value_or_exit(VCPKG_LINE_INFO); REQUIRE(install_plan.size() == 1); check_name_and_version(install_plan.install_actions[0], "c", {"1", 0}); @@ -1570,13 +1568,9 @@ TEST_CASE ("version install overrides", "[versionplan]") SECTION ("relaxed") { - auto install_plan = create_versioned_install_plan(vp, - bp, - var_provider, - {Dependency{"b"}}, - {DependencyOverride{"b", "1"}, DependencyOverride{"c", "1"}}, - toplevel_spec()) - .value_or_exit(VCPKG_LINE_INFO); + auto install_plan = + create_versioned_install_plan(vp, bp, var_provider, {Dependency{"b"}}, {bdo, cdo}, toplevel_spec()) + .value_or_exit(VCPKG_LINE_INFO); REQUIRE(install_plan.size() == 1); check_name_and_version(install_plan.install_actions[0], "b", {"1", 0}); @@ -1600,13 +1594,10 @@ TEST_CASE ("version install transitive overrides", "[versionplan]") bp.v["b"] = {"2", 0}; bp.v["c"] = {"2", 1}; + DependencyOverride bdo{"b", Version{"1", 0}, VersionScheme::String}; + DependencyOverride cdo{"c", Version{"1", 0}, VersionScheme::String}; WITH_EXPECTED(install_plan, - create_versioned_install_plan(vp, - bp, - var_provider, - {Dependency{"b"}}, - {DependencyOverride{"b", "1"}, DependencyOverride{"c", "1"}}, - toplevel_spec())); + create_versioned_install_plan(vp, bp, var_provider, {Dependency{"b"}}, {bdo, cdo}, toplevel_spec())); REQUIRE(install_plan.size() == 2); check_name_and_version(install_plan.install_actions[0], "c", {"1", 0}); @@ -2153,7 +2144,7 @@ TEST_CASE ("version overlay ports", "[versionplan]") Dependency{"a", {}, {}, {VersionConstraintKind::Minimum, "1", 1}}, }, { - DependencyOverride{"a", "2", 0}, + DependencyOverride{"a", Version{"2", 0}, VersionScheme::String}, }, toplevel_spec()) .value_or_exit(VCPKG_LINE_INFO); @@ -2163,18 +2154,19 @@ TEST_CASE ("version overlay ports", "[versionplan]") } SECTION ("override") { - auto install_plan = create_versioned_install_plan(vp, - bp, - oprovider, - var_provider, - { - Dependency{"a"}, - }, - { - DependencyOverride{"a", "2", 0}, - }, - toplevel_spec()) - .value_or_exit(VCPKG_LINE_INFO); + auto install_plan = + create_versioned_install_plan(vp, + bp, + oprovider, + var_provider, + { + Dependency{"a"}, + }, + { + DependencyOverride{"a", Version{"2", 0}, VersionScheme::String}, + }, + toplevel_spec()) + .value_or_exit(VCPKG_LINE_INFO); REQUIRE(install_plan.size() == 1); check_name_and_version(install_plan.install_actions[0], "a", {"overlay", 0}); @@ -2203,13 +2195,23 @@ TEST_CASE ("respect supports expression", "[versionplan]") { // override from non supported to supported version MockOverlayProvider oprovider; - install_plan = create_versioned_install_plan( - vp, bp, oprovider, var_provider, {Dependency{"a"}}, {DependencyOverride{"a", "1", 1}}, toplevel_spec()); + install_plan = create_versioned_install_plan(vp, + bp, + oprovider, + var_provider, + {Dependency{"a"}}, + {DependencyOverride{"a", Version{"1", 1}, VersionScheme::String}}, + toplevel_spec()); CHECK(install_plan.has_value()); // override from supported to non supported version bp.v["a"] = {"1", 1}; - install_plan = create_versioned_install_plan( - vp, bp, oprovider, var_provider, {Dependency{"a"}}, {DependencyOverride{"a", "1", 0}}, toplevel_spec()); + install_plan = create_versioned_install_plan(vp, + bp, + oprovider, + var_provider, + {Dependency{"a"}}, + {DependencyOverride{"a", Version{"1", 0}, VersionScheme::String}}, + toplevel_spec()); CHECK_FALSE(install_plan.has_value()); } } @@ -2246,7 +2248,7 @@ TEST_CASE ("respect supports expressions of features", "[versionplan]") oprovider, var_provider, {Dependency{"a", {{"x"}}}}, - {DependencyOverride{"a", "1", 1}}, + {DependencyOverride{"a", Version{"1", 1}, VersionScheme::String}}, toplevel_spec()); CHECK(install_plan.has_value()); // override from supported to non supported version @@ -2256,7 +2258,7 @@ TEST_CASE ("respect supports expressions of features", "[versionplan]") oprovider, var_provider, {Dependency{"a", {{"x"}}}}, - {DependencyOverride{"a", "1", 0}}, + {DependencyOverride{"a", Version{"1", 0}, VersionScheme::String}}, toplevel_spec()); CHECK_FALSE(install_plan.has_value()); } diff --git a/src/vcpkg-test/manifests.cpp b/src/vcpkg-test/manifests.cpp index 9411dfb071..83f51f8d68 100644 --- a/src/vcpkg-test/manifests.cpp +++ b/src/vcpkg-test/manifests.cpp @@ -390,7 +390,11 @@ TEST_CASE ("manifest overrides embedded port version", "[manifests]") ] })json"); REQUIRE(parsed.has_value()); - CHECK((*parsed.get())->core_paragraph->overrides.at(0).port_version == 1); + { + const auto& first_override = (*parsed.get())->core_paragraph->overrides.at(0); + CHECK(first_override.version == Version{"abcd", 1}); + CHECK(first_override.scheme == VersionScheme::String); + } parsed = test_parse_port_manifest(R"json({ "name": "zlib", @@ -403,7 +407,11 @@ TEST_CASE ("manifest overrides embedded port version", "[manifests]") ] })json"); REQUIRE(parsed.has_value()); - CHECK((*parsed.get())->core_paragraph->overrides.at(0).port_version == 1); + { + const auto& first_override = (*parsed.get())->core_paragraph->overrides.at(0); + CHECK(first_override.version == Version{"2018-01-01", 1}); + CHECK(first_override.scheme == VersionScheme::Date); + } parsed = test_parse_port_manifest(R"json({ "name": "zlib", @@ -416,7 +424,11 @@ TEST_CASE ("manifest overrides embedded port version", "[manifests]") ] })json"); REQUIRE(parsed.has_value()); - CHECK((*parsed.get())->core_paragraph->overrides.at(0).port_version == 1); + { + const auto& first_override = (*parsed.get())->core_paragraph->overrides.at(0); + CHECK(first_override.version == Version{"1.2", 1}); + CHECK(first_override.scheme == VersionScheme::Relaxed); + } parsed = test_parse_port_manifest(R"json({ "name": "zlib", @@ -429,7 +441,11 @@ TEST_CASE ("manifest overrides embedded port version", "[manifests]") ] })json"); REQUIRE(parsed.has_value()); - CHECK((*parsed.get())->core_paragraph->overrides.at(0).port_version == 1); + { + const auto& first_override = (*parsed.get())->core_paragraph->overrides.at(0); + CHECK(first_override.version == Version{"1.2.0", 1}); + CHECK(first_override.scheme == VersionScheme::Semver); + } } TEST_CASE ("manifest constraints", "[manifests]") @@ -565,9 +581,9 @@ TEST_CASE ("manifest builtin-baseline", "[manifests]") REQUIRE(pgh.core_paragraph->dependencies[0].constraint.port_version == 1); REQUIRE(pgh.core_paragraph->dependencies[0].constraint.type == VersionConstraintKind::Minimum); REQUIRE(pgh.core_paragraph->overrides.size() == 1); - REQUIRE(pgh.core_paragraph->overrides[0].version_scheme == VersionScheme::String); - REQUIRE(pgh.core_paragraph->overrides[0].version == "abcd"); - REQUIRE(pgh.core_paragraph->overrides[0].port_version == 0); + const auto& first_override = pgh.core_paragraph->overrides[0]; + REQUIRE(first_override.version == Version{"abcd", 0}); + REQUIRE(first_override.scheme == VersionScheme::String); REQUIRE(pgh.core_paragraph->builtin_baseline.value_or("does not have a value") == "089fa4de7dca22c67dcab631f618d5cd0697c8d4"); REQUIRE(!pgh.check_against_feature_flags({}, feature_flags_without_versioning)); @@ -602,9 +618,9 @@ TEST_CASE ("manifest builtin-baseline", "[manifests]") REQUIRE(pgh.core_paragraph->dependencies[0].constraint.port_version == 1); REQUIRE(pgh.core_paragraph->dependencies[0].constraint.type == VersionConstraintKind::Minimum); REQUIRE(pgh.core_paragraph->overrides.size() == 1); - REQUIRE(pgh.core_paragraph->overrides[0].version_scheme == VersionScheme::String); - REQUIRE(pgh.core_paragraph->overrides[0].version == "abcd"); - REQUIRE(pgh.core_paragraph->overrides[0].port_version == 0); + const auto& first_override = pgh.core_paragraph->overrides[0]; + REQUIRE(first_override.version == Version{"abcd", 0}); + REQUIRE(first_override.scheme == VersionScheme::String); REQUIRE(!pgh.core_paragraph->builtin_baseline.has_value()); REQUIRE(!pgh.check_against_feature_flags({}, feature_flags_without_versioning)); REQUIRE(!pgh.check_against_feature_flags({}, feature_flags_with_versioning)); @@ -679,8 +695,9 @@ TEST_CASE ("manifest overrides", "[manifests]") auto& pgh = **m_pgh.get(); REQUIRE(Json::stringify(serialize_manifest(pgh), Json::JsonStyle::with_spaces(4)) == std::get<0>(v)); REQUIRE(pgh.core_paragraph->overrides.size() == 1); - REQUIRE(pgh.core_paragraph->overrides[0].version_scheme == std::get<1>(v)); - REQUIRE(pgh.core_paragraph->overrides[0].version == std::get<2>(v)); + const auto& first_override = pgh.core_paragraph->overrides[0]; + REQUIRE(first_override.version == Version{std::get<2>(v).to_string(), 0}); + REQUIRE(first_override.scheme == std::get<1>(v)); REQUIRE(!pgh.check_against_feature_flags({}, feature_flags_without_versioning)); REQUIRE(pgh.check_against_feature_flags({}, feature_flags_with_versioning)); } @@ -732,10 +749,16 @@ TEST_CASE ("manifest overrides", "[manifests]") auto& pgh = **m_pgh.get(); REQUIRE(Json::stringify(serialize_manifest(pgh), Json::JsonStyle::with_spaces(4)) == raw); REQUIRE(pgh.core_paragraph->overrides.size() == 2); - REQUIRE(pgh.core_paragraph->overrides[0].name == "abc"); - REQUIRE(pgh.core_paragraph->overrides[0].port_version == 5); - REQUIRE(pgh.core_paragraph->overrides[1].name == "abcd"); - REQUIRE(pgh.core_paragraph->overrides[1].port_version == 7); + { + const auto& first_override = pgh.core_paragraph->overrides[0]; + const auto& second_override = pgh.core_paragraph->overrides[1]; + REQUIRE(first_override.name == "abc"); + REQUIRE(first_override.version == Version{"hello", 5}); + REQUIRE(first_override.scheme == VersionScheme::String); + REQUIRE(second_override.name == "abcd"); + REQUIRE(second_override.version == Version{"hello", 7}); + REQUIRE(second_override.scheme == VersionScheme::String); + } REQUIRE(!pgh.check_against_feature_flags({}, feature_flags_without_versioning)); REQUIRE(pgh.check_against_feature_flags({}, feature_flags_with_versioning)); diff --git a/src/vcpkg/dependencies.cpp b/src/vcpkg/dependencies.cpp index fa7fccd69c..766536ef01 100644 --- a/src/vcpkg/dependencies.cpp +++ b/src/vcpkg/dependencies.cpp @@ -2007,7 +2007,7 @@ namespace vcpkg provider, bprovider, oprovider, var_provider, options.host_triplet, options.packages_dir); for (auto&& o : overrides) { - vpg.add_override(o.name, {o.version, o.port_version}); + vpg.add_override(o.name, o.version); } vpg.solve_with_roots(deps, toplevel); diff --git a/src/vcpkg/sourceparagraph.cpp b/src/vcpkg/sourceparagraph.cpp index 7ec557738a..11f62606b4 100644 --- a/src/vcpkg/sourceparagraph.cpp +++ b/src/vcpkg/sourceparagraph.cpp @@ -71,10 +71,9 @@ namespace vcpkg bool operator==(const DependencyOverride& lhs, const DependencyOverride& rhs) { - if (lhs.version_scheme != rhs.version_scheme) return false; - if (lhs.port_version != rhs.port_version) return false; if (lhs.name != rhs.name) return false; if (lhs.version != rhs.version) return false; + if (lhs.scheme != rhs.scheme) return false; return lhs.extra_info == rhs.extra_info; } bool operator!=(const DependencyOverride& lhs, const DependencyOverride& rhs); @@ -758,22 +757,6 @@ namespace vcpkg return t; } - static void visit_impl(const LocalizedString& type_name, - Json::Reader& r, - const Json::Object& obj, - std::string& name, - std::string& version, - VersionScheme& version_scheme, - int& port_version) - { - r.required_object_field(type_name, obj, NAME, name, Json::PackageNameDeserializer::instance); - - auto schemed_version = visit_required_schemed_deserializer(type_name, r, obj, true); - version = schemed_version.version.text(); - version_scheme = schemed_version.scheme; - port_version = schemed_version.version.port_version(); - } - virtual Optional visit_object(Json::Reader& r, const Json::Object& obj) const override { DependencyOverride dep; @@ -786,7 +769,11 @@ namespace vcpkg } } - visit_impl(type_name(), r, obj, dep.name, dep.version, dep.version_scheme, dep.port_version); + const auto type_name = this->type_name(); + r.required_object_field(type_name, obj, NAME, dep.name, Json::PackageNameDeserializer::instance); + auto schemed_version = visit_required_schemed_deserializer(type_name, r, obj, true); + dep.version = std::move(schemed_version.version); + dep.scheme = schemed_version.scheme; return dep; } @@ -1823,7 +1810,7 @@ namespace vcpkg dep_obj.insert(DependencyOverrideDeserializer::NAME, Json::Value::string(dep.name)); - serialize_schemed_version(dep_obj, dep.version_scheme, dep.version, dep.port_version); + serialize_schemed_version(dep_obj, dep.scheme, dep.version.text(), dep.version.port_version()); }; auto serialize_license = diff --git a/src/vcpkg/versions.cpp b/src/vcpkg/versions.cpp index 7b09d4df50..c02d3aa597 100644 --- a/src/vcpkg/versions.cpp +++ b/src/vcpkg/versions.cpp @@ -341,6 +341,8 @@ namespace vcpkg return lhs.scheme == rhs.scheme && lhs.version == rhs.version; } + bool operator!=(const SchemedVersion& lhs, const SchemedVersion& rhs) { return !(lhs == rhs); } + StringLiteral to_string_literal(VersionScheme scheme) { static constexpr StringLiteral MISSING = "missing";