From fe65d8940e8c3193c9a1170ad96ab13a833e8f8d Mon Sep 17 00:00:00 2001 From: Robert Schumacher Date: Tue, 1 Dec 2020 14:27:56 -0800 Subject: [PATCH 1/5] [vcpkg] Refactor deserializers to reduce duplicate functionality --- toolsrc/include/vcpkg/base/util.h | 10 ++ toolsrc/include/vcpkg/versiondeserializers.h | 30 +++- toolsrc/include/vcpkg/versiont.h | 2 +- toolsrc/src/vcpkg/commands.porthistory.cpp | 6 +- toolsrc/src/vcpkg/sourceparagraph.cpp | 108 ++---------- toolsrc/src/vcpkg/versiondeserializers.cpp | 171 +++++++++++++------ 6 files changed, 173 insertions(+), 154 deletions(-) diff --git a/toolsrc/include/vcpkg/base/util.h b/toolsrc/include/vcpkg/base/util.h index ca5104c234c631..b5ad1d04f05e57 100644 --- a/toolsrc/include/vcpkg/base/util.h +++ b/toolsrc/include/vcpkg/base/util.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include @@ -28,6 +29,15 @@ namespace vcpkg::Util { return std::find(container.begin(), container.end(), item) != container.end(); } + template + std::vector concat(View r1, View r2) + { + std::vector v; + v.reserve(r1.size() + r2.size()); + v.insert(v.end(), r1.begin(), r1.end()); + v.insert(v.end(), r2.begin(), r2.end()); + return v; + } } namespace Sets diff --git a/toolsrc/include/vcpkg/versiondeserializers.h b/toolsrc/include/vcpkg/versiondeserializers.h index f5ffda101e2293..7a043d35f1c782 100644 --- a/toolsrc/include/vcpkg/versiondeserializers.h +++ b/toolsrc/include/vcpkg/versiondeserializers.h @@ -13,20 +13,32 @@ namespace vcpkg struct VersionDbEntry { VersionT version; - Versions::Scheme scheme; + Versions::Scheme scheme = Versions::Scheme::String; std::string git_tree; - - VersionDbEntry(const std::string& version_string, - int port_version, - Versions::Scheme scheme, - const std::string& git_tree) - : version(VersionT(version_string, port_version)), scheme(scheme), git_tree(git_tree) - { - } }; Json::IDeserializer& get_versiont_deserializer_instance(); + struct SchemedVersion + { + Versions::Scheme scheme = Versions::Scheme::String; + VersionT versiont; + }; + + Optional visit_optional_schemed_deserializer(StringView parent_type, + Json::Reader& r, + const Json::Object& obj); + SchemedVersion visit_required_schemed_deserializer(StringView parent_type, + Json::Reader& r, + const Json::Object& obj); + View schemed_deserializer_fields(); + + void serialize_schemed_version(Json::Object& out_obj, + Versions::Scheme scheme, + const std::string& version, + int port_version, + bool always_emit_port_version = false); + ExpectedS>> parse_baseline_file(Files::Filesystem& fs, StringView baseline_name, const fs::path& baseline_file_path); diff --git a/toolsrc/include/vcpkg/versiont.h b/toolsrc/include/vcpkg/versiont.h index 7819f98759d042..910183b9d49223 100644 --- a/toolsrc/include/vcpkg/versiont.h +++ b/toolsrc/include/vcpkg/versiont.h @@ -23,7 +23,7 @@ namespace vcpkg private: std::string m_text; - int m_port_version; + int m_port_version = 0; }; struct VersionDiff diff --git a/toolsrc/src/vcpkg/commands.porthistory.cpp b/toolsrc/src/vcpkg/commands.porthistory.cpp index 045562136bc9b9..7efb36bd18f078 100644 --- a/toolsrc/src/vcpkg/commands.porthistory.cpp +++ b/toolsrc/src/vcpkg/commands.porthistory.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include namespace vcpkg::Commands::PortHistory @@ -230,8 +231,9 @@ namespace vcpkg::Commands::PortHistory { Json::Object object; object.insert("git-tree", Json::Value::string(version.git_tree)); - object.insert("version-string", Json::Value::string(version.version)); - object.insert("port-version", Json::Value::integer(version.port_version)); + + serialize_schemed_version( + object, Versions::Scheme::String, version.version, version.port_version, true); versions_json.push_back(std::move(object)); } diff --git a/toolsrc/src/vcpkg/sourceparagraph.cpp b/toolsrc/src/vcpkg/sourceparagraph.cpp index f05ebb619e130c..4bf64cb06e8267 100644 --- a/toolsrc/src/vcpkg/sourceparagraph.cpp +++ b/toolsrc/src/vcpkg/sourceparagraph.cpp @@ -11,6 +11,7 @@ #include #include #include +#include namespace vcpkg { @@ -548,23 +549,11 @@ namespace vcpkg 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, - }; - + static const StringView u[] = {NAME}; + static const auto t = Util::Vectors::concat(schemed_deserializer_fields(), u); return t; } @@ -576,40 +565,12 @@ namespace vcpkg 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); + auto schemed_version = visit_required_schemed_deserializer(type_name, r, obj); + version = schemed_version.versiont.text(); + version_scheme = schemed_version.scheme; + port_version = schemed_version.versiont.port_version(); } virtual Optional visit_object(Json::Reader& r, const Json::Object& obj) override @@ -634,11 +595,6 @@ namespace vcpkg 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: @@ -909,15 +865,6 @@ namespace vcpkg virtual StringView type_name() const override { return "a manifest"; } constexpr static StringLiteral NAME = "name"; - - // Default is a relaxed semver-like version - constexpr static StringLiteral VERSION_RELAXED = "version"; - constexpr static StringLiteral VERSION_SEMVER = "version-semver"; - constexpr static StringLiteral VERSION_DATE = "version-date"; - // Legacy version string, accepts arbitrary string values. - constexpr static StringLiteral VERSION_STRING = "version-string"; - - constexpr static StringLiteral PORT_VERSION = "port-version"; constexpr static StringLiteral MAINTAINERS = "maintainers"; constexpr static StringLiteral DESCRIPTION = "description"; constexpr static StringLiteral HOMEPAGE = "homepage"; @@ -932,13 +879,8 @@ namespace vcpkg virtual Span valid_fields() const override { - static const StringView t[] = { + static const StringView u[] = { NAME, - VERSION_STRING, - VERSION_RELAXED, - VERSION_SEMVER, - VERSION_DATE, - PORT_VERSION, MAINTAINERS, DESCRIPTION, HOMEPAGE, @@ -951,6 +893,7 @@ namespace vcpkg SUPPORTS, OVERRIDES, }; + static const auto t = Util::Vectors::concat(schemed_deserializer_fields(), u); return t; } @@ -1014,11 +957,6 @@ namespace vcpkg ManifestDeserializer ManifestDeserializer::instance; constexpr StringLiteral ManifestDeserializer::NAME; - constexpr StringLiteral ManifestDeserializer::VERSION_STRING; - constexpr StringLiteral ManifestDeserializer::VERSION_RELAXED; - constexpr StringLiteral ManifestDeserializer::VERSION_SEMVER; - constexpr StringLiteral ManifestDeserializer::VERSION_DATE; - constexpr StringLiteral ManifestDeserializer::PORT_VERSION; constexpr StringLiteral ManifestDeserializer::MAINTAINERS; constexpr StringLiteral ManifestDeserializer::DESCRIPTION; constexpr StringLiteral ManifestDeserializer::HOMEPAGE; @@ -1329,17 +1267,6 @@ 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) @@ -1349,12 +1276,7 @@ namespace vcpkg 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)); + serialize_schemed_version(dep_obj, dep.version_scheme, dep.version, dep.port_version); }; Json::Object obj; @@ -1365,12 +1287,12 @@ namespace vcpkg } obj.insert(ManifestDeserializer::NAME, Json::Value::string(scf.core_paragraph->name)); - obj.insert(version_field(scf.core_paragraph->version_scheme), Json::Value::string(scf.core_paragraph->version)); - if (scf.core_paragraph->port_version != 0 || debug) - { - obj.insert(ManifestDeserializer::PORT_VERSION, Json::Value::integer(scf.core_paragraph->port_version)); - } + serialize_schemed_version(obj, + scf.core_paragraph->version_scheme, + scf.core_paragraph->version, + scf.core_paragraph->port_version, + debug); serialize_paragraph(obj, ManifestDeserializer::MAINTAINERS, scf.core_paragraph->maintainers); serialize_paragraph(obj, ManifestDeserializer::DESCRIPTION, scf.core_paragraph->description); diff --git a/toolsrc/src/vcpkg/versiondeserializers.cpp b/toolsrc/src/vcpkg/versiondeserializers.cpp index 276b70e4fa817a..2fa987bde66034 100644 --- a/toolsrc/src/vcpkg/versiondeserializers.cpp +++ b/toolsrc/src/vcpkg/versiondeserializers.cpp @@ -1,71 +1,144 @@ +#include + #include using namespace vcpkg; using namespace vcpkg::Versions; +static constexpr StringLiteral VERSION_RELAXED = "version"; +static constexpr StringLiteral VERSION_SEMVER = "version-semver"; +static constexpr StringLiteral VERSION_STRING = "version-string"; +static constexpr StringLiteral VERSION_DATE = "version-date"; +static constexpr StringLiteral PORT_VERSION = "port-version"; + +namespace vcpkg +{ + SchemedVersion visit_required_schemed_deserializer(StringView parent_type, Json::Reader& r, const Json::Object& obj) + { + auto maybe_schemed_version = visit_optional_schemed_deserializer(parent_type, r, obj); + if (auto p = maybe_schemed_version.get()) + { + return std::move(*p); + } + else + { + r.add_generic_error(parent_type, "expected a versioning field (example: ", VERSION_STRING, ")"); + return {}; + } + } + Optional visit_optional_schemed_deserializer(StringView parent_type, + Json::Reader& r, + const Json::Object& obj) + { + Versions::Scheme version_scheme = Versions::Scheme::String; + std::string version; + int port_version = 0; + + 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"}; + + 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; + bool has_port_version = + r.optional_object_field(obj, PORT_VERSION, port_version, Json::NaturalNumberDeserializer::instance); + + if (num_versions == 0) + { + if (!has_port_version) + { + return nullopt; + } + else + { + r.add_generic_error(parent_type, "unexpected \"port_version\" without a versioning field"); + } + } + else if (num_versions > 1) + { + r.add_generic_error(parent_type, "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); + } + + return SchemedVersion{version_scheme, {version, port_version}}; + } + + View schemed_deserializer_fields() + { + static const StringView t[] = {VERSION_RELAXED, VERSION_SEMVER, VERSION_STRING, VERSION_DATE, PORT_VERSION}; + return t; + } + + void serialize_schemed_version(Json::Object& out_obj, + Versions::Scheme scheme, + const std::string& version, + int port_version, + bool always_emit_port_version) + { + auto version_field = [](Versions::Scheme version_scheme) { + switch (version_scheme) + { + case Versions::Scheme::String: return VERSION_STRING; + case Versions::Scheme::Semver: return VERSION_SEMVER; + case Versions::Scheme::Relaxed: return VERSION_RELAXED; + case Versions::Scheme::Date: return VERSION_DATE; + default: Checks::unreachable(VCPKG_LINE_INFO); + } + }; + + if (port_version != 0 || always_emit_port_version) + { + out_obj.insert(PORT_VERSION, Json::Value::integer(port_version)); + } + + out_obj.insert(version_field(scheme), Json::Value::string(version)); + } + +} + namespace { struct VersionDbEntryDeserializer final : Json::IDeserializer { - static constexpr StringLiteral VERSION_RELAXED = "version"; - static constexpr StringLiteral VERSION_SEMVER = "version-semver"; - static constexpr StringLiteral VERSION_STRING = "version-string"; - static constexpr StringLiteral VERSION_DATE = "version-date"; - static constexpr StringLiteral PORT_VERSION = "port-version"; static constexpr StringLiteral GIT_TREE = "git-tree"; StringView type_name() const override { return "a version database entry"; } View valid_fields() const override { - static const StringView t[] = { - VERSION_RELAXED, VERSION_SEMVER, VERSION_STRING, VERSION_DATE, PORT_VERSION, GIT_TREE}; + static const StringView u[] = {GIT_TREE}; + static const auto t = vcpkg::Util::Vectors::concat(schemed_deserializer_fields(), u); return t; } Optional visit_object(Json::Reader& r, const Json::Object& obj) override { - std::string version; - int port_version = 0; - std::string git_tree; - Versions::Scheme version_scheme = Versions::Scheme::String; - - // Code copy-and-paste'd from sourceparagraph.cpp - 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"}; + VersionDbEntry ret; + + auto schemed_version = visit_required_schemed_deserializer(type_name(), r, obj); + ret.scheme = schemed_version.scheme; + ret.version = std::move(schemed_version.versiont); + static Json::StringDeserializer git_tree_deserializer("a git object SHA"); - 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); - r.required_object_field(type_name(), obj, GIT_TREE, git_tree, git_tree_deserializer); + r.required_object_field(type_name(), obj, GIT_TREE, ret.git_tree, git_tree_deserializer); - return VersionDbEntry(version, port_version, version_scheme, git_tree); + return std::move(ret); } static VersionDbEntryDeserializer instance; @@ -115,7 +188,7 @@ namespace StringView type_name() const override { return "a version object"; } View valid_fields() const override { - static const StringView t[] = {"version-string", "port-version"}; + static const StringView t[] = {VERSION_STRING, PORT_VERSION}; return t; } @@ -124,8 +197,8 @@ namespace std::string version; int port_version = 0; - r.required_object_field(type_name(), obj, "version-string", version, version_deserializer); - r.optional_object_field(obj, "port-version", port_version, Json::NaturalNumberDeserializer::instance); + r.required_object_field(type_name(), obj, VERSION_STRING, version, version_deserializer); + r.optional_object_field(obj, PORT_VERSION, port_version, Json::NaturalNumberDeserializer::instance); return VersionT{std::move(version), port_version}; } From b82e99dd3fed1c9cd1d084881f89dd5986101b72 Mon Sep 17 00:00:00 2001 From: Robert Schumacher Date: Thu, 3 Dec 2020 12:28:29 -0800 Subject: [PATCH 2/5] [vcpkg] Error on '#' in version strings to avoid confusion with port-version --- toolsrc/include/vcpkg/base/jsonreader.h | 3 +- toolsrc/include/vcpkg/versiondeserializers.h | 1 + toolsrc/src/vcpkg-test/manifests.cpp | 97 ++++++++++++++++++++ toolsrc/src/vcpkg/sourceparagraph.cpp | 6 +- toolsrc/src/vcpkg/versiondeserializers.cpp | 38 ++++++-- 5 files changed, 134 insertions(+), 11 deletions(-) diff --git a/toolsrc/include/vcpkg/base/jsonreader.h b/toolsrc/include/vcpkg/base/jsonreader.h index 02c1936e251e57..bea60e7ce5b644 100644 --- a/toolsrc/include/vcpkg/base/jsonreader.h +++ b/toolsrc/include/vcpkg/base/jsonreader.h @@ -33,13 +33,14 @@ namespace vcpkg::Json virtual Optional visit_object(Reader&, const Object&); virtual View valid_fields() const; + virtual ~IDeserializer() = default; + protected: IDeserializer() = default; IDeserializer(const IDeserializer&) = default; IDeserializer& operator=(const IDeserializer&) = default; IDeserializer(IDeserializer&&) = default; IDeserializer& operator=(IDeserializer&&) = default; - virtual ~IDeserializer() = default; }; struct Reader diff --git a/toolsrc/include/vcpkg/versiondeserializers.h b/toolsrc/include/vcpkg/versiondeserializers.h index 7a043d35f1c782..249d685cbbbb16 100644 --- a/toolsrc/include/vcpkg/versiondeserializers.h +++ b/toolsrc/include/vcpkg/versiondeserializers.h @@ -18,6 +18,7 @@ namespace vcpkg }; Json::IDeserializer& get_versiont_deserializer_instance(); + std::unique_ptr> make_version_deserializer(StringLiteral type_name); struct SchemedVersion { diff --git a/toolsrc/src/vcpkg-test/manifests.cpp b/toolsrc/src/vcpkg-test/manifests.cpp index 32aa11f8a7a73d..69a5d348b3d74e 100644 --- a/toolsrc/src/vcpkg-test/manifests.cpp +++ b/toolsrc/src/vcpkg-test/manifests.cpp @@ -115,6 +115,103 @@ TEST_CASE ("manifest versioning", "[manifests]") "version-semver": "1.2.3-rc3" })json", true); + + test_parse_manifest(R"json({ + "name": "zlib", + "version-string": "abcd#1" + })json", + true); + test_parse_manifest(R"json({ + "name": "zlib", + "version": "abcd#1" + })json", + true); + test_parse_manifest(R"json({ + "name": "zlib", + "version-date": "abcd#1" + })json", + true); + test_parse_manifest(R"json({ + "name": "zlib", + "version-semver": "abcd#1" + })json", + true); +} + +TEST_CASE ("manifest constraints error hash", "[manifests]") +{ + test_parse_manifest(R"json({ + "name": "zlib", + "version-string": "abcd", + "dependencies": [ + { + "name": "b", + "version=": "5#1" + } + ] +} +)json", + true); + + test_parse_manifest(R"json({ + "name": "zlib", + "version-string": "abcd", + "dependencies": [ + { + "name": "d", + "version>=": "2018-09-01#1" + } + ] +})json", + true); +} + +TEST_CASE ("manifest overrides error hash", "[manifests]") +{ + test_parse_manifest(R"json({ + "name": "zlib", + "version-string": "abcd", + "overrides": [ + { + "name": "d", + "version-string": "abcd#1" + } + ] +})json", + true); + test_parse_manifest(R"json({ + "name": "zlib", + "version-string": "abcd", + "overrides": [ + { + "name": "d", + "version-date": "2018-01-01#1" + } + ] +})json", + true); + test_parse_manifest(R"json({ + "name": "zlib", + "version-string": "abcd", + "overrides": [ + { + "name": "d", + "version": "1.2#1" + } + ] +})json", + true); + test_parse_manifest(R"json({ + "name": "zlib", + "version-string": "abcd", + "overrides": [ + { + "name": "d", + "version-semver": "1.2#1" + } + ] +})json", + true); } TEST_CASE ("manifest constraints", "[manifests]") diff --git a/toolsrc/src/vcpkg/sourceparagraph.cpp b/toolsrc/src/vcpkg/sourceparagraph.cpp index 4bf64cb06e8267..f80a1494e5d963 100644 --- a/toolsrc/src/vcpkg/sourceparagraph.cpp +++ b/toolsrc/src/vcpkg/sourceparagraph.cpp @@ -488,12 +488,12 @@ namespace vcpkg r.optional_object_field(obj, PLATFORM, dep.platform, PlatformExprDeserializer::instance); - static Json::StringDeserializer version_deserializer{"a version"}; + static auto version_deserializer = make_version_deserializer("a version"); auto has_eq_constraint = - r.optional_object_field(obj, VERSION_EQ, dep.constraint.value, version_deserializer); + 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); + 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); diff --git a/toolsrc/src/vcpkg/versiondeserializers.cpp b/toolsrc/src/vcpkg/versiondeserializers.cpp index 2fa987bde66034..d0b76b3b7cb3c1 100644 --- a/toolsrc/src/vcpkg/versiondeserializers.cpp +++ b/toolsrc/src/vcpkg/versiondeserializers.cpp @@ -11,8 +11,32 @@ static constexpr StringLiteral VERSION_STRING = "version-string"; static constexpr StringLiteral VERSION_DATE = "version-date"; static constexpr StringLiteral PORT_VERSION = "port-version"; +namespace +{ + struct VersionDeserializer final : Json::IDeserializer + { + VersionDeserializer(StringLiteral type) : m_type(type) { } + StringView type_name() const override { return m_type; } + + Optional visit_string(Json::Reader& r, StringView sv) override + { + if (std::find(sv.begin(), sv.end(), '#') != sv.end()) + { + r.add_generic_error(type_name(), "invalid character in version text: '#'"); + } + return sv.to_string(); + } + StringLiteral m_type; + }; +} + namespace vcpkg { + std::unique_ptr> make_version_deserializer(StringLiteral type_name) + { + return std::make_unique(type_name); + } + SchemedVersion visit_required_schemed_deserializer(StringView parent_type, Json::Reader& r, const Json::Object& obj) { auto maybe_schemed_version = visit_optional_schemed_deserializer(parent_type, r, obj); @@ -34,10 +58,10 @@ namespace vcpkg std::string version; int port_version = 0; - 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 VersionDeserializer version_exact_deserializer{"an exact version string"}; + static VersionDeserializer version_relaxed_deserializer{"a relaxed version string"}; + static VersionDeserializer version_semver_deserializer{"a semantic version string"}; + static VersionDeserializer version_date_deserializer{"a date version string"}; 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); @@ -197,16 +221,16 @@ namespace std::string version; int port_version = 0; + static VersionDeserializer version_deserializer{"version"}; + r.required_object_field(type_name(), obj, VERSION_STRING, version, version_deserializer); r.optional_object_field(obj, PORT_VERSION, port_version, Json::NaturalNumberDeserializer::instance); return VersionT{std::move(version), port_version}; } - - static Json::StringDeserializer version_deserializer; static VersionTDeserializer instance; }; - Json::StringDeserializer VersionTDeserializer::version_deserializer{"version"}; + VersionTDeserializer VersionTDeserializer::instance; } From 3e4913c8e127883f21afa270b9e5ea3d36bc72e5 Mon Sep 17 00:00:00 2001 From: Robert Schumacher Date: Thu, 3 Dec 2020 12:36:15 -0800 Subject: [PATCH 3/5] [vcpkg] Improve error message --- toolsrc/src/vcpkg/versiondeserializers.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/toolsrc/src/vcpkg/versiondeserializers.cpp b/toolsrc/src/vcpkg/versiondeserializers.cpp index d0b76b3b7cb3c1..81d6bf23a9ba5d 100644 --- a/toolsrc/src/vcpkg/versiondeserializers.cpp +++ b/toolsrc/src/vcpkg/versiondeserializers.cpp @@ -20,9 +20,17 @@ namespace Optional visit_string(Json::Reader& r, StringView sv) override { - if (std::find(sv.begin(), sv.end(), '#') != sv.end()) + StringView pv(std::find(sv.begin(), sv.end(), '#'), sv.end()); + if (pv.size() == 1) { - r.add_generic_error(type_name(), "invalid character in version text: '#'"); + r.add_generic_error(type_name(), "invalid character '#' in version text"); + } + else if (pv.size() > 1) + { + r.add_generic_error(type_name(), + "invalid character '#' in version text. Did you mean \"port-version\": ", + pv.substr(1), + "?"); } return sv.to_string(); } From 14c0644421c63cb802e7716e379164171a03bd52 Mon Sep 17 00:00:00 2001 From: Robert Schumacher Date: Thu, 3 Dec 2020 14:32:31 -0800 Subject: [PATCH 4/5] [vcpkg] Reorder field output --- toolsrc/src/vcpkg/versiondeserializers.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/toolsrc/src/vcpkg/versiondeserializers.cpp b/toolsrc/src/vcpkg/versiondeserializers.cpp index 81d6bf23a9ba5d..1557f4be9540b1 100644 --- a/toolsrc/src/vcpkg/versiondeserializers.cpp +++ b/toolsrc/src/vcpkg/versiondeserializers.cpp @@ -134,12 +134,12 @@ namespace vcpkg } }; + out_obj.insert(version_field(scheme), Json::Value::string(version)); + if (port_version != 0 || always_emit_port_version) { out_obj.insert(PORT_VERSION, Json::Value::integer(port_version)); } - - out_obj.insert(version_field(scheme), Json::Value::string(version)); } } From 9bb0ece94e896a3966265075c57620c2d5240f6b Mon Sep 17 00:00:00 2001 From: Robert Schumacher Date: Thu, 3 Dec 2020 14:46:24 -0800 Subject: [PATCH 5/5] [vcpkg] Fix tests --- toolsrc/src/vcpkg-test/manifests.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/toolsrc/src/vcpkg-test/manifests.cpp b/toolsrc/src/vcpkg-test/manifests.cpp index 69a5d348b3d74e..f03caf3bf76bd7 100644 --- a/toolsrc/src/vcpkg-test/manifests.cpp +++ b/toolsrc/src/vcpkg-test/manifests.cpp @@ -383,13 +383,13 @@ TEST_CASE ("manifest overrides", "[manifests]") "overrides": [ { "name": "abc", - "port-version": 5, - "version-string": "hello" + "version-string": "hello", + "port-version": 5 }, { "name": "abcd", - "port-version": 7, - "version-string": "hello" + "version-string": "hello", + "port-version": 7 } ] }