From 0575ec4e2084c553fa56e3d645233b118936a2d5 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Tue, 30 Apr 2019 20:26:36 +0300 Subject: Add support for description-type package manifest value --- libbpkg/manifest.cxx | 292 ++++++++++++++++++++++++++++++++++--- libbpkg/manifest.hxx | 43 +++++- tests/manifest/driver.cxx | 103 +++++++------ tests/manifest/testscript | 357 +++++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 705 insertions(+), 90 deletions(-) diff --git a/libbpkg/manifest.cxx b/libbpkg/manifest.cxx index 55556f0..dc3ff86 100644 --- a/libbpkg/manifest.cxx +++ b/libbpkg/manifest.cxx @@ -19,6 +19,7 @@ #include #include // casecmp(), lcase(), alnum(), // digit(), xdigit(), next_word() +#include #include #include #include @@ -1391,6 +1392,104 @@ namespace bpkg match_classes (cs, im, expr, r); } + // text_type + // + string + to_string (text_type t) + { + switch (t) + { + case text_type::plain: return "text/plain"; + case text_type::github_mark: return "text/markdown;variant=GFM"; + case text_type::common_mark: return "text/markdown;variant=CommonMark"; + } + + assert (false); // Can't be here. + return string (); + } + + optional + to_text_type (const string& t) + { + auto bad_type = [] (const string& d) {throw invalid_argument (d);}; + + // Parse the media type representation (see RFC2045 for details) into the + // type/subtype value and the parameter list. Note: we don't support + // parameter quoting and comments for simplicity. + // + size_t p (t.find (';')); + const string& tp (p != string::npos ? trim (string (t, 0, p)) : t); + + small_vector, 1> ps; + + while (p != string::npos) + { + // Extract parameter name. + // + size_t b (p + 1); + p = t.find ('=', b); + + if (p == string::npos) + bad_type ("missing '='"); + + string n (trim (string (t, b, p - b))); + + // Extract parameter value. + // + b = p + 1; + p = t.find (';', b); + + string v (trim (string (t, + b, + p != string::npos ? p - b : string::npos))); + + ps.emplace_back (move (n), move (v)); + } + + // Calculate the resulting text type, failing on unrecognized media type, + // unexpected parameter name or value. + // + // Note that type, subtype, and parameter names are matched + // case-insensitively. + // + optional r; + + // Currently only the plain and markdown text types are allowed. Later we + // can potentially introduce some other text types. + // + if (casecmp (tp, "text/plain") == 0) + { + // Currently, we don't expect parameters for plain text. Later we can + // potentially introduce some plain text variants. + // + if (ps.empty ()) + r = text_type::plain; + } + else if (casecmp (tp, "text/markdown") == 0) + { + // Currently, a single optional variant parameter with the two possible + // values is allowed for markdown. Later we can potentially introduce + // some other markdown variants. + // + if (ps.empty () || + (ps.size () == 1 && casecmp (ps[0].first, "variant") == 0)) + { + // Note that markdown variants are matched case-insensitively (see + // RFC7763 for details). + // + string v; + if (ps.empty () || casecmp (v = move (ps[0].second), "GFM") == 0) + r = text_type::github_mark; + else if (casecmp (v, "CommonMark") == 0) + r = text_type::common_mark; + } + } + else if (casecmp (tp, "text/", 5) != 0) + bad_type ("text type expected"); + + return r; + } + // pkg_package_manifest // static build_class_expr @@ -1543,6 +1642,13 @@ namespace bpkg // parsed. // vector dependencies; + + // We will cache the description and its type values to validate them + // later, after both are parsed. + // + optional description; + optional description_type; + for (nv = p.next (); !nv.empty (); nv = p.next ()) { string& n (nv.name); @@ -1643,9 +1749,9 @@ namespace bpkg } else if (n == "description") { - if (m.description) + if (description) { - if (m.description->file) + if (description->name == "description-file") bad_name ("package description and description-file are " "mutually exclusive"); else @@ -1655,32 +1761,30 @@ namespace bpkg if (v.empty ()) bad_value ("empty package description"); - m.description = text_file (move (v)); + description = move (nv); } else if (n == "description-file") { if (flag (package_manifest_flags::forbid_file)) bad_name ("package description-file not allowed"); - if (m.description) + if (description) { - if (m.description->file) + if (description->name == "description-file") bad_name ("package description-file redefinition"); else bad_name ("package description-file and description are " "mutually exclusive"); } - auto vc (parser::split_comment (v)); - path p (move (vc.first)); - - if (p.empty ()) - bad_value ("no path in package description-file"); - - if (p.absolute ()) - bad_value ("package description-file path is absolute"); + description = move (nv); + } + else if (n == "description-type") + { + if (description_type) + bad_name ("package description-type redefinition"); - m.description = text_file (move (p), move (vc.second)); + description_type = move (nv); } else if (n == "changes") { @@ -1900,6 +2004,74 @@ namespace bpkg else if (m.license_alternatives.empty ()) bad_value ("no project license specified"); + // Verify that description is specified if the description type is + // specified. + // + if (description_type && !description) + bad_value ("no package description for specified description type"); + + // Validate (and set) description and its type. + // + if (description) + { + // Restore as bad_value() uses its line/column. + // + nv = move (*description); + + string& v (nv.value); + + if (nv.name == "description-file") + { + auto vc (parser::split_comment (v)); + + path p; + try + { + p = path (move (vc.first)); + } + catch (const invalid_path& e) + { + bad_value (string ("invalid package description file: ") + + e.what ()); + } + + if (p.empty ()) + bad_value ("no path in package description-file"); + + if (p.absolute ()) + bad_value ("package description-file path is absolute"); + + m.description = text_file (move (p), move (vc.second)); + } + else + m.description = text_file (move (v)); + + if (description_type) + m.description_type = move (description_type->value); + + // Verify the description type. + // + try + { + m.effective_description_type (iu); + } + catch (const invalid_argument& e) + { + if (description_type) + { + // Restore as bad_value() uses its line/column. + // + nv = move (*description_type); + + bad_value (string ("invalid package description type: ") + + e.what ()); + } + else + bad_value (string ("invalid package description file: ") + + e.what ()); + } + } + // Now, when the version manifest value is parsed, we can parse the // dependencies and complete their constrains, if requested. // @@ -1993,6 +2165,21 @@ namespace bpkg m.dependencies.push_back (da); } + // @@ If the required description-type value is absent we will not fail + // until toolchain 0.11.0 is released and will set it to plain text + // instead. Not doing so we will fail to build packages coming from + // older pkg repositories. In particular, we will fail to verify that + // the public packages are still buildable with the queued toolchain. + // + if (m.description && + !m.description_type && + flag (package_manifest_flags::require_description_type)) +#if 0 + bad_name ("no package description type specified"); +#else + m.description_type = "text/plain"; +#endif + if (!m.location && flag (package_manifest_flags::require_location)) bad_name ("no package location specified"); @@ -2008,9 +2195,10 @@ namespace bpkg move (nv), iu, false /* complete_depends */, - package_manifest_flags::forbid_file | - package_manifest_flags::require_location | - package_manifest_flags::forbid_fragment | + package_manifest_flags::forbid_file | + package_manifest_flags::require_description_type | + package_manifest_flags::require_location | + package_manifest_flags::forbid_fragment | package_manifest_flags::forbid_incomplete_depends); } @@ -2053,6 +2241,33 @@ namespace bpkg p, move (nv), function (), iu, cd, fl, *this); } + optional package_manifest:: + effective_description_type (bool iu) const + { + if (!description) + throw logic_error ("absent description"); + + optional r; + + if (description_type) + r = to_text_type (*description_type); + else if (description->file) + { + string ext (description->path.extension ()); + if (ext.empty () || casecmp (ext, "txt") == 0) + r = text_type::plain; + else if (casecmp (ext, "md") == 0 || casecmp (ext, "markdown") == 0) + r = text_type::github_mark; + } + else + r = text_type::plain; + + if (!r && !iu) + throw invalid_argument ("unknown text type"); + + return r; + } + void package_manifest:: override (const vector& nvs, const string& name) { @@ -2144,7 +2359,7 @@ namespace bpkg static const string changes_file ("changes-file"); void package_manifest:: - load_files (const function& loader) + load_files (const function& loader, bool iu) { auto load = [&loader] (const string& n, const path& p) { @@ -2158,8 +2373,32 @@ namespace bpkg // Load the description-file manifest value. // - if (description && description->file) - description = text_file (load (description_file, description->path)); + if (description) + { + // Make the description type explicit. + // + optional t (effective_description_type (iu)); // Can throw. + + assert (t || iu); // Can only be absent if we ignore unknown. + + if (!description_type && t) + description_type = to_string (*t); + + // At this point the description type can only be absent if the + // description comes from a file. Otherwise, we would end up with the + // plain text. + // + assert (description_type || description->file); + + if (description->file) + { + if (!description_type) + description_type = "text/unknown; extension=" + + description->path.extension (); + + description = text_file (load (description_file, description->path)); + } + } // Load the changes-file manifest values. // @@ -2226,6 +2465,9 @@ namespace bpkg m.description->comment)); else s.next ("description", m.description->text); + + if (m.description_type) + s.next ("description-type", *m.description_type); } for (const auto& c: m.changes) @@ -2552,8 +2794,14 @@ namespace bpkg d + " for " + p.name.string () + "-" + p.version.string ()); }; - if (p.description && p.description->file) - bad_value ("forbidden description-file"); + if (p.description) + { + if (p.description->file) + bad_value ("forbidden description-file"); + + if (!p.description_type) + bad_value ("no valid description-type"); + } for (const auto& c: p.changes) if (c.file) diff --git a/libbpkg/manifest.hxx b/libbpkg/manifest.hxx index 8dca593..2d1e609 100644 --- a/libbpkg/manifest.hxx +++ b/libbpkg/manifest.hxx @@ -430,7 +430,8 @@ namespace bpkg forbid_incomplete_depends = 0x10, require_location = 0x20, - require_sha256sum = 0x40 + require_sha256sum = 0x40, + require_description_type = 0x80 }; inline package_manifest_flags @@ -594,6 +595,28 @@ namespace bpkg return os << bce.string (); } + enum class text_type + { + plain, + common_mark, + github_mark + }; + + LIBBPKG_EXPORT std::string + to_string (text_type); + + // Throw std::invalid_argument if the argument is not a well-formed text + // type. Otherwise, return nullopt for an unknown text variant. + // + LIBBPKG_EXPORT butl::optional + to_text_type (const std::string&); // May throw std::invalid_argument. + + inline std::ostream& + operator<< (std::ostream& os, text_type t) + { + return os << to_string (t); + } + class LIBBPKG_EXPORT package_manifest { public: @@ -610,6 +633,7 @@ namespace bpkg std::vector license_alternatives; strings tags; butl::optional description; + butl::optional description_type; std::vector changes; butl::optional url; butl::optional doc_url; @@ -635,6 +659,17 @@ namespace bpkg const package_name& effective_project () const noexcept {return project ? *project : name;} + // Return the description type value if present, text_type::github_mark if + // the description refers to a file with the .md or .markdown extension + // and text_type::plain if it refers to a file with the .txt extension or + // no extension or the description does not come from a file. Depending on + // the ignore_unknown value either throw std::invalid_argument or return + // nullopt if the description value or the file extension is unknown. + // Throw std::logic_error if the description value is nullopt. + // + butl::optional + effective_description_type (bool ignore_unknown = false) const; + public: package_manifest () = default; @@ -716,6 +751,9 @@ namespace bpkg // Load the *-file manifest values using the specified load function that // returns the file contents passing through any exception it may throw. + // Set the potentially absent description type value to the effective + // description type. If the effective type is nullopt then assign a + // synthetic unknown type. // // Note that if the returned file contents is empty, load_files() makes // sure that this is allowed by the value's semantics throwing @@ -726,7 +764,8 @@ namespace bpkg const butl::path& value); void - load_files (const std::function&); + load_files (const std::function&, + bool ignore_unknown = false); }; // Create individual package manifest. diff --git a/tests/manifest/driver.cxx b/tests/manifest/driver.cxx index 5f8a0f5..6ef6193 100644 --- a/tests/manifest/driver.cxx +++ b/tests/manifest/driver.cxx @@ -20,7 +20,7 @@ using namespace bpkg; // Usages: // // argv[0] (-pp|-dp|-gp|-pr|-dr|-gr|-s) -// argv[0] [-c] -p +// argv[0] -p -c -i // argv[0] -ec // // In the first form read and parse manifest list from stdin and serialize it @@ -35,8 +35,12 @@ using namespace bpkg; // -s parse signature manifest // // In the second form read and parse the package manifest from stdin and -// serialize it to stdout. Complete the dependency constraints if -c is -// specified. Note: -c, if specified, should go before -p on the command line. +// serialize it to stdout. +// +// -c complete the dependency constraints +// -i ignore unknown +// +// Note: the above options should go after -p on the command line. // // In the third form read and parse dependency constraints from stdin and // roundtrip them to stdout together with their effective constraints, @@ -45,18 +49,8 @@ using namespace bpkg; int main (int argc, char* argv[]) { - assert (argc <= 3); - string opt (argv[1]); - - bool complete_depends (opt == "-c"); - - if (complete_depends) - { - opt = argv[2]; - assert (opt == "-p"); - } - - assert ((opt == "-ec" || complete_depends) == (argc == 3)); + assert (argc >= 2); + string mode (argv[1]); cout.exceptions (ios_base::failbit | ios_base::badbit); @@ -65,8 +59,49 @@ main (int argc, char* argv[]) try { - if (opt == "-ec") + if (mode == "-p") { + bool complete_depends (false); + bool ignore_unknown (false); + + for (int i (2); i != argc; ++i) + { + string o (argv[i]); + + if (o == "-c") + complete_depends = true; + else if (o == "-i") + ignore_unknown = true; + else + assert (false); + } + + cin.exceptions (ios_base::failbit | ios_base::badbit); + + package_manifest ( + p, + [] (version& v) + { + // Emulate populating the snapshot information for the latest + // snapshot. + // + if (butl::optional sv = + parse_standard_version (v.string ())) + { + if (sv->latest_snapshot ()) + { + sv->snapshot_sn = 123; + v = version (sv->string ()); + } + } + }, + ignore_unknown, + complete_depends).serialize (s); + } + else if (mode == "-ec") + { + assert (argc == 3); + version v (argv[2]); cin.exceptions (ios_base::badbit); @@ -84,41 +119,23 @@ main (int argc, char* argv[]) } else { + assert (argc == 2); + cin.exceptions (ios_base::failbit | ios_base::badbit); - if (opt == "-p") - package_manifest ( - p, - [] (version& v) - { - // Emulate populating the snapshot information for the latest - // snapshot. - // - if (butl::optional sv = - parse_standard_version (v.string ())) - { - if (sv->latest_snapshot ()) - { - sv->snapshot_sn = 123; - v = version (sv->string ()); - } - } - }, - false /* ignore_unknown */, - complete_depends).serialize (s); - else if (opt == "-pp") + if (mode == "-pp") pkg_package_manifests (p).serialize (s); - else if (opt == "-dp") + else if (mode == "-dp") dir_package_manifests (p).serialize (s); - else if (opt == "-gp") + else if (mode == "-gp") git_package_manifests (p).serialize (s); - else if (opt == "-pr") + else if (mode == "-pr") pkg_repository_manifests (p).serialize (s); - else if (opt == "-dr") + else if (mode == "-dr") dir_repository_manifests (p).serialize (s); - else if (opt == "-gr") + else if (mode == "-gr") git_repository_manifests (p).serialize (s); - else if (opt == "-s") + else if (mode == "-s") signature_manifest (p).serialize (s); else assert (false); diff --git a/tests/manifest/testscript b/tests/manifest/testscript index 99f0132..acf9d3b 100644 --- a/tests/manifest/testscript +++ b/tests/manifest/testscript @@ -100,7 +100,7 @@ : dependency-constraint-version : - $* -c -p <'stdin:6:10: error: invalid dependency constraint: min version is greater than max version' != 0 + $* -p -c <'stdin:6:10: error: invalid dependency constraint: min version is greater than max version' != 0 : 1 name: foo version: 2.0.0 @@ -117,12 +117,37 @@ : manifest : { + test.options += -p + + : invalid + : + { + : description-file + : + $* <>~%EOE% != 0 + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description-file: /README + EOI + %( + stdin:6:19: error: package description-file path is absolute + %| + stdin:6:19: error: invalid package description file: invalid filesystem path + %) + EOE + } + : complete : { + test.options += -c + : final : - $* -c -p <>EOO + $* <>EOO : 1 name: foo version: 2.0.0 @@ -140,7 +165,7 @@ : non-standard : - $* -c -p <>EOO + $* <>EOO : 1 name: foo version: 2.0.0-x @@ -158,7 +183,7 @@ : non-standard-shortcut : - $* -c -p <>EOE != 0 + $* <>EOE != 0 : 1 name: foo version: 2.0.0-x @@ -171,7 +196,7 @@ : latest-snapshot : - $* -c -p <>EOO + $* <>EOO : 1 name: foo version: 2.0.0-a.0.z @@ -192,7 +217,7 @@ : incomplete : - $* -p <>EOF + $* <>EOF : 1 name: foo version: 2.0.0 @@ -200,6 +225,308 @@ license: LGPLv2 depends: bar == $ | libbaz ~$ | libbox ^$ | libfox [1.0 $) EOF + + : description-type + : + { + : absent + : + $* <>EOO + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description: libfoo is a very modern C++ XML parser. + EOI + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description: libfoo is a very modern C++ XML parser. + EOO + + : not-text + : + $* <>EOE != 0 + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description: libfoo is a very modern C++ XML parser. + description-type: image/gif + EOI + stdin:7:19: error: invalid package description type: text type expected + EOE + + : deducing + : + { + : fail + : + $* <>EOE != 0 + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description-file: README.rtf + EOI + stdin:6:19: error: invalid package description file: unknown text type + EOE + + : ignore-unknown + : + $* -i <>EOO + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description-file: README.rtf + EOI + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description-file: README.rtf + EOO + } + + : unknown + : + { + : fail + : + $* <>EOE != 0 + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description: libfoo is a very modern C++ XML parser. + description-type: text/markdowns + EOI + stdin:7:19: error: invalid package description type: unknown text type + EOE + + : ignore + : + $* -i <>EOO + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description: libfoo is a very modern C++ XML parser. + description-type: text/markdowns + EOI + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description: libfoo is a very modern C++ XML parser. + description-type: text/markdowns + EOO + } + + : plain + : + { + : valid + : + $* <>EOO + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description: libfoo is a very modern C++ XML parser. + description-type: text/plain + EOI + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description: libfoo is a very modern C++ XML parser. + description-type: text/plain + EOO + + : invalid + : + $* <>EOE != 0 + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description: libfoo is a very modern C++ XML parser. + description-type: text/plain; + EOI + stdin:7:19: error: invalid package description type: missing '=' + EOE + } + + : markdown + : + { + : default + : + $* <>EOO + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description: libfoo is a very modern C++ XML parser. + description-type: text/markdown + EOI + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description: libfoo is a very modern C++ XML parser. + description-type: text/markdown + EOO + + : gfm + : + $* <>EOO + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description: libfoo is a very modern C++ XML parser. + description-type: text/markdown; variant=GFM + EOI + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description: libfoo is a very modern C++ XML parser. + description-type: text/markdown; variant=GFM + EOO + + : common-mark + : + $* <>EOO + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description: libfoo is a very modern C++ XML parser. + description-type: text/markdown; variant=CommonMark + EOI + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description: libfoo is a very modern C++ XML parser. + description-type: text/markdown; variant=CommonMark + EOO + + : invalid-variant + : + { + : fail + : + $* <>EOE != 0 + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description: libfoo is a very modern C++ XML parser. + description-type: text/markdown; variant=Original + EOI + stdin:7:19: error: invalid package description type: unknown text type + EOE + + : ignore + : + $* -i <>EOO + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description: libfoo is a very modern C++ XML parser. + description-type: text/markdown; variant=Original + EOI + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description: libfoo is a very modern C++ XML parser. + description-type: text/markdown; variant=Original + EOO + } + + : invalid-parameter + : + { + : fail + : + $* <>EOE != 0 + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description: libfoo is a very modern C++ XML parser. + description-type: text/markdown; variants=GFM + EOI + stdin:7:19: error: invalid package description type: unknown text type + EOE + + : ignore + : + $* -i <>EOO + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description: libfoo is a very modern C++ XML parser. + description-type: text/markdown; variants=GFM + EOI + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + description: libfoo is a very modern C++ XML parser. + description-type: text/markdown; variants=GFM + EOO + } + } + } + + : builds + : + { + : invalid + : + { + : empty + : + $* <"stdin:2:9: error: invalid package builds: class expression separator ':' expected" != 0 + : 1 + builds: default -gcc + EOI + } + } } : manifest-list @@ -219,6 +546,7 @@ license: BSD tags: c++, xml, parser, serializer, pull, streaming, modern description: libfoo is a very modern C++ XML parser. + description-type: text/plain changes: 1.2.3+2: applied upstream patch for critical bug bar changes: 1.2.3+1: applied upstream patch for critical bug foo url: http://www.example.org/projects/libfoo/; libfoo project page url @@ -323,23 +651,6 @@ fragment: ca602c2d46b0dca7a9ebc856871767b0ba6b74f3 EOF } - - : builds - : - { - : invalid - : - { - : empty - : - $* -pp <"stdin:4:9: error: invalid package builds: class expression separator ':' expected" != 0 - : 1 - sha256sum: a2b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 - : - builds: default -gcc - EOI - } - } } : repositories -- cgit v1.1