Improve process run_*() API

11 files changed, 138 insertions, 66 deletions

diff --git a/libbuild2/cc/gcc.cxx b/libbuild2/cc/gcc.cxx
index 5857709..91a04f6 100644
--- a/libbuild2/cc/gcc.cxx
+++ b/libbuild2/cc/gcc.cxx
@@ -219,11 +219,14 @@ namespace build2
 
         is.close (); // Don't block.
       }
-      catch (const io_error&)
+      catch (const io_error& e)
       {
-        pr.wait ();
-        fail << "error reading " << x_lang << " compiler -print-search-dirs "
-             << "output";
+        if (run_wait (args, pr))
+          fail << "io error reading " << args[0] << " -print-search-dirs "
+               << "output: " << e;
+
+        // If the child process has failed then assume the io error was caused
+        // by that and let run_finish() deal with it.
       }
 
       run_finish (args, pr);
diff --git a/libbuild2/cc/guess.cxx b/libbuild2/cc/guess.cxx
index 053a4cb..d5fc884 100644
--- a/libbuild2/cc/guess.cxx
+++ b/libbuild2/cc/guess.cxx
@@ -224,7 +224,7 @@ namespace build2
         // that.
       }
 
-      if (!run_finish (args.data (), pr, false /* error */, l))
+      if (!run_finish_code (args.data (), pr, l))
         r = "none";
 
       if (r.empty ())
@@ -2077,7 +2077,7 @@ namespace build2
         // that.
       }
 
-      if (!run_finish (args.data (), pr, false /* error */, l))
+      if (!run_finish_code (args.data (), pr, l))
         fail << "unable to extract MSVC information from " << xp;
 
       if (const char* w = (
diff --git a/libbuild2/cc/link-rule.cxx b/libbuild2/cc/link-rule.cxx
index ccb8dc6..6969ef2 100644
--- a/libbuild2/cc/link-rule.cxx
+++ b/libbuild2/cc/link-rule.cxx
@@ -2127,9 +2127,12 @@ namespace build2
                 }
                 catch (const io_error& e)
                 {
-                  if (pr.wait ()) // Ignore if child failed.
+                  if (run_wait (args, pr))
                     fail << "unable to pipe resource file to " << args[0]
                          << ": " << e;
+
+                  // If the child process has failed then assume the io error
+                  // was caused by that and let run_finish() deal with it.
                 }
 
                 run_finish (args, pr);
diff --git a/libbuild2/cc/msvc.cxx b/libbuild2/cc/msvc.cxx
index c7022fc..1bdd2bb 100644
--- a/libbuild2/cc/msvc.cxx
+++ b/libbuild2/cc/msvc.cxx
@@ -356,7 +356,7 @@ namespace build2
         // that.
       }
 
-      if (!run_finish (args, pr, false, s))
+      if (!run_finish_code (args, pr, s))
         return otype::e;
 
       if (obj && dll)
diff --git a/libbuild2/functions-process.cxx b/libbuild2/functions-process.cxx
index 2cc3385..63bdf61 100644
--- a/libbuild2/functions-process.cxx
+++ b/libbuild2/functions-process.cxx
@@ -65,10 +65,10 @@ namespace build2
   }
 
   static process
-  start (const scope*,
-         const process_path& pp,
-         const strings& args,
-         cstrings& cargs)
+  run_start (const scope*,
+             const process_path& pp,
+             const strings& args,
+             cstrings& cargs)
   {
     cargs.reserve (args.size () + 2);
     cargs.push_back (pp.recall_string ());
@@ -85,23 +85,13 @@ namespace build2
                       -1              /* stdout */);
   }
 
-  static void
-  finish (cstrings& args, process& pr, bool io)
-  {
-    run_finish (args, pr);
-
-    if (io)
-      fail << "error reading " << args[0] << " output";
-  }
-
   static value
   run (const scope* s, const process_path& pp, const strings& args)
   {
     cstrings cargs;
-    process pr (start (s, pp, args, cargs));
+    process pr (run_start (s, pp, args, cargs));
 
     string v;
-    bool io (false);
     try
     {
       ifdstream is (move (pr.in_ofd));
@@ -113,15 +103,16 @@ namespace build2
 
       is.close (); // Detect errors.
     }
-    catch (const io_error&)
+    catch (const io_error& e)
     {
-      // Presumably the child process failed and issued diagnostics so let
-      // finish() try to deal with that first.
-      //
-      io = true;
+      if (run_wait (cargs, pr))
+        fail << "io error reading " << cargs[0] << " output: " << e;
+
+      // If the child process has failed then assume the io error was
+      // caused by that and let run_finish() deal with it.
     }
 
-    finish (cargs, pr, io);
+    run_finish (cargs, pr);
 
     names r;
     r.push_back (to_name (move (trim (v))));
@@ -141,10 +132,9 @@ namespace build2
     regex re (parse_regex (pat, regex::ECMAScript));
 
     cstrings cargs;
-    process pr (start (s, pp, args, cargs));
+    process pr (run_start (s, pp, args, cargs));
 
     names r;
-    bool io (false);
     try
     {
       ifdstream is (move (pr.in_ofd), ifdstream::badbit);
@@ -167,15 +157,16 @@ namespace build2
 
       is.close (); // Detect errors.
     }
-    catch (const io_error&)
+    catch (const io_error& e)
     {
-      // Presumably the child process failed and issued diagnostics so let
-      // finish() try to deal with that first.
-      //
-      io = true;
+      if (run_wait (cargs, pr))
+        fail << "io error reading " << cargs[0] << " output: " << e;
+
+      // If the child process has failed then assume the io error was
+      // caused by that and let run_finish() deal with it.
     }
 
-    finish (cargs, pr, io);
+    run_finish (cargs, pr);
 
     return value (move (r));
   }
diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx
index f79f80e..59eeafd 100644
--- a/libbuild2/parser.cxx
+++ b/libbuild2/parser.cxx
@@ -1597,7 +1597,6 @@ namespace build2
                            dir_path ()  /* cwd    */,
                            nullptr      /* env    */,
                            l));
-    bool bad (false);
     try
     {
       // While a failing process could write garbage to stdout, for simplicity
@@ -1627,19 +1626,17 @@ namespace build2
 
       is.close (); // Detect errors.
     }
-    catch (const io_error&)
+    catch (const io_error& e)
     {
-      // Presumably the child process failed and issued diagnostics so let
-      // run_finish() try to deal with that first.
-      //
-      bad = true;
+      if (run_wait (cargs, pr, l))
+        fail (l) << "io error reading " << cargs[0] << " output: " << e;
+
+      // If the child process has failed then assume the io error was
+      // caused by that and let run_finish() deal with it.
     }
 
     run_finish (cargs, pr, l);
 
-    if (bad)
-      fail (l) << "error reading " << args[0] << " output";
-
     next_after_newline (t, tt);
   }
 
diff --git a/libbuild2/utility.cxx b/libbuild2/utility.cxx
index 20f0c22..0c44a59 100644
--- a/libbuild2/utility.cxx
+++ b/libbuild2/utility.cxx
@@ -252,11 +252,22 @@ namespace build2
   }
 
   bool
-  run_finish (const char* args[],
-              process& pr,
-              bool err,
-              const string& l,
-              const location& loc)
+  run_wait (const char* args[], process& pr, const location& loc)
+  try
+  {
+    return pr.wait ();
+  }
+  catch (const process_error& e)
+  {
+    fail (loc) << "unable to execute " << args[0] << ": " << e << endf;
+  }
+
+  bool
+  run_finish_impl (const char* args[],
+                   process& pr,
+                   bool err,
+                   const string& l,
+                   const location& loc)
   try
   {
     tracer trace ("run_finish");
@@ -297,6 +308,13 @@ namespace build2
     fail (loc) << "unable to execute " << args[0] << ": " << e << endf;
   }
 
+  void
+  run_io_error (const char* args[], const io_error& e)
+  {
+    fail << "io error reading " << args[0] << " output: " << e << endf;
+  }
+
+
   const string       empty_string;
   const path         empty_path;
   const dir_path     empty_dir_path;
diff --git a/libbuild2/utility.hxx b/libbuild2/utility.hxx
index 5c6e984..a6ed8e0 100644
--- a/libbuild2/utility.hxx
+++ b/libbuild2/utility.hxx
@@ -242,25 +242,39 @@ namespace build2
   [[noreturn]] LIBBUILD2_SYMEXPORT void
   run_search_fail (const path&, const location& = location ());
 
+  // Wait for process termination returning true if the process exited
+  // normally with a zero code and false otherwise. The latter case is
+  // normally followed up with a call to run_finish().
+  //
+  LIBBUILD2_SYMEXPORT bool
+  run_wait (const char* args[], process&, const location& = location ());
+
+  bool
+  run_wait (cstrings& args, process&, const location& = location ());
+
   // Wait for process termination. Issue diagnostics and throw failed in case
   // of abnormal termination. If the process has terminated normally but with
-  // a non-zero exit status, then, if error is true, assume the diagnostics
-  // has already been issued and throw failed as well. Otherwise (error is
-  // false), return false. The last argument is used in cooperation with
-  // run_start() in case STDERR is redirected to STDOUT.
+  // a non-zero exit status, then assume the diagnostics has already been
+  // issued and just throw failed. The last argument is used in cooperation
+  // with run_start() in case STDERR is redirected to STDOUT.
   //
-  LIBBUILD2_SYMEXPORT bool
+  void
   run_finish (const char* args[],
               process&,
-              bool error = true,
               const string& = string (),
               const location& = location ());
 
-  inline void
-  run_finish (cstrings& args, process& pr, const location& l = location ())
-  {
-    run_finish (args.data (), pr, true, string (), l);
-  }
+  void
+  run_finish (cstrings& args, process& pr, const location& l = location ());
+
+  // As above but if the process has exited normally with a non-zero code,
+  // then return false rather than throwing.
+  //
+  bool
+  run_finish_code (const char* args[],
+                   process&,
+                   const string& = string (),
+                   const location& = location ());
 
   // Start a process with the specified arguments. If in is -1, then redirect
   // STDIN to a pipe (can also be -2 to redirect to /dev/null or equivalent).
diff --git a/libbuild2/utility.ixx b/libbuild2/utility.ixx
index 514d4ee..4b841f7 100644
--- a/libbuild2/utility.ixx
+++ b/libbuild2/utility.ixx
@@ -6,6 +6,45 @@
 
 namespace build2
 {
+  inline bool
+  run_wait (cstrings& args, process& pr, const location& loc)
+  {
+    return run_wait (args.data (), pr, loc);
+  }
+
+  // Note: currently this function is also used in a run() implementations.
+  //
+  LIBBUILD2_SYMEXPORT bool
+  run_finish_impl (const char*[],
+                   process&,
+                   bool error,
+                   const string&,
+                   const location& = location ());
+
+  inline void
+  run_finish (const char* args[],
+              process& pr,
+              const string& l,
+              const location& loc)
+  {
+    run_finish_impl (args, pr, true /* error */, l, loc);
+  }
+
+  inline void
+  run_finish (cstrings& args, process& pr, const location& loc)
+  {
+    run_finish (args.data (), pr, string (), loc);
+  }
+
+  inline bool
+  run_finish_code (const char* args[],
+                   process& pr,
+                   const string& l,
+                   const location& loc)
+  {
+    return run_finish_impl (args, pr, false /* error */, l, loc);
+  }
+
   inline void
   hash_path (sha256& cs, const path& p, const dir_path& prefix)
   {
diff --git a/libbuild2/utility.txx b/libbuild2/utility.txx
index c183930..aadb181 100644
--- a/libbuild2/utility.txx
+++ b/libbuild2/utility.txx
@@ -56,6 +56,9 @@ namespace build2
     return p;
   }
 
+  [[noreturn]] LIBBUILD2_SYMEXPORT void
+  run_io_error (const char*[], const io_error&);
+
   template <typename T, typename F>
   T
   run (uint16_t verbosity,
@@ -102,12 +105,16 @@ namespace build2
 
       is.close ();
     }
-    catch (const io_error&)
+    catch (const io_error& e)
     {
-      // Presumably the child process failed. Let run_finish() deal with that.
+      if (run_wait (args, pr))
+        run_io_error (args, e);
+
+      // If the child process has failed then assume the io error was
+      // caused by that and let run_finish() deal with it.
     }
 
-    if (!(run_finish (args, pr, err, l) || ignore_exit))
+    if (!(run_finish_impl (args, pr, err, l) || ignore_exit))
       r = T ();
 
     return r;
diff --git a/libbuild2/version/snapshot-git.cxx b/libbuild2/version/snapshot-git.cxx
index 7dcb1f3..30dea6d 100644
--- a/libbuild2/version/snapshot-git.cxx
+++ b/libbuild2/version/snapshot-git.cxx
@@ -187,7 +187,7 @@ namespace build2
         // that.
       }
 
-      if (!run_finish (args, pr, false /* error */, l))
+      if (!run_finish_code (args, pr, l))
       {
         // Presumably new repository without HEAD. Return uncommitted snapshot
         // with UNIX epoch as timestamp.