Make rep_create to convert file-type manifest values to corresponding inline-type ones

7 files changed, 286 insertions, 104 deletions

diff --git a/bpkg/archive b/bpkg/archive
new file mode 100644
index 0000000..4d90384
--- /dev/null
+++ b/bpkg/archive
@@ -0,0 +1,38 @@
+// file      : bpkg/archive -*- C++ -*-
+// copyright : Copyright (c) 2014-2016 Code Synthesis Ltd
+// license   : MIT; see accompanying LICENSE file
+
+#ifndef BPKG_ARCHIVE
+#define BPKG_ARCHIVE
+
+#include <butl/process>
+
+#include <bpkg/types>
+#include <bpkg/utility>
+
+#include <bpkg/common-options>
+
+namespace bpkg
+{
+  // Return the package directory based on the package archive.
+  //
+  dir_path
+  package_dir (const path& archive);
+
+  // Start the process of extracting the specified file from the archive. If
+  // error is false, then redirect STDERR to STDOUT (this can be used, for
+  // example, to suppress diagnostics).
+  //
+  butl::process
+  start_extract (const common_options&,
+                 const path& archive,
+                 const path& file,
+                 bool error = true);
+
+  // Start as above and then extract the file content as a string.
+  //
+  string
+  extract (const common_options&, const path& archive, const path& file);
+}
+
+#endif // BPKG_ARCHIVE
diff --git a/bpkg/archive.cxx b/bpkg/archive.cxx
new file mode 100644
index 0000000..b0c7ab7
--- /dev/null
+++ b/bpkg/archive.cxx
@@ -0,0 +1,120 @@
+// file      : bpkg/archive.cxx -*- C++ -*-
+// copyright : Copyright (c) 2014-2016 Code Synthesis Ltd
+// license   : MIT; see accompanying LICENSE file
+
+#include <bpkg/archive>
+
+#include <butl/process>
+#include <butl/fdstream>
+
+#include <bpkg/diagnostics>
+
+using namespace std;
+using namespace butl;
+
+namespace bpkg
+{
+  dir_path
+  package_dir (const path& a)
+  {
+    // Strip the top-level extension and, as a special case, if the second-
+    // level extension is .tar, strip that as well (e.g., .tar.bz2).
+    //
+    path d (a.leaf ().base ());
+    if (const char* e = d.extension ())
+    {
+      if (e == string ("tar"))
+        d = d.base ();
+    }
+    return path_cast<dir_path> (d);
+  }
+
+  process
+  start_extract (const common_options& co,
+                 const path& a,
+                 const path& f,
+                 bool err)
+  {
+    assert (!f.empty () && f.relative ());
+
+    cstrings args {co.tar ().string ().c_str ()};
+
+    // Add extra options.
+    //
+    for (const string& o: co.tar_option ())
+      args.push_back (o.c_str ());
+
+    // -O/--to-stdout -- extract to STDOUT.
+    //
+    args.push_back ("-O");
+
+    args.push_back ("-xf");
+    args.push_back (a.string ().c_str ());
+    args.push_back (f.string ().c_str ());
+    args.push_back (nullptr);
+
+    if (verb >= 2)
+      print_process (args);
+
+    try
+    {
+      // If err is false, then redirect STDERR to STDOUT.
+      //
+      return process (args.data (), 0, -1, (err ? 2 : 1));
+    }
+    catch (const process_error& e)
+    {
+      error << "unable to execute " << args[0] << ": " << e.what ();
+
+      if (e.child ())
+        exit (1);
+
+      throw failed ();
+    }
+  }
+
+  string
+  extract (const common_options& o, const path& a, const path& f)
+  try
+  {
+    process pr (start_extract (o, a, f));
+
+    try
+    {
+      ifdstream is (pr.in_ofd);
+
+      // Do not throw when eofbit is set (end of stream reached), and
+      // when failbit is set (getline() failed to extract any character).
+      //
+      is.exceptions (ifdstream::badbit);
+
+      string s;
+      getline (is, s, '\0');
+
+      if (pr.wait ())
+        return s;
+
+      // Fall through.
+    }
+    catch (const ifdstream::failure&)
+    {
+      // Child exit status doesn't matter. Just wait for the process
+      // completion and fall through.
+      //
+      pr.wait ();
+    }
+
+    // While it is reasonable to assuming the child process issued diagnostics
+    // if exited with an error status, tar, specifically, doesn't mention the
+    // archive name. So print the error message whatever the child exit status
+    // is.
+    //
+    error << "unable to extract " << f << " from " << a;
+    throw failed ();
+  }
+  catch (const process_error& e)
+  {
+    error << "unable to extract " << f << " from " << a << ": " << e.what ();
+    throw failed ();
+  }
+}
diff --git a/bpkg/buildfile b/bpkg/buildfile
index 50a68e6..5984b53 100644
--- a/bpkg/buildfile
+++ b/bpkg/buildfile
@@ -8,6 +8,7 @@ import libs += libodb%lib{odb}
 import libs += libodb-sqlite%lib{odb-sqlite}
 
 exe{bpkg}:                                                                   \
+{hxx         cxx}{ archive          }                                        \
 {hxx            }{ bpkg-version     }                                        \
 {            cxx}{ bpkg             } {hxx ixx cxx}{ bpkg-options          } \
 {hxx         cxx}{ cfg-add          } {hxx ixx cxx}{ cfg-add-options       } \
diff --git a/bpkg/pkg-verify.cxx b/bpkg/pkg-verify.cxx
index 292ea86..3c79cbd 100644
--- a/bpkg/pkg-verify.cxx
+++ b/bpkg/pkg-verify.cxx
@@ -11,6 +11,7 @@
 
 #include <bpkg/manifest-parser>
 
+#include <bpkg/archive>
 #include <bpkg/diagnostics>
 
 using namespace std;
@@ -20,135 +21,110 @@ namespace bpkg
 {
   package_manifest
   pkg_verify (const common_options& co, const path& af, bool iu, bool diag)
+  try
   {
-    // Figure out the package directory. Strip the top-level extension
-    // and, as a special case, if the second-level extension is .tar,
-    // strip that as well (e.g., .tar.bz2).
-    //
-    path pd (af.leaf ().base ());
-    if (const char* e = pd.extension ())
-    {
-      if (e == string ("tar"))
-        pd = pd.base ();
-    }
-
-    // Extract the manifest.
-    //
+    dir_path pd (package_dir (af));
     path mf (pd / path ("manifest"));
 
-    cstrings args {co.tar ().string ().c_str ()};
-
-    // Add extra options.
+    // If diag is false, we need to make tar not print any diagnostics.
+    // There doesn't seem to be an option to suppress this and the only
+    // way is to redirect STDERR to something like /dev/null. To keep
+    // things simple, we are going to redirect it to STDOUT, which we
+    // in turn redirect to a pipe and use to parse the manifest data.
+    // If things go badly for tar and it starts spitting errors instead
+    // of the manifest, the manifest parser will fail. But that's ok
+    // since we assume that the child error is always the reason for
+    // the manifest parsing failure.
     //
-    for (const string& o: co.tar_option ())
-      args.push_back (o.c_str ());
+    process pr (start_extract (co, af, mf, diag));
 
-    // -O/--to-stdout -- extract to STDOUT.
-    //
-    args.push_back ("-O");
-
-    args.push_back ("-xf");
-    args.push_back (af.string ().c_str ());
-    args.push_back (mf.string ().c_str ());
-    args.push_back (nullptr);
-
-    if (verb >= 2)
-      print_process (args);
+    ifdstream is (pr.in_ofd);
+    is.exceptions (ifdstream::badbit | ifdstream::failbit);
 
     try
     {
-      // If diag is false, we need to make tar not print any diagnostics.
-      // There doesn't seem to be an option to suppress this and the only
-      // way is to redirect STDERR to something like /dev/null. To keep
-      // things simple, we are going to redirect it to STDOUT, which we
-      // in turn redirect to a pipe and use to parse the manifest data.
-      // If things go badly for tar and it starts spitting errors instead
-      // of the manifest, the manifest parser will fail. But that's ok
-      // since we assume that the child error is always the reason for
-      // the manifest parsing failure.
-      //
-      process pr (args.data (), 0, -1, (diag ? 2 : 1));
+      manifest_parser mp (is, mf.string ());
+      package_manifest m (mp, iu);
+      is.close ();
 
-      try
+      if (pr.wait ())
       {
-        ifdstream is (pr.in_ofd);
-        is.exceptions (ifdstream::badbit | ifdstream::failbit);
-
-        manifest_parser mp (is, mf.string ());
-        package_manifest m (mp, iu);
-        is.close ();
+        // Verify package archive/directory is <name>-<version>.
+        //
+        dir_path ed (m.name + "-" + m.version.string ());
 
-        if (pr.wait ())
-        {
-          // Verify package archive/directory is <name>-<version>.
-          //
-          path ed (m.name + "-" + m.version.string ());
-
-          if (pd != ed)
-          {
-            if (diag)
-              error << "package archive/directory name mismatch in " << af <<
-                info << "extracted from archive '" << pd << "'" <<
-                info << "expected from manifest '" << ed << "'";
-
-            throw failed ();
-          }
-
-          return m;
-        }
-
-        // Child existed with an error, fall through.
-      }
-      // Ignore these exceptions if the child process exited with
-      // an error status since that's the source of the failure.
-      //
-      catch (const manifest_parsing& e)
-      {
-        if (pr.wait ())
+        if (pd != ed)
         {
           if (diag)
-            error (e.name, e.line, e.column) << e.description <<
-              info << "package archive " << af;
+            error << "package archive/directory name mismatch in " << af <<
+              info << "extracted from archive '" << pd << "'" <<
+              info << "expected from manifest '" << ed << "'";
 
           throw failed ();
         }
-      }
-      catch (const ifdstream::failure&)
-      {
-        if (pr.wait ())
-        {
-          if (diag)
-            error << "unable to extract " << mf << " from " << af;
 
-          throw failed ();
-        }
+        return m;
       }
 
-      // We should only get here if the child exited with an error
-      // status.
+      // Child exited with an error, fall through.
+    }
+    // Ignore these exceptions if the child process exited with
+    // an error status since that's the source of the failure.
+    //
+    catch (const manifest_parsing& e)
+    {
+      // Before we used to just close the file descriptor to signal to the
+      // other end that we are not interested in the rest. But tar doesn't
+      // take this very well (SIGPIPE). So now we are going to skip until
+      // the end.
       //
-      assert (!pr.wait ());
+      if (!is.eof ())
+        is.ignore (numeric_limits<streamsize>::max ());
+      is.close ();
 
-      // While it is reasonable to assuming the child process issued
-      // diagnostics, tar, specifically, doesn't mention the archive
-      // name.
-      //
-      if (diag)
-        error << af << " does not appear to be a bpkg package";
+      if (pr.wait ())
+      {
+        if (diag)
+          error (e.name, e.line, e.column) << e.description <<
+            info << "package archive " << af;
 
-      throw failed ();
+        throw failed ();
+      }
     }
-    catch (const process_error& e)
+    catch (const ifdstream::failure&)
     {
-      // Note: this is not an "invalid package" case, so no diag check.
-      //
-      error << "unable to execute " << args[0] << ": " << e.what ();
+      is.close ();
 
-      if (e.child ())
-        exit (1);
+      if (pr.wait ())
+      {
+        if (diag)
+          error << "unable to extract " << mf << " from " << af;
 
-      throw failed ();
+        throw failed ();
+      }
     }
+
+    // We should only get here if the child exited with an error
+    // status.
+    //
+    assert (!pr.wait ());
+
+    // While it is reasonable to assuming the child process issued
+    // diagnostics, tar, specifically, doesn't mention the archive
+    // name.
+    //
+    if (diag)
+      error << af << " does not appear to be a bpkg package";
+
+    throw failed ();
+  }
+  catch (const process_error& e)
+  {
+    // Note: this is not an "invalid package" case, so no diag check.
+    //
+    error << "unable to extract manifest file from " << af << ": "
+          << e.what ();
+    throw failed ();
   }
 
   package_manifest
diff --git a/bpkg/rep-create.cxx b/bpkg/rep-create.cxx
index c714445..9c6f275 100644
--- a/bpkg/rep-create.cxx
+++ b/bpkg/rep-create.cxx
@@ -13,8 +13,9 @@
 #include <bpkg/manifest>
 #include <bpkg/manifest-serializer>
 
-#include <bpkg/checksum>
 #include <bpkg/fetch>
+#include <bpkg/archive>
+#include <bpkg/checksum>
 #include <bpkg/diagnostics>
 
 #include <bpkg/pkg-verify>
@@ -110,6 +111,39 @@ namespace bpkg
       //
       m.location = a.leaf (root);
 
+      dir_path pd (m.name + "-" + m.version.string ());
+
+      // Expand the description-file manifest value.
+      //
+      if (m.description && m.description->file)
+      {
+        path f (pd / m.description->path);
+        string s (extract (o, a, f));
+
+        if (s.empty ())
+          fail << "description-file value in manifest of package archive "
+               << a << " references empty file " << f;
+
+        m.description = text_file (move (s));
+      }
+
+      // Expand the changes-file manifest values.
+      //
+      for (auto& c: m.changes)
+      {
+        if (c.file)
+        {
+          path f (pd / c.path);
+          string s (extract (o, a, f));
+
+          if (s.empty ())
+            fail << "changes-file value in manifest of package archive " << a
+                 << " references empty file " << f;
+
+          c = text_file (move (s));
+        }
+      }
+
       package_key k {m.name, m.version}; // Argument evaluation order.
       auto r (map.emplace (move (k), package_data {a, move (m)}));
 
diff --git a/tests/repository/1/common/bar/unstable/libbar-1.1.1.tar.gz b/tests/repository/1/common/bar/unstable/libbar-1.1.1.tar.gz
index 889dc90..a0fbdc3 100644
--- a/tests/repository/1/common/bar/unstable/libbar-1.1.1.tar.gz
+++ b/tests/repository/1/common/bar/unstable/libbar-1.1.1.tar.gz
diff --git a/tests/test.sh b/tests/test.sh
index 87dfa98..9345d35 100755
--- a/tests/test.sh
+++ b/tests/test.sh
@@ -225,11 +225,24 @@ name: libbar
 version: 1.1.1
 summary: libbar
 license: MIT
+description: \\
+libbar is a very modern C++ XML parser.
+
+It has an API that we believe should have already been in Boost or even in
+the C++ standard library.
+
+\\
+changes: \\
+* Applied upstream patch for critical bug bar.
+
+* Applied upstream patch for critical bug foo.
+
+\\
 url: http://example.org
 email: pkg@example.org
 depends: libfoo >= 1.1.0
 location: libbar-1.1.1.tar.gz
-sha256sum: abb073e9729bbc914f26ed0dedad9f5ead2c73a36cbc1e8f2702d6fde7634ff3
+sha256sum: d09700602ff78ae405b6d4850e34660e939d27676e015a23b549884497c8bb45
 EOF