From c6b1d1dd870b3370d0a09fb4600e3a6b03326f35 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 23 Nov 2022 08:57:45 +0200 Subject: Rework diag_buffer interface to facilitate correct destruction order --- libbuild2/bin/def-rule.cxx | 30 ++-- libbuild2/cc/compile-rule.cxx | 45 +++--- libbuild2/cc/gcc.cxx | 6 +- libbuild2/cc/guess.cxx | 4 +- libbuild2/cc/link-rule.cxx | 24 +-- libbuild2/cc/msvc.cxx | 6 +- libbuild2/diagnostics.cxx | 28 ++-- libbuild2/diagnostics.hxx | 127 +++++++++++++--- libbuild2/diagnostics.ixx | 63 ++++++++ libbuild2/install/rule.cxx | 8 +- libbuild2/parser.cxx | 2 +- libbuild2/script/run.cxx | 42 ++++-- libbuild2/test/rule.cxx | 34 ++++- libbuild2/utility.cxx | 292 +++++++++++++++++-------------------- libbuild2/utility.hxx | 153 ++----------------- libbuild2/utility.ixx | 48 ------ libbuild2/version/snapshot-git.cxx | 2 +- 17 files changed, 444 insertions(+), 470 deletions(-) (limited to 'libbuild2') 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 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&& buf, + const char* args0, + auto_fd&&, + fdstream_mode = fdstream_mode::skip); + + diag_buffer (context&, + vector&& 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&& 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&& 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&& 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&& 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& 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 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 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 - T - run (diag_buffer&, - uint16_t verbosity, - const process_env&, - const char* const* args, - F&&, - sha256* checksum = nullptr); - - template - inline T - run (diag_buffer& dbuf, - uint16_t verbosity, - const process_env& pe, - const cstrings& args, - F&& f, - sha256* checksum = nullptr) - { - return run (dbuf, - verbosity, - pe, args.data (), - forward (f), - checksum); - } - - template inline T run (context&, const process_env&, @@ -666,31 +601,6 @@ namespace build2 template inline T - run (diag_buffer&, - const process_env&, - const char* const* args, - uint16_t finish_verbosity, - F&&, - sha256* checksum = nullptr); - - template - inline T - run (diag_buffer& dbuf, - const process_env& pe, - const cstrings& args, - uint16_t finish_verbosity, - F&& f, - sha256* checksum = nullptr) - { - return run (dbuf, - pe, args.data (), - finish_verbosity, - forward (f), - checksum); - } - - template - inline T run (context& ctx, uint16_t verbosity, const char* args[], @@ -724,37 +634,6 @@ namespace build2 error, ignore_exit, checksum); } - template - 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 (dbuf, - verbosity, - pp, args, - forward (f), - checksum); - } - - template - inline T - run (diag_buffer& dbuf, - uint16_t verbosity, - cstrings& args, - F&& f, - sha256* checksum = nullptr) - { - return run (dbuf, - verbosity, - args.data (), - forward (f), - checksum); - } - // As above but run a program without any arguments or with one argument. // // run @@ -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 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 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 - inline T run (context& ctx, const process_env& pe, const char* const* args, @@ -221,30 +197,6 @@ namespace build2 return r; } - template - 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 -- cgit v1.1