From 72e9ec3d7028765836851b515d80816f2da74060 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Thu, 27 Oct 2022 11:00:47 +0200 Subject: Initial work on child process diagnostics buffering Currently this is implemented for C/C++ compile and link rules. --- libbuild2/bin/def-rule.cxx | 3 + libbuild2/cc/compile-rule.cxx | 91 +++++++++------ libbuild2/cc/link-rule.cxx | 106 ++++++++++------- libbuild2/cc/msvc.cxx | 25 +++-- libbuild2/diagnostics.cxx | 256 +++++++++++++++++++++++++++++++++++++++++- libbuild2/diagnostics.hxx | 175 ++++++++++++++++++++++++++++- libbuild2/in/rule.cxx | 3 + libbuild2/script/run.cxx | 2 +- libbuild2/types.hxx | 2 + libbuild2/utility.cxx | 2 + 10 files changed, 578 insertions(+), 87 deletions(-) diff --git a/libbuild2/bin/def-rule.cxx b/libbuild2/bin/def-rule.cxx index c0e82fb..421074c 100644 --- a/libbuild2/bin/def-rule.cxx +++ b/libbuild2/bin/def-rule.cxx @@ -751,6 +751,9 @@ namespace build2 return target_state::changed; } + // @@ TODO: split into rule_name (string) and rule_version (integer) + // on next increment. + // const string def_rule::rule_id_ {"bin.def 2"}; } } diff --git a/libbuild2/cc/compile-rule.cxx b/libbuild2/cc/compile-rule.cxx index fda97a0..8d507e7 100644 --- a/libbuild2/cc/compile-rule.cxx +++ b/libbuild2/cc/compile-rule.cxx @@ -264,6 +264,10 @@ namespace build2 compile_rule:: compile_rule (data&& d, const scope& rs) : common (move (d)), + // + // @@ TODO: split into rule_name (string) and rule_version (integer) + // on next increment. + // rule_id (string (x) += ".compile 6") { // Locate the header cache (see enter_header() for details). @@ -3961,6 +3965,8 @@ namespace build2 // diagnostics (if things go badly we will restart with this // support). // + // @@ TODO: diag_buffer + // if (drmp == nullptr) // Dependency info goes to stdout. { assert (!sense_diag); // Note: could support with fdselect(). @@ -3970,7 +3976,7 @@ namespace build2 // pr = process ( cpath, - args.data (), + args, 0, -1, cclass == compiler_class::msvc ? 1 : gen ? 2 : -2, @@ -3980,7 +3986,7 @@ namespace build2 else // Dependency info goes to a temporary file. { pr = process (cpath, - args.data (), + args, mod_mapper ? -1 : 0, mod_mapper ? -1 : 2, // Send stdout to stderr. gen ? 2 : sense_diag ? -1 : -2, @@ -4890,10 +4896,10 @@ namespace build2 print_process (args); // We don't want to see warnings multiple times so ignore all - // diagnostics. + // diagnostics (thus no need for diag_buffer). // pr = process (cpath, - args.data (), + args, 0, -1, -2, nullptr, // CWD env.empty () ? nullptr : env.data ()); @@ -6327,7 +6333,7 @@ namespace build2 // Filter cl.exe noise (msvc.cxx). // void - msvc_filter_cl (ifdstream&, const path& src); + msvc_filter_cl (diag_buffer&, const path& src); // Append header unit-related options. // @@ -6674,7 +6680,7 @@ namespace build2 small_vector module_args; // Module options storage. size_t out_i (0); // Index of the -o option. - size_t lang_n (0); // Number of lang options. + //size_t lang_n (0); // Number of lang options. @@ TMP switch (cclass) { @@ -7025,7 +7031,7 @@ namespace build2 args.push_back ("-c"); } - lang_n = append_lang_options (args, md); + /*lang_n = */append_lang_options (args, md); // @@ TMP if (md.pp == preprocessed::all) { @@ -7074,6 +7080,8 @@ namespace build2 // the source file, not its preprocessed version (so that it's easy to // copy and re-run, etc). Only at level 3 and above print the real deal. // + // @@ TODO: why don't we print env (here and/or below)? Also link rule. + // if (verb == 1) text << x_name << ' ' << s; else if (verb == 2) @@ -7081,12 +7089,17 @@ namespace build2 // If we have the (partially) preprocessed output, switch to that. // + // But we remember the original source/position to restore later. + // bool psrc (md.psrc); bool ptmp (psrc && md.psrc.temporary); + pair osrc; if (psrc) { args.pop_back (); // nullptr + osrc.second = args.back (); args.pop_back (); // sp + osrc.first = args.size (); sp = &md.psrc.path (); @@ -7096,6 +7109,8 @@ namespace build2 { case compiler_type::gcc: { + // @@ TMP +#if 0 // The -fpreprocessed is implied by .i/.ii. But not when compiling // a header unit (there is no .hi/.hii). // @@ -7109,6 +7124,15 @@ namespace build2 // for (; lang_n != 0; --lang_n) args.pop_back (); +#else + // -fpreprocessed is implied by .i/.ii unless compiling a header + // unit (there is no .hi/.hii). Also, we would need to pop -x + // since it takes precedence over the extension, which would mess + // up our osrc logic. So in the end it feels like always passing + // explicit -fpreprocessed is the way to go. + // + args.push_back ("-fpreprocessed"); +#endif args.push_back ("-fdirectives-only"); break; @@ -7145,6 +7169,8 @@ 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 @@ -7163,45 +7189,36 @@ namespace build2 file_cache::read psrcr (psrc ? md.psrc.open () : file_cache::read ()); // VC cl.exe sends diagnostics to stdout. It also prints the file - // name being compiled as the first line. So for cl.exe we redirect - // stdout to a pipe, filter that noise out, and send the rest to - // stderr. + // name being compiled as the first line. So for cl.exe we filter + // that noise out. // - // For other compilers redirect stdout to stderr, in case any of - // them tries to pull off something similar. For sane compilers this - // should be harmless. + // For other compilers also redirect stdout to stderr, in case any + // of them tries to pull off something similar. For sane compilers + // this should be harmless. // bool filter (ctype == compiler_type::msvc); process pr (cpath, - args.data (), - 0, (filter ? -1 : 2), 2, + args, + 0, 2, dbuf.open (args[0], filter), nullptr, // CWD env.empty () ? nullptr : env.data ()); if (filter) - { - try - { - ifdstream is ( - move (pr.in_ofd), fdstream_mode::text, ifdstream::badbit); - - msvc_filter_cl (is, *sp); + msvc_filter_cl (dbuf, *sp); - // If anything remains in the stream, send it all to stderr. - // Note that the eof check is important: if the stream is at - // eof, this and all subsequent writes to the diagnostics stream - // will fail (and you won't see a thing). - // - if (is.peek () != ifdstream::traits_type::eof ()) - diag_stream_lock () << is.rdbuf (); + dbuf.read (); - is.close (); - } - catch (const io_error&) {} // Assume exits with error. + // Restore the original source if we switched to preprocessed. + // + if (psrc) + { + args.resize (osrc.first); + args.push_back (osrc.second); + args.push_back (nullptr); } - run_finish (args, pr); + dbuf.finish (args, pr); } catch (const process_error& e) { @@ -7252,12 +7269,14 @@ namespace build2 try { process pr (cpath, - args.data (), - 0, 2, 2, + args, + 0, 2, dbuf.open (args[0]), nullptr, // CWD env.empty () ? nullptr : env.data ()); - run_finish (args, pr); + dbuf.read (); + + dbuf.finish (args, pr); } catch (const process_error& e) { diff --git a/libbuild2/cc/link-rule.cxx b/libbuild2/cc/link-rule.cxx index a5fa7bc..5d212e6 100644 --- a/libbuild2/cc/link-rule.cxx +++ b/libbuild2/cc/link-rule.cxx @@ -254,6 +254,10 @@ namespace build2 link_rule:: link_rule (data&& d) : common (move (d)), + // + // @@ TODO: split into rule_name (string) and rule_version (integer) + // on next increment. + // rule_id (string (x) += ".link 3") { } @@ -2671,7 +2675,7 @@ namespace build2 // Filter link.exe noise (msvc.cxx). // void - msvc_filter_link (ifdstream&, const file&, otype); + msvc_filter_link (diag_buffer&, const file&, otype); // Translate target CPU to the link.exe/lib.exe /MACHINE option. // @@ -2879,6 +2883,8 @@ 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. @@ -2929,12 +2935,15 @@ namespace build2 try { + // We assume that what we write to stdin is small enough to + // fit into the pipe's buffer without blocking. + // process pr (rc, args, - -1 /* stdin */, - 1 /* stdout */, - 2 /* stderr */, - nullptr /* cwd */, + -1 /* stdin */, + 1 /* stdout */, + dbuf.open (args[0]) /* stderr */, + nullptr /* cwd */, env_ptrs.empty () ? nullptr : env_ptrs.data ()); try @@ -2960,7 +2969,8 @@ namespace build2 // was caused by that and let run_finish() deal with it. } - run_finish (args, pr); + dbuf.read (); + dbuf.finish (args, pr, 2 /* verbosity */); } catch (const process_error& e) { @@ -3823,8 +3833,21 @@ namespace build2 else if (verb == 2) print_process (args); + // Do any necessary fixups to the command line to make it runnable. + // + // Notice the split in the diagnostics: at verbosity level 1 we print + // the "logical" command line while at level 2 and above -- what we are + // actually executing. + // + // We also need to save the original for the diag_buffer::close() call + // below if at verbosity level 1. + // + cstrings oargs; + // Adjust linker parallelism. // + // Note that we are not going to bother with oargs for this. + // string jobs_arg; scheduler::alloc_guard jobs_extra; @@ -3875,12 +3898,6 @@ namespace build2 } } - // Do any necessary fixups to the command line to make it runnable. - // - // Notice the split in the diagnostics: at verbosity level 1 we print - // the "logical" command line while at level 2 and above -- what we are - // actually executing. - // // On Windows we need to deal with the command line length limit. The // best workaround seems to be passing (part of) the command line in an // "options file" ("response file" in Microsoft's terminology). Both @@ -3966,14 +3983,15 @@ namespace build2 fail << "unable to write to " << f << ": " << e; } + if (verb == 1) + oargs = args; + // Replace input arguments with @file. // targ = '@' + f.string (); args.resize (args_input); args.push_back (targ.c_str()); args.push_back (nullptr); - - //@@ TODO: leave .t file if linker failed and verb > 2? } } #endif @@ -3996,51 +4014,48 @@ namespace build2 { // VC tools (both lib.exe and link.exe) send diagnostics to stdout. // Also, link.exe likes to print various gratuitous messages. So for - // link.exe we redirect stdout to a pipe, filter that noise out, and - // send the rest to stderr. + // link.exe we filter that noise out. // // For lib.exe (and any other insane linker that may try to pull off // something like this) we are going to redirect stdout to stderr. // For sane compilers this should be harmless. // // Note that we don't need this for LLD's link.exe replacement which - // is quiet. + // is thankfully quiet. // bool filter (tsys == "win32-msvc" && !lt.static_library () && cast (rs["bin.ld.id"]) != "msvc-lld"); process pr (*ld, - args.data (), - 0 /* stdin */, - (filter ? -1 : 2) /* stdout */, - 2 /* stderr */, - nullptr /* cwd */, + args, + 0 /* stdin */, + 2 /* stdout */, + dbuf.open (args[0], filter) /* stderr */, + nullptr /* cwd */, env_ptrs.empty () ? nullptr : env_ptrs.data ()); if (filter) + msvc_filter_link (dbuf, t, ot); + + dbuf.read (); + { - try - { - ifdstream is ( - move (pr.in_ofd), fdstream_mode::text, ifdstream::badbit); + bool e (pr.wait ()); - msvc_filter_link (is, t, ot); +#ifdef _WIN32 + // Keep the options file if we have shown it. + // + if (!e && verb > 2) + trm.cancel (); +#endif - // If anything remains in the stream, send it all to stderr. - // Note that the eof check is important: if the stream is at - // eof, this and all subsequent writes to the diagnostics stream - // will fail (and you won't see a thing). - // - if (is.peek () != ifdstream::traits_type::eof ()) - diag_stream_lock () << is.rdbuf (); + dbuf.close (oargs.empty () ? args : oargs, *pr.exit); - is.close (); - } - catch (const io_error&) {} // Assume exits with error. + if (!e) + throw failed (); } - run_finish (args, pr); jobs_extra.deallocate (); } catch (const process_error& e) @@ -4100,10 +4115,25 @@ namespace build2 print_process (args); if (!ctx.dry_run) + { +#if 0 run (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 + } } // For Windows generate (or clean up) rpath-emulating assembly. diff --git a/libbuild2/cc/msvc.cxx b/libbuild2/cc/msvc.cxx index f95cab0..9aff347 100644 --- a/libbuild2/cc/msvc.cxx +++ b/libbuild2/cc/msvc.cxx @@ -164,18 +164,21 @@ namespace build2 // Filter cl.exe and link.exe noise. // + // Note: must be followed with the dbuf.read() call. + // void - msvc_filter_cl (ifdstream& is, const path& src) + msvc_filter_cl (diag_buffer& dbuf, const path& src) + try { // While it appears VC always prints the source name (event if the // file does not exist), let's do a sanity check. Also handle the // command line errors/warnings which come before the file name. // - for (string l; !eof (getline (is, l)); ) + for (string l; !eof (getline (dbuf.is, l)); ) { if (l != src.leaf ().string ()) { - diag_stream_lock () << l << endl; + dbuf.write (l, true /* newline */); if (msvc_sense_diag (l, 'D').first != string::npos) continue; @@ -184,14 +187,19 @@ namespace build2 break; } } + catch (const io_error&) + { + // Let the following diag_buffer::read() call deal with this. + } void - msvc_filter_link (ifdstream& is, const file& t, otype lt) + msvc_filter_link (diag_buffer& dbuf, const file& t, otype lt) + try { // Filter lines until we encounter something we don't recognize. We also // have to assume the messages can be translated. // - for (string l; getline (is, l); ) + for (string l; getline (dbuf.is, l); ) { // " Creating library foo\foo.dll.lib and object foo\foo.dll.exp" // @@ -216,12 +224,15 @@ namespace build2 // /INCREMENTAL causes linker to sometimes issue messages but now I // can't quite reproduce it. - // - diag_stream_lock () << l << endl; + dbuf.write (l, true /* newline */); break; } } + catch (const io_error&) + { + // Let the following diag_buffer::read() call deal with this. + } void msvc_extract_header_search_dirs (const strings& v, dir_paths& r) diff --git a/libbuild2/diagnostics.cxx b/libbuild2/diagnostics.cxx index a2a8444..7bdfea2 100644 --- a/libbuild2/diagnostics.cxx +++ b/libbuild2/diagnostics.cxx @@ -3,7 +3,7 @@ #include -#include // strchr() +#include // strchr(), memcpy() #include @@ -13,6 +13,7 @@ #include using namespace std; +using namespace butl; namespace build2 { @@ -46,6 +47,259 @@ namespace build2 // const int stream_verb_index = ostream::xalloc (); + // diag_buffer + // + process::pipe diag_buffer:: + open (const char* args0, bool force, bool blocking) + { + assert (state_ == state::closed && args0 != nullptr); + + assert (blocking); // @@ TODO (also in read() and close () below). + + serial = ctx_.sched.serial (); + + process::pipe r; + if (!serial || force) + { + 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)); + + is.open (move (p.in), fdstream_mode::text, ifdstream::badbit); + } + 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; + } + + bool diag_buffer:: + read () + { + assert (state_ == state::opened); + + bool r; + if (is.is_open ()) + { + try + { + if (is.blocking ()) + { + if (serial) + { + // This is the case where we are called after custom processing. + // + assert (buf.empty ()); + + // Note that the eof check is important: if the stream is at eof, + // this and all subsequent writes to the diagnostics stream will + // fail (and you won't see a thing). + // + if (is.peek () != ifdstream::traits_type::eof ()) + { + // Holding the diag lock while waiting for diagnostics from the + // child process would be a bad idea in the parallel build. But + // it should be harmless in serial. + // + diag_stream_lock l; + *diag_stream << is.rdbuf (); + } + } + else + { + fdstreambuf& sb (*static_cast (is.rdbuf ())); + + while (is.peek () != istream::traits_type::eof ()) + { + const char* p (sb.gptr ()); + size_t n (sb.egptr () - p); + + // Allocate at least fdstreambuf::buffer_size to reduce + // reallocations and memory fragmentation. + // + size_t i (buf.size ()); + if (i == 0 && n < fdstreambuf::buffer_size) + buf.reserve (fdstreambuf::buffer_size); + + buf.resize (i + n); + memcpy (buf.data () + i, p, n); + + sb.gbump (static_cast (n)); + } + } + + r = false; + } + else + { + // @@ TODO (maybe we can unify the two?) + r = true; + } + + if (!r) + is.close (); + } + catch (const io_error& e) + { + // For now we assume (here and pretty much everywhere else) that the + // output can't fail. + // + fail << "unable to read from " << args0 << " stderr:" << e; + } + } + else + r = false; + + if (!r) + state_ = state::eof; + + return r; + } + + void diag_buffer:: + write (const string& s, bool nl) + { + // Similar logic to read() above. + // + if (serial) + { + assert (buf.empty ()); + + diag_stream_lock l; + *diag_stream << s; + if (nl) + *diag_stream << '\n'; + } + else + { + size_t n (s.size () + (nl ? 1 : 0)); + + size_t i (buf.size ()); + if (i == 0 && n < fdstreambuf::buffer_size) + buf.reserve (fdstreambuf::buffer_size); + + buf.resize (i + n); + memcpy (buf.data () + i, s.c_str (), s.size ()); + + if (nl) + buf.back () = '\n'; + } + } + + void diag_buffer:: + close (const char* const* args, size_t args_size, + const process_exit& pe, + uint16_t v, + const location& loc) + { + assert (state_ != state::closed); + + // We may still be in the open state in case of custom processing. + // + if (state_ == state::opened) + { + if (is.is_open ()) + { + try + { + // @@ TODO: is it ok to call peek() in non-blocking? + // + assert (is.peek () == ifdstream::traits_type::eof ()); + is.close (); + } + catch (const io_error& e) + { + fail << "unable to read from " << args0 << " stderr:" << e; + } + } + + state_ = state::eof; + } + + // We need to make sure the command line we print on the unsuccessful exit + // is inseparable from any buffered diagnostics. So we prepare the record + // first and then write both while holding the diagnostics stream lock. + // + diag_record dr; + if (!pe) + { + // Note: see similar code in run_finish_impl(). + + // 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. + // + dr << error (loc) << "process " << args[0] << " " << pe; + + if (verb >= 1 && verb <= v) + { + dr << info << "command line: "; + print_process (dr, args, args_size); + } + } + + if (!buf.empty () || !dr.empty ()) + { + diag_stream_lock l; + + if (!buf.empty ()) + diag_stream->write (buf.data (), static_cast (buf.size ())); + + if (!dr.empty ()) + dr.flush ([] (const butl::diag_record& r) + { + // Similar to default_writer(). + // + *diag_stream << r.os.str () << '\n'; + }); + + diag_stream->flush (); + } + + buf.clear (); + args0 = nullptr; + state_ = state::closed; + } + + void diag_buffer:: + finish (const char* const* args, size_t args_size, + process& pr, + uint16_t v, + const location& loc) + { + // Note: see similar code in run_finish_impl(). + // + try + { + pr.wait (); + } + catch (const process_error& e) + { + fail (loc) << "unable to execute " << args[0] << ": " << e << endf; + } + + const process_exit& pe (*pr.exit); + + close (args, args_size, pe, v, loc); + + if (!pe) + throw failed (); + } + + // print_process() + // void print_process (const char* const* args, size_t n) { diff --git a/libbuild2/diagnostics.hxx b/libbuild2/diagnostics.hxx index 179b5c9..a56fd64 100644 --- a/libbuild2/diagnostics.hxx +++ b/libbuild2/diagnostics.hxx @@ -21,10 +21,177 @@ namespace build2 // class failed: public std::exception {}; - // Print process commmand line. If the number of elements is specified - // (or the second version is used), then it will print the piped multi- - // process command line, if present. In this case, the expected format - // is as follows: + // Diagnostics buffer. + // + // The purpose of this class is to handle diagnostics from child processes, + // where handle can mean: + // + // - Buffer it (to avoid interleaving in parallel builds). + // + // - Stream it (if the input can be split into diagnostic records). + // + // - Do nothing (in serial builds). + // + // This class is also responsible for converting the diagnostics into the + // structured form (which means it may need to buffer even in serial + // builds). + // + class LIBBUILD2_SYMEXPORT diag_buffer + { + public: + explicit + diag_buffer (context& c): ctx_ (c) {} + + public: + // If buffering is necessary or force is true, open a pipe and return the + // child process end of it. Otherwise, return stderr. If blocking is + // false, 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 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. + // + // All the below functions handle io errors, issue suitable diagnostics, + // and throw failed. If an exception is thrown from any of them, then the + // instance should not be used any further. + // + process::pipe + open (const char* args0, bool force = false, bool blocking = true); + + // Read the diagnostics from the parent end of the pipe if one was opened + // and buffer/stream it as necessary. Return true if there could be more + // diagnostics to read (only possible in the non-blocking mode). + // + // Instead of calling this function you can perform custom reading and, if + // necessary, buffering of the diagnostics by accessing the input stream + // (is) and underlying buffer (buf) directly. This can be used to filter, + // split the diagnostics into records according to a certain format, etc. + // Note that such custom processing implementation should maintain the + // overall semantics of diagnostics buffering in that it may only omit + // buffering in the serial case or if the diagnostics can be streamed in + // atomic records. See also write() below. + // + // The input stream is opened in the text mode and has the badbit but not + // failbit exception mask. The custom processing should also be compatible + // with the stream mode (blocking or non). If buffering is performed, then + // depending on the expected diagnostics the custom processing may want to + // reserve an appropriate initial buffer size to avoid unnecessary + // reallocation. As a convenience, in the blocking mode, if the stream + // still contains some diagnostics, then it can be handled by calling + // read(). This is helpful when needing to process only the inital part of + // the diagnostics. + // + bool + read (); + + // Close the parent end of the pipe if one was opened and write out any + // buffered diagnostics. + // + // If the verbosity level is between 1 and the specified value and the + // child process exited with non-0 code, then print the command line after + // the diagnostics. Normally the specified verbosity will be 1 and the + // command line args represent the verbosity level 2 (logical) command + // line. The semantics os the args/args_size arguments is the same as + // in print_process() below. + // + // If the diag_buffer instance is destroyed before calling close(), then + // any buffered diagnostics is discarded. + // + // @@ TODO: need overload with process_env (see print_process). + // + void + close (const cstrings& args, + const process_exit& pe, + uint16_t verbosity = 1, + const location& loc = {}) + { + close (args.data (), args.size (), pe, verbosity, loc); + } + + void + close (const char* const* args, + const process_exit& pe, + uint16_t verbosity = 1, + const location& loc = {}) + { + close (args, 0, pe, verbosity, loc); + } + + void + close (const char* const* args, size_t args_size, + const process_exit& pe, + uint16_t verbosity = 1, + const location& loc = {}); + + + // This version calls close() plus it first waits for the process to + // finish and later throws failed if it didn't exit with 0 code (so + // similar to run_finish ()). + // + void + finish (const cstrings& args, + process& pr, + uint16_t verbosity = 1, + const location& loc = {}) + { + finish (args.data (), args.size (), pr, verbosity, loc); + } + + void + finish (const char* const* args, + process& pr, + uint16_t verbosity = 1, + const location& loc = {}) + { + finish (args, 0, pr, verbosity, loc); + } + + void + finish (const char* const* args, size_t args_size, + process&, + uint16_t verbosity = 1, + const location& = {}); + + // Direct access to the underlying stream and buffer for custom processing + // (see read() above for details). + // + public: + ifdstream is; + vector buf; + const char* args0; + bool serial; + + // Buffer or stream a fragment of diagnostics as necessary. If newline + // is true, also add a trailing newline. + // + // This function is normally called from a custom diagnostics processing + // implementation (see read() above for details). + // + void + write (const string&, bool newline); + + private: + // Note that we don't seem to need a custom destructor to achieve the + // desired semantics: we can assume the process has exited before we are + // destroyed (because we supply stderr to its constructor) which means + // closing fdstream without reading any futher should be ok. + // + enum class state {closed, opened, eof}; + + context& ctx_; + state state_ = state::closed; + }; + + // Print process commmand line. If the number of elements is specified (or + // the const cstrings& version is used), then it will print the piped multi- + // process command line, if present. In this case, the expected format is as + // follows: // // name1 arg arg ... nullptr // name2 arg arg ... nullptr diff --git a/libbuild2/in/rule.cxx b/libbuild2/in/rule.cxx index d07adfc..761a882 100644 --- a/libbuild2/in/rule.cxx +++ b/libbuild2/in/rule.cxx @@ -147,6 +147,9 @@ namespace build2 // First should come the rule name/version. // + // @@ TODO: split into rule_name (string) and rule_version (integer) + // on next increment. + // if (dd.expect (rule_id_ + " 1") != nullptr) l4 ([&]{trace << "rule mismatch forcing update of " << t;}); diff --git a/libbuild2/script/run.cxx b/libbuild2/script/run.cxx index ca04443..1971bd1 100644 --- a/libbuild2/script/run.cxx +++ b/libbuild2/script/run.cxx @@ -1329,7 +1329,7 @@ namespace build2 } catch (const io_error& e) { - fail (ll) << "unable to read from " << cmd_path (c) << " output: " + fail (ll) << "unable to read from " << cmd_path (c) << " stdout: " << e; } } diff --git a/libbuild2/types.hxx b/libbuild2/types.hxx index 6b4022d..d93da6d 100644 --- a/libbuild2/types.hxx +++ b/libbuild2/types.hxx @@ -373,8 +373,10 @@ namespace build2 using butl::sha256; // + // using butl::process; using butl::process_env; + using butl::process_exit; using butl::process_path; using butl::process_error; diff --git a/libbuild2/utility.cxx b/libbuild2/utility.cxx index 3eeeeaa..e73cd61 100644 --- a/libbuild2/utility.cxx +++ b/libbuild2/utility.cxx @@ -281,6 +281,8 @@ namespace build2 if (pr.wait ()) return true; + // Note: see similar code in diag_buffer::close/finish(). + // const process_exit& e (*pr.exit); if (!e.normal ()) -- cgit v1.1