aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2022-11-23 08:57:45 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2022-11-23 08:57:45 +0200
commitc6b1d1dd870b3370d0a09fb4600e3a6b03326f35 (patch)
treef400b0d4a67bd1e067b545372e7742c5665f5caf
parentf19959de304afaff2b3d539c9bef1f493ede5fbd (diff)
Rework diag_buffer interface to facilitate correct destruction order
-rw-r--r--libbuild2/bin/def-rule.cxx30
-rw-r--r--libbuild2/cc/compile-rule.cxx45
-rw-r--r--libbuild2/cc/gcc.cxx6
-rw-r--r--libbuild2/cc/guess.cxx4
-rw-r--r--libbuild2/cc/link-rule.cxx24
-rw-r--r--libbuild2/cc/msvc.cxx6
-rw-r--r--libbuild2/diagnostics.cxx28
-rw-r--r--libbuild2/diagnostics.hxx127
-rw-r--r--libbuild2/diagnostics.ixx63
-rw-r--r--libbuild2/install/rule.cxx8
-rw-r--r--libbuild2/parser.cxx2
-rw-r--r--libbuild2/script/run.cxx42
-rw-r--r--libbuild2/test/rule.cxx34
-rw-r--r--libbuild2/utility.cxx292
-rw-r--r--libbuild2/utility.hxx153
-rw-r--r--libbuild2/utility.ixx48
-rw-r--r--libbuild2/version/snapshot-git.cxx2
17 files changed, 444 insertions, 470 deletions
diff --git a/libbuild2/bin/def-rule.cxx b/libbuild2/bin/def-rule.cxx
index 2879ca9..0998c89 100644
--- a/libbuild2/bin/def-rule.cxx
+++ b/libbuild2/bin/def-rule.cxx
@@ -727,8 +727,6 @@ namespace build2
if (verb == 1)
print_diag ("def", t);
- diag_buffer dbuf (ctx);
-
// Extract symbols from each object file.
//
symbols syms;
@@ -752,24 +750,24 @@ namespace build2
process pr (
run_start (nm,
args,
- 0 /* stdin */,
- -1 /* stdout */,
- dbuf.open (args[0],
- false /* force */,
- fdstream_mode::non_blocking |
- fdstream_mode::skip) /* stderr */));
+ 0 /* stdin */,
+ -1 /* stdout */,
+ diag_buffer::pipe (ctx) /* stderr */));
+
+ // Note that while we read both streams until eof in the normal
+ // circumstances, we cannot use fdstream_mode::skip for the exception
+ // case on both of them: we may end up being blocked trying to read
+ // one stream while the process may be blocked writing to the other.
+ // So in case of an exception we only skip the diagnostics and close
+ // stdout hard. The latter should happen first so the order of the
+ // dbuf/is variables is important.
+ //
+ diag_buffer dbuf (ctx, args[0], pr, (fdstream_mode::non_blocking |
+ fdstream_mode::skip));
bool io (false);
try
{
- // Note that while we read both streams until eof in the normal
- // circumstances, we cannot use fdstream_mode::skip for the
- // exception case on both of them: we may end up being blocked
- // trying to read one stream while the process may be blocked
- // writing to the other. So in case of an exception we only skip the
- // diagnostics and close stdout hard. The latter should happen first
- // so the order of the dbuf/is variables is important.
- //
ifdstream is (move (pr.in_ofd),
fdstream_mode::non_blocking,
ifdstream::badbit);
diff --git a/libbuild2/cc/compile-rule.cxx b/libbuild2/cc/compile-rule.cxx
index 6ca1f0a..874674d 100644
--- a/libbuild2/cc/compile-rule.cxx
+++ b/libbuild2/cc/compile-rule.cxx
@@ -4009,13 +4009,14 @@ namespace build2
args,
-1,
-1,
- dbuf.open (args[0],
- false /* force */,
- fdstream_mode::non_blocking |
- fdstream_mode::skip),
+ diag_buffer::pipe (ctx),
nullptr, // CWD
env.empty () ? nullptr : env.data ());
+ dbuf.open (args[0],
+ move (pr.in_efd),
+ fdstream_mode::non_blocking |
+ fdstream_mode::skip);
try
{
gcc_module_mapper_state mm_state (skip_count, imports);
@@ -4126,8 +4127,6 @@ namespace build2
// diagnostics (if things go badly we will restart with this
// support).
//
- using pipe = process::pipe;
-
if (drmp == nullptr) // Dependency info goes to stdout.
{
assert (!sense_diag); // Note: could support if necessary.
@@ -4135,39 +4134,42 @@ namespace build2
// For VC with /P the dependency info and diagnostics all go
// to stderr so redirect it to stdout.
//
- pipe err (
- cclass == compiler_class::msvc ? pipe {-1, 1} : // stdout
- !gen ? pipe {-1, -2} : // null
- dbuf.open (args[0],
- sense_diag /* force */,
- fdstream_mode::non_blocking));
+ int err (
+ cclass == compiler_class::msvc ? 1 : // stdout
+ !gen ? -2 : // /dev/null
+ diag_buffer::pipe (ctx, sense_diag /* force */));
pr = process (
cpath,
args,
0,
-1,
- move (err),
+ err,
nullptr, // CWD
env.empty () ? nullptr : env.data ());
+
+ dbuf.open (args[0],
+ move (pr.in_efd),
+ fdstream_mode::non_blocking); // Skip on stdout.
}
else // Dependency info goes to temporary file.
{
// Since we only need to read from one stream (dbuf) let's
// use the simpler blocking setup.
//
- pipe err (
- !gen && !sense_diag ? pipe {-1, -2} : // null
- dbuf.open (args[0], sense_diag /* force */));
+ int err (
+ !gen && !sense_diag ? -2 : // /dev/null
+ diag_buffer::pipe (ctx, sense_diag /* force */));
pr = process (cpath,
args,
0,
2, // Send stdout to stderr.
- move (err),
+ err,
nullptr, // CWD
env.empty () ? nullptr : env.data ());
+ dbuf.open (args[0], move (pr.in_efd));
dbuf.read (sense_diag /* force */);
if (sense_diag)
@@ -7343,8 +7345,6 @@ namespace build2
if (verb >= 3)
print_process (args);
- diag_buffer dbuf (ctx);
-
// @@ DRYRUN: Currently we discard the (partially) preprocessed file on
// dry-run which is a waste. Even if we keep the file around (like we do
// for the error case; see above), we currently have no support for
@@ -7374,10 +7374,12 @@ namespace build2
process pr (cpath,
args,
- 0, 2, dbuf.open (args[0], filter),
+ 0, 2, diag_buffer::pipe (ctx, filter /* force */),
nullptr, // CWD
env.empty () ? nullptr : env.data ());
+ diag_buffer dbuf (ctx, args[0], pr);
+
if (filter)
msvc_filter_cl (dbuf, *sp);
@@ -7444,10 +7446,11 @@ namespace build2
{
process pr (cpath,
args,
- 0, 2, dbuf.open (args[0]),
+ 0, 2, diag_buffer::pipe (ctx),
nullptr, // CWD
env.empty () ? nullptr : env.data ());
+ diag_buffer dbuf (ctx, args[0], pr);
dbuf.read ();
run_finish (dbuf, args, pr, 1 /* verbosity */);
}
diff --git a/libbuild2/cc/gcc.cxx b/libbuild2/cc/gcc.cxx
index 0045c27..755b0d8 100644
--- a/libbuild2/cc/gcc.cxx
+++ b/libbuild2/cc/gcc.cxx
@@ -137,9 +137,9 @@ namespace build2
process pr (run_start (
env,
args,
- -2, /* stdin */
- -2, /* stdout */
- {-1, -1} /* stderr */));
+ -2, /* stdin */
+ -2, /* stdout */
+ -1 /* stderr */));
try
{
ifdstream is (
diff --git a/libbuild2/cc/guess.cxx b/libbuild2/cc/guess.cxx
index a0d7ee4..7a2ede9 100644
--- a/libbuild2/cc/guess.cxx
+++ b/libbuild2/cc/guess.cxx
@@ -186,7 +186,7 @@ namespace build2
args,
-1 /* stdin */,
-1 /* stdout */,
- {-1, 1} /* stderr (to stdout) */));
+ 1 /* stderr (to stdout) */));
string l, r;
try
{
@@ -2179,7 +2179,7 @@ namespace build2
args,
-2 /* stdin (to /dev/null) */,
-1 /* stdout */,
- {-1, 1} /* stderr (to stdout) */));
+ 1 /* stderr (to stdout) */));
clang_msvc_info r;
diff --git a/libbuild2/cc/link-rule.cxx b/libbuild2/cc/link-rule.cxx
index 9bf86e6..20c6c62 100644
--- a/libbuild2/cc/link-rule.cxx
+++ b/libbuild2/cc/link-rule.cxx
@@ -2892,8 +2892,6 @@ namespace build2
env_ptrs.push_back (nullptr);
}
- diag_buffer dbuf (ctx);
-
// If targeting Windows, take care of the manifest.
//
path manifest; // Manifest itself (msvc) or compiled object file.
@@ -2949,12 +2947,14 @@ namespace build2
//
process pr (rc,
args,
- -1 /* stdin */,
- 1 /* stdout */,
- dbuf.open (args[0]) /* stderr */,
- nullptr /* cwd */,
+ -1 /* stdin */,
+ 1 /* stdout */,
+ diag_buffer::pipe (ctx) /* stderr */,
+ nullptr /* cwd */,
env_ptrs.empty () ? nullptr : env_ptrs.data ());
+ diag_buffer dbuf (ctx, args[0], pr);
+
try
{
ofdstream os (move (pr.out_fd));
@@ -4038,12 +4038,14 @@ namespace build2
process pr (*ld,
args,
- 0 /* stdin */,
- 2 /* stdout */,
- dbuf.open (args[0], filter) /* stderr */,
- nullptr /* cwd */,
+ 0 /* stdin */,
+ 2 /* stdout */,
+ diag_buffer::pipe (ctx, filter /* force */) /* stderr */,
+ nullptr /* cwd */,
env_ptrs.empty () ? nullptr : env_ptrs.data ());
+ diag_buffer dbuf (ctx, args[0], pr);
+
if (filter)
msvc_filter_link (dbuf, t, ot);
@@ -4127,7 +4129,7 @@ namespace build2
if (!ctx.dry_run)
{
- run (dbuf,
+ run (ctx,
rl,
args,
1 /* finish_verbosity */,
diff --git a/libbuild2/cc/msvc.cxx b/libbuild2/cc/msvc.cxx
index 8b9c05b..8fcbb0b 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 */,
- {-1, 1} /* stderr (to stdout) */));
+ 0 /* stdin */,
+ -1 /* stdout */,
+ 1 /* stderr (to stdout) */));
bool obj (false), dll (false);
string s;
diff --git a/libbuild2/diagnostics.cxx b/libbuild2/diagnostics.cxx
index 3246cae..bc74db3 100644
--- a/libbuild2/diagnostics.cxx
+++ b/libbuild2/diagnostics.cxx
@@ -498,41 +498,35 @@ namespace build2
// diag_buffer
//
- process::pipe diag_buffer::
- open (const char* args0, bool force, fdstream_mode m)
+
+ int diag_buffer::
+ pipe (context& ctx, bool force)
+ {
+ return (ctx.sched.serial () || ctx.no_diag_buffer) && !force ? 2 : -1;
+ }
+
+ void diag_buffer::
+ open (const char* args0, auto_fd&& fd, fdstream_mode m)
{
assert (state_ == state::closed && args0 != nullptr);
serial = ctx_.sched.serial ();
nobuf = !serial && ctx_.no_diag_buffer;
- process::pipe r;
- if (!(serial || nobuf) || force)
+ if (fd != nullfd)
{
try
{
- fdpipe p (fdopen_pipe ());
-
- // Note that we must return non-owning fd to our end of the pipe (see
- // the process class for details).
- //
- r = process::pipe (p.in.get (), move (p.out));
-
- m |= fdstream_mode::text;
-
- is.open (move (p.in), m);
+ is.open (move (fd), m | fdstream_mode::text);
}
catch (const io_error& e)
{
fail << "unable to read from " << args0 << " stderr: " << e;
}
}
- else
- r = process::pipe (-1, 2);
this->args0 = args0;
state_ = state::opened;
- return r;
}
void diag_buffer::
diff --git a/libbuild2/diagnostics.hxx b/libbuild2/diagnostics.hxx
index 9b9f6d8..397d3c0 100644
--- a/libbuild2/diagnostics.hxx
+++ b/libbuild2/diagnostics.hxx
@@ -730,29 +730,59 @@ namespace build2
//
// - Do nothing (in serial builds or if requested not to buffer).
//
- // In the future this class will also be responsible for converting the
+ // In the future this class may also be responsible for converting the
// diagnostics into the structured form (which means it may need to buffer
// even in serial builds).
//
+ // The typical usage is as follows:
+ //
+ // process pr (..., diag_buffer::pipe (ctx));
+ // diag_buffer dbuf (ctx, args[0], pr); // Skip.
+ // ifdstream is (move (pr.in_ofd)); // No skip.
+ // ofdstream os (move (pr.out_fd));
+ //
+ // The reason for this somewhat roundabout setup is to make sure the
+ // diag_buffer instance is destroyed before the process instance. This is
+ // important in case an exception is thrown where we want to make sure all
+ // our pipe ends are closed before we wait for the process exit (which
+ // happens in the process destructor).
+ //
+ // And speaking of the destruction order, another thing to keep in mind is
+ // that only one stream can use the skip mode (fdstream_mode::skip; because
+ // skipping is performed in the blocking mode) and the stream that skips
+ // should come first so that all other streams are destroyed/closed before
+ // it (failed that, we may end up in a deadlock). For example:
+ //
+ // process pr (..., diag_buffer::pipe (ctx));
+ // ifdstream is (move (pr.in_ofd), fdstream_mode::skip); // Skip.
+ // diag_buffer dbuf (ctx, args[0], pr, fdstream_mode::none); // No skip.
+ // ofdstream os (move (pr.out_fd));
+ //
class LIBBUILD2_SYMEXPORT diag_buffer
{
public:
- explicit
- diag_buffer (context& c): is (ifdstream::badbit), ctx_ (c) {}
+ // If buffering is necessary or force is true, return an "instruction"
+ // (-1) to the process class constructor to open a pipe and redirect
+ // stderr to it. Otherwise, return an "instruction" to inherit stderr (2).
+ //
+ // The force flag is normally used if custom diagnostics processing is
+ // required (filter, split, etc; see read() below).
+ //
+ // Note that the diagnostics buffer must be opened (see below) regardless
+ // of the pipe() result.
+ //
+ static int
+ pipe (context&, bool force = false);
- public:
- // If buffering is necessary or force is true, open a pipe and return the
- // child process end of it. Otherwise, return stderr. If mode is
- // non_blocking, then make reading from the parent end of the pipe
- // non-blocking.
+ // Open the diagnostics buffer given the parent end of the pipe (normally
+ // process:in_efd). If it is nullfd, then assume no buffering is
+ // necessary. If mode is non_blocking, then make reading from the parent
+ // end of the pipe non-blocking.
//
// The args0 argument is the child process program name for diagnostics.
// It is expected to remain valid until the call to close() and should
// normally be the same as args[0] passed to close().
//
- // The force flag is normally used if custom diagnostics processing is
- // required (filter, split, etc; see read() below).
- //
// Note that the same buffer can go through multiple open-read-close
// sequences, for example, to execute multiple commands.
//
@@ -764,10 +794,68 @@ namespace build2
// only the last stream to be destroyed can normally have the skip mode
// since in case of an exception, skipping will be blocking.
//
- process::pipe
+ diag_buffer (context&,
+ const char* args0,
+ auto_fd&&,
+ fdstream_mode = fdstream_mode::skip);
+
+ // As above, but the parrent end of the pipe (process:in_efd) is passed
+ // via a process instance.
+ //
+ diag_buffer (context&,
+ const char* args0,
+ process&,
+ fdstream_mode = fdstream_mode::skip);
+
+ // As above but with support for the underlying buffer reuse.
+ //
+ // Note that in most cases reusing the buffer is probably not worth the
+ // trouble because we normally don't expect any diagnostics in the common
+ // case. However, if needed, it can be arranged, for example:
+ //
+ // vector<char> buf;
+ //
+ // {
+ // process pr (...);
+ // diag_buffer dbuf (ctx, move (buf), args[0], pr);
+ // dbuf.read ();
+ // dbuf.close ();
+ // buf = move (dbuf.buf);
+ // }
+ //
+ // {
+ // ...
+ // }
+ //
+ // Note also that while there is no guarantee the underlying buffer is
+ // moved when, say, the vector is empty, all the main implementations
+ // always steal the buffer.
+ //
+ diag_buffer (context&,
+ vector<char>&& buf,
+ const char* args0,
+ auto_fd&&,
+ fdstream_mode = fdstream_mode::skip);
+
+ diag_buffer (context&,
+ vector<char>&& buf,
+ const char* args0,
+ process&,
+ fdstream_mode = fdstream_mode::skip);
+
+ // Separate construction and opening.
+ //
+ // Note: be careful with the destruction order (see above for details).
+ //
+ explicit
+ diag_buffer (context&);
+
+ diag_buffer (context&, vector<char>&& buf);
+
+ void
open (const char* args0,
- bool force = false,
- fdstream_mode mode = fdstream_mode::skip);
+ auto_fd&&,
+ fdstream_mode = fdstream_mode::skip);
// Open the buffer in the state as if after read() returned false, that
// is, the stream corresponding to the parent's end of the pipe reached
@@ -849,20 +937,17 @@ namespace build2
//
void
close (const cstrings& args,
- const process_exit& pe,
+ const process_exit&,
uint16_t verbosity,
bool omit_normal = false,
- const location& loc = {})
- {
- close (args.data (), pe, verbosity, omit_normal, loc);
- }
+ const location& = {});
void
close (const char* const* args,
- const process_exit& pe,
+ const process_exit&,
uint16_t verbosity,
bool omit_normal = false,
- const location& loc = {});
+ const location& = {});
// 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/diagnostics.ixx b/libbuild2/diagnostics.ixx
index a082290..273dfad 100644
--- a/libbuild2/diagnostics.ixx
+++ b/libbuild2/diagnostics.ixx
@@ -3,6 +3,8 @@
namespace build2
{
+ // print_diag()
+ //
LIBBUILD2_SYMEXPORT void
print_diag_impl (const char*, target_key*, target_key&&, const char*);
@@ -60,4 +62,65 @@ namespace build2
{
print_diag (p, l, path_name (&r), c);
}
+
+ // diag_buffer
+ //
+ inline diag_buffer::
+ diag_buffer (context& ctx)
+ : is (ifdstream::badbit), ctx_ (ctx)
+ {
+ }
+
+ inline diag_buffer::
+ diag_buffer (context& ctx, vector<char>&& b)
+ : is (ifdstream::badbit), buf (move (b)), ctx_ (ctx)
+ {
+ buf.clear ();
+ }
+
+ inline diag_buffer::
+ diag_buffer (context& ctx, const char* args0, auto_fd&& fd, fdstream_mode m)
+ : diag_buffer (ctx)
+ {
+ open (args0, move (fd), m);
+ }
+
+ inline diag_buffer::
+ diag_buffer (context& ctx, const char* args0, process& pr, fdstream_mode m)
+ : diag_buffer (ctx)
+ {
+ open (args0, move (pr.in_efd), m);
+ }
+
+ inline diag_buffer::
+ diag_buffer (context& ctx,
+ vector<char>&& b,
+ const char* args0,
+ auto_fd&& fd,
+ fdstream_mode m)
+ : diag_buffer (ctx, move (b))
+ {
+ open (args0, move (fd), m);
+ }
+
+ inline diag_buffer::
+ diag_buffer (context& ctx,
+ vector<char>&& b,
+ const char* args0,
+ process& pr,
+ fdstream_mode m)
+ : diag_buffer (ctx, move (b))
+ {
+ open (args0, move (pr.in_efd), m);
+ }
+
+ inline void diag_buffer::
+ close (const cstrings& args,
+ const process_exit& pe,
+ uint16_t verbosity,
+ bool omit_normal,
+ const location& loc)
+ {
+ close (args.data (), pe, verbosity, omit_normal, loc);
+ }
}
diff --git a/libbuild2/install/rule.cxx b/libbuild2/install/rule.cxx
index 36a06fa..8818ea3 100644
--- a/libbuild2/install/rule.cxx
+++ b/libbuild2/install/rule.cxx
@@ -1198,11 +1198,11 @@ namespace build2
print_diag ("uninstall -d", chd);
}
- diag_buffer dbuf (rs.ctx);
process pr (run_start (pp, args,
- 0 /* stdin */,
- 1 /* stdout */,
- dbuf.open (args[0]) /* stderr */));
+ 0 /* stdin */,
+ 1 /* stdout */,
+ diag_buffer::pipe (rs.ctx) /* stderr */));
+ diag_buffer dbuf (rs.ctx, args[0], pr);
dbuf.read ();
r = run_finish_code (
dbuf,
diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx
index c761d09..c8f66de 100644
--- a/libbuild2/parser.cxx
+++ b/libbuild2/parser.cxx
@@ -3133,7 +3133,7 @@ namespace build2
cargs,
0 /* stdin */,
-1 /* stdout */,
- {-1, 2} /* stderr */,
+ 2 /* stderr */,
nullptr /* env */,
dir_path () /* cwd */,
l));
diff --git a/libbuild2/script/run.cxx b/libbuild2/script/run.cxx
index cbd750f..550fdc1 100644
--- a/libbuild2/script/run.cxx
+++ b/libbuild2/script/run.cxx
@@ -2059,31 +2059,45 @@ namespace build2
{
case redirect_type::pass:
{
- if (dfd == 2) // stderr?
+ try
{
- // Deduce the args0 argument similar to cmd_path().
- //
- process::pipe ep (
+ if (dfd == 2) // stderr?
+ {
+ fdpipe p;
+ if (diag_buffer::pipe (env.context) == -1) // Are we buffering?
+ p = fdopen_pipe ();
+
+ // @@ TODO: perhaps this lambda should return process::pipe?
+ // (see the simple test rule implementatio for an
+ // example).
+ //
+ // Note that we must return non-owning fd to our end of the
+ // pipe (see the process class for details).
+ //
+ // process::pipe r (p.in.get (), move (p.out));
+
+ // Deduce the args0 argument similar to cmd_path().
+ //
+ // Note that we must open the diag buffer regardless of the
+ // diag_buffer::pipe() result.
+ //
pc.dbuf.open ((c.program.initial == nullptr
? c.program.recall.string ().c_str ()
: c.program.recall_string ()),
- false /* force */,
- fdstream_mode::non_blocking));
+ move (p.in),
+ fdstream_mode::non_blocking);
- if (ep.out != 2) // Are we buffering stderr?
- {
- ep.own_out = false;
- return auto_fd (ep.out);
+ if (p.out != nullfd)
+ return move (p.out);
+
+ // Fall through.
}
- }
- try
- {
return fddup (dfd);
}
catch (const io_error& e)
{
- fail (ll) << "unable to duplicate " << what << ": " << e;
+ fail (ll) << "unable to redirect " << what << ": " << e;
}
}
diff --git a/libbuild2/test/rule.cxx b/libbuild2/test/rule.cxx
index 5cdd8fb..5992414 100644
--- a/libbuild2/test/rule.cxx
+++ b/libbuild2/test/rule.cxx
@@ -958,13 +958,37 @@ namespace build2
process p;
{
- process::pipe ep (pp.dbuf.open (args[0],
- pp.force_dbuf,
- fdstream_mode::non_blocking));
+ process::pipe ep;
+ {
+ fdpipe p;
+ if (diag_buffer::pipe (t.ctx, pp.force_dbuf) == -1) // Buffering?
+ {
+ try
+ {
+ p = fdopen_pipe ();
+ }
+ catch (const io_error& e)
+ {
+ fail << "unable to redirect stderr: " << e;
+ }
+
+ // Note that we must return non-owning fd to our end of the pipe
+ // (see the process class for details).
+ //
+ ep = process::pipe (p.in.get (), move (p.out));
+ }
+ else
+ ep = process::pipe (-1, 2);
+
+ // Note that we must open the diag buffer regardless of the
+ // diag_buffer::pipe() result.
+ //
+ pp.dbuf.open (args[0], move (p.in), fdstream_mode::non_blocking);
+ }
p = (prev == nullptr
- ? process (args, 0, out, ep.out) // First process.
- : process (args, *prev->proc, out, ep.out)); // Next process.
+ ? process (args, 0, out, move (ep)) // First process.
+ : process (args, *prev->proc, out, move (ep))); // Next process.
}
pp.proc = &p;
diff --git a/libbuild2/utility.cxx b/libbuild2/utility.cxx
index 3ce0026..5a58287 100644
--- a/libbuild2/utility.cxx
+++ b/libbuild2/utility.cxx
@@ -218,7 +218,7 @@ namespace build2
const char* const* args,
int in,
int out,
- process::pipe err,
+ int err,
const location& l)
try
{
@@ -232,7 +232,7 @@ namespace build2
args,
in,
out,
- move (err),
+ err,
pe.cwd != nullptr ? pe.cwd->string ().c_str () : nullptr,
pe.vars);
}
@@ -379,26 +379,17 @@ namespace build2
}
else
{
- diag_buffer dbuf (ctx);
- run (dbuf, pe, args, v);
+ process pr (run_start (pe,
+ args,
+ 0 /* stdin */,
+ 1 /* stdout */,
+ diag_buffer::pipe (ctx) /* stderr */));
+ diag_buffer dbuf (ctx, args[0], pr);
+ dbuf.read ();
+ run_finish (dbuf, args, pr, v);
}
}
- void
- run (diag_buffer& dbuf,
- const process_env& pe,
- const char* const* args,
- uint16_t v)
- {
- process pr (run_start (pe,
- args,
- 0 /* stdin */,
- 1 /* stdout */,
- dbuf.open (args[0]) /* stderr */));
- dbuf.read ();
- run_finish (dbuf, args, pr, v);
- }
-
bool
run (context& ctx,
uint16_t verbosity,
@@ -413,101 +404,73 @@ namespace build2
{
assert (!err || !ignore_exit);
- if (err && ctx.phase != run_phase::load)
+ if (!err || ctx.phase == run_phase::load)
{
- diag_buffer dbuf (ctx);
- run (dbuf,
- verbosity,
- pe, args,
- finish_verbosity,
- f,
- tr,
- checksum);
- return true;
- }
+ process pr (run_start (verbosity,
+ pe,
+ args,
+ 0 /* stdin */,
+ -1 /* stdout */,
+ err ? 2 : 1 /* stderr */));
+
+ string l; // Last line of output.
+ try
+ {
+ ifdstream is (move (pr.in_ofd), fdstream_mode::skip);
- process pr (run_start (verbosity,
- pe,
- args,
- 0 /* stdin */,
- -1 /* stdout */,
- {-1, err ? 2 : 1}));
+ bool empty (true);
- string l; // Last line of output.
- try
- {
- ifdstream is (move (pr.in_ofd), fdstream_mode::skip);
+ // Make sure we keep the last line.
+ //
+ for (bool last (is.peek () == ifdstream::traits_type::eof ());
+ !last && getline (is, l); )
+ {
+ last = (is.peek () == ifdstream::traits_type::eof ());
- bool empty (true);
+ if (tr)
+ trim (l);
- // Make sure we keep the last line.
- //
- for (bool last (is.peek () == ifdstream::traits_type::eof ());
- !last && getline (is, l); )
- {
- last = (is.peek () == ifdstream::traits_type::eof ());
+ if (checksum != nullptr)
+ checksum->append (l);
- if (tr)
- trim (l);
+ if (empty)
+ {
+ empty = f (l, last);
- if (checksum != nullptr)
- checksum->append (l);
+ if (!empty && checksum == nullptr)
+ break;
+ }
+ }
- if (empty)
- {
- empty = f (l, last);
+ is.close ();
+ }
+ catch (const io_error& e)
+ {
+ if (run_wait (args, pr))
+ fail << "io error reading " << args[0] << " output: " << e << endf;
- if (!empty && checksum == nullptr)
- break;
- }
+ // If the child process has failed then assume the io error was
+ // caused by that and let run_finish() deal with it.
}
- is.close ();
+ // Omit normal exit code diagnostics if err is false.
+ //
+ if (!(run_finish_impl (args, pr, err, l, finish_verbosity, !err) ||
+ ignore_exit))
+ return false;
}
- catch (const io_error& e)
+ else
{
- if (run_wait (args, pr))
- fail << "io error reading " << args[0] << " output: " << e << endf;
-
- // If the child process has failed then assume the io error was
- // caused by that and let run_finish() deal with it.
- }
-
- // Omit normal exit code diagnostics if err is false.
- //
- if (!(run_finish_impl (args, pr, err, l, finish_verbosity, !err) ||
- ignore_exit))
- return false;
-
- return true;
- }
-
- void
- run (diag_buffer& dbuf,
- uint16_t verbosity,
- const process_env& pe,
- const char* const* args,
- uint16_t finish_verbosity,
- const function<bool (string&, bool)>& f,
- bool tr,
- sha256* checksum)
- {
- // We have to use the non-blocking setup since we have to read from stdout
- // and stderr simultaneously.
- //
- process pr (
- run_start (verbosity,
- pe,
- args,
- 0 /* stdin */,
- -1 /* stdout */,
- dbuf.open (args[0],
- false /* force */,
- fdstream_mode::non_blocking |
- fdstream_mode::skip) /* stderr */));
+ // We have to use the non-blocking setup since we have to read from stdout
+ // and stderr simultaneously.
+ //
+ process pr (run_start (verbosity,
+ pe,
+ args,
+ 0 /* stdin */,
+ -1 /* stdout */,
+ diag_buffer::pipe (ctx) /* stderr */));
- try
- {
// Note that while we read both streams until eof in the normal
// circumstances, we cannot use fdstream_mode::skip for the exception
// case on both of them: we may end up being blocked trying to read one
@@ -516,95 +479,102 @@ namespace build2
// hard. The latter should happen first so the order of the dbuf/is
// variables is important.
//
- ifdstream is (move (pr.in_ofd),
- fdstream_mode::non_blocking,
- ifdstream::badbit);
+ diag_buffer dbuf (ctx, args[0], pr, (fdstream_mode::non_blocking |
+ fdstream_mode::skip));
+ try
+ {
+ ifdstream is (move (pr.in_ofd),
+ fdstream_mode::non_blocking,
+ ifdstream::badbit);
- bool empty (true);
+ bool empty (true);
- // Read until we reach EOF on all streams.
- //
- // Note that if dbuf is not opened, then we automatically get an
- // inactive nullfd entry.
- //
- fdselect_set fds {is.fd (), dbuf.is.fd ()};
- fdselect_state& ist (fds[0]);
- fdselect_state& dst (fds[1]);
+ // Read until we reach EOF on all streams.
+ //
+ // Note that if dbuf is not opened, then we automatically get an
+ // inactive nullfd entry.
+ //
+ fdselect_set fds {is.fd (), dbuf.is.fd ()};
+ fdselect_state& ist (fds[0]);
+ fdselect_state& dst (fds[1]);
- // To detect the last line we are going keep the previous line and
- // only call the function once we've read the next.
- //
- optional<string> pl;
+ // To detect the last line we are going keep the previous line and
+ // only call the function once we've read the next.
+ //
+ optional<string> pl;
- for (string l; ist.fd != nullfd || dst.fd != nullfd; )
- {
- if (ist.fd != nullfd && getline_non_blocking (is, l))
+ for (string l; ist.fd != nullfd || dst.fd != nullfd; )
{
- if (eof (is))
+ if (ist.fd != nullfd && getline_non_blocking (is, l))
{
- if (pl && empty)
- f (*pl, true /* last */);
+ if (eof (is))
+ {
+ if (pl && empty)
+ f (*pl, true /* last */);
- ist.fd = nullfd;
- }
- else
- {
- if (checksum != nullptr || empty)
+ ist.fd = nullfd;
+ }
+ else
{
- if (tr)
- trim (l);
+ if (checksum != nullptr || empty)
+ {
+ if (tr)
+ trim (l);
- if (checksum != nullptr)
- checksum->append (l);
+ if (checksum != nullptr)
+ checksum->append (l);
- if (empty)
- {
- if (pl)
+ if (empty)
{
- if ((empty = f (*pl, false /* last */)))
+ if (pl)
+ {
+ if ((empty = f (*pl, false /* last */)))
swap (l, *pl);
- // Note that we cannot bail out like in the other version
- // since we don't have the skip mode on is. Plus, we might
- // still have the diagnostics.
+ // Note that we cannot bail out like in the other version
+ // since we don't have the skip mode on is. Plus, we might
+ // still have the diagnostics.
+ }
+ else
+ pl = move (l);
}
- else
- pl = move (l);
}
+
+ l.clear ();
}
- l.clear ();
+ continue;
}
- continue;
- }
+ ifdselect (fds);
- ifdselect (fds);
+ if (dst.ready)
+ {
+ if (!dbuf.read ())
+ dst.fd = nullfd;
+ }
+ }
- if (dst.ready)
+ is.close ();
+ }
+ catch (const io_error& e)
+ {
+ if (run_wait (args, pr))
{
- if (!dbuf.read ())
- dst.fd = nullfd;
+ // Note that we will drop the diagnostics in this case since reading
+ // it could have been the cause of this error.
+ //
+ fail << "io error reading " << args[0] << " output: " << e << endf;
}
- }
- is.close ();
- }
- catch (const io_error& e)
- {
- if (run_wait (args, pr))
- {
- // Note that we will drop the diagnostics in this case since reading
- // it could have been the cause of this error.
- //
- fail << "io error reading " << args[0] << " output: " << e << endf;
+ // If the child process has failed then assume the io error was caused
+ // by that and let run_finish() deal with it.
}
- // If the child process has failed then assume the io error was
- // caused by that and let run_finish() deal with it.
+ run_finish_impl (dbuf, args, pr, true /* fail */, finish_verbosity);
}
- run_finish_impl (dbuf, args, pr, true /* fail */, finish_verbosity);
+ return true;
}
cstrings
diff --git a/libbuild2/utility.hxx b/libbuild2/utility.hxx
index 5fffc8c..c12fae7 100644
--- a/libbuild2/utility.hxx
+++ b/libbuild2/utility.hxx
@@ -276,7 +276,7 @@ namespace build2
const char* const* args,
int in = 0,
int out = 1,
- process::pipe = {-1, 2},
+ int err = 2,
const location& = {});
inline process
@@ -285,10 +285,10 @@ namespace build2
const cstrings& args,
int in = 0,
int out = 1,
- process::pipe err = {-1, 2},
+ int err = 2,
const location& l = {})
{
- return run_start (verbosity, pe, args.data (), in, out, move (err), l);
+ return run_start (verbosity, pe, args.data (), in, out, err, l);
}
inline process
@@ -296,10 +296,10 @@ namespace build2
const char* const* args,
int in = 0,
int out = 1,
- process::pipe err = {-1, 2},
+ int err = 2,
const location& l = {})
{
- return run_start (verb_never, pe, args, in, out, move (err), l);
+ return run_start (verb_never, pe, args, in, out, err, l);
}
inline process
@@ -307,10 +307,10 @@ namespace build2
const cstrings& args,
int in = 0,
int out = 1,
- process::pipe err = {-1, 2},
+ int err = 2,
const location& l = {})
{
- return run_start (pe, args.data (), in, out, move (err), l);
+ return run_start (pe, args.data (), in, out, err, l);
}
// As above, but search for the process (including updating args[0]) and
@@ -321,7 +321,7 @@ namespace build2
const char* args[],
int in = 0,
int out = 1,
- process::pipe err = {-1, 2},
+ int err = 2,
const char* const* env = nullptr,
const dir_path& cwd = {},
const location& l = {})
@@ -329,7 +329,7 @@ namespace build2
process_path pp (run_search (args[0], l));
return run_start (verbosity,
process_env (pp, cwd, env), args,
- in, out, move (err),
+ in, out, err,
l);
}
@@ -338,15 +338,12 @@ namespace build2
cstrings& args,
int in = 0,
int out = 1,
- process::pipe err = {-1, 2},
+ int err = 2,
const char* const* env = nullptr,
const dir_path& cwd = {},
const location& l = {})
{
- return run_start (verbosity,
- args.data (),
- in, out, move (err),
- env, cwd, l);
+ return run_start (verbosity, args.data (), in, out, err, env, cwd, l);
}
// Wait for process termination returning true if the process exited
@@ -485,12 +482,6 @@ namespace build2
const char* const* args,
uint16_t finish_verbosity);
- LIBBUILD2_SYMEXPORT void
- run (diag_buffer&,
- const process_env& pe,
- const char* const* args,
- uint16_t finish_verbosity);
-
inline void
run (context& ctx,
const process_env& pe,
@@ -500,15 +491,6 @@ namespace build2
run (ctx, pe, args.data (), finish_verbosity);
}
- inline void
- run (diag_buffer& dbuf,
- const process_env& pe,
- const cstrings& args,
- uint16_t finish_verbosity)
- {
- run (dbuf, pe, args.data (), finish_verbosity);
- }
-
// As above but pass cwd/env vars as arguments rather than as part of
// process_env.
//
@@ -524,17 +506,6 @@ namespace build2
}
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, finish_verbosity);
- }
-
- inline void
run (context& ctx,
const process_path& p,
const cstrings& args,
@@ -545,17 +516,6 @@ namespace build2
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 (), 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.
@@ -611,31 +571,6 @@ namespace build2
}
template <typename T, typename F>
- T
- run (diag_buffer&,
- uint16_t verbosity,
- const process_env&,
- const char* const* args,
- F&&,
- sha256* checksum = nullptr);
-
- template <typename T, typename F>
- inline T
- run (diag_buffer& dbuf,
- uint16_t verbosity,
- const process_env& pe,
- const cstrings& args,
- F&& f,
- sha256* checksum = nullptr)
- {
- return run<T> (dbuf,
- verbosity,
- pe, args.data (),
- forward<F> (f),
- checksum);
- }
-
- template <typename T, typename F>
inline T
run (context&,
const process_env&,
@@ -666,31 +601,6 @@ namespace build2
template <typename T, typename F>
inline T
- run (diag_buffer&,
- const process_env&,
- const char* const* args,
- uint16_t finish_verbosity,
- F&&,
- 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,
- sha256* checksum = nullptr)
- {
- return run<T> (dbuf,
- pe, args.data (),
- finish_verbosity,
- forward<F> (f),
- checksum);
- }
-
- template <typename T, typename F>
- inline T
run (context& ctx,
uint16_t verbosity,
const char* args[],
@@ -724,37 +634,6 @@ namespace build2
error, ignore_exit, checksum);
}
- template <typename T, typename F>
- inline T
- run (diag_buffer& dbuf,
- uint16_t verbosity,
- const char* args[],
- F&& f,
- sha256* checksum = nullptr)
- {
- process_path pp (run_search (args[0]));
- return run<T> (dbuf,
- verbosity,
- pp, args,
- forward<F> (f),
- checksum);
- }
-
- template <typename T, typename F>
- inline T
- run (diag_buffer& dbuf,
- uint16_t verbosity,
- cstrings& args,
- F&& f,
- sha256* checksum = nullptr)
- {
- return run<T> (dbuf,
- verbosity,
- args.data (),
- forward<F> (f),
- checksum);
- }
-
// As above but run a program without any arguments or with one argument.
//
// run <prog>
@@ -862,16 +741,6 @@ namespace build2
bool ignore_exit = false,
sha256* checksum = nullptr);
- LIBBUILD2_SYMEXPORT void
- run (diag_buffer& dbuf,
- uint16_t verbosity,
- const process_env&,
- const char* const* args,
- uint16_t finish_verbosity,
- const function<bool (string& line, bool last)>&,
- bool trim = true,
- sha256* checksum = nullptr);
-
// Concatenate the program path and arguments into a shallow NULL-terminated
// vector of C-strings.
//
diff --git a/libbuild2/utility.ixx b/libbuild2/utility.ixx
index 3948623..58ea8db 100644
--- a/libbuild2/utility.ixx
+++ b/libbuild2/utility.ixx
@@ -169,30 +169,6 @@ namespace build2
template <typename T, typename F>
inline T
- run (diag_buffer& dbuf,
- uint16_t verbosity,
- const process_env& pe,
- const char* const* args,
- F&& f,
- sha256* checksum)
- {
- T r;
- run (dbuf,
- verbosity,
- pe, args,
- verbosity - 1,
- [&r, &f] (string& l, bool last) // Small function optimmization.
- {
- r = f (l, last);
- return r.empty ();
- },
- true /* trim */,
- checksum);
- return r;
- }
-
- template <typename T, typename F>
- inline T
run (context& ctx,
const process_env& pe,
const char* const* args,
@@ -221,30 +197,6 @@ namespace build2
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,
- sha256* checksum)
- {
- T r;
- run (dbuf,
- verb_never,
- pe, args,
- finish_verbosity,
- [&r, &f] (string& l, bool last)
- {
- r = f (l, last);
- return r.empty ();
- },
- true /* trim */,
- checksum);
- 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 ad12c9f..ab0224a 100644
--- a/libbuild2/version/snapshot-git.cxx
+++ b/libbuild2/version/snapshot-git.cxx
@@ -127,7 +127,7 @@ namespace build2
args,
0 /* stdin */,
-1 /* stdout */,
- {-1, 1} /* stderr (to stdout) */));
+ 1 /* stderr (to stdout) */));
string l;
try