Reimplement tar invocations to do manual decompression

This is needed to prevent tar from forking, which doesn't work reliably
on MSYS2.

5 files changed, 124 insertions, 38 deletions

diff --git a/bpkg/archive b/bpkg/archive
index 918734c..1f33538 100644
--- a/bpkg/archive
+++ b/bpkg/archive
@@ -20,10 +20,12 @@ namespace bpkg
   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
+  // error is false, then redirect STDERR to /dev/null (this can be used, for
   // example, to suppress diagnostics).
   //
-  butl::process
+  // Return a pair of processes that form a pipe. Wait on the second first.
+  //
+  pair<butl::process, butl::process>
   start_extract (const common_options&,
                  const path& archive,
                  const path& file,
diff --git a/bpkg/archive.cxx b/bpkg/archive.cxx
index 4e12e28..54ab8ea 100644
--- a/bpkg/archive.cxx
+++ b/bpkg/archive.cxx
@@ -26,7 +26,7 @@ namespace bpkg
     return path_cast<dir_path> (d);
   }
 
-  process
+  pair<process, process>
   start_extract (const common_options& co,
                  const path& a,
                  const path& f,
@@ -34,7 +34,30 @@ namespace bpkg
   {
     assert (!f.empty () && f.relative ());
 
-    cstrings args {co.tar ().string ().c_str ()};
+    cstrings args;
+
+    // See if we need to decompress.
+    //
+    {
+      string e (a.extension ());
+
+      if      (e == "gz")    args.push_back ("gzip");
+      else if (e == "bzip2") args.push_back ("bzip2");
+      else if (e == "xz")    args.push_back ("xz");
+      else if (e != "tar")
+        fail << "unknown compression method in " << a;
+    }
+
+    size_t i (0); // The tar command line start.
+    if (!args.empty ())
+    {
+      args.push_back ("-dc");
+      args.push_back (a.string ().c_str ());
+      args.push_back (nullptr);
+      i = args.size ();
+    }
+
+    args.push_back (co.tar ().string ().c_str ());
 
     // Add extra options.
     //
@@ -55,7 +78,7 @@ namespace bpkg
 #endif
 
     args.push_back ("-xf");
-    args.push_back (a.string ().c_str ());
+    args.push_back (i == 0 ? a.string ().c_str () : "-");
 
     // MSYS tar doesn't find archived file if it's path is provided in Windows
     // notation.
@@ -64,21 +87,42 @@ namespace bpkg
     args.push_back (fs.c_str ());
 
     args.push_back (nullptr);
+    args.push_back (nullptr); // Pipe end.
 
+    size_t what;
     try
     {
-      process_path pp (process::path_search (args[0]));
+      process_path dpp;
+      process_path tpp;
+
+      process dpr;
+      process tpr;
+
+      if (i != 0)
+        dpp = process::path_search (args[what = 0]);
+
+      tpp = process::path_search (args[what = i]);
 
       if (verb >= 2)
         print_process (args);
 
       // If err is false, then redirect STDERR to STDOUT.
       //
-      return process (pp, args.data (), 0, -1, (err ? 2 : 1));
+      auto_fd nfd (err ? nullfd : fdnull ());
+
+      if (i != 0)
+      {
+        dpr = process (dpp, &args[what = 0], 0,   -1, (err ? 2 : nfd.get ()));
+        tpr = process (tpp, &args[what = i], dpr, -1, (err ? 2 : nfd.get ()));
+      }
+      else
+        tpr = process (tpp, &args[what = 0], 0, -1, (err ? 2 : nfd.get ()));
+
+      return make_pair (move (dpr), move (tpr));
     }
     catch (const process_error& e)
     {
-      error << "unable to execute " << args[0] << ": " << e;
+      error << "unable to execute " << args[what] << ": " << e;
 
       if (e.child)
         exit (1);
@@ -91,20 +135,20 @@ namespace bpkg
   extract (const common_options& o, const path& a, const path& f)
   try
   {
-    process pr (start_extract (o, a, f));
+    pair<process, process> pr (start_extract (o, a, f));
 
     try
     {
       // Do not throw when eofbit is set (end of stream reached), and
       // when failbit is set (getline() failed to extract any character).
       //
-      ifdstream is (move (pr.in_ofd), ifdstream::badbit);
+      ifdstream is (move (pr.second.in_ofd), ifdstream::badbit);
 
       string s;
       getline (is, s, '\0');
       is.close ();
 
-      if (pr.wait ())
+      if (pr.second.wait () && pr.first.wait ())
         return s;
 
       // Fall through.
@@ -114,7 +158,7 @@ namespace bpkg
       // Child exit status doesn't matter. Just wait for the process
       // completion and fall through.
       //
-      pr.wait (); // Check throw.
+      pr.second.wait (); pr.first.wait (); // Check throw.
     }
 
     // While it is reasonable to assuming the child process issued diagnostics
diff --git a/bpkg/pkg-unpack.cxx b/bpkg/pkg-unpack.cxx
index d4a255c..9aead38 100644
--- a/bpkg/pkg-unpack.cxx
+++ b/bpkg/pkg-unpack.cxx
@@ -163,7 +163,30 @@ namespace bpkg
     //
     auto_rm_r arm (d);
 
-    cstrings args {co.tar ().string ().c_str ()};
+    cstrings args;
+
+    // See if we need to decompress.
+    //
+    {
+      string e (a.extension ());
+
+      if      (e == "gz")    args.push_back ("gzip");
+      else if (e == "bzip2") args.push_back ("bzip2");
+      else if (e == "xz")    args.push_back ("xz");
+      else if (e != "tar")
+        fail << "unknown compression method in package " << a;
+    }
+
+    size_t i (0); // The tar command line start.
+    if (!args.empty ())
+    {
+      args.push_back ("-dc");
+      args.push_back (a.string ().c_str ());
+      args.push_back (nullptr);
+      i = args.size ();
+    }
+
+    args.push_back (co.tar ().string ().c_str ());
 
     // Add extra options.
     //
@@ -185,28 +208,45 @@ namespace bpkg
 #endif
 
     args.push_back ("-xf");
-    args.push_back (a.string ().c_str ());
+    args.push_back (i == 0 ? a.string ().c_str () : "-");
     args.push_back (nullptr);
+    args.push_back (nullptr); // Pipe end.
 
+    size_t what;
     try
     {
-      process_path pp (process::path_search (args[0]));
+      process_path dpp;
+      process_path tpp;
+
+      process dpr;
+      process tpr;
+
+      if (i != 0)
+        dpp = process::path_search (args[what = 0]);
+
+      tpp = process::path_search (args[what = i]);
 
       if (verb >= 2)
         print_process (args);
 
-      process pr (pp, args.data ());
+      if (i != 0)
+      {
+        dpr = process (dpp, &args[what = 0], 0, -1);
+        tpr = process (tpp, &args[what = i], dpr);
+      }
+      else
+        tpr = process (tpp, &args[what = 0]);
 
       // While it is reasonable to assuming the child process issued
-      // diagnostics, tar, specifically, doesn't mention the archive
-      // name.
+      // diagnostics, tar, specifically, doesn't mention the archive name.
       //
-      if (!pr.wait ())
+      if (!(what = i, tpr.wait ()) ||
+          !(what = 0, dpr.wait ()))
         fail << "unable to extract package archive " << a;
     }
     catch (const process_error& e)
     {
-      error << "unable to execute " << args[0] << ": " << e;
+      error << "unable to execute " << args[what] << ": " << e;
 
       if (e.child)
         exit (1);
diff --git a/bpkg/pkg-verify.cxx b/bpkg/pkg-verify.cxx
index cd51d54..275c710 100644
--- a/bpkg/pkg-verify.cxx
+++ b/bpkg/pkg-verify.cxx
@@ -23,26 +23,27 @@ namespace bpkg
     dir_path pd (package_dir (af));
     path mf (pd / path ("manifest"));
 
-    // 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.
+    // 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.
     //
-    process pr (start_extract (co, af, mf, diag));
+    // 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.
+    //
+    pair<process, process> pr (start_extract (co, af, mf, diag));
+
+    auto wait = [&pr] () {return pr.second.wait () && pr.first.wait ();};
 
     try
     {
-      ifdstream is (move (pr.in_ofd), fdstream_mode::skip);
+      ifdstream is (move (pr.second.in_ofd), fdstream_mode::skip);
       manifest_parser mp (is, mf.string ());
       package_manifest m (mp, iu);
       is.close ();
 
-      if (pr.wait ())
+      if (wait ())
       {
         // Verify package archive/directory is <name>-<version>.
         //
@@ -68,7 +69,7 @@ namespace bpkg
     //
     catch (const manifest_parsing& e)
     {
-      if (pr.wait ())
+      if (wait ())
       {
         if (diag)
           error (e.name, e.line, e.column) << e.description <<
@@ -79,7 +80,7 @@ namespace bpkg
     }
     catch (const io_error&)
     {
-      if (pr.wait ())
+      if (wait ())
       {
         if (diag)
           error << "unable to extract " << mf << " from " << af;
@@ -91,7 +92,7 @@ namespace bpkg
     // We should only get here if the child exited with an error
     // status.
     //
-    assert (!pr.wait ());
+    assert (!wait ());
 
     // While it is reasonable to assuming the child process issued
     // diagnostics, tar, specifically, doesn't mention the archive
diff --git a/tests/rep-create.test b/tests/rep-create.test
index acc6957..01ab7f8 100644
--- a/tests/rep-create.test
+++ b/tests/rep-create.test
@@ -176,9 +176,8 @@
     $clone_rep;
     touch stable/foo;
 
-    $* stable/ 2>>/~%EOE% != 0
-      %.+
-      error: stable/foo does not appear to be a bpkg package
+    $* stable/ 2>>/EOE != 0
+      error: unknown compression method in stable/foo
       EOE
   }
 }