From 046c4d8017a2c709f506816d4f73d734ccbeaef7 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Thu, 10 Mar 2016 11:37:08 +0300 Subject: Replace description and change classes with text_file class --- bpkg/manifest | 67 ++++++++++++----------- bpkg/manifest.cxx | 138 ++++++++++++++++++++++++++++++++++++++---------- tests/manifest/packages | 2 - 3 files changed, 147 insertions(+), 60 deletions(-) diff --git a/bpkg/manifest b/bpkg/manifest index 29ba6fe..a6eeb05 100644 --- a/bpkg/manifest +++ b/bpkg/manifest @@ -170,23 +170,40 @@ namespace bpkg // description // description-file + // change + // change-file // - class description: public std::string + class text_file { public: + using path_type = butl::path; + bool file; + + union + { + std::string text; + path_type path; + }; + std::string comment; - // Description constructor. + // File text constructor. // explicit - description (std::string d = "") - : std::string (std::move (d)), file (false) {} + text_file (std::string t = ""): file (false), text (std::move (t)) {} - // Description file constructor. + // File reference constructor. // - description (std::string f, std::string c) - : std::string (std::move (f)), file (true), comment (std::move (c)) {} + text_file (path_type p, std::string c) + : file (true), path (std::move (p)), comment (std::move (c)) {} + + text_file (text_file&&); + text_file (const text_file&); + text_file& operator= (text_file&&); + text_file& operator= (const text_file&); + + ~text_file (); }; // license @@ -200,26 +217,6 @@ namespace bpkg licenses (std::string c = ""): comment (std::move (c)) {} }; - // change - // change-file - // - class change: public std::string - { - public: - bool file; - std::string comment; - - // Change constructor. - // - explicit - change (std::string c = ""): std::string (std::move (c)), file (false) {} - - // Change file constructor. - // - change (std::string f, std::string c) - : std::string (std::move (f)), file (true), comment (std::move (c)) {} - }; - // url // package-url // @@ -326,7 +323,6 @@ namespace bpkg using priority_type = bpkg::priority; using url_type = bpkg::url; using email_type = bpkg::email; - using description_type = bpkg::description; std::string name; version_type version; @@ -334,8 +330,8 @@ namespace bpkg std::string summary; std::vector license_alternatives; strings tags; - butl::optional description; - std::vector changes; + butl::optional description; + std::vector changes; url_type url; butl::optional package_url; email_type email; @@ -349,13 +345,24 @@ namespace bpkg butl::optional sha256sum; public: + // Create individual package manifest. + // package_manifest (manifest_parser&, bool ignore_unknown = false); + + // Create an element of the package list manifest. + // package_manifest (manifest_parser&, manifest_name_value start, bool ignore_unknown = false); void serialize (manifest_serializer&) const; + + private: + package_manifest (manifest_parser&, + manifest_name_value start, + bool in_list, + bool ignore_unknown); }; class package_manifests: public std::vector diff --git a/bpkg/manifest.cxx b/bpkg/manifest.cxx index 2cad721..3f937a8 100644 --- a/bpkg/manifest.cxx +++ b/bpkg/manifest.cxx @@ -526,6 +526,54 @@ namespace bpkg return v; } + // text_file + // + text_file:: + ~text_file () + { + if (file) + path.~path_type (); + else + text.~string (); + } + + text_file:: + text_file (text_file&& f): file (f.file), comment (move (f.comment)) + { + if (file) + new (&path) path_type (move (f.path)); + else + new (&text) string (move (f.text)); + } + + text_file:: + text_file (const text_file& f): file (f.file), comment (f.comment) + { + if (file) + new (&path) path_type (f.path); + else + new (&text) string (f.text); + } + + text_file& text_file:: + operator= (text_file&& f) + { + if (this != &f) + { + this->~text_file (); + new (this) text_file (move (f)); // Assume noexcept move-construction. + } + return *this; + } + + text_file& text_file:: + operator= (const text_file& f) + { + if (this != &f) + *this = text_file (f); // Reduce to move-assignment. + return *this; + } + // depends // @@ -610,7 +658,7 @@ namespace bpkg // package_manifest:: package_manifest (parser& p, bool iu) - : package_manifest (p, p.next (), iu) // Delegate + : package_manifest (p, p.next (), false, iu) // Delegate { // Make sure this is the end. // @@ -622,6 +670,12 @@ namespace bpkg package_manifest:: package_manifest (parser& p, name_value nv, bool iu) + : package_manifest (p, nv, true, iu) // Delegate + { + } + + package_manifest:: + package_manifest (parser& p, name_value nv, bool il, bool iu) { auto bad_name ([&p, &nv](const string& d) { throw parsing (p.name (), nv.name_line, nv.name_column, d);}); @@ -713,10 +767,13 @@ namespace bpkg if (v.empty ()) bad_value ("empty package description"); - description = description_type (move (v)); + description = text_file (move (v)); } else if (n == "description-file") { + if (il) + bad_name ("package description-file not allowed"); + if (description) { if (description->file) @@ -731,7 +788,12 @@ namespace bpkg if (v.empty ()) bad_value ("no path in package description-file"); - description = description_type (move (v), move (c)); + path p (v); + + if (p.absolute ()) + bad_value ("package description-file path is absolute"); + + description = text_file (move (p), move (c)); } else if (n == "changes") { @@ -742,12 +804,20 @@ namespace bpkg } else if (n == "changes-file") { + if (il) + bad_name ("package changes-file not allowed"); + string c (split_comment (v)); if (v.empty ()) bad_value ("no path in package changes-file"); - changes.emplace_back (move (v), move (c)); + path p (v); + + if (p.absolute ()) + bad_value ("package changes-file path is absolute"); + + changes.emplace_back (move (p), move (c)); } else if (n == "url") { @@ -1055,10 +1125,11 @@ namespace bpkg dependencies.push_back (da); } - // Manifest list names. Currently we don't check it is indeed a list. - // else if (n == "location") { + if (!il) + bad_name ("package location not allowed"); + if (location) bad_name ("package location redefinition"); @@ -1081,6 +1152,9 @@ namespace bpkg } else if (n == "sha256sum") { + if (!il) + bad_name ("package sha256sum not allowed"); + if (sha256sum) bad_name ("package sha256sum redefinition"); @@ -1107,6 +1181,15 @@ namespace bpkg bad_value ("no project email specified"); else if (license_alternatives.empty ()) bad_value ("no project license specified"); + + if (il) + { + if (!location) + bad_name ("no package location specified"); + + if (!sha256sum) + bad_name ("no package sha256sum specified"); + } } void package_manifest:: @@ -1115,6 +1198,9 @@ namespace bpkg // @@ Should we check that all non-optional values are specified ? // @@ Should we check that values are valid: name is not empty, version // release is not empty, sha256sum is a proper string, ... + // @@ Currently we don't know if we are serializing the individual package + // manifest or the package list manifest, so can't ensure all values + // allowed in the current context (location, sha256sum, *-file values). // s.next ("", "1"); // Start of manifest. @@ -1140,17 +1226,18 @@ namespace bpkg { if (description->file) s.next ("description-file", - add_comment (*description, description->comment)); + add_comment ( + description->path.string (), description->comment)); else - s.next ("description", *description); + s.next ("description", description->text); } for (const auto& c: changes) { if (c.file) - s.next ("changes-file", add_comment (c, c.comment)); + s.next ("changes-file", add_comment (c.path.string (), c.comment)); else - s.next ("changes", c); + s.next ("changes", c.text); } s.next ("url", add_comment (url, url.comment)); @@ -1232,19 +1319,8 @@ namespace bpkg // Parse package manifests. // - for (nv = p.next (); !nv.empty (); ) - { + for (nv = p.next (); !nv.empty (); nv = p.next ()) push_back (package_manifest (p, nv, iu)); - nv = p.next (); - - const package_manifest& m (back ()); - - if (!m.location) - bad_name ("package location expected"); - - if (!m.sha256sum) - bad_name ("package sha256sum expected"); - } } void package_manifests:: @@ -1262,19 +1338,25 @@ namespace bpkg // for (const package_manifest& p: *this) { - auto no_value = [&p, &s](const string& d) + auto bad_value = [&p, &s](const string& d) { throw - serialization ( - s.name (), - d + " for " + p.name + "-" + p.version.string ()); + serialization ( + s.name (), d + " for " + p.name + "-" + p.version.string ()); }; + if (p.description && p.description->file) + bad_value ("forbidden description-file"); + + for (const auto& c: p.changes) + if (c.file) + bad_value ("forbidden changes-file"); + if (!p.location) - no_value ("no valid location"); + bad_value ("no valid location"); if (!p.sha256sum) - no_value ("no valid sha256sum"); + bad_value ("no valid sha256sum"); p.serialize (s); } diff --git a/tests/manifest/packages b/tests/manifest/packages index dda9392..7c076ea 100644 --- a/tests/manifest/packages +++ b/tests/manifest/packages @@ -11,7 +11,6 @@ tags: c++, xml, parser, serializer, pull, streaming, modern description: libfoo is a very modern C++ XML parser. changes: 1.2.3+2: applied upstream patch for critical bug bar changes: 1.2.3+1: applied upstream patch for critical bug foo -changes-file: NEWS url: http://www.example.org/projects/libfoo/; libfoo project page url package-url: http://www.example.org/projects/libbar/1.2.3+2; package url email: libfoo-users@example.org; Public mailing list, posts by non-members\ @@ -36,7 +35,6 @@ version: 3.4A.5+6 summary: Modern bar management framework license: LGPLv2 tags: c++, xml, modern -description-file: README; Comprehensive description url: http://www.example.org/projects/libbar/ email: libbar-users@example.org depends: libbaz (1- 2-) | libbaz [3 4-) | libbaz (5 6] | libbaz [7 8] -- cgit v1.1