From 142ca37b51eafe478572cb63e9df3b065b519f97 Mon Sep 17 00:00:00 2001 From: Robert Schumacher Date: Fri, 23 Oct 2020 04:22:38 -0700 Subject: [PATCH 1/5] [vcpkg] Implement constraints in manifests --- toolsrc/include/vcpkg/packagespec.h | 15 ++++++ toolsrc/include/vcpkg/versions.h | 10 ++++ toolsrc/src/vcpkg-test/manifests.cpp | 68 +++++++++++++++++++++++++++ toolsrc/src/vcpkg/packagespec.cpp | 8 ++++ toolsrc/src/vcpkg/sourceparagraph.cpp | 51 +++++++++++++++++++- 5 files changed, 151 insertions(+), 1 deletion(-) diff --git a/toolsrc/include/vcpkg/packagespec.h b/toolsrc/include/vcpkg/packagespec.h index a362d80680eb38..5a4790116196bb 100644 --- a/toolsrc/include/vcpkg/packagespec.h +++ b/toolsrc/include/vcpkg/packagespec.h @@ -6,6 +6,7 @@ #include #include +#include namespace vcpkg::Parse { @@ -133,11 +134,25 @@ namespace vcpkg static ExpectedS from_string(const std::string& input); }; + struct DependencyConstraint + { + Versions::Constraint::Type type = Versions::Constraint::Type::None; + std::string value; + int port_version = 0; + + friend bool operator==(const DependencyConstraint& lhs, const DependencyConstraint& rhs); + friend bool operator!=(const DependencyConstraint& lhs, const DependencyConstraint& rhs) + { + return !(lhs == rhs); + } + }; + struct Dependency { std::string name; std::vector features; PlatformExpression::Expr platform; + DependencyConstraint constraint; Json::Object extra_info; diff --git a/toolsrc/include/vcpkg/versions.h b/toolsrc/include/vcpkg/versions.h index f0304c788514ea..a1e65e5dab2fa1 100644 --- a/toolsrc/include/vcpkg/versions.h +++ b/toolsrc/include/vcpkg/versions.h @@ -11,4 +11,14 @@ namespace vcpkg::Versions Semver, Date }; + + struct Constraint + { + enum class Type + { + None, + Minimum, + Exact + }; + }; } diff --git a/toolsrc/src/vcpkg-test/manifests.cpp b/toolsrc/src/vcpkg-test/manifests.cpp index be087584ed1715..5b003274db981a 100644 --- a/toolsrc/src/vcpkg-test/manifests.cpp +++ b/toolsrc/src/vcpkg-test/manifests.cpp @@ -111,6 +111,74 @@ TEST_CASE ("manifest versioning", "[manifests]") true); } +TEST_CASE ("manifest constraints", "[manifests]") +{ + std::string raw = R"json({ + "name": "zlib", + "version-string": "abcd", + "dependencies": [ + "a", + { + "name": "b", + "port-version": 12, + "version=": "5" + }, + { + "$extra": null, + "name": "c" + }, + { + "name": "d", + "version>=": "2018-09-01" + } + ] +} +)json"; + auto m_pgh = test_parse_manifest(raw); + + REQUIRE(m_pgh.has_value()); + auto& pgh = **m_pgh.get(); + REQUIRE(Json::stringify(serialize_manifest(pgh), Json::JsonStyle::with_spaces(4)) == raw); + REQUIRE(pgh.core_paragraph->dependencies.size() == 4); + REQUIRE(pgh.core_paragraph->dependencies[0].name == "a"); + REQUIRE(pgh.core_paragraph->dependencies[0].constraint == + DependencyConstraint{Versions::Constraint::Type::None, "", 0}); + REQUIRE(pgh.core_paragraph->dependencies[1].name == "b"); + REQUIRE(pgh.core_paragraph->dependencies[1].constraint == + DependencyConstraint{Versions::Constraint::Type::Exact, "5", 12}); + REQUIRE(pgh.core_paragraph->dependencies[2].name == "c"); + REQUIRE(pgh.core_paragraph->dependencies[2].constraint == + DependencyConstraint{Versions::Constraint::Type::None, "", 0}); + REQUIRE(pgh.core_paragraph->dependencies[3].name == "d"); + REQUIRE(pgh.core_paragraph->dependencies[3].constraint == + DependencyConstraint{Versions::Constraint::Type::Minimum, "2018-09-01", 0}); + + test_parse_manifest(R"json({ + "name": "zlib", + "version-string": "abcd", + "dependencies": [ + { + "name": "d", + "version=": "2018-09-01", + "version>=": "2018-09-01" + } + ] + })json", + true); + + test_parse_manifest(R"json({ + "name": "zlib", + "version-string": "abcd", + "dependencies": [ + { + "name": "d", + "port-version": 5 + } + ] + })json", + true); +} + TEST_CASE ("manifest construct maximum", "[manifests]") { auto m_pgh = test_parse_manifest(R"json({ diff --git a/toolsrc/src/vcpkg/packagespec.cpp b/toolsrc/src/vcpkg/packagespec.cpp index 0feb8412a418a1..3357d2676097cf 100644 --- a/toolsrc/src/vcpkg/packagespec.cpp +++ b/toolsrc/src/vcpkg/packagespec.cpp @@ -259,6 +259,14 @@ namespace vcpkg return ret; } + bool operator==(const DependencyConstraint& lhs, const DependencyConstraint& rhs) + { + if (lhs.type != rhs.type) return false; + if (lhs.value != rhs.value) return false; + return lhs.port_version == rhs.port_version; + } + bool operator!=(const DependencyConstraint& lhs, const DependencyConstraint& rhs); + bool operator==(const Dependency& lhs, const Dependency& rhs) { if (lhs.name != rhs.name) return false; diff --git a/toolsrc/src/vcpkg/sourceparagraph.cpp b/toolsrc/src/vcpkg/sourceparagraph.cpp index 802e6d6a39c09e..542947aa34b0e0 100644 --- a/toolsrc/src/vcpkg/sourceparagraph.cpp +++ b/toolsrc/src/vcpkg/sourceparagraph.cpp @@ -390,6 +390,9 @@ namespace vcpkg constexpr static StringLiteral FEATURES = "features"; constexpr static StringLiteral DEFAULT_FEATURES = "default-features"; constexpr static StringLiteral PLATFORM = "platform"; + constexpr static StringLiteral PORT_VERSION = "port-version"; + constexpr static StringLiteral VERSION_EQ = "version="; + constexpr static StringLiteral VERSION_GE = "version>="; virtual Span valid_fields() const override { @@ -398,6 +401,9 @@ namespace vcpkg FEATURES, DEFAULT_FEATURES, PLATFORM, + PORT_VERSION, + VERSION_EQ, + VERSION_GE, }; return t; @@ -442,6 +448,34 @@ namespace vcpkg r.optional_object_field(obj, PLATFORM, dep.platform, PlatformExprDeserializer::instance); + static Json::StringDeserializer version_deserializer{"a version"}; + + auto has_eq_constraint = + r.optional_object_field(obj, VERSION_EQ, dep.constraint.value, version_deserializer); + auto has_ge_constraint = + r.optional_object_field(obj, VERSION_GE, dep.constraint.value, version_deserializer); + auto has_port_ver = r.optional_object_field( + obj, PORT_VERSION, dep.constraint.port_version, Json::NaturalNumberDeserializer::instance); + + if (has_eq_constraint) + { + dep.constraint.type = Versions::Constraint::Type::Exact; + if (has_ge_constraint) + { + r.add_generic_error(type_name(), "cannot have both exact and minimum constraints simultaneously"); + } + } + else if (has_ge_constraint) + { + dep.constraint.type = Versions::Constraint::Type::Minimum; + } + else if (has_port_ver) // does not have a primary constraint + { + r.add_generic_error( + type_name(), + "\"port-version\" cannot be used without a primary constraint (\"version=\" or \"version>=\")"); + } + return dep; } @@ -1072,7 +1106,8 @@ namespace vcpkg } }; auto serialize_dependency = [&](Json::Array& arr, const Dependency& dep) { - if (dep.features.empty() && dep.platform.is_empty() && dep.extra_info.is_empty()) + if (dep.features.empty() && dep.platform.is_empty() && dep.extra_info.is_empty() && + dep.constraint.type == Versions::Constraint::Type::None) { arr.push_back(Json::Value::string(dep.name)); } @@ -1096,6 +1131,20 @@ namespace vcpkg serialize_optional_array(dep_obj, DependencyDeserializer::FEATURES, features_copy); serialize_optional_string(dep_obj, DependencyDeserializer::PLATFORM, to_string(dep.platform)); + if (dep.constraint.port_version != 0) + { + dep_obj.insert(DependencyDeserializer::PORT_VERSION, + Json::Value::integer(dep.constraint.port_version)); + } + + if (dep.constraint.type == Versions::Constraint::Type::Exact) + { + dep_obj.insert(DependencyDeserializer::VERSION_EQ, Json::Value::string(dep.constraint.value)); + } + else if (dep.constraint.type == Versions::Constraint::Type::Minimum) + { + dep_obj.insert(DependencyDeserializer::VERSION_GE, Json::Value::string(dep.constraint.value)); + } } }; From e99daf4d88be0b95cc2af7bec1adb616966381af Mon Sep 17 00:00:00 2001 From: Robert Schumacher Date: Fri, 23 Oct 2020 04:50:34 -0700 Subject: [PATCH 2/5] [vcpkg] Add SourceControlFile::check_against_feature_flags to prevent accidentally ignoring versioning fields --- toolsrc/include/vcpkg/sourceparagraph.h | 16 +++----- toolsrc/src/vcpkg/install.cpp | 9 ++++- toolsrc/src/vcpkg/portfileprovider.cpp | 14 ++----- toolsrc/src/vcpkg/sourceparagraph.cpp | 50 +++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 21 deletions(-) diff --git a/toolsrc/include/vcpkg/sourceparagraph.h b/toolsrc/include/vcpkg/sourceparagraph.h index 21e5a4ef9fcaf8..598054a16c12db 100644 --- a/toolsrc/include/vcpkg/sourceparagraph.h +++ b/toolsrc/include/vcpkg/sourceparagraph.h @@ -2,6 +2,8 @@ #include +#include + #include #include #include @@ -80,16 +82,7 @@ namespace vcpkg /// struct SourceControlFile { - SourceControlFile clone() const - { - SourceControlFile ret; - ret.core_paragraph = std::make_unique(*core_paragraph); - for (const auto& feat_ptr : feature_paragraphs) - { - ret.feature_paragraphs.push_back(std::make_unique(*feat_ptr)); - } - return ret; - } + SourceControlFile clone() const; static Parse::ParseExpected parse_manifest_file(const fs::path& path_to_manifest, const Json::Object& object); @@ -104,6 +97,9 @@ namespace vcpkg Optional find_feature(const std::string& featurename) const; Optional&> find_dependencies_for_feature(const std::string& featurename) const; + Optional check_against_feature_flags(const std::string& origin, + const FeatureFlagSettings& flags) const; + friend bool operator==(const SourceControlFile& lhs, const SourceControlFile& rhs); friend bool operator!=(const SourceControlFile& lhs, const SourceControlFile& rhs) { return !(lhs == rhs); } }; diff --git a/toolsrc/src/vcpkg/install.cpp b/toolsrc/src/vcpkg/install.cpp index a83c6c9765712b..90c7970e7b6170 100644 --- a/toolsrc/src/vcpkg/install.cpp +++ b/toolsrc/src/vcpkg/install.cpp @@ -789,7 +789,7 @@ namespace vcpkg::Install Metrics::g_metrics.lock()->track_property("x-write-nuget-packages-config", "defined"); pkgsconfig = fs::u8path(it_pkgsconfig->second); } - auto manifest_path = paths.get_manifest_path().value_or_exit(VCPKG_LINE_INFO); + const auto& manifest_path = paths.get_manifest_path().value_or_exit(VCPKG_LINE_INFO); auto maybe_manifest_scf = SourceControlFile::parse_manifest_file(manifest_path, *manifest); if (!maybe_manifest_scf) { @@ -798,8 +798,15 @@ namespace vcpkg::Install "more information.\n"); Checks::exit_fail(VCPKG_LINE_INFO); } + auto& manifest_scf = *maybe_manifest_scf.value_or_exit(VCPKG_LINE_INFO); + if (auto maybe_error = + manifest_scf.check_against_feature_flags(fs::u8string(manifest_path), paths.get_feature_flags())) + { + Checks::exit_with_message(VCPKG_LINE_INFO, maybe_error.value_or_exit(VCPKG_LINE_INFO)); + } + std::vector features; auto manifest_feature_it = options.multisettings.find(OPTION_MANIFEST_FEATURE); if (manifest_feature_it != options.multisettings.end()) diff --git a/toolsrc/src/vcpkg/portfileprovider.cpp b/toolsrc/src/vcpkg/portfileprovider.cpp index aaeda0bb10514f..5019fd362121df 100644 --- a/toolsrc/src/vcpkg/portfileprovider.cpp +++ b/toolsrc/src/vcpkg/portfileprovider.cpp @@ -161,6 +161,10 @@ namespace vcpkg::PortFileProvider } if (auto p = maybe_port.get()) { + auto maybe_error = p->source_control_file->check_against_feature_flags(fs::u8string(p->source_location), + paths.get_feature_flags()); + if (maybe_error) return std::move(*maybe_error.get()); + cache_it = cache.emplace(spec, std::move(*p)).first; } } @@ -171,16 +175,6 @@ namespace vcpkg::PortFileProvider } else { - if (!paths.get_feature_flags().versions) - { - if (cache_it->second.source_control_file->core_paragraph->version_scheme != Versions::Scheme::String) - { - return Strings::concat( - "Port definition rejected because the `", - VcpkgCmdArguments::VERSIONS_FEATURE, - "` feature flag is disabled.\nThis can be fixed by using the \"version-string\" field."); - } - } return cache_it->second; } } diff --git a/toolsrc/src/vcpkg/sourceparagraph.cpp b/toolsrc/src/vcpkg/sourceparagraph.cpp index 542947aa34b0e0..529ed8a4806952 100644 --- a/toolsrc/src/vcpkg/sourceparagraph.cpp +++ b/toolsrc/src/vcpkg/sourceparagraph.cpp @@ -10,6 +10,7 @@ #include #include #include +#include namespace vcpkg { @@ -913,6 +914,17 @@ namespace vcpkg constexpr StringLiteral ManifestDeserializer::DEFAULT_FEATURES; constexpr StringLiteral ManifestDeserializer::SUPPORTS; + SourceControlFile SourceControlFile::clone() const + { + SourceControlFile ret; + ret.core_paragraph = std::make_unique(*core_paragraph); + for (const auto& feat_ptr : feature_paragraphs) + { + ret.feature_paragraphs.push_back(std::make_unique(*feat_ptr)); + } + return ret; + } + Parse::ParseExpected SourceControlFile::parse_manifest_file(const fs::path& path_to_manifest, const Json::Object& manifest) { @@ -937,6 +949,44 @@ namespace vcpkg } } + Optional SourceControlFile::check_against_feature_flags(const std::string& origin, + const FeatureFlagSettings& flags) const + { + if (!flags.versions) + { + if (core_paragraph->version_scheme != Versions::Scheme::String) + { + return Strings::concat(origin, + " was rejected because it uses a non-string version scheme and the `", + VcpkgCmdArguments::VERSIONS_FEATURE, + "` feature flag is disabled.\nThis can be fixed by using \"version-string\"."); + } + + auto check_deps = [&](View deps) -> Optional { + for (auto&& dep : deps) + { + if (dep.constraint.type != Versions::Constraint::Type::None) + { + return Strings::concat(origin, + " was rejected because it uses constraints and the `", + VcpkgCmdArguments::VERSIONS_FEATURE, + "` feature flag is disabled.\nThis can be fixed by removing uses of " + "\"version>=\" and \"version=\"."); + } + } + return nullopt; + }; + + if (auto r = check_deps(core_paragraph->dependencies)) return r; + + for (auto&& fpgh : feature_paragraphs) + { + if (auto r = check_deps(fpgh->dependencies)) return r; + } + } + return nullopt; + } + void print_error_message(Span> error_info_list) { Checks::check_exit(VCPKG_LINE_INFO, error_info_list.size() > 0); From f8898e031e3fba7374e779ec816c6e1df94fe4bc Mon Sep 17 00:00:00 2001 From: Robert Schumacher Date: Fri, 23 Oct 2020 04:52:10 -0700 Subject: [PATCH 3/5] [vcpkg] Switch check_against_feature_flags to accept fs::path --- toolsrc/include/vcpkg/sourceparagraph.h | 2 +- toolsrc/src/vcpkg/install.cpp | 3 +-- toolsrc/src/vcpkg/portfileprovider.cpp | 4 ++-- toolsrc/src/vcpkg/sourceparagraph.cpp | 6 +++--- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/toolsrc/include/vcpkg/sourceparagraph.h b/toolsrc/include/vcpkg/sourceparagraph.h index 598054a16c12db..2f03c02ed01b78 100644 --- a/toolsrc/include/vcpkg/sourceparagraph.h +++ b/toolsrc/include/vcpkg/sourceparagraph.h @@ -97,7 +97,7 @@ namespace vcpkg Optional find_feature(const std::string& featurename) const; Optional&> find_dependencies_for_feature(const std::string& featurename) const; - Optional check_against_feature_flags(const std::string& origin, + Optional check_against_feature_flags(const fs::path& origin, const FeatureFlagSettings& flags) const; friend bool operator==(const SourceControlFile& lhs, const SourceControlFile& rhs); diff --git a/toolsrc/src/vcpkg/install.cpp b/toolsrc/src/vcpkg/install.cpp index 90c7970e7b6170..15749970815e55 100644 --- a/toolsrc/src/vcpkg/install.cpp +++ b/toolsrc/src/vcpkg/install.cpp @@ -801,8 +801,7 @@ namespace vcpkg::Install auto& manifest_scf = *maybe_manifest_scf.value_or_exit(VCPKG_LINE_INFO); - if (auto maybe_error = - manifest_scf.check_against_feature_flags(fs::u8string(manifest_path), paths.get_feature_flags())) + if (auto maybe_error = manifest_scf.check_against_feature_flags(manifest_path, paths.get_feature_flags())) { Checks::exit_with_message(VCPKG_LINE_INFO, maybe_error.value_or_exit(VCPKG_LINE_INFO)); } diff --git a/toolsrc/src/vcpkg/portfileprovider.cpp b/toolsrc/src/vcpkg/portfileprovider.cpp index 5019fd362121df..f5beeacf424648 100644 --- a/toolsrc/src/vcpkg/portfileprovider.cpp +++ b/toolsrc/src/vcpkg/portfileprovider.cpp @@ -161,8 +161,8 @@ namespace vcpkg::PortFileProvider } if (auto p = maybe_port.get()) { - auto maybe_error = p->source_control_file->check_against_feature_flags(fs::u8string(p->source_location), - paths.get_feature_flags()); + auto maybe_error = + p->source_control_file->check_against_feature_flags(p->source_location, paths.get_feature_flags()); if (maybe_error) return std::move(*maybe_error.get()); cache_it = cache.emplace(spec, std::move(*p)).first; diff --git a/toolsrc/src/vcpkg/sourceparagraph.cpp b/toolsrc/src/vcpkg/sourceparagraph.cpp index 529ed8a4806952..b4b2608995efe6 100644 --- a/toolsrc/src/vcpkg/sourceparagraph.cpp +++ b/toolsrc/src/vcpkg/sourceparagraph.cpp @@ -949,14 +949,14 @@ namespace vcpkg } } - Optional SourceControlFile::check_against_feature_flags(const std::string& origin, + Optional SourceControlFile::check_against_feature_flags(const fs::path& origin, const FeatureFlagSettings& flags) const { if (!flags.versions) { if (core_paragraph->version_scheme != Versions::Scheme::String) { - return Strings::concat(origin, + return Strings::concat(fs::u8string(origin), " was rejected because it uses a non-string version scheme and the `", VcpkgCmdArguments::VERSIONS_FEATURE, "` feature flag is disabled.\nThis can be fixed by using \"version-string\"."); @@ -967,7 +967,7 @@ namespace vcpkg { if (dep.constraint.type != Versions::Constraint::Type::None) { - return Strings::concat(origin, + return Strings::concat(fs::u8string(origin), " was rejected because it uses constraints and the `", VcpkgCmdArguments::VERSIONS_FEATURE, "` feature flag is disabled.\nThis can be fixed by removing uses of " From 60c8ac40845af7314eed11e840dcd248be775216 Mon Sep 17 00:00:00 2001 From: Robert Schumacher Date: Fri, 23 Oct 2020 06:13:00 -0700 Subject: [PATCH 4/5] [vcpkg] Implement overrides parsing in manifests --- toolsrc/include/vcpkg/packagespec.h | 13 ++ toolsrc/include/vcpkg/sourceparagraph.h | 1 + toolsrc/src/vcpkg-test/manifests.cpp | 134 ++++++++++++++++ toolsrc/src/vcpkg/packagespec.cpp | 10 ++ toolsrc/src/vcpkg/sourceparagraph.cpp | 198 +++++++++++++++++++----- 5 files changed, 313 insertions(+), 43 deletions(-) diff --git a/toolsrc/include/vcpkg/packagespec.h b/toolsrc/include/vcpkg/packagespec.h index 5a4790116196bb..d45fb018232051 100644 --- a/toolsrc/include/vcpkg/packagespec.h +++ b/toolsrc/include/vcpkg/packagespec.h @@ -160,6 +160,19 @@ namespace vcpkg friend bool operator!=(const Dependency& lhs, const Dependency& rhs) { return !(lhs == rhs); } }; + struct DependencyOverride + { + std::string name; + Versions::Scheme version_scheme = Versions::Scheme::String; + std::string version; + int port_version = 0; + + Json::Object extra_info; + + friend bool operator==(const DependencyOverride& lhs, const DependencyOverride& rhs); + friend bool operator!=(const DependencyOverride& lhs, const DependencyOverride& rhs) { return !(lhs == rhs); } + }; + struct ParsedQualifiedSpecifier { std::string name; diff --git a/toolsrc/include/vcpkg/sourceparagraph.h b/toolsrc/include/vcpkg/sourceparagraph.h index 2f03c02ed01b78..9a55f541b9d3aa 100644 --- a/toolsrc/include/vcpkg/sourceparagraph.h +++ b/toolsrc/include/vcpkg/sourceparagraph.h @@ -65,6 +65,7 @@ namespace vcpkg std::string homepage; std::string documentation; std::vector dependencies; + std::vector overrides; std::vector default_features; std::string license; // SPDX license expression diff --git a/toolsrc/src/vcpkg-test/manifests.cpp b/toolsrc/src/vcpkg-test/manifests.cpp index 5b003274db981a..998677d217ca65 100644 --- a/toolsrc/src/vcpkg-test/manifests.cpp +++ b/toolsrc/src/vcpkg-test/manifests.cpp @@ -43,6 +43,9 @@ static Parse::ParseExpected test_parse_manifest(StringView sv return res; } +static const FeatureFlagSettings feature_flags_with_versioning{false, false, false, true}; +static const FeatureFlagSettings feature_flags_without_versioning{false, false, false, false}; + TEST_CASE ("manifest construct minimum", "[manifests]") { auto m_pgh = test_parse_manifest(R"json({ @@ -58,6 +61,8 @@ TEST_CASE ("manifest construct minimum", "[manifests]") REQUIRE(pgh.core_paragraph->maintainers.empty()); REQUIRE(pgh.core_paragraph->description.empty()); REQUIRE(pgh.core_paragraph->dependencies.empty()); + + REQUIRE(!pgh.check_against_feature_flags({}, feature_flags_without_versioning)); } TEST_CASE ("manifest versioning", "[manifests]") @@ -138,6 +143,8 @@ TEST_CASE ("manifest constraints", "[manifests]") REQUIRE(m_pgh.has_value()); auto& pgh = **m_pgh.get(); + REQUIRE(pgh.check_against_feature_flags({}, feature_flags_without_versioning)); + REQUIRE(!pgh.check_against_feature_flags({}, feature_flags_with_versioning)); REQUIRE(Json::stringify(serialize_manifest(pgh), Json::JsonStyle::with_spaces(4)) == raw); REQUIRE(pgh.core_paragraph->dependencies.size() == 4); REQUIRE(pgh.core_paragraph->dependencies[0].name == "a"); @@ -179,6 +186,131 @@ TEST_CASE ("manifest constraints", "[manifests]") true); } +TEST_CASE ("manifest overrides", "[manifests]") +{ + std::tuple data[] = { + {R"json({ + "name": "zlib", + "version-date": "2020-01-01", + "overrides": [ + { + "name": "abc", + "version-string": "abcd" + } + ] +} +)json", + Versions::Scheme::String, + "abcd"}, + {R"json({ + "name": "zlib", + "version": "1.2.3.4.5", + "overrides": [ + { + "name": "abc", + "version-date": "2020-01-01" + } + ] +} +)json", + Versions::Scheme::Date, + "2020-01-01"}, + {R"json({ + "name": "zlib", + "version-date": "2020-01-01", + "overrides": [ + { + "name": "abc", + "version": "1.2.3.4.5" + } + ] +} +)json", + Versions::Scheme::Relaxed, + "1.2.3.4.5"}, + {R"json({ + "name": "zlib", + "version-date": "2020-01-01", + "overrides": [ + { + "name": "abc", + "version-semver": "1.2.3-rc3" + } + ] +} +)json", + Versions::Scheme::Semver, + "1.2.3-rc3"}, + }; + for (auto v : data) + { + auto m_pgh = test_parse_manifest(std::get<0>(v)); + + REQUIRE(m_pgh.has_value()); + 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)); + REQUIRE(pgh.check_against_feature_flags({}, feature_flags_without_versioning)); + REQUIRE(!pgh.check_against_feature_flags({}, feature_flags_with_versioning)); + } + + test_parse_manifest(R"json({ + "name": "zlib", + "version-string": "abcd", + "overrides": [ + { + "name": "abc", + "version-semver": "1.2.3-rc3", + "version-string": "1.2.3-rc3" + } + ]})json", + true); + + test_parse_manifest(R"json({ + "name": "zlib", + "version-string": "abcd", + "overrides": [ + { + "name": "abc", + "port-version": 5 + } + ]})json", + true); + + std::string raw = R"json({ + "name": "zlib", + "version-string": "abcd", + "overrides": [ + { + "name": "abc", + "port-version": 5, + "version-string": "hello" + }, + { + "name": "abcd", + "port-version": 7, + "version-string": "hello" + } + ] +} +)json"; + auto m_pgh = test_parse_manifest(raw); + + REQUIRE(m_pgh.has_value()); + 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); + + REQUIRE(pgh.check_against_feature_flags({}, feature_flags_without_versioning)); + REQUIRE(!pgh.check_against_feature_flags({}, feature_flags_with_versioning)); +} + TEST_CASE ("manifest construct maximum", "[manifests]") { auto m_pgh = test_parse_manifest(R"json({ @@ -248,6 +380,8 @@ TEST_CASE ("manifest construct maximum", "[manifests]") REQUIRE(pgh.feature_paragraphs[1]->description.size() == 2); REQUIRE(pgh.feature_paragraphs[1]->description[0] == "son of the fire lord"); REQUIRE(pgh.feature_paragraphs[1]->description[1] == "firebending 師父"); + + REQUIRE(!pgh.check_against_feature_flags({}, feature_flags_without_versioning)); } TEST_CASE ("SourceParagraph manifest two dependencies", "[manifests]") diff --git a/toolsrc/src/vcpkg/packagespec.cpp b/toolsrc/src/vcpkg/packagespec.cpp index 3357d2676097cf..484da6c654b4f5 100644 --- a/toolsrc/src/vcpkg/packagespec.cpp +++ b/toolsrc/src/vcpkg/packagespec.cpp @@ -277,4 +277,14 @@ namespace vcpkg return true; } bool operator!=(const Dependency& lhs, const Dependency& rhs); + + bool operator==(const DependencyOverride& lhs, const DependencyOverride& rhs) + { + if (lhs.version_scheme != lhs.version_scheme) return false; + if (lhs.port_version != lhs.port_version) return false; + if (lhs.name != rhs.name) return false; + if (lhs.version != lhs.version) return false; + return lhs.extra_info == rhs.extra_info; + } + bool operator!=(const DependencyOverride& lhs, const DependencyOverride& rhs); } diff --git a/toolsrc/src/vcpkg/sourceparagraph.cpp b/toolsrc/src/vcpkg/sourceparagraph.cpp index b4b2608995efe6..83e872befdcab5 100644 --- a/toolsrc/src/vcpkg/sourceparagraph.cpp +++ b/toolsrc/src/vcpkg/sourceparagraph.cpp @@ -501,6 +501,106 @@ namespace vcpkg constexpr StringLiteral DependencyDeserializer::FEATURES; constexpr StringLiteral DependencyDeserializer::DEFAULT_FEATURES; constexpr StringLiteral DependencyDeserializer::PLATFORM; + constexpr StringLiteral DependencyDeserializer::VERSION_EQ; + constexpr StringLiteral DependencyDeserializer::VERSION_GE; + constexpr StringLiteral DependencyDeserializer::PORT_VERSION; + + struct DependencyOverrideDeserializer : Json::IDeserializer + { + virtual StringView type_name() const override { return "an override"; } + + constexpr static StringLiteral NAME = "name"; + constexpr static StringLiteral PORT_VERSION = "port-version"; + constexpr static StringLiteral VERSION_RELAXED = "version"; + constexpr static StringLiteral VERSION_SEMVER = "version-semver"; + constexpr static StringLiteral VERSION_DATE = "version-date"; + constexpr static StringLiteral VERSION_STRING = "version-string"; + + virtual Span valid_fields() const override + { + static const StringView t[] = { + NAME, + PORT_VERSION, + VERSION_RELAXED, + VERSION_SEMVER, + VERSION_DATE, + VERSION_STRING, + }; + + return t; + } + + static void visit_impl(StringView type_name, + Json::Reader& r, + const Json::Object& obj, + std::string& name, + std::string& version, + Versions::Scheme& version_scheme, + int& port_version) + { + static Json::StringDeserializer version_exact_deserializer{"an exact version string"}; + static Json::StringDeserializer version_relaxed_deserializer{"a relaxed version string"}; + static Json::StringDeserializer version_semver_deserializer{"a semantic version string"}; + static Json::StringDeserializer version_date_deserializer{"a date version string"}; + + r.required_object_field(type_name, obj, NAME, name, Json::IdentifierDeserializer::instance); + bool has_exact = r.optional_object_field(obj, VERSION_STRING, version, version_exact_deserializer); + bool has_relax = r.optional_object_field(obj, VERSION_RELAXED, version, version_relaxed_deserializer); + bool has_semver = r.optional_object_field(obj, VERSION_SEMVER, version, version_semver_deserializer); + bool has_date = r.optional_object_field(obj, VERSION_DATE, version, version_date_deserializer); + int num_versions = (int)has_exact + (int)has_relax + (int)has_semver + (int)has_date; + if (num_versions == 0) + { + r.add_generic_error(type_name, "expected a versioning field (example: ", VERSION_STRING, ")"); + } + else if (num_versions > 1) + { + r.add_generic_error(type_name, "expected only one versioning field"); + } + else + { + if (has_exact) + version_scheme = Versions::Scheme::String; + else if (has_relax) + version_scheme = Versions::Scheme::Relaxed; + else if (has_semver) + version_scheme = Versions::Scheme::Semver; + else if (has_date) + version_scheme = Versions::Scheme::Date; + else + Checks::unreachable(VCPKG_LINE_INFO); + } + + r.optional_object_field(obj, PORT_VERSION, port_version, Json::NaturalNumberDeserializer::instance); + } + + virtual Optional visit_object(Json::Reader& r, const Json::Object& obj) override + { + DependencyOverride dep; + + for (const auto& el : obj) + { + if (Strings::starts_with(el.first, "$")) + { + dep.extra_info.insert_or_replace(el.first.to_string(), el.second); + } + } + + visit_impl(type_name(), r, obj, dep.name, dep.version, dep.version_scheme, dep.port_version); + + return dep; + } + + static DependencyOverrideDeserializer instance; + }; + DependencyOverrideDeserializer DependencyOverrideDeserializer::instance; + + constexpr StringLiteral DependencyOverrideDeserializer::NAME; + constexpr StringLiteral DependencyOverrideDeserializer::VERSION_STRING; + constexpr StringLiteral DependencyOverrideDeserializer::VERSION_RELAXED; + constexpr StringLiteral DependencyOverrideDeserializer::VERSION_SEMVER; + constexpr StringLiteral DependencyOverrideDeserializer::VERSION_DATE; + constexpr StringLiteral DependencyOverrideDeserializer::PORT_VERSION; // reasoning for these two distinct types -- FeatureDeserializer and ArrayFeatureDeserializer: // `"features"` may be defined in one of two ways: @@ -790,6 +890,7 @@ namespace vcpkg constexpr static StringLiteral FEATURES = "features"; constexpr static StringLiteral DEFAULT_FEATURES = "default-features"; constexpr static StringLiteral SUPPORTS = "supports"; + constexpr static StringLiteral OVERRIDES = "overrides"; virtual Span valid_fields() const override { @@ -810,6 +911,7 @@ namespace vcpkg FEATURES, DEFAULT_FEATURES, SUPPORTS, + OVERRIDES, }; return t; @@ -832,48 +934,21 @@ namespace vcpkg } } - static Json::StringDeserializer version_exact_deserializer{"an exact version string"}; - static Json::StringDeserializer version_relaxed_deserializer{"a relaxed version string"}; - static Json::StringDeserializer version_semver_deserializer{"a semantic version string"}; - static Json::StringDeserializer version_date_deserializer{"a date version string"}; static Json::StringDeserializer url_deserializer{"a url"}; constexpr static StringView type_name = "vcpkg.json"; - r.required_object_field(type_name, obj, NAME, spgh->name, Json::IdentifierDeserializer::instance); - bool has_exact = r.optional_object_field(obj, VERSION_STRING, spgh->version, version_exact_deserializer); - bool has_relax = r.optional_object_field(obj, VERSION_RELAXED, spgh->version, version_relaxed_deserializer); - bool has_semver = r.optional_object_field(obj, VERSION_SEMVER, spgh->version, version_semver_deserializer); - bool has_date = r.optional_object_field(obj, VERSION_DATE, spgh->version, version_date_deserializer); - int num_versions = (int)has_exact + (int)has_relax + (int)has_semver + (int)has_date; - if (num_versions == 0) - { - r.add_generic_error(type_name, "expected a versioning field (example: ", VERSION_RELAXED, ")"); - } - else if (num_versions > 1) - { - r.add_generic_error(type_name, "expected only one versioning field"); - } - else - { - if (has_exact) - spgh->version_scheme = Versions::Scheme::String; - else if (has_relax) - spgh->version_scheme = Versions::Scheme::Relaxed; - else if (has_semver) - spgh->version_scheme = Versions::Scheme::Semver; - else if (has_date) - spgh->version_scheme = Versions::Scheme::Date; - else - Checks::unreachable(VCPKG_LINE_INFO); - } + DependencyOverrideDeserializer::visit_impl( + type_name, r, obj, spgh->name, spgh->version, spgh->version_scheme, spgh->port_version); - r.optional_object_field(obj, PORT_VERSION, spgh->port_version, Json::NaturalNumberDeserializer::instance); r.optional_object_field(obj, MAINTAINERS, spgh->maintainers, Json::ParagraphDeserializer::instance); r.optional_object_field(obj, DESCRIPTION, spgh->description, Json::ParagraphDeserializer::instance); r.optional_object_field(obj, HOMEPAGE, spgh->homepage, url_deserializer); r.optional_object_field(obj, DOCUMENTATION, spgh->documentation, url_deserializer); r.optional_object_field(obj, LICENSE, spgh->license, LicenseExpressionDeserializer::instance); r.optional_object_field(obj, DEPENDENCIES, spgh->dependencies, DependencyArrayDeserializer::instance); + static Json::ArrayDeserializer overrides_deserializer{ + "an array of overrides"}; + r.optional_object_field(obj, OVERRIDES, spgh->overrides, overrides_deserializer); if (obj.contains(DEV_DEPENDENCIES)) { @@ -913,6 +988,7 @@ namespace vcpkg constexpr StringLiteral ManifestDeserializer::FEATURES; constexpr StringLiteral ManifestDeserializer::DEFAULT_FEATURES; constexpr StringLiteral ManifestDeserializer::SUPPORTS; + constexpr StringLiteral ManifestDeserializer::OVERRIDES; SourceControlFile SourceControlFile::clone() const { @@ -983,6 +1059,14 @@ namespace vcpkg { if (auto r = check_deps(fpgh->dependencies)) return r; } + + if (core_paragraph->overrides.size() != 0) + { + return Strings::concat(fs::u8string(origin), + " was rejected because it uses overrides and the `", + VcpkgCmdArguments::VERSIONS_FEATURE, + "` feature flag is disabled.\nThis can be fixed by removing \"overrides\"."); + } } return nullopt; } @@ -1198,6 +1282,34 @@ namespace vcpkg } }; + auto version_field = [](Versions::Scheme version_scheme) { + switch (version_scheme) + { + case Versions::Scheme::String: return ManifestDeserializer::VERSION_STRING; + case Versions::Scheme::Semver: return ManifestDeserializer::VERSION_SEMVER; + case Versions::Scheme::Relaxed: return ManifestDeserializer::VERSION_RELAXED; + case Versions::Scheme::Date: return ManifestDeserializer::VERSION_DATE; + default: Checks::unreachable(VCPKG_LINE_INFO); + } + }; + + auto serialize_override = [&](Json::Array& arr, const DependencyOverride& dep) { + auto& dep_obj = arr.push_back(Json::Object()); + for (const auto& el : dep.extra_info) + { + dep_obj.insert(el.first.to_string(), el.second); + } + + dep_obj.insert(DependencyOverrideDeserializer::NAME, Json::Value::string(dep.name)); + + if (dep.port_version != 0) + { + dep_obj.insert(DependencyDeserializer::PORT_VERSION, Json::Value::integer(dep.port_version)); + } + + dep_obj.insert(version_field(dep.version_scheme), Json::Value::string(dep.version)); + }; + Json::Object obj; for (const auto& el : scf.core_paragraph->extra_info) @@ -1206,17 +1318,7 @@ namespace vcpkg } obj.insert(ManifestDeserializer::NAME, Json::Value::string(scf.core_paragraph->name)); - auto version_field = [&] { - switch (scf.core_paragraph->version_scheme) - { - case Versions::Scheme::String: return ManifestDeserializer::VERSION_STRING; - case Versions::Scheme::Semver: return ManifestDeserializer::VERSION_SEMVER; - case Versions::Scheme::Relaxed: return ManifestDeserializer::VERSION_RELAXED; - case Versions::Scheme::Date: return ManifestDeserializer::VERSION_DATE; - default: Checks::unreachable(VCPKG_LINE_INFO); - } - }(); - obj.insert(version_field, Json::Value::string(scf.core_paragraph->version)); + obj.insert(version_field(scf.core_paragraph->version_scheme), Json::Value::string(scf.core_paragraph->version)); if (scf.core_paragraph->port_version != 0 || debug) { @@ -1268,6 +1370,16 @@ namespace vcpkg } } + if (!scf.core_paragraph->overrides.empty() || debug) + { + auto& overrides = obj.insert(ManifestDeserializer::OVERRIDES, Json::Array()); + + for (const auto& over : scf.core_paragraph->overrides) + { + serialize_override(overrides, over); + } + } + if (debug) { obj.insert("TYPE", Json::Value::string(Type::to_string(scf.core_paragraph->type))); From 43e3999d8ebdb04e70fb7ccee0f794c9cc300fad Mon Sep 17 00:00:00 2001 From: Robert Schumacher Date: Wed, 28 Oct 2020 04:54:57 -0700 Subject: [PATCH 5/5] [vcpkg] Address CR comments --- toolsrc/src/vcpkg/packagespec.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/toolsrc/src/vcpkg/packagespec.cpp b/toolsrc/src/vcpkg/packagespec.cpp index 484da6c654b4f5..a40d30340a1bc5 100644 --- a/toolsrc/src/vcpkg/packagespec.cpp +++ b/toolsrc/src/vcpkg/packagespec.cpp @@ -280,10 +280,10 @@ namespace vcpkg bool operator==(const DependencyOverride& lhs, const DependencyOverride& rhs) { - if (lhs.version_scheme != lhs.version_scheme) return false; - if (lhs.port_version != lhs.port_version) return false; + 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 != lhs.version) return false; + if (lhs.version != rhs.version) return false; return lhs.extra_info == rhs.extra_info; } bool operator!=(const DependencyOverride& lhs, const DependencyOverride& rhs);