Replace description and change classes with text_file class

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<licenses> license_alternatives;
     strings tags;
-    butl::optional<description_type> description;
-    std::vector<change> changes;
+    butl::optional<text_file> description;
+    std::vector<text_file> changes;
     url_type url;
     butl::optional<url_type> package_url;
     email_type email;
@@ -349,13 +345,24 @@ namespace bpkg
     butl::optional<std::string> 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<package_manifest>
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]