Skip to content

Commit 9b151e6

Browse files
authored
Use Version in DependencyOverride rather than reinventing it. (#1238)
1 parent 3d1f382 commit 9b151e6

File tree

7 files changed

+93
-79
lines changed

7 files changed

+93
-79
lines changed

include/vcpkg/sourceparagraph.h

+2-3
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,8 @@ namespace vcpkg
7070
struct DependencyOverride
7171
{
7272
std::string name;
73-
std::string version;
74-
int port_version = 0;
75-
VersionScheme version_scheme = VersionScheme::String;
73+
Version version;
74+
VersionScheme scheme;
7675

7776
Json::Object extra_info;
7877

include/vcpkg/versions.h

+1
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ namespace vcpkg
7373
Version version;
7474

7575
friend bool operator==(const SchemedVersion& lhs, const SchemedVersion& rhs);
76+
friend bool operator!=(const SchemedVersion& lhs, const SchemedVersion& rhs);
7677
};
7778

7879
StringLiteral to_string_literal(VersionScheme scheme);

src/vcpkg-test/dependencies.cpp

+41-39
Original file line numberDiff line numberDiff line change
@@ -1554,29 +1554,23 @@ TEST_CASE ("version install overrides", "[versionplan]")
15541554
bp.v["b"] = {"2", 0};
15551555
bp.v["c"] = {"2", 0};
15561556

1557+
DependencyOverride bdo{"b", Version{"1", 0}, VersionScheme::String};
1558+
DependencyOverride cdo{"c", Version{"1", 0}, VersionScheme::String};
15571559
SECTION ("string")
15581560
{
1559-
auto install_plan = create_versioned_install_plan(vp,
1560-
bp,
1561-
var_provider,
1562-
{Dependency{"c"}},
1563-
{DependencyOverride{"b", "1"}, DependencyOverride{"c", "1"}},
1564-
toplevel_spec())
1565-
.value_or_exit(VCPKG_LINE_INFO);
1561+
auto install_plan =
1562+
create_versioned_install_plan(vp, bp, var_provider, {Dependency{"c"}}, {bdo, cdo}, toplevel_spec())
1563+
.value_or_exit(VCPKG_LINE_INFO);
15661564

15671565
REQUIRE(install_plan.size() == 1);
15681566
check_name_and_version(install_plan.install_actions[0], "c", {"1", 0});
15691567
}
15701568

15711569
SECTION ("relaxed")
15721570
{
1573-
auto install_plan = create_versioned_install_plan(vp,
1574-
bp,
1575-
var_provider,
1576-
{Dependency{"b"}},
1577-
{DependencyOverride{"b", "1"}, DependencyOverride{"c", "1"}},
1578-
toplevel_spec())
1579-
.value_or_exit(VCPKG_LINE_INFO);
1571+
auto install_plan =
1572+
create_versioned_install_plan(vp, bp, var_provider, {Dependency{"b"}}, {bdo, cdo}, toplevel_spec())
1573+
.value_or_exit(VCPKG_LINE_INFO);
15801574

15811575
REQUIRE(install_plan.size() == 1);
15821576
check_name_and_version(install_plan.install_actions[0], "b", {"1", 0});
@@ -1600,13 +1594,10 @@ TEST_CASE ("version install transitive overrides", "[versionplan]")
16001594
bp.v["b"] = {"2", 0};
16011595
bp.v["c"] = {"2", 1};
16021596

1597+
DependencyOverride bdo{"b", Version{"1", 0}, VersionScheme::String};
1598+
DependencyOverride cdo{"c", Version{"1", 0}, VersionScheme::String};
16031599
WITH_EXPECTED(install_plan,
1604-
create_versioned_install_plan(vp,
1605-
bp,
1606-
var_provider,
1607-
{Dependency{"b"}},
1608-
{DependencyOverride{"b", "1"}, DependencyOverride{"c", "1"}},
1609-
toplevel_spec()));
1600+
create_versioned_install_plan(vp, bp, var_provider, {Dependency{"b"}}, {bdo, cdo}, toplevel_spec()));
16101601

16111602
REQUIRE(install_plan.size() == 2);
16121603
check_name_and_version(install_plan.install_actions[0], "c", {"1", 0});
@@ -2153,7 +2144,7 @@ TEST_CASE ("version overlay ports", "[versionplan]")
21532144
Dependency{"a", {}, {}, {VersionConstraintKind::Minimum, "1", 1}},
21542145
},
21552146
{
2156-
DependencyOverride{"a", "2", 0},
2147+
DependencyOverride{"a", Version{"2", 0}, VersionScheme::String},
21572148
},
21582149
toplevel_spec())
21592150
.value_or_exit(VCPKG_LINE_INFO);
@@ -2163,18 +2154,19 @@ TEST_CASE ("version overlay ports", "[versionplan]")
21632154
}
21642155
SECTION ("override")
21652156
{
2166-
auto install_plan = create_versioned_install_plan(vp,
2167-
bp,
2168-
oprovider,
2169-
var_provider,
2170-
{
2171-
Dependency{"a"},
2172-
},
2173-
{
2174-
DependencyOverride{"a", "2", 0},
2175-
},
2176-
toplevel_spec())
2177-
.value_or_exit(VCPKG_LINE_INFO);
2157+
auto install_plan =
2158+
create_versioned_install_plan(vp,
2159+
bp,
2160+
oprovider,
2161+
var_provider,
2162+
{
2163+
Dependency{"a"},
2164+
},
2165+
{
2166+
DependencyOverride{"a", Version{"2", 0}, VersionScheme::String},
2167+
},
2168+
toplevel_spec())
2169+
.value_or_exit(VCPKG_LINE_INFO);
21782170

21792171
REQUIRE(install_plan.size() == 1);
21802172
check_name_and_version(install_plan.install_actions[0], "a", {"overlay", 0});
@@ -2203,13 +2195,23 @@ TEST_CASE ("respect supports expression", "[versionplan]")
22032195
{
22042196
// override from non supported to supported version
22052197
MockOverlayProvider oprovider;
2206-
install_plan = create_versioned_install_plan(
2207-
vp, bp, oprovider, var_provider, {Dependency{"a"}}, {DependencyOverride{"a", "1", 1}}, toplevel_spec());
2198+
install_plan = create_versioned_install_plan(vp,
2199+
bp,
2200+
oprovider,
2201+
var_provider,
2202+
{Dependency{"a"}},
2203+
{DependencyOverride{"a", Version{"1", 1}, VersionScheme::String}},
2204+
toplevel_spec());
22082205
CHECK(install_plan.has_value());
22092206
// override from supported to non supported version
22102207
bp.v["a"] = {"1", 1};
2211-
install_plan = create_versioned_install_plan(
2212-
vp, bp, oprovider, var_provider, {Dependency{"a"}}, {DependencyOverride{"a", "1", 0}}, toplevel_spec());
2208+
install_plan = create_versioned_install_plan(vp,
2209+
bp,
2210+
oprovider,
2211+
var_provider,
2212+
{Dependency{"a"}},
2213+
{DependencyOverride{"a", Version{"1", 0}, VersionScheme::String}},
2214+
toplevel_spec());
22132215
CHECK_FALSE(install_plan.has_value());
22142216
}
22152217
}
@@ -2246,7 +2248,7 @@ TEST_CASE ("respect supports expressions of features", "[versionplan]")
22462248
oprovider,
22472249
var_provider,
22482250
{Dependency{"a", {{"x"}}}},
2249-
{DependencyOverride{"a", "1", 1}},
2251+
{DependencyOverride{"a", Version{"1", 1}, VersionScheme::String}},
22502252
toplevel_spec());
22512253
CHECK(install_plan.has_value());
22522254
// override from supported to non supported version
@@ -2256,7 +2258,7 @@ TEST_CASE ("respect supports expressions of features", "[versionplan]")
22562258
oprovider,
22572259
var_provider,
22582260
{Dependency{"a", {{"x"}}}},
2259-
{DependencyOverride{"a", "1", 0}},
2261+
{DependencyOverride{"a", Version{"1", 0}, VersionScheme::String}},
22602262
toplevel_spec());
22612263
CHECK_FALSE(install_plan.has_value());
22622264
}

src/vcpkg-test/manifests.cpp

+39-16
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,11 @@ TEST_CASE ("manifest overrides embedded port version", "[manifests]")
390390
]
391391
})json");
392392
REQUIRE(parsed.has_value());
393-
CHECK((*parsed.get())->core_paragraph->overrides.at(0).port_version == 1);
393+
{
394+
const auto& first_override = (*parsed.get())->core_paragraph->overrides.at(0);
395+
CHECK(first_override.version == Version{"abcd", 1});
396+
CHECK(first_override.scheme == VersionScheme::String);
397+
}
394398

395399
parsed = test_parse_port_manifest(R"json({
396400
"name": "zlib",
@@ -403,7 +407,11 @@ TEST_CASE ("manifest overrides embedded port version", "[manifests]")
403407
]
404408
})json");
405409
REQUIRE(parsed.has_value());
406-
CHECK((*parsed.get())->core_paragraph->overrides.at(0).port_version == 1);
410+
{
411+
const auto& first_override = (*parsed.get())->core_paragraph->overrides.at(0);
412+
CHECK(first_override.version == Version{"2018-01-01", 1});
413+
CHECK(first_override.scheme == VersionScheme::Date);
414+
}
407415

408416
parsed = test_parse_port_manifest(R"json({
409417
"name": "zlib",
@@ -416,7 +424,11 @@ TEST_CASE ("manifest overrides embedded port version", "[manifests]")
416424
]
417425
})json");
418426
REQUIRE(parsed.has_value());
419-
CHECK((*parsed.get())->core_paragraph->overrides.at(0).port_version == 1);
427+
{
428+
const auto& first_override = (*parsed.get())->core_paragraph->overrides.at(0);
429+
CHECK(first_override.version == Version{"1.2", 1});
430+
CHECK(first_override.scheme == VersionScheme::Relaxed);
431+
}
420432

421433
parsed = test_parse_port_manifest(R"json({
422434
"name": "zlib",
@@ -429,7 +441,11 @@ TEST_CASE ("manifest overrides embedded port version", "[manifests]")
429441
]
430442
})json");
431443
REQUIRE(parsed.has_value());
432-
CHECK((*parsed.get())->core_paragraph->overrides.at(0).port_version == 1);
444+
{
445+
const auto& first_override = (*parsed.get())->core_paragraph->overrides.at(0);
446+
CHECK(first_override.version == Version{"1.2.0", 1});
447+
CHECK(first_override.scheme == VersionScheme::Semver);
448+
}
433449
}
434450

435451
TEST_CASE ("manifest constraints", "[manifests]")
@@ -565,9 +581,9 @@ TEST_CASE ("manifest builtin-baseline", "[manifests]")
565581
REQUIRE(pgh.core_paragraph->dependencies[0].constraint.port_version == 1);
566582
REQUIRE(pgh.core_paragraph->dependencies[0].constraint.type == VersionConstraintKind::Minimum);
567583
REQUIRE(pgh.core_paragraph->overrides.size() == 1);
568-
REQUIRE(pgh.core_paragraph->overrides[0].version_scheme == VersionScheme::String);
569-
REQUIRE(pgh.core_paragraph->overrides[0].version == "abcd");
570-
REQUIRE(pgh.core_paragraph->overrides[0].port_version == 0);
584+
const auto& first_override = pgh.core_paragraph->overrides[0];
585+
REQUIRE(first_override.version == Version{"abcd", 0});
586+
REQUIRE(first_override.scheme == VersionScheme::String);
571587
REQUIRE(pgh.core_paragraph->builtin_baseline.value_or("does not have a value") ==
572588
"089fa4de7dca22c67dcab631f618d5cd0697c8d4");
573589
REQUIRE(!pgh.check_against_feature_flags({}, feature_flags_without_versioning));
@@ -602,9 +618,9 @@ TEST_CASE ("manifest builtin-baseline", "[manifests]")
602618
REQUIRE(pgh.core_paragraph->dependencies[0].constraint.port_version == 1);
603619
REQUIRE(pgh.core_paragraph->dependencies[0].constraint.type == VersionConstraintKind::Minimum);
604620
REQUIRE(pgh.core_paragraph->overrides.size() == 1);
605-
REQUIRE(pgh.core_paragraph->overrides[0].version_scheme == VersionScheme::String);
606-
REQUIRE(pgh.core_paragraph->overrides[0].version == "abcd");
607-
REQUIRE(pgh.core_paragraph->overrides[0].port_version == 0);
621+
const auto& first_override = pgh.core_paragraph->overrides[0];
622+
REQUIRE(first_override.version == Version{"abcd", 0});
623+
REQUIRE(first_override.scheme == VersionScheme::String);
608624
REQUIRE(!pgh.core_paragraph->builtin_baseline.has_value());
609625
REQUIRE(!pgh.check_against_feature_flags({}, feature_flags_without_versioning));
610626
REQUIRE(!pgh.check_against_feature_flags({}, feature_flags_with_versioning));
@@ -679,8 +695,9 @@ TEST_CASE ("manifest overrides", "[manifests]")
679695
auto& pgh = **m_pgh.get();
680696
REQUIRE(Json::stringify(serialize_manifest(pgh), Json::JsonStyle::with_spaces(4)) == std::get<0>(v));
681697
REQUIRE(pgh.core_paragraph->overrides.size() == 1);
682-
REQUIRE(pgh.core_paragraph->overrides[0].version_scheme == std::get<1>(v));
683-
REQUIRE(pgh.core_paragraph->overrides[0].version == std::get<2>(v));
698+
const auto& first_override = pgh.core_paragraph->overrides[0];
699+
REQUIRE(first_override.version == Version{std::get<2>(v).to_string(), 0});
700+
REQUIRE(first_override.scheme == std::get<1>(v));
684701
REQUIRE(!pgh.check_against_feature_flags({}, feature_flags_without_versioning));
685702
REQUIRE(pgh.check_against_feature_flags({}, feature_flags_with_versioning));
686703
}
@@ -732,10 +749,16 @@ TEST_CASE ("manifest overrides", "[manifests]")
732749
auto& pgh = **m_pgh.get();
733750
REQUIRE(Json::stringify(serialize_manifest(pgh), Json::JsonStyle::with_spaces(4)) == raw);
734751
REQUIRE(pgh.core_paragraph->overrides.size() == 2);
735-
REQUIRE(pgh.core_paragraph->overrides[0].name == "abc");
736-
REQUIRE(pgh.core_paragraph->overrides[0].port_version == 5);
737-
REQUIRE(pgh.core_paragraph->overrides[1].name == "abcd");
738-
REQUIRE(pgh.core_paragraph->overrides[1].port_version == 7);
752+
{
753+
const auto& first_override = pgh.core_paragraph->overrides[0];
754+
const auto& second_override = pgh.core_paragraph->overrides[1];
755+
REQUIRE(first_override.name == "abc");
756+
REQUIRE(first_override.version == Version{"hello", 5});
757+
REQUIRE(first_override.scheme == VersionScheme::String);
758+
REQUIRE(second_override.name == "abcd");
759+
REQUIRE(second_override.version == Version{"hello", 7});
760+
REQUIRE(second_override.scheme == VersionScheme::String);
761+
}
739762

740763
REQUIRE(!pgh.check_against_feature_flags({}, feature_flags_without_versioning));
741764
REQUIRE(pgh.check_against_feature_flags({}, feature_flags_with_versioning));

src/vcpkg/dependencies.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -2007,7 +2007,7 @@ namespace vcpkg
20072007
provider, bprovider, oprovider, var_provider, options.host_triplet, options.packages_dir);
20082008
for (auto&& o : overrides)
20092009
{
2010-
vpg.add_override(o.name, {o.version, o.port_version});
2010+
vpg.add_override(o.name, o.version);
20112011
}
20122012

20132013
vpg.solve_with_roots(deps, toplevel);

src/vcpkg/sourceparagraph.cpp

+7-20
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,9 @@ namespace vcpkg
7171

7272
bool operator==(const DependencyOverride& lhs, const DependencyOverride& rhs)
7373
{
74-
if (lhs.version_scheme != rhs.version_scheme) return false;
75-
if (lhs.port_version != rhs.port_version) return false;
7674
if (lhs.name != rhs.name) return false;
7775
if (lhs.version != rhs.version) return false;
76+
if (lhs.scheme != rhs.scheme) return false;
7877
return lhs.extra_info == rhs.extra_info;
7978
}
8079
bool operator!=(const DependencyOverride& lhs, const DependencyOverride& rhs);
@@ -758,22 +757,6 @@ namespace vcpkg
758757
return t;
759758
}
760759

761-
static void visit_impl(const LocalizedString& type_name,
762-
Json::Reader& r,
763-
const Json::Object& obj,
764-
std::string& name,
765-
std::string& version,
766-
VersionScheme& version_scheme,
767-
int& port_version)
768-
{
769-
r.required_object_field(type_name, obj, NAME, name, Json::PackageNameDeserializer::instance);
770-
771-
auto schemed_version = visit_required_schemed_deserializer(type_name, r, obj, true);
772-
version = schemed_version.version.text();
773-
version_scheme = schemed_version.scheme;
774-
port_version = schemed_version.version.port_version();
775-
}
776-
777760
virtual Optional<DependencyOverride> visit_object(Json::Reader& r, const Json::Object& obj) const override
778761
{
779762
DependencyOverride dep;
@@ -786,7 +769,11 @@ namespace vcpkg
786769
}
787770
}
788771

789-
visit_impl(type_name(), r, obj, dep.name, dep.version, dep.version_scheme, dep.port_version);
772+
const auto type_name = this->type_name();
773+
r.required_object_field(type_name, obj, NAME, dep.name, Json::PackageNameDeserializer::instance);
774+
auto schemed_version = visit_required_schemed_deserializer(type_name, r, obj, true);
775+
dep.version = std::move(schemed_version.version);
776+
dep.scheme = schemed_version.scheme;
790777

791778
return dep;
792779
}
@@ -1823,7 +1810,7 @@ namespace vcpkg
18231810

18241811
dep_obj.insert(DependencyOverrideDeserializer::NAME, Json::Value::string(dep.name));
18251812

1826-
serialize_schemed_version(dep_obj, dep.version_scheme, dep.version, dep.port_version);
1813+
serialize_schemed_version(dep_obj, dep.scheme, dep.version.text(), dep.version.port_version());
18271814
};
18281815

18291816
auto serialize_license =

src/vcpkg/versions.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,8 @@ namespace vcpkg
341341
return lhs.scheme == rhs.scheme && lhs.version == rhs.version;
342342
}
343343

344+
bool operator!=(const SchemedVersion& lhs, const SchemedVersion& rhs) { return !(lhs == rhs); }
345+
344346
StringLiteral to_string_literal(VersionScheme scheme)
345347
{
346348
static constexpr StringLiteral MISSING = "missing";

0 commit comments

Comments
 (0)