Make process exit diagnostics in executable metadata extraction consistent

4 files changed, 35 insertions, 15 deletions

diff --git a/libbuild2/file.cxx b/libbuild2/file.cxx
index a3039a2..8666520 100644
--- a/libbuild2/file.cxx
+++ b/libbuild2/file.cxx
@@ -1862,13 +1862,13 @@ namespace build2
     try
     {
       // Note: not using run_*() functions since need to be able to suppress
-      // all errors, including inability to exec.
+      // all errors, including abnormal, inability to exec, etc., in case of
+      // optional import. Also, no need to buffer diagnostics since in the
+      // serial load.
       //
       if (verb >= 3)
         print_process (args);
 
-      // @@ DBUF: diag
-
       process pr (pp,
                   args,
                   -2           /* stdin  to /dev/null                 */,
@@ -1924,10 +1924,19 @@ namespace build2
             return r;
 
           if (!opt)
-            error (loc) << "invalid metadata signature in " << args[0]
-                        << " output" <<
+          {
+            diag_record dr;
+            dr << error (loc) << "invalid metadata signature in " << args[0]
+               << " output" <<
               info << "expected '" << s << "'";
 
+            if (verb >= 1 && verb <= 2)
+            {
+              dr << info << "command line: ";
+              print_process (dr, args);
+            }
+          }
+
           goto fail;
         }
 
@@ -1943,16 +1952,27 @@ namespace build2
       if (pr.wait ())
       {
         if (!opt)
-          error (loc) << "unable to read metadata from " << args[0];
+          error (loc) << "io error reading metadata from " << args[0];
       }
       else
       {
         // The child process presumably issued diagnostics but if it didn't,
-        // the result will be very confusing. So let's issue something
-        // generic for good measure.
+        // the result will be very confusing. So let's issue something generic
+        // for good measure. But also make it consistent with diagnostics
+        // issued by run_finish().
         //
         if (!opt)
-          error (loc) << "unable to extract metadata from " << args[0];
+        {
+          diag_record dr;
+          dr << error (loc) << "unable to extract metadata from " << args[0] <<
+            info << "process " << args[0] << " " << *pr.exit;
+
+          if (verb >= 1 && verb <= 2)
+          {
+            dr << info << "command line: ";
+            print_process (dr, args);
+          }
+        }
       }
 
       goto fail;
@@ -1968,8 +1988,7 @@ namespace build2
       goto fail;
     }
 
-    fail:
-
+  fail:
     if (opt)
     {
       metadata_cache.insert (pp.effect_string (), true);
diff --git a/libbuild2/functions-process.cxx b/libbuild2/functions-process.cxx
index be10a26..948f581 100644
--- a/libbuild2/functions-process.cxx
+++ b/libbuild2/functions-process.cxx
@@ -177,7 +177,8 @@ namespace build2
       diag_record dr;
       dr << fail << "builtin " << bn << " " << process_exit (rs);
 
-      // @@ TMP TODO: this and need to print command line at verbosite >= 3
+      // @@ DBUF
+      // @@ TMP: this and need to print command line at verbosite >= 3
       //    line for the process case.
       //
 #if 0
diff --git a/libbuild2/test/rule.cxx b/libbuild2/test/rule.cxx
index 4e97968..7c27b63 100644
--- a/libbuild2/test/rule.cxx
+++ b/libbuild2/test/rule.cxx
@@ -678,7 +678,7 @@ namespace build2
       //
       optional<process_exit> pe;
 
-      // @@ DBUF
+      // @@ DBUF: buffering and run_finish_impl()-like non-0 exit diagnostics.
 
       try
       {
diff --git a/libbuild2/utility.cxx b/libbuild2/utility.cxx
index 75791ee..5659771 100644
--- a/libbuild2/utility.cxx
+++ b/libbuild2/utility.cxx
@@ -314,8 +314,8 @@ namespace build2
     {
       // 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().
+      // exit). Let's print this always and see how it wears. Note that we now
+      // rely on this in, for example, process_finish(), extract_metadata().
       //
       // Note: make sure keep the above trace if decide not to print.
       //