More work on child process diagnostics buffering

7 files changed, 124 insertions, 136 deletions

diff --git a/libbuild2/cc/compile-rule.cxx b/libbuild2/cc/compile-rule.cxx
index 3a750a1..7221175 100644
--- a/libbuild2/cc/compile-rule.cxx
+++ b/libbuild2/cc/compile-rule.cxx
@@ -4608,7 +4608,7 @@ namespace build2
               continue;
             }
             else
-              run_finish (args, pr); // Throws.
+              run_finish (args, pr); // Throws. @@ DBUF
           }
           catch (const process_error& e)
           {
@@ -5008,7 +5008,7 @@ namespace build2
             info << "then run failing command to display compiler diagnostics";
         }
         else
-          run_finish (args, pr); // Throws.
+          run_finish (args, pr); // Throws. @@ DBUF
       }
       catch (const process_error& e)
       {
@@ -7218,7 +7218,7 @@ namespace build2
             args.push_back (nullptr);
           }
 
-          dbuf.finish (args, pr);
+          run_finish (dbuf, args, pr);
         }
         catch (const process_error& e)
         {
@@ -7275,8 +7275,7 @@ namespace build2
                         env.empty () ? nullptr : env.data ());
 
             dbuf.read ();
-
-            dbuf.finish (args, pr);
+            run_finish (dbuf, args, pr);
           }
           catch (const process_error& e)
           {
diff --git a/libbuild2/cc/gcc.cxx b/libbuild2/cc/gcc.cxx
index 7999c82..6b24516 100644
--- a/libbuild2/cc/gcc.cxx
+++ b/libbuild2/cc/gcc.cxx
@@ -132,113 +132,94 @@ namespace build2
       if (verb >= 3)
         print_process (env, args);
 
+      // Open pipe to stderr, redirect stdin and stdout to /dev/null.
+      //
+      process pr (run_start (
+                    env,
+                    args,
+                    -2,      /* stdin */
+                    -2,      /* stdout */
+                    {-1, -1} /* stderr */));
       try
       {
-        //@@ TODO: why don't we use run_start() here? Because it's unable to
-        //   open pipe for stderr and we need to change it first, for example,
-        //   making the err parameter a file descriptor rather than a flag.
-        //
+        ifdstream is (
+          move (pr.in_efd), fdstream_mode::skip, ifdstream::badbit);
 
-        // Open pipe to stderr, redirect stdin and stdout to /dev/null.
+        // Normally the system header paths appear between the following
+        // lines:
         //
-        process pr (xc,
-                    args.data (),
-                    -2,     /* stdin */
-                    -2,     /* stdout */
-                    -1,     /* stderr */
-                    nullptr /* cwd */,
-                    env.vars);
-
-        try
+        // #include <...> search starts here:
+        // End of search list.
+        //
+        // The exact text depends on the current locale. What we can rely on
+        // is the presence of the "#include <...>" substring in the "opening"
+        // line and the fact that the paths are indented with a single space
+        // character, unlike the "closing" line.
+        //
+        // Note that on Mac OS we will also see some framework paths among
+        // system header paths, followed with a comment. For example:
+        //
+        //  /Library/Frameworks (framework directory)
+        //
+        // For now we ignore framework paths and to filter them out we will
+        // only consider valid paths to existing directories, skipping those
+        // which we fail to normalize or stat. @@ Maybe this is a bit too
+        // loose, especially compared to gcc_library_search_dirs()?
+        //
+        string s;
+        for (bool found (false); getline (is, s); )
         {
-          ifdstream is (
-            move (pr.in_efd), fdstream_mode::skip, ifdstream::badbit);
-
-          // Normally the system header paths appear between the following
-          // lines:
-          //
-          // #include <...> search starts here:
-          // End of search list.
-          //
-          // The exact text depends on the current locale. What we can rely on
-          // is the presence of the "#include <...>" substring in the
-          // "opening" line and the fact that the paths are indented with a
-          // single space character, unlike the "closing" line.
-          //
-          // Note that on Mac OS we will also see some framework paths among
-          // system header paths, followed with a comment. For example:
-          //
-          //  /Library/Frameworks (framework directory)
-          //
-          // For now we ignore framework paths and to filter them out we will
-          // only consider valid paths to existing directories, skipping those
-          // which we fail to normalize or stat. @@ Maybe this is a bit too
-          // loose, especially compared to gcc_library_search_dirs()?
-          //
-          string s;
-          for (bool found (false); getline (is, s); )
+          if (!found)
+            found = s.find ("#include <...>") != string::npos;
+          else
           {
-            if (!found)
-              found = s.find ("#include <...>") != string::npos;
-            else
-            {
-              if (s[0] != ' ')
-                break;
+            if (s[0] != ' ')
+              break;
 
-              dir_path d;
-              try
-              {
-                string ds (s, 1, s.size () - 1);
+            dir_path d;
+            try
+            {
+              string ds (s, 1, s.size () - 1);
 
 #ifdef _WIN32
-                if (path_traits::is_separator (ds[0]))
-                  add_current_drive (ds);
+              if (path_traits::is_separator (ds[0]))
+                add_current_drive (ds);
 #endif
-                d = dir_path (move (ds));
+              d = dir_path (move (ds));
 
-                if (d.relative () || !exists (d, true))
-                  continue;
-
-                d.normalize ();
-              }
-              catch (const invalid_path&)
-              {
+              if (d.relative () || !exists (d, true))
                 continue;
-              }
 
-              if (find (r.begin (), r.end (), d) == r.end ())
-                r.emplace_back (move (d));
+              d.normalize ();
             }
+            catch (const invalid_path&)
+            {
+              continue;
+            }
+
+            if (find (r.begin (), r.end (), d) == r.end ())
+              r.emplace_back (move (d));
           }
+        }
 
-          is.close (); // Don't block.
+        is.close (); // Don't block.
 
-          if (!pr.wait ())
-          {
-            // We have read stderr so better print some diagnostics.
-            //
-            diag_record dr (fail);
+        if (!run_wait (args, pr))
+        {
+          // We have read stderr so better print some diagnostics.
+          //
+          diag_record dr (fail);
 
-            dr << "failed to extract " << x_lang << " header search paths" <<
-              info << "command line: ";
+          dr << "failed to extract " << x_lang << " header search paths" <<
+            info << "command line: ";
 
-            print_process (dr, args);
-          }
-        }
-        catch (const io_error&)
-        {
-          pr.wait ();
-          fail << "error reading " << x_lang << " compiler -v -E output";
+          print_process (dr, args);
         }
       }
-      catch (const process_error& e)
+      catch (const io_error&)
       {
-        error << "unable to execute " << args[0] << ": " << e;
-
-        if (e.child)
-          exit (1);
-
-        throw failed ();
+        run_wait (args, pr);
+        fail << "error reading " << x_lang << " compiler -v -E output";
       }
 
       // It's highly unlikely not to have any system directories. More likely
@@ -302,6 +283,9 @@ namespace build2
 
       // Open pipe to stdout.
       //
+      // Note: this function is called in the serial load phase and so no
+      // diagnostics buffering is needed.
+      //
       process pr (run_start (env,
                              args,
                              0, /* stdin */
diff --git a/libbuild2/cc/guess.cxx b/libbuild2/cc/guess.cxx
index e851ea8..5e8dbbc 100644
--- a/libbuild2/cc/guess.cxx
+++ b/libbuild2/cc/guess.cxx
@@ -181,12 +181,12 @@ namespace build2
       // could also be because there is something wrong with the compiler or
       // options but that we simply leave to blow up later).
       //
-      process pr (run_start (3     /* verbosity */,
+      process pr (run_start (3       /* verbosity */,
                              xp,
                              args,
-                             -1    /* stdin */,
-                             -1    /* stdout */,
-                             false /* error  */));
+                             -1      /* stdin */,
+                             -1      /* stdout */,
+                             {-1, 1} /* stderr (to stdout) */));
       string l, r;
       try
       {
@@ -817,7 +817,8 @@ namespace build2
     // Note: allowed to change pre if succeeds.
     //
     static guess_result
-    guess (const char* xm,
+    guess (context& ctx,
+           const char* xm,
            lang xl,
            const path& xc,
            const strings& x_mo,
@@ -1009,7 +1010,7 @@ namespace build2
 #endif
 
       string cache;
-      auto run = [&cs, &env, &args, &cache] (
+      auto run = [&ctx, &cs, &env, &args, &cache] (
         const char* o,
         auto&& f,
         bool checksum = false) -> guess_result
@@ -1017,9 +1018,10 @@ namespace build2
         args[args.size () - 2] = o;
         cache.clear ();
         return build2::run<guess_result> (
+          ctx,
           3                          /* verbosity */,
           env,
-          args.data (),
+          args,
           forward<decltype (f)> (f),
           false                      /* error */,
           false                      /* ignore_exit */,
@@ -1318,7 +1320,11 @@ namespace build2
         //
         const char* evars[] = {"CL=", "_CL_=", nullptr};
 
-        r = build2::run<guess_result> (3, process_env (xp, evars), f, false);
+        r = build2::run<guess_result> (ctx,
+                                       3,
+                                       process_env (xp, evars),
+                                       f,
+                                       false);
 
         if (r.empty ())
         {
@@ -1634,7 +1640,8 @@ namespace build2
                                      "LIB", "LINK", "_LINK_", nullptr};
 
     static compiler_info
-    guess_msvc (const char* xm,
+    guess_msvc (context&,
+                const char* xm,
                 lang xl,
                 const path& xc,
                 const string* xv,
@@ -1905,7 +1912,8 @@ namespace build2
       "SDKROOT", "MACOSX_DEPLOYMENT_TARGET", nullptr};
 
     static compiler_info
-    guess_gcc (const char* xm,
+    guess_gcc (context& ctx,
+               const char* xm,
                lang xl,
                const path& xc,
                const string* xv,
@@ -2018,7 +2026,7 @@ namespace build2
         //
         auto f = [] (string& l, bool) {return move (l);};
 
-        t = run<string> (3, xp, args.data (), f, false);
+        t = run<string> (ctx, 3, xp, args, f, false);
 
         if (t.empty ())
         {
@@ -2026,7 +2034,7 @@ namespace build2
                         << "falling back to -dumpmachine";});
 
           args[args.size () - 2] = "-dumpmachine";
-          t = run<string> (3, xp, args.data (), f, false);
+          t = run<string> (ctx, 3, xp, args, f, false);
         }
 
         if (t.empty ())
@@ -2169,9 +2177,9 @@ namespace build2
       process pr (run_start (3     /* verbosity */,
                              xp,
                              args,
-                             -2    /* stdin  (/dev/null) */,
-                             -1    /* stdout             */,
-                             false /* error  (2>&1)      */));
+                             -2      /* stdin  (to /dev/null) */,
+                             -1      /* stdout                */,
+                             {-1, 1} /* stderr (to stdout)    */));
 
       clang_msvc_info r;
 
@@ -2357,7 +2365,8 @@ namespace build2
       nullptr};
 
     static compiler_info
-    guess_clang (const char* xm,
+    guess_clang (context& ctx,
+                 const char* xm,
                  lang xl,
                  const path& xc,
                  const string* xv,
@@ -2604,7 +2613,7 @@ namespace build2
         // for LC_ALL.
         //
         auto f = [] (string& l, bool) {return move (l);};
-        t = run<string> (3, xp, args.data (), f, false);
+        t = run<string> (ctx, 3, xp, args, f, false);
 
         if (t.empty ())
           fail << "unable to extract target architecture from " << xc
@@ -2832,7 +2841,8 @@ namespace build2
     }
 
     static compiler_info
-    guess_icc (const char* xm,
+    guess_icc (context& ctx,
+               const char* xm,
                lang xl,
                const path& xc,
                const string* xv,
@@ -2896,7 +2906,7 @@ namespace build2
         //
         // @@ TODO: running without the mode options.
         //
-        s = run<string> (3, env, "-V", f, false);
+        s = run<string> (ctx, 3, env, "-V", f, false);
 
         if (s.empty ())
           fail << "unable to extract signature from " << xc << " -V output";
@@ -3022,7 +3032,7 @@ namespace build2
 
         // The -V output is sent to STDERR.
         //
-        t = run<string> (3, env, args.data (), f, false);
+        t = run<string> (ctx, 3, env, args, f, false);
 
         if (t.empty ())
           fail << "unable to extract target architecture from " << xc
@@ -3073,7 +3083,7 @@ namespace build2
         //
         {
           auto f = [] (string& l, bool) {return move (l);};
-          t = run<string> (3, xp, "-dumpmachine", f);
+          t = run<string> (ctx, 3, xp, "-dumpmachine", f);
         }
 
         if (t.empty ())
@@ -3154,7 +3164,8 @@ namespace build2
     static global_cache<compiler_info> cache;
 
     const compiler_info&
-    guess (const char* xm,
+    guess (context& ctx,
+           const char* xm,
            lang xl,
            const string& ec,
            const path& xc,
@@ -3228,7 +3239,7 @@ namespace build2
 
       if (pre.type != invalid_compiler_type)
       {
-        gr = guess (xm, xl, xc, x_mo, xi, pre, cs);
+        gr = guess (ctx, xm, xl, xc, x_mo, xi, pre, cs);
 
         if (gr.empty ())
         {
@@ -3244,13 +3255,14 @@ namespace build2
       }
 
       if (gr.empty ())
-        gr = guess (xm, xl, xc, x_mo, xi, pre, cs);
+        gr = guess (ctx, xm, xl, xc, x_mo, xi, pre, cs);
 
       if (gr.empty ())
         fail << "unable to guess " << xl << " compiler type of " << xc <<
           info << "use config." << xm << ".id to specify explicitly";
 
       compiler_info (*gf) (
+        context&,
         const char*, lang, const path&, const string*, const string*,
         const strings&,
         const strings*, const strings*,
@@ -3270,7 +3282,8 @@ namespace build2
       case compiler_type::icc: gf = &guess_icc;     break;
       }
 
-      compiler_info r (gf (xm, xl, xc, xv, xt,
+      compiler_info r (gf (ctx,
+                           xm, xl, xc, xv, xt,
                            x_mo, c_po, x_po, c_co, x_co, c_lo, x_lo,
                            move (gr), cs));
 
diff --git a/libbuild2/cc/guess.hxx b/libbuild2/cc/guess.hxx
index 53acc15..7cbbd87 100644
--- a/libbuild2/cc/guess.hxx
+++ b/libbuild2/cc/guess.hxx
@@ -253,7 +253,8 @@ namespace build2
     // that most of it will be the same, at least for C and C++.
     //
     const compiler_info&
-    guess (const char* xm,        // Module (for var names in diagnostics).
+    guess (context&,
+           const char* xm,        // Module (for var names in diagnostics).
            lang xl,               // Language.
            const string& ec,      // Environment checksum.
            const path& xc,        // Compiler path.
diff --git a/libbuild2/cc/link-rule.cxx b/libbuild2/cc/link-rule.cxx
index 5d212e6..ff03fd9 100644
--- a/libbuild2/cc/link-rule.cxx
+++ b/libbuild2/cc/link-rule.cxx
@@ -2970,7 +2970,7 @@ namespace build2
                 }
 
                 dbuf.read ();
-                dbuf.finish (args, pr, 2 /* verbosity */);
+                run_finish (dbuf, args, pr, 2 /* verbosity */);
               }
               catch (const process_error& e)
               {
@@ -4116,23 +4116,10 @@ namespace build2
 
         if (!ctx.dry_run)
         {
-#if 0
-          run (rl,
+          run (dbuf,
+               rl,
                args,
-               dir_path () /* cwd */,
                env_ptrs.empty () ? nullptr : env_ptrs.data ());
-#else
-          process pr (rl,
-                      args,
-                      0                   /* stdin  */,
-                      1                   /* stdout */,
-                      dbuf.open (args[0]) /* stderr */,
-                      nullptr             /* cwd    */,
-                      env_ptrs.empty () ? nullptr : env_ptrs.data ());
-
-          dbuf.read ();
-          dbuf.finish (args, pr);
-#endif
         }
       }
 
diff --git a/libbuild2/cc/module.cxx b/libbuild2/cc/module.cxx
index 6583001..9063ebc 100644
--- a/libbuild2/cc/module.cxx
+++ b/libbuild2/cc/module.cxx
@@ -30,6 +30,8 @@ namespace build2
     {
       tracer trace (x, "guess_init");
 
+      context& ctx (rs.ctx);
+
       bool cc_loaded (cast_false<bool> (rs["cc.core.guess.loaded"]));
 
       // Adjust module priority (compiler). Also order cc module before us
@@ -157,6 +159,7 @@ namespace build2
         // we are now folding *.std options into mode options.
         //
         x_info = &build2::cc::guess (
+          ctx,
           x, x_lang,
           rs.root_extra->environment_checksum,
           move (xc),
@@ -183,7 +186,8 @@ namespace build2
 
         if (config_sub)
         {
-          ct = run<string> (3,
+          ct = run<string> (ctx,
+                            3,
                             *config_sub,
                             xi.target.c_str (),
                             [] (string& l, bool) {return move (l);});
diff --git a/libbuild2/cc/msvc.cxx b/libbuild2/cc/msvc.cxx
index 765b1c6..3165602 100644
--- a/libbuild2/cc/msvc.cxx
+++ b/libbuild2/cc/msvc.cxx
@@ -433,9 +433,9 @@ namespace build2
       //
       process pr (run_start (ld,
                              args,
-                             0     /* stdin */,
-                             -1    /* stdout */,
-                             false /* error */));
+                             0       /* stdin */,
+                             -1      /* stdout */,
+                             {-1, 1} /* stderr (to stdout) */));
 
       bool obj (false), dll (false);
       string s;