From 3bc0fc4c4496c345c79734dcd6dc56d44119aebf Mon Sep 17 00:00:00 2001
From: Boris Kolpackov <boris@codesynthesis.com>
Date: Tue, 8 Nov 2022 10:34:22 +0200
Subject: Make process exit diagnostics consistent

In particular, we now always print error message on non-0 exit except
in cases where such exit is ignored.
---
 build2/cli/rule.cxx                |   2 +-
 libbuild2/bin/def-rule.cxx         |   2 +-
 libbuild2/cc/compile-rule.cxx      |   8 +-
 libbuild2/cc/gcc.cxx               |   2 +-
 libbuild2/cc/guess.cxx             |   4 +-
 libbuild2/cc/link-rule.cxx         |   9 ++-
 libbuild2/cc/msvc.cxx              |   2 +-
 libbuild2/diagnostics.cxx          |  13 +---
 libbuild2/diagnostics.hxx          |  23 +++---
 libbuild2/dist/operation.cxx       |  18 +++--
 libbuild2/file.cxx                 |   2 +
 libbuild2/functions-process.cxx    |  26 ++++---
 libbuild2/install/rule.cxx         |  30 +++++--
 libbuild2/parser.cxx               |   2 +-
 libbuild2/test/rule.cxx            |   4 +-
 libbuild2/utility.cxx              | 103 +++++++++++++++---------
 libbuild2/utility.hxx              | 155 ++++++++++++++++++++++++-------------
 libbuild2/utility.ixx              | 129 ++++++++++++++++++++++++++----
 libbuild2/version/snapshot-git.cxx |   2 +-
 tests/directive/run.testscript     |   3 +-
 20 files changed, 370 insertions(+), 169 deletions(-)

diff --git a/build2/cli/rule.cxx b/build2/cli/rule.cxx
index 9e3ffa6..1f82699 100644
--- a/build2/cli/rule.cxx
+++ b/build2/cli/rule.cxx
@@ -329,7 +329,7 @@ namespace build2
 
       if (!ctx.dry_run)
       {
-        run (ctx, pp, args);
+        run (ctx, pp, args, 1 /* finish_verbosity */);
         dd.check_mtime (tp);
       }
 
diff --git a/libbuild2/bin/def-rule.cxx b/libbuild2/bin/def-rule.cxx
index d887be7..b4fbe0f 100644
--- a/libbuild2/bin/def-rule.cxx
+++ b/libbuild2/bin/def-rule.cxx
@@ -785,7 +785,7 @@ namespace build2
           io = true;
         }
 
-        if (!run_finish_code (dbuf, args, pr) || io)
+        if (!run_finish_code (dbuf, args, pr, 1 /* verbosity */) || io)
           fail << "unable to extract symbols from " << arg;
       }
 
diff --git a/libbuild2/cc/compile-rule.cxx b/libbuild2/cc/compile-rule.cxx
index e5ff914..2534058 100644
--- a/libbuild2/cc/compile-rule.cxx
+++ b/libbuild2/cc/compile-rule.cxx
@@ -4769,7 +4769,7 @@ namespace build2
                 throw failed ();
               }
               else
-                run_finish (args, pr);
+                run_finish (args, pr, 2 /* verbosity */);
             }
           }
           catch (const process_error& e)
@@ -5172,7 +5172,7 @@ namespace build2
             info << "then run failing command to display compiler diagnostics";
         }
         else
-          run_finish (args, pr); // Throws.
+          run_finish (args, pr, 2 /* verbosity */); // Throws.
       }
       catch (const process_error& e)
       {
@@ -7382,7 +7382,7 @@ namespace build2
             args.push_back (nullptr);
           }
 
-          run_finish (dbuf, args, pr);
+          run_finish (dbuf, args, pr, 1 /* verbosity */);
         }
         catch (const process_error& e)
         {
@@ -7439,7 +7439,7 @@ namespace build2
                         env.empty () ? nullptr : env.data ());
 
             dbuf.read ();
-            run_finish (dbuf, args, pr);
+            run_finish (dbuf, args, pr, 1 /* verbosity */);
           }
           catch (const process_error& e)
           {
diff --git a/libbuild2/cc/gcc.cxx b/libbuild2/cc/gcc.cxx
index 6b24516..0045c27 100644
--- a/libbuild2/cc/gcc.cxx
+++ b/libbuild2/cc/gcc.cxx
@@ -320,7 +320,7 @@ namespace build2
         // by that and let run_finish() deal with it.
       }
 
-      run_finish (args, pr);
+      run_finish (args, pr, 2 /* verbosity */);
 
       if (l.empty ())
         fail << "unable to extract " << x_lang << " compiler system library "
diff --git a/libbuild2/cc/guess.cxx b/libbuild2/cc/guess.cxx
index 5e8dbbc..a0d7ee4 100644
--- a/libbuild2/cc/guess.cxx
+++ b/libbuild2/cc/guess.cxx
@@ -222,7 +222,7 @@ namespace build2
         // that.
       }
 
-      if (!run_finish_code (args.data (), pr, l))
+      if (!run_finish_code (args.data (), pr, l, 2 /* verbosity */))
         r = "none";
 
       if (r.empty ())
@@ -2331,7 +2331,7 @@ namespace build2
         // that.
       }
 
-      if (!run_finish_code (args.data (), pr, l))
+      if (!run_finish_code (args.data (), pr, l, 2 /* verbosity */))
         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 af04ac7..c2ba8e7 100644
--- a/libbuild2/cc/link-rule.cxx
+++ b/libbuild2/cc/link-rule.cxx
@@ -3992,7 +3992,7 @@ namespace build2
       }
 #endif
 
-      if (verb > 2)
+      if (verb >= 3)
         print_process (args);
 
       // Remove the target file if any of the subsequent (after the linker)
@@ -4042,11 +4042,13 @@ namespace build2
 #ifdef _WIN32
             // Keep the options file if we have shown it.
             //
-            if (!e && verb > 2)
+            if (!e && verb >= 3)
               trm.cancel ();
 #endif
 
-            dbuf.close (oargs.empty () ? args : oargs, *pr.exit);
+            dbuf.close (oargs.empty () ? args : oargs,
+                        *pr.exit,
+                        1 /* verbosity */);
 
             if (!e)
               throw failed ();
@@ -4115,6 +4117,7 @@ namespace build2
           run (dbuf,
                rl,
                args,
+               1 /* finish_verbosity */,
                env_ptrs.empty () ? nullptr : env_ptrs.data ());
         }
       }
diff --git a/libbuild2/cc/msvc.cxx b/libbuild2/cc/msvc.cxx
index 3165602..8b9c05b 100644
--- a/libbuild2/cc/msvc.cxx
+++ b/libbuild2/cc/msvc.cxx
@@ -491,7 +491,7 @@ namespace build2
         io = true;
       }
 
-      if (!run_finish_code (args, pr, s) || io)
+      if (!run_finish_code (args, pr, s, 2 /* verbosity */) || io)
       {
         diag_record dr;
         dr << warn << "unable to detect " << l << " library type, ignoring" <<
diff --git a/libbuild2/diagnostics.cxx b/libbuild2/diagnostics.cxx
index 9433cd8..110641a 100644
--- a/libbuild2/diagnostics.cxx
+++ b/libbuild2/diagnostics.cxx
@@ -347,8 +347,8 @@ namespace build2
   close (const char* const* args,
          const process_exit& pe,
          uint16_t v,
-         const location& loc,
-         bool omit_normall)
+         bool omit_normal,
+         const location& loc)
   {
     tracer trace ("diag_buffer::close");
 
@@ -363,19 +363,12 @@ namespace build2
     {
       // Note: see similar code in run_finish_impl().
       //
-      if (omit_normall && pe.normal ())
+      if (omit_normal && pe.normal ())
       {
         l4 ([&]{trace << "process " << args[0] << " " << pe;});
       }
       else
       {
-        // It's unclear whether we should print this only if printing the
-        // command line (we could also do things differently for
-        // normal/abnormal exit). Let's print this always and see how it
-        // wears.
-        //
-        // Note: make sure keep the above trace is not printing.
-        //
         dr << error (loc) << "process " << args[0] << " " << pe;
 
         if (verb >= 1 && verb <= v)
diff --git a/libbuild2/diagnostics.hxx b/libbuild2/diagnostics.hxx
index 9d516be..2505e2d 100644
--- a/libbuild2/diagnostics.hxx
+++ b/libbuild2/diagnostics.hxx
@@ -623,9 +623,9 @@ namespace build2
     // If the child process exited abnormally or normally with non-0 code,
     // then print the error diagnostics to this effect. Additionally, if the
     // verbosity level is between 1 and the specified value, then print the
-    // command line as info after the error. If omit_normall is true, then
-    // don't print either for the normal exit (usually used when process
-    // failure can be tolerated).
+    // command line as info after the error. If omit_normal is true, then
+    // don't print either for the normal exit (usually used for custom
+    // diagnostics or when process failure can be tolerated).
     //
     // Normally the specified verbosity will be 1 and the command line args
     // represent the verbosity level 2 (logical) command line. Note that args
@@ -637,24 +637,25 @@ namespace build2
     //
     // Note: see also run_finish(diag_buffer&).
     //
-    // @@ TODO: need overload with process_env (see print_process).
+    // @@ TODO: need overload with process_env (see print_process). Also in
+    //          run_finish_impl().
     //
     void
     close (const cstrings& args,
            const process_exit& pe,
-           uint16_t verbosity = 1,
-           const location& loc = {},
-           bool omit_normall = false)
+           uint16_t verbosity,
+           bool omit_normal = false,
+           const location& loc = {})
     {
-      close (args.data (), pe, verbosity, loc, omit_normall);
+      close (args.data (), pe, verbosity, omit_normal, loc);
     }
 
     void
     close (const char* const* args,
            const process_exit& pe,
-           uint16_t verbosity = 1,
-           const location& loc = {},
-           bool omit_normall = false);
+           uint16_t verbosity,
+           bool omit_normal = false,
+           const location& loc = {});
 
     // As above but with a custom diag record for the child exit diagnostics,
     // if any. Note that if the diag record has the fail epilogue, then this
diff --git a/libbuild2/dist/operation.cxx b/libbuild2/dist/operation.cxx
index f6d00e4..078a8e2 100644
--- a/libbuild2/dist/operation.cxx
+++ b/libbuild2/dist/operation.cxx
@@ -741,7 +741,7 @@ namespace build2
       if (verb >= 2)
         print_process (args);
 
-      run (ctx, cmd, args);
+      run (ctx, cmd, args, 1 /* finish_verbosity */);
     }
 
     // install <file> <dir>[/<name>]
@@ -784,7 +784,7 @@ namespace build2
       if (verb >= 2)
         print_process (args);
 
-      run (t.ctx, cmd, args);
+      run (t.ctx, cmd, args, 1 /* finish_verbosity */);
 
       return d / (n.empty () ? relf.leaf () : n);
     }
@@ -957,10 +957,17 @@ namespace build2
                          out_fd.get ()     /* stdout */);
 
         cpr.in_ofd.reset (); // Close the archiver's stdout on our side.
-        run_finish (args.data () + i, cpr);
       }
 
-      run_finish (args.data (), apr);
+      // Delay throwing until we diagnose both ends of the pipe.
+      //
+      if (!run_finish_code (args.data (),
+                            apr,
+                            1     /* verbosity */,
+                            false /* omit_normal */) ||
+          !(i == 0 || run_finish_code (args.data () + i, cpr, 1, false)))
+        throw failed ();
+
 
       out_rm.cancel ();
       return ap;
@@ -1030,7 +1037,8 @@ namespace build2
                                args,
                                0             /* stdin  */,
                                c_fd.get ()   /* stdout */));
-        run_finish (args, pr);
+
+        run_finish (args, pr, 1 /* verbosity */);
       }
       else
       {
diff --git a/libbuild2/file.cxx b/libbuild2/file.cxx
index dd4623b..a3039a2 100644
--- a/libbuild2/file.cxx
+++ b/libbuild2/file.cxx
@@ -1867,6 +1867,8 @@ namespace build2
       if (verb >= 3)
         print_process (args);
 
+      // @@ DBUF: diag
+
       process pr (pp,
                   args,
                   -2           /* stdin  to /dev/null                 */,
diff --git a/libbuild2/functions-process.cxx b/libbuild2/functions-process.cxx
index 28b7a99..be10a26 100644
--- a/libbuild2/functions-process.cxx
+++ b/libbuild2/functions-process.cxx
@@ -174,7 +174,21 @@ namespace build2
       // While assuming that the builtin has issued the diagnostics on failure
       // we still print the error message (see process_finish() for details).
       //
-      fail << bn << " builtin " << process_exit (rs) << endf;
+      diag_record dr;
+      dr << fail << "builtin " << bn << " " << process_exit (rs);
+
+      // @@ TMP TODO: this and need to print command line at verbosite >= 3
+      //    line for the process case.
+      //
+#if 0
+      if (verb >= 1 && verb <= 2)
+      {
+        dr << info << "command line: ";
+        print_process (dr, args);
+      }
+#endif
+
+      dr << endf;
     }
     catch (const system_error& e)
     {
@@ -328,15 +342,7 @@ namespace build2
   void
   process_finish (const scope*, const cstrings& args, process& pr)
   {
-    try
-    {
-      if (!pr.wait ())
-        fail << "process " << args[0] << " " << *pr.exit;
-    }
-    catch (const process_error& e)
-    {
-      fail << "unable to execute " << args[0] << ": " << e;
-    }
+    run_finish (args, pr, 2 /* verbosity */);
   }
 
   // Run a process.
diff --git a/libbuild2/install/rule.cxx b/libbuild2/install/rule.cxx
index aa13169..8fd180c 100644
--- a/libbuild2/install/rule.cxx
+++ b/libbuild2/install/rule.cxx
@@ -246,7 +246,7 @@ namespace build2
       //
       // Remember that we are called twice: first during update for install
       // (pre-operation) and then during install. During the former, we rely
-      // on the normall update rule to resolve the group members. During the
+      // on the normal update rule to resolve the group members. During the
       // latter, there will be no rule to do this but the group will already
       // have been resolved by the pre-operation.
       //
@@ -848,7 +848,9 @@ namespace build2
           text << "install " << chd;
       }
 
-      run (ctx, pp, args);
+      run (ctx,
+           pp, args,
+           verb >= verbosity ? 1 : verb_never /* finish_verbosity */);
     }
 
     void file_rule::
@@ -903,7 +905,9 @@ namespace build2
       }
 
       if (!ctx.dry_run)
-        run (ctx, pp, args);
+        run (ctx,
+             pp, args,
+             verb >= verbosity ? 1 : verb_never /* finish_verbosity */);
     }
 
     void file_rule::
@@ -946,7 +950,9 @@ namespace build2
       }
 
       if (!ctx.dry_run)
-        run (ctx, pp, args);
+        run (ctx,
+             pp, args,
+             verb >= verbosity ? 1 : verb_never /* finish_verbosity */);
 #else
       // The -f part.
       //
@@ -1185,7 +1191,10 @@ namespace build2
                                  1                   /* stdout */,
                                  dbuf.open (args[0]) /* stderr */));
           dbuf.read ();
-          r = run_finish_code (dbuf, args, pr);
+          r = run_finish_code (
+            dbuf,
+            args, pr,
+            verb >= verbosity ? 1 : verb_never /* verbosity */);
         }
 
         if (!r)
@@ -1280,11 +1289,16 @@ namespace build2
 
         process_path pp (run_search (args[0]));
 
-        if (verb >= verbosity && verb >= 2)
-          print_process (args);
+        if (verb >= verbosity)
+        {
+          if (verb >= 2)
+            print_process (args);
+        }
 
         if (!ctx.dry_run)
-          run (ctx, pp, args);
+          run (ctx,
+               pp, args,
+               verb >= verbosity ? 1 : verb_never /* finish_verbosity */);
       }
 
       return true;
diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx
index 9b0c7a3..c761d09 100644
--- a/libbuild2/parser.cxx
+++ b/libbuild2/parser.cxx
@@ -3174,7 +3174,7 @@ namespace build2
       // caused by that and let run_finish() deal with it.
     }
 
-    run_finish (cargs, pr, l);
+    run_finish (cargs, pr, 2 /* verbosity */, false /* omit_normal */, l);
 
     next_after_newline (t, tt);
   }
diff --git a/libbuild2/test/rule.cxx b/libbuild2/test/rule.cxx
index 142bded..4e97968 100644
--- a/libbuild2/test/rule.cxx
+++ b/libbuild2/test/rule.cxx
@@ -70,7 +70,7 @@ namespace build2
       {
         // Remember that we are called twice: first during update for test
         // (pre-operation) and then during test. During the former, we rely on
-        // the normall update rule to resolve the group members. During the
+        // the normal update rule to resolve the group members. During the
         // latter, there will be no rule to do this but the group will already
         // have been resolved by the pre-operation.
         //
@@ -678,6 +678,8 @@ namespace build2
       //
       optional<process_exit> pe;
 
+      // @@ DBUF
+
       try
       {
         // Wait for a process to complete until the deadline is reached and
diff --git a/libbuild2/utility.cxx b/libbuild2/utility.cxx
index 556c4a3..d4d632c 100644
--- a/libbuild2/utility.cxx
+++ b/libbuild2/utility.cxx
@@ -240,7 +240,7 @@ namespace build2
   {
     if (e.child)
     {
-      // Note: run_finish() expects this exact message.
+      // Note: run_finish_impl() below expects this exact message.
       //
       cerr << "unable to execute " << args[0] << ": " << e << endl;
 
@@ -270,6 +270,8 @@ namespace build2
                    process& pr,
                    bool f,
                    const string& l,
+                   uint16_t v,
+                   bool omit_normal,
                    const location& loc)
   {
     tracer trace ("run_finish");
@@ -284,36 +286,55 @@ namespace build2
       fail (loc) << "unable to execute " << args[0] << ": " << e << endf;
     }
 
-    // Note: see similar code in diag_buffer::close/finish().
-    //
-    const process_exit& e (*pr.exit);
-
-    if (!e.normal ())
-      fail (loc) << "process " << args[0] << " " << e;
-
-    // Normall but non-zero exit status.
+    // Note: see similar code in diag_buffer::close().
     //
-    // Even if the user asked to suppress diagnostiscs, one error that we
-    // want to let through is the inability to execute the program itself.
-    // We cannot reserve a special exit status to signal this so we will
-    // just have to compare the output. This particular situation will
-    // result in a single error line printed by run_start() above.
-    //
-    if (l.compare (0, 18, "unable to execute ") == 0)
-      error (loc) << l;
+    const process_exit& pe (*pr.exit);
+    bool ne (pe.normal ());
 
-    if (f)
+    if (omit_normal && ne)
     {
       // While we assume diagnostics has already been issued (to stderr), if
       // that's not the case, it's a real pain to debug. So trace it. (And
       // if you think that doesn't happen in sensible programs, check GCC
       // bug #107448).
       //
-      l4 ([&]{trace << "process " << args[0] << " " << e;});
+      l4 ([&]{trace << "process " << args[0] << " " << pe;});
+    }
+    else
+    {
+      // Even if the user redirected the diagnostiscs, one error that we want
+      // to let through is the inability to execute the program itself. We
+      // cannot reserve a special exit status to signal this so we will just
+      // have to compare the output. This particular situation will result in
+      // a single error line printed by run_start() above.
+      //
+      // Shouldn't we treat this as abnormal termination? But if we didn't
+      // redirect stderr to stdout, then this will be handled as normal exit.
+      // So let's be consistent even if wrong.
+      //
+      if (ne && l.compare (0, 18, "unable to execute ") == 0)
+        error (loc) << l;
 
-      throw failed ();
+      // It's unclear whether we should print this only if printing the
+      // command line (we could also do things differently for normal/abnormal
+      // exit). Let's print this always and see how it wears. Note that we
+      // now rely on this in, for example, process_finish().
+      //
+      // Note: make sure keep the above trace if decide not to print.
+      //
+      diag_record dr;
+      dr << error (loc) << "process " << args[0] << " " << pe;
+
+      if (verb >= 1 && verb <= v)
+      {
+        dr << info << "command line: ";
+        print_process (dr, args);
+      }
     }
 
+    if (f || !ne)
+      throw failed ();
+
     return false;
   }
 
@@ -323,6 +344,7 @@ namespace build2
                    process& pr,
                    bool f,
                    uint16_t v,
+                   bool on,
                    const location& loc)
   {
     try
@@ -336,34 +358,40 @@ namespace build2
 
     const process_exit& pe (*pr.exit);
 
-    dbuf.close (args, pe, v, loc, !f /* omit_normall */);
+    dbuf.close (args, pe, v, on, loc);
 
     if (pe)
       return true;
 
-    if (f)
+    if (f || !pe.normal ())
       throw failed ();
 
     return false;
   }
 
   void
-  run (context& ctx, const process_env& pe, const char* const* args)
+  run (context& ctx,
+       const process_env& pe,
+       const char* const* args,
+       uint16_t v)
   {
     if (ctx.phase == run_phase::load)
     {
       process pr (run_start (pe, args));
-      run_finish (args, pr);
+      run_finish (args, pr, v);
     }
     else
     {
       diag_buffer dbuf (ctx);
-      run (dbuf, pe, args);
+      run (dbuf, pe, args, v);
     }
   }
 
   void
-  run (diag_buffer& dbuf, const process_env& pe, const char* const* args)
+  run (diag_buffer& dbuf,
+       const process_env& pe,
+       const char* const* args,
+       uint16_t v)
   {
     process pr (run_start (pe,
                            args,
@@ -371,7 +399,7 @@ namespace build2
                            1                   /* stdout */,
                            dbuf.open (args[0]) /* stderr */));
     dbuf.read ();
-    run_finish (dbuf, args, pr);
+    run_finish (dbuf, args, pr, v);
   }
 
   bool
@@ -379,6 +407,7 @@ namespace build2
        uint16_t verbosity,
        const process_env& pe,
        const char* const* args,
+       uint16_t finish_verbosity,
        const function<bool (string&, bool)>& f,
        bool tr,
        bool err,
@@ -388,7 +417,12 @@ namespace build2
     if (err && ctx.phase != run_phase::load)
     {
       diag_buffer dbuf (ctx);
-      return run (dbuf, verbosity, pe, args, f, tr, ignore_exit, checksum);
+      return run (dbuf,
+                  verbosity,
+                  pe, args,
+                  finish_verbosity,
+                  f,
+                  tr, ignore_exit, checksum);
     }
 
     process pr (run_start (verbosity,
@@ -438,7 +472,7 @@ namespace build2
       // caused by that and let run_finish() deal with it.
     }
 
-    if (!(run_finish_impl (args, pr, err, l) || ignore_exit))
+    if (!(run_finish_impl (args, pr, err, l, finish_verbosity) || ignore_exit))
       return false;
 
     return true;
@@ -449,6 +483,7 @@ namespace build2
        uint16_t verbosity,
        const process_env& pe,
        const char* const* args,
+       uint16_t finish_verbosity,
        const function<bool (string&, bool)>& f,
        bool tr,
        bool ignore_exit,
@@ -566,15 +601,7 @@ namespace build2
       // caused by that and let run_finish() deal with it.
     }
 
-    if (ignore_exit)
-    {
-      if (!run_finish_code (dbuf, args, pr, verbosity - 1))
-        return false;
-    }
-    else
-      run_finish (dbuf, args, pr, verbosity - 1);
-
-    return true;
+    return run_finish_impl (dbuf, args, pr, !ignore_exit, finish_verbosity);
   }
 
   fdpipe
diff --git a/libbuild2/utility.hxx b/libbuild2/utility.hxx
index 9f269bc..87d325b 100644
--- a/libbuild2/utility.hxx
+++ b/libbuild2/utility.hxx
@@ -228,7 +228,7 @@ namespace build2
 
   // Basic process utilities.
   //
-  // The run*() functions with process_path assume that you are printing
+  // The run*() functions with process_path/_env assume that you are printing
   // the process command line yourself.
 
   // Search for a process executable. Issue diagnostics and throw failed in
@@ -359,45 +359,82 @@ namespace build2
   bool
   run_wait (const 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 assume the diagnostics has already been
-  // issued and just throw failed. The line argument is used in cooperation
-  // with run_start() in case stderr is redirected to stdout (see the
+  // Wait for process termination, issues diagnostics, and throw failed.
+  //
+  // If the child process exited abnormally or normally with non-0 code, then
+  // print the error diagnostics to this effect. Additionally, if the
+  // verbosity level is between 1 and the specified value, then print the
+  // command line as info after the error. If omit_normal is true, then don't
+  // print either for the normal exit (usually used for custom diagnostics or
+  // when process failure can be tolerated).
+  //
+  // Normally the specified verbosity will be 1 and the command line args
+  // represent the verbosity level 2 (logical) command line. Or, to put it
+  // another way, it should be 1 less than what gets passed to run_start().
+  // Note that args should only represent a single command in a pipe (see
+  // print_process() for details).
+  //
+  // See also diag_buffer::close().
+  //
+  // The line argument is used in cooperation with run_start() to diagnose a
+  // failure to exec in case stderr is redirected to stdout (see the
   // implementation for details).
   //
   void
   run_finish (const char* const* args,
               process&,
-              const string& line = string (),
+              uint16_t verbosity,
+              bool omit_normal = false,
               const location& = location ());
 
   void
   run_finish (const cstrings& args,
               process&,
+              uint16_t verbosity,
+              bool omit_normal = false,
+              const location& = location ());
+
+  void
+  run_finish (const char* const* args,
+              process&,
+              const string& line,
+              uint16_t verbosity,
+              bool omit_normal = false,
               const location& = location ());
 
   // As above but if the process has exited normally with a non-zero code,
   // then return false rather than throwing.
   //
+  // Note that the normal non-0 exit diagnostics is omitted by default
+  // assuming appropriate custom diagnostics will be issued, if required.
+  //
   bool
   run_finish_code (const char* const* args,
                    process&,
-                   const string& = string (),
+                   uint16_t verbosity,
+                   bool omit_normal = true,
                    const location& = location ());
 
   bool
   run_finish_code (const cstrings& args,
                    process&,
+                   uint16_t verbosity,
+                   bool omit_normal = true,
+                   const location& = location ());
+
+  bool
+  run_finish_code (const char* const* args,
+                   process&,
+                   const string&,
+                   uint16_t verbosity,
+                   bool omit_normal = true,
                    const location& = location ());
 
   // As above but with diagnostics buffering.
   //
   // Specifically, this version first waits for the process termination, then
-  // calls diag_buffer::close(verbosity), and finally throws failed if the
-  // process didn't exit with 0 code. Note that what gets printed in case of
-  // normal termination with non-0 code is different compared to the above
-  // versions (see diag_buffer::close() for details).
+  // calls diag_buffer::close(verbosity, omit_normal), and finally throws
+  // failed if the process didn't exit with 0 code.
   //
   class diag_buffer;
 
@@ -405,33 +442,38 @@ namespace build2
   run_finish (diag_buffer&,
               const char* const* args,
               process&,
-              uint16_t verbosity = 1,
+              uint16_t verbosity,
+              bool omit_normal = false,
               const location& = location ());
 
   void
   run_finish (diag_buffer&,
               const cstrings& args,
               process&,
-              uint16_t verbosity = 1,
+              uint16_t verbosity,
+              bool omit_normal = false,
               const location& = location ());
 
   // As above but if the process has exited normally with a non-zero code,
-  // then return false rather than throwing. Note: diag_buffer::close() is
-  // called with omit_normall=true assuming appropriate custom diagnostics
-  // will be issued, if required.
+  // then return false rather than throwing.
+  //
+  // Note that the normal non-0 exit diagnostics is omitted by default
+  // assuming appropriate custom diagnostics will be issued, if required.
   //
   bool
   run_finish_code (diag_buffer&,
                    const char* const* args,
                    process&,
-                   uint16_t verbosity = 1,
+                   uint16_t verbosity,
+                   bool omit_normal = true,
                    const location& = location ());
 
   bool
   run_finish_code (diag_buffer&,
                    const cstrings& args,
                    process&,
-                   uint16_t verbosity = 1,
+                   uint16_t verbosity,
+                   bool omit_normal = true,
                    const location& = location ());
 
   // Run the process with the specified arguments by calling the above start
@@ -440,27 +482,31 @@ namespace build2
   LIBBUILD2_SYMEXPORT void
   run (context&,
        const process_env& pe, // Implicit-constructible from process_path.
-       const char* const* args);
+       const char* const* args,
+       uint16_t finish_verbosity);
 
   LIBBUILD2_SYMEXPORT void
   run (diag_buffer&,
        const process_env& pe,
-       const char* const* args);
+       const char* const* args,
+       uint16_t finish_verbosity);
 
   inline void
   run (context& ctx,
        const process_env& pe,
-       const cstrings& args)
+       const cstrings& args,
+       uint16_t finish_verbosity)
   {
-    run (ctx, pe, args.data ());
+    run (ctx, pe, args.data (), finish_verbosity);
   }
 
   inline void
   run (diag_buffer& dbuf,
        const process_env& pe,
-       const cstrings& args)
+       const cstrings& args,
+       uint16_t finish_verbosity)
   {
-    run (dbuf, pe, args.data ());
+    run (dbuf, pe, args.data (), finish_verbosity);
   }
 
   // As above but pass cwd/env vars as arguments rather than as part of
@@ -470,47 +516,52 @@ namespace build2
   run (context& ctx,
        const process_path& p,
        const char* const* args,
+       uint16_t finish_verbosity,
        const char* const* env,
        const dir_path& cwd = {})
   {
-    run (ctx, process_env (p, cwd, env), args);
+    run (ctx, process_env (p, cwd, env), args, finish_verbosity);
   }
 
   inline void
   run (diag_buffer& dbuf,
        const process_path& p,
        const char* const* args,
+       uint16_t finish_verbosity,
        const char* const* env,
        const dir_path& cwd = {})
   {
-    run (dbuf, process_env (p, cwd, env), args);
+    run (dbuf, process_env (p, cwd, env), args, finish_verbosity);
   }
 
   inline void
   run (context& ctx,
        const process_path& p,
        const cstrings& args,
+       uint16_t finish_verbosity,
        const char* const* env,
        const dir_path& cwd = {})
   {
-    run (ctx, p, args.data (), env, cwd);
+    run (ctx, p, args.data (), finish_verbosity, env, cwd);
   }
 
   inline void
   run (diag_buffer& dbuf,
        const process_path& p,
        const cstrings& args,
+       uint16_t finish_verbosity,
        const char* const* env,
        const dir_path& cwd = {})
   {
-    run (dbuf, p, args.data (), env, cwd);
+    run (dbuf, p, args.data (), finish_verbosity, env, cwd);
   }
 
   // Start the process as above and then call the specified function on each
   // trimmed line of the output until it returns a non-empty object T (tested
   // with T::empty()) which is then returned to the caller.
   //
-  // If verbosity is specified, print the process commands line at that level.
+  // If verbosity is specified, print the process commands line at that level
+  // (with the verbosite-1 value passed run_finish()).
   //
   // If error is false, then redirecting stderr to stdout (can be used to
   // suppress and/or analyze diagnostics from the child process). Otherwise,
@@ -522,9 +573,9 @@ namespace build2
   //
   // T (string& line, bool last)
   //
-  // If ignore_exit is true, then the program's exit status is ignored (if it
-  // is false and the program exits with the non-zero status, then an empty T
-  // instance is returned).
+  // If ignore_exit is true, then the program's normal exit status is ignored
+  // (if it is false and the program exits with the non-zero status, then an
+  // empty T instance is returned).
   //
   // If checksum is not NULL, then feed it the content of each trimmed line
   // (including those that come after the callback returns non-empty object).
@@ -587,26 +638,21 @@ namespace build2
 
   template <typename T, typename F>
   inline T
-  run (context& ctx,
-       const process_env& pe,
+  run (context&,
+       const process_env&,
        const char* const* args,
-       F&& f,
+       uint16_t finish_verbosity,
+       F&&,
        bool error = true,
        bool ignore_exit = false,
-       sha256* checksum = nullptr)
-  {
-    return run<T> (ctx,
-                   verb_never,
-                   pe, args,
-                   forward<F> (f),
-                   error, ignore_exit, checksum);
-  }
+       sha256* checksum = nullptr);
 
   template <typename T, typename F>
   inline T
   run (context& ctx,
        const process_env& pe,
        const cstrings& args,
+       uint16_t finish_verbosity,
        F&& f,
        bool error = true,
        bool ignore_exit = false,
@@ -614,37 +660,34 @@ namespace build2
   {
     return run<T> (ctx,
                    pe, args.data (),
+                   finish_verbosity,
                    forward<F> (f),
                    error, ignore_exit, checksum);
   }
 
   template <typename T, typename F>
   inline T
-  run (diag_buffer& dbuf,
-       const process_env& pe,
+  run (diag_buffer&,
+       const process_env&,
        const char* const* args,
-       F&& f,
+       uint16_t finish_verbosity,
+       F&&,
        bool ignore_exit = false,
-       sha256* checksum = nullptr)
-  {
-    return run<T> (dbuf,
-                   verb_never,
-                   pe, args,
-                   forward<F> (f),
-                   ignore_exit, checksum);
-  }
+       sha256* checksum = nullptr);
 
   template <typename T, typename F>
   inline T
   run (diag_buffer& dbuf,
        const process_env& pe,
        const cstrings& args,
+       uint16_t finish_verbosity,
        F&& f,
        bool ignore_exit = false,
        sha256* checksum = nullptr)
   {
     return run<T> (dbuf,
                    pe, args.data (),
+                   finish_verbosity,
                    forward<F> (f),
                    ignore_exit, checksum);
   }
@@ -816,6 +859,7 @@ namespace build2
        uint16_t verbosity,
        const process_env&,
        const char* const* args,
+       uint16_t finish_verbosity,
        const function<bool (string& line, bool last)>&,
        bool trim = true,
        bool err = true,
@@ -827,6 +871,7 @@ namespace build2
        uint16_t verbosity,
        const process_env&,
        const char* const* args,
+       uint16_t finish_verbosity,
        const function<bool (string& line, bool last)>&,
        bool trim = true,
        bool ignore_exit = false,
diff --git a/libbuild2/utility.ixx b/libbuild2/utility.ixx
index c122ec9..2447303 100644
--- a/libbuild2/utility.ixx
+++ b/libbuild2/utility.ixx
@@ -11,14 +11,16 @@ namespace build2
     return run_wait (args.data (), pr, loc);
   }
 
-  // Note: currently this function is also used in run() implementations.
+  // Note: these functions are also used in the run() implementations.
   //
   LIBBUILD2_SYMEXPORT bool
   run_finish_impl (const char* const*,
                    process&,
                    bool fail,
                    const string&,
-                   const location& = location ());
+                   uint16_t,
+                   bool = false,
+                   const location& = {});
 
   LIBBUILD2_SYMEXPORT bool
   run_finish_impl (diag_buffer&,
@@ -26,36 +28,69 @@ namespace build2
                    process&,
                    bool fail,
                    uint16_t,
-                   const location&);
+                   bool = false,
+                   const location& = {});
 
   inline void
   run_finish (const char* const* args,
               process& pr,
-              const string& l,
+              uint16_t v,
+              bool on,
+              const location& loc)
+  {
+    run_finish_impl (args, pr, true /* fail */, string (), v, on, loc);
+  }
+
+  inline void
+  run_finish (const cstrings& args,
+              process& pr,
+              uint16_t v,
+              bool on,
               const location& loc)
   {
-    run_finish_impl (args, pr, true /* fail */, l, loc);
+    run_finish (args.data (), pr, v, on, loc);
   }
 
   inline void
-  run_finish (const cstrings& args, process& pr, const location& loc)
+  run_finish (const char* const* args,
+              process& pr,
+              const string& l,
+              uint16_t v,
+              bool on,
+              const location& loc)
   {
-    run_finish (args.data (), pr, string (), loc);
+    run_finish_impl (args, pr, true, l, v, on, loc);
   }
 
   inline bool
   run_finish_code (const char* const* args,
                    process& pr,
-                   const string& l,
+                   uint16_t v,
+                   bool on,
+                   const location& loc)
+  {
+    return run_finish_impl (args, pr, false, string (), v, on, loc);
+  }
+
+  inline bool
+  run_finish_code (const cstrings& args,
+                   process& pr,
+                   uint16_t v,
+                   bool on,
                    const location& loc)
   {
-    return run_finish_impl (args, pr, false /* fail */, l, loc);
+    return run_finish_code (args.data (), pr, v, on, loc);
   }
 
   inline bool
-  run_finish_code (const cstrings& args, process& pr, const location& loc)
+  run_finish_code (const char* const* args,
+                   process& pr,
+                   const string& l,
+                   uint16_t v,
+                   bool on,
+                   const location& loc)
   {
-    return run_finish_code (args.data (), pr, string (), loc);
+    return run_finish_impl (args, pr, false, l, v, on, loc);
   }
 
   inline void
@@ -63,9 +98,10 @@ namespace build2
               const char* const* args,
               process& pr,
               uint16_t v,
+              bool on,
               const location& loc)
   {
-    run_finish_impl (dbuf, args, pr, true /* fail */, v, loc);
+    run_finish_impl (dbuf, args, pr, true /* fail */, v, on, loc);
   }
 
   inline void
@@ -73,9 +109,10 @@ namespace build2
               const cstrings& args,
               process& pr,
               uint16_t v,
+              bool on,
               const location& loc)
   {
-    run_finish_impl (dbuf, args.data (), pr, true /* fail */, v, loc);
+    run_finish_impl (dbuf, args.data (), pr, true, v, on, loc);
   }
 
   inline bool
@@ -83,9 +120,10 @@ namespace build2
                    const char* const* args,
                    process& pr,
                    uint16_t v,
+                   bool on,
                    const location& loc)
   {
-    return run_finish_impl (dbuf, args, pr, false /* fail */, v, loc);
+    return run_finish_impl (dbuf, args, pr, false, v, on, loc);
   }
 
   inline bool
@@ -93,9 +131,10 @@ namespace build2
                    const cstrings& args,
                    process& pr,
                    uint16_t v,
+                   bool on,
                    const location& loc)
   {
-    return run_finish_impl (dbuf, args.data (), pr, false /* fail */, v, loc);
+    return run_finish_impl (dbuf, args.data (), pr, false, v, on, loc);
   }
 
   template <typename T, typename F>
@@ -113,6 +152,7 @@ namespace build2
     if (!run (ctx,
               verbosity,
               pe, args,
+              verbosity - 1,
               [&r, &f] (string& l, bool last) // Small function optimmization.
               {
                 r = f (l, last);
@@ -141,6 +181,7 @@ namespace build2
     if (!run (dbuf,
               verbosity,
               pe, args,
+              verbosity - 1,
               [&r, &f] (string& l, bool last) // Small function optimmization.
               {
                 r = f (l, last);
@@ -154,6 +195,64 @@ namespace build2
     return r;
   }
 
+  template <typename T, typename F>
+  inline T
+  run (context& ctx,
+       const process_env& pe,
+       const char* const* args,
+       uint16_t finish_verbosity,
+       F&& f,
+       bool err,
+       bool ignore_exit,
+       sha256* checksum)
+  {
+    T r;
+    if (!run (ctx,
+              verb_never,
+              pe, args,
+              finish_verbosity,
+              [&r, &f] (string& l, bool last)
+              {
+                r = f (l, last);
+                return r.empty ();
+              },
+              true /* trim */,
+              err,
+              ignore_exit,
+              checksum))
+      r = T ();
+
+    return r;
+  }
+
+  template <typename T, typename F>
+  inline T
+  run (diag_buffer& dbuf,
+       const process_env& pe,
+       const char* const* args,
+       uint16_t finish_verbosity,
+       F&& f,
+       bool ignore_exit,
+       sha256* checksum)
+  {
+    T r;
+    if (!run (dbuf,
+              verb_never,
+              pe, args,
+              finish_verbosity,
+              [&r, &f] (string& l, bool last)
+              {
+                r = f (l, last);
+                return r.empty ();
+              },
+              true /* trim */,
+              ignore_exit,
+              checksum))
+      r = T ();
+
+    return r;
+  }
+
   inline void
   hash_path (sha256& cs, const path& p, const dir_path& prefix)
   {
diff --git a/libbuild2/version/snapshot-git.cxx b/libbuild2/version/snapshot-git.cxx
index c4168bf..ad12c9f 100644
--- a/libbuild2/version/snapshot-git.cxx
+++ b/libbuild2/version/snapshot-git.cxx
@@ -206,7 +206,7 @@ namespace build2
         // that.
       }
 
-      if (run_finish_code (args, pr, l))
+      if (run_finish_code (args, pr, l, 2 /* verbosity */))
       {
         if (r.sn == 0)
           fail << "unable to extract git commit id/date for " << rep_root;
diff --git a/tests/directive/run.testscript b/tests/directive/run.testscript
index 199dd5f..ecff0fe 100644
--- a/tests/directive/run.testscript
+++ b/tests/directive/run.testscript
@@ -25,8 +25,9 @@ EOI
 : bad-exit
 :
 cat <'assert false' >=buildfile;
-$* <"$run" 2>>EOE != 0
+$* <"$run" 2>>~/EOE/ != 0
 buildfile:1:1: error: assertion failed
+/<stdin>:1:5: error: process .+ exited with code 1/
 EOE
 
 : bad-output
-- 
cgit v1.1