From f7f22db6030464f63eb942da04b3d5e10351f770 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 2 Nov 2022 09:56:31 +0200 Subject: More work on child process diagnostics buffering --- libbuild2/bin/def-rule.cxx | 158 ++++++++--- libbuild2/bin/guess.cxx | 42 +-- libbuild2/bin/guess.hxx | 8 +- libbuild2/bin/init.cxx | 14 +- libbuild2/cc/compile-rule.cxx | 9 +- libbuild2/cc/gcc.cxx | 154 +++++------ libbuild2/cc/guess.cxx | 63 +++-- libbuild2/cc/guess.hxx | 3 +- libbuild2/cc/link-rule.cxx | 19 +- libbuild2/cc/module.cxx | 6 +- libbuild2/cc/msvc.cxx | 6 +- libbuild2/context.cxx | 3 +- libbuild2/diagnostics.cxx | 173 ++++++------ libbuild2/diagnostics.hxx | 86 +++--- libbuild2/dist/operation.cxx | 32 +-- libbuild2/functions-process.cxx | 47 +++- libbuild2/install/rule.cxx | 17 +- libbuild2/parser.cxx | 7 +- libbuild2/script/run.cxx | 2 + libbuild2/types.hxx | 1 + libbuild2/utility.cxx | 309 +++++++++++++++++++-- libbuild2/utility.hxx | 531 ++++++++++++++++++++++++++++--------- libbuild2/utility.ixx | 128 ++++++++- libbuild2/utility.txx | 64 ----- libbuild2/version/snapshot-git.cxx | 17 +- libbuild2/version/snapshot.cxx | 4 +- 26 files changed, 1312 insertions(+), 591 deletions(-) (limited to 'libbuild2') diff --git a/libbuild2/bin/def-rule.cxx b/libbuild2/bin/def-rule.cxx index 421074c..257fad8 100644 --- a/libbuild2/bin/def-rule.cxx +++ b/libbuild2/bin/def-rule.cxx @@ -33,8 +33,10 @@ namespace build2 }; static void - read_dumpbin (istream& is, symbols& syms) + read_dumpbin (diag_buffer& dbuf, ifdstream& is, symbols& syms) { + // Note: io_error is handled by the caller. + // Lines that describe symbols look like: // // 0 1 2 3 4 5 6 @@ -72,29 +74,27 @@ namespace build2 // // Note that an UNDEF data symbol with non-zero OFFSET is a "common // symbol", equivalent to the nm `C` type. - - // Map of read-only (.rdata, .xdata) and uninitialized (.bss) sections - // to their types (R and B, respectively). If a section is not found in - // this map, then it's assumed to be normal data (.data). // - map sections; - - string l; - while (!eof (getline (is, l))) + // We keep a map of read-only (.rdata, .xdata) and uninitialized (.bss) + // sections to their types (R and B, respectively). If a section is not + // found in this map, then it's assumed to be normal data (.data). + // + auto parse_line = [&syms, + secs = map ()] (const string& l) mutable { size_t b (0), e (0), n; // IDX (note that it can be more than 3 characters). // if (next_word (l, b, e) == 0) - continue; + return; // OFFSET (always 8 characters). // n = next_word (l, b, e); if (n != 8) - continue; + return; string off (l, b, n); @@ -103,7 +103,7 @@ namespace build2 n = next_word (l, b, e); if (n == 0) - continue; + return; string sec (l, b, n); @@ -112,7 +112,7 @@ namespace build2 n = next_word (l, b, e); if (l.compare (b, n, "notype") != 0) - continue; + return; bool dat; if (l[e] == ' ' && l[e + 1] == '(' && l[e + 2] == ')') @@ -128,7 +128,7 @@ namespace build2 n = next_word (l, b, e); if (n == 0) - continue; + return; string vis (l, b, n); @@ -137,14 +137,14 @@ namespace build2 n = next_word (l, b, e); if (n != 1 || l[b] != '|') - continue; + return; // SYMNAME // n = next_word (l, b, e); if (n == 0) - continue; + return; string s (l, b, n); @@ -162,23 +162,23 @@ namespace build2 }; if (cmp (".rdata", 6) || - cmp (".xdata", 6)) sections.emplace (move (sec), 'R'); - else if (cmp (".bss", 4)) sections.emplace (move (sec), 'B'); + cmp (".xdata", 6)) secs.emplace (move (sec), 'R'); + else if (cmp (".bss", 4)) secs.emplace (move (sec), 'B'); - continue; + return; } // We can only export extern symbols. // if (vis != "External") - continue; + return; if (dat) { if (sec != "UNDEF") { - auto i (sections.find (sec)); - switch (i == sections.end () ? 'D' : i->second) + auto i (secs.find (sec)); + switch (i == secs.end () ? 'D' : i->second) { case 'D': syms.d.insert (move (s)); break; case 'R': syms.r.insert (move (s)); break; @@ -196,20 +196,54 @@ namespace build2 if (sec != "UNDEF") syms.t.insert (move (s)); } + }; + + // Read until we reach EOF on all streams. + // + // Note that if dbuf is not opened, then we automatically get inactive + // nullfd entry. + // + fdselect_set fds {is.fd (), dbuf.is.fd ()}; + fdselect_state& ist (fds[0]); + fdselect_state& dst (fds[1]); + + for (string l; ist.fd != nullfd || dst.fd != nullfd; ) + { + if (ist.fd != nullfd && getline_non_blocking (is, l)) + { + if (eof (is)) + ist.fd = nullfd; + else + { + parse_line (l); + l.clear (); + } + + continue; + } + + ifdselect (fds); + + if (dst.ready) + { + if (!dbuf.read ()) + dst.fd = nullfd; + } } } static void - read_posix_nm (istream& is, symbols& syms) + read_posix_nm (diag_buffer& dbuf, ifdstream& is, symbols& syms) { + // Note: io_error is handled by the caller. + // Lines that describe symbols look like: // // // // The types that we are interested in are T, D, R, and B. // - string l; - while (!eof (getline (is, l))) + auto parse_line = [&syms] (const string& l) { size_t b (0), e (0), n; @@ -218,7 +252,7 @@ namespace build2 n = next_word (l, b, e); if (n == 0) - continue; + return; string s (l, b, n); @@ -227,7 +261,7 @@ namespace build2 n = next_word (l, b, e); if (n != 1) - continue; + return; switch (l[b]) { @@ -238,6 +272,39 @@ namespace build2 case 'C': syms.c.insert (move (s)); break; case 'T': syms.t.insert (move (s)); break; } + }; + + // Read until we reach EOF on all streams. + // + // Note that if dbuf is not opened, then we automatically get inactive + // nullfd entry. + // + fdselect_set fds {is.fd (), dbuf.is.fd ()}; + fdselect_state& ist (fds[0]); + fdselect_state& dst (fds[1]); + + for (string l; ist.fd != nullfd || dst.fd != nullfd; ) + { + if (ist.fd != nullfd && getline_non_blocking (is, l)) + { + if (eof (is)) + ist.fd = nullfd; + else + { + parse_line (l); + l.clear (); + } + + continue; + } + + ifdselect (fds); + + if (dst.ready) + { + if (!dbuf.read ()) + dst.fd = nullfd; + } } } @@ -656,6 +723,8 @@ namespace build2 if (verb == 1) text << "def " << t; + diag_buffer dbuf (ctx); + // Extract symbols from each object file. // symbols syms; @@ -674,22 +743,37 @@ namespace build2 // Both dumpbin.exe and nm send their output to stdout. While nm sends // diagnostics to stderr, dumpbin sends it to stdout together with the - // output. + // output. To keep things uniform we will buffer stderr in both cases. // - process pr (run_start (nm, - args, - 0 /* stdin */, - -1 /* stdout */)); + process pr ( + run_start (nm, + args, + 0 /* stdin */, + -1 /* stdout */, + dbuf.open (args[0], + false /* force */, + fdstream_mode::non_blocking | + fdstream_mode::skip) /* stderr */)); + bool io (false); try { - ifdstream is ( - move (pr.in_ofd), fdstream_mode::skip, ifdstream::badbit); + // 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); if (lid == "msvc" || nid == "msvc") - read_dumpbin (is, syms); + read_dumpbin (dbuf, is, syms); else - read_posix_nm (is, syms); + read_posix_nm (dbuf, is, syms); is.close (); } @@ -701,7 +785,7 @@ namespace build2 io = true; } - if (!run_finish_code (args.data (), pr) || io) + if (!run_finish_code (dbuf, args, pr) || io) fail << "unable to extract symbols from " << arg; } diff --git a/libbuild2/bin/guess.cxx b/libbuild2/bin/guess.cxx index e935913..e9759b8 100644 --- a/libbuild2/bin/guess.cxx +++ b/libbuild2/bin/guess.cxx @@ -92,7 +92,7 @@ namespace build2 static global_cache ar_cache; const ar_info& - guess_ar (const path& ar, const path* rl, const char* paths) + guess_ar (context& ctx, const path& ar, const path* rl, const char* paths) { tracer trace ("bin::guess_ar"); @@ -234,7 +234,11 @@ namespace build2 // (yes, it goes to stdout) but that seems harmless. // sha256 cs; - arr = run (3, are, "--version", f, false, false, &cs); + arr = run (ctx, + 3, + are, "--version", + f, + false , false, &cs); if (!arr.empty ()) arr.checksum = cs.string (); @@ -254,10 +258,10 @@ namespace build2 : guess_result (); }; - // Redirect STDERR to STDOUT and ignore exit status. + // Redirect stderr to stdout and ignore exit status. // sha256 cs; - arr = run (3, are, f, false, true, &cs); + arr = run (ctx, 3, are, f, false, true, &cs); if (!arr.empty ()) { @@ -300,7 +304,11 @@ namespace build2 }; sha256 cs; - rlr = run (3, rle, "--version", f, false, false, &cs); + rlr = run (ctx, + 3, + rle, "--version", + f, + false, false, &cs); if (!rlr.empty ()) rlr.checksum = cs.string (); @@ -317,10 +325,10 @@ namespace build2 : guess_result (); }; - // Redirect STDERR to STDOUT and ignore exit status. + // Redirect stderr to stdout and ignore exit status. // sha256 cs; - rlr = run (3, rle, f, false, true, &cs); + rlr = run (ctx, 3, rle, f, false, true, &cs); if (!rlr.empty ()) { @@ -385,7 +393,7 @@ namespace build2 static global_cache ld_cache; const ld_info& - guess_ld (const path& ld, const char* paths) + guess_ld (context& ctx, const path& ld, const char* paths) { tracer trace ("bin::guess_ld"); @@ -497,12 +505,12 @@ namespace build2 : guess_result (move (id), move (l), move (ver))); }; - // Redirect STDERR to STDOUT and ignore exit status. Note that in case + // Redirect stderr to stdout and ignore exit status. Note that in case // of link.exe we will hash the diagnostics (yes, it goes to stdout) // but that seems harmless. // sha256 cs; - r = run (3, env, "--version", f, false, true, &cs); + r = run (ctx, 3, env, "--version", f, false, true, &cs); if (!r.empty ()) r.checksum = cs.string (); @@ -533,7 +541,7 @@ namespace build2 }; sha256 cs; - r = run (3, env, "-v", f, false, false, &cs); + r = run (ctx, 3, env, "-v", f, false, false, &cs); if (!r.empty ()) r.checksum = cs.string (); @@ -560,7 +568,7 @@ namespace build2 // option. // sha256 cs; - r = run (3, env, "-version", f, false, false, &cs); + r = run (ctx, 3, env, "-version", f, false, false, &cs); if (!r.empty ()) r.checksum = cs.string (); @@ -598,7 +606,7 @@ namespace build2 static global_cache rc_cache; const rc_info& - guess_rc (const path& rc, const char* paths) + guess_rc (context& ctx, const path& rc, const char* paths) { tracer trace ("bin::guess_rc"); @@ -654,7 +662,7 @@ namespace build2 // option. // sha256 cs; - r = run (3, env, "--version", f, false, false, &cs); + r = run (ctx, 3, env, "--version", f, false, false, &cs); if (!r.empty ()) r.checksum = cs.string (); @@ -687,7 +695,7 @@ namespace build2 }; sha256 cs; - r = run (3, env, "/?", f, false, false, &cs); + r = run (ctx, 3, env, "/?", f, false, false, &cs); if (!r.empty ()) r.checksum = cs.string (); @@ -715,7 +723,7 @@ namespace build2 static global_cache nm_cache; const nm_info& - guess_nm (const path& nm, const char* paths) + guess_nm (context& ctx, const path& nm, const char* paths) { tracer trace ("bin::guess_nm"); @@ -799,7 +807,7 @@ namespace build2 // option. // sha256 cs; - r = run (3, env, "--version", f, false, false, &cs); + r = run (ctx, 3, env, "--version", f, false, false, &cs); if (!r.empty ()) r.checksum = cs.string (); diff --git a/libbuild2/bin/guess.hxx b/libbuild2/bin/guess.hxx index 52c0e1b..7dc7b33 100644 --- a/libbuild2/bin/guess.hxx +++ b/libbuild2/bin/guess.hxx @@ -54,7 +54,7 @@ namespace build2 // attemplated and the returned ranlib_* members will be left empty. // const ar_info& - guess_ar (const path& ar, const path* ranlib, const char* paths); + guess_ar (context&, const path& ar, const path* ranlib, const char* paths); // ld information. // @@ -100,7 +100,7 @@ namespace build2 }; const ld_info& - guess_ld (const path& ld, const char* paths); + guess_ld (context&, const path& ld, const char* paths); // rc information. // @@ -132,7 +132,7 @@ namespace build2 }; const rc_info& - guess_rc (const path& rc, const char* paths); + guess_rc (context&, const path& rc, const char* paths); // nm information. // @@ -166,7 +166,7 @@ namespace build2 }; const nm_info& - guess_nm (const path& nm, const char* paths); + guess_nm (context&, const path& nm, const char* paths); } } diff --git a/libbuild2/bin/init.cxx b/libbuild2/bin/init.cxx index 4b6090a..d7c5b5a 100644 --- a/libbuild2/bin/init.cxx +++ b/libbuild2/bin/init.cxx @@ -236,9 +236,9 @@ namespace build2 // if (!hint && config_sub) { - s = run (3, - *config_sub, - s.c_str (), + s = run (ctx, + 3, + *config_sub, s.c_str (), [] (string& l, bool) {return move (l);}); l5 ([&]{trace << "config.sub target: '" << s << "'";}); } @@ -692,7 +692,7 @@ namespace build2 nullptr, config::save_default_commented))); - const ar_info& ari (guess_ar (ar, ranlib, pat.paths)); + const ar_info& ari (guess_ar (rs.ctx, ar, ranlib, pat.paths)); // If this is a configuration with new values, then print the report // at verbosity level 2 and up (-v). @@ -843,7 +843,7 @@ namespace build2 path (apply_pattern (ld_d, pat.pattern)), config::save_default_commented))); - const ld_info& ldi (guess_ld (ld, pat.paths)); + const ld_info& ldi (guess_ld (rs.ctx, ld, pat.paths)); // If this is a configuration with new values, then print the report // at verbosity level 2 and up (-v). @@ -1004,7 +1004,7 @@ namespace build2 path (apply_pattern (rc_d, pat.pattern)), config::save_default_commented))); - const rc_info& rci (guess_rc (rc, pat.paths)); + const rc_info& rci (guess_rc (rs.ctx, rc, pat.paths)); // If this is a configuration with new values, then print the report // at verbosity level 2 and up (-v). @@ -1116,7 +1116,7 @@ namespace build2 path (apply_pattern (nm_d, pat.pattern)), config::save_default_commented))); - const nm_info& nmi (guess_nm (nm, pat.paths)); + const nm_info& nmi (guess_nm (rs.ctx, nm, pat.paths)); // If this is a configuration with new values, then print the report // at verbosity level 2 and up (-v). diff --git a/libbuild2/cc/compile-rule.cxx b/libbuild2/cc/compile-rule.cxx index 3a750a1..7221175 100644 --- a/libbuild2/cc/compile-rule.cxx +++ b/libbuild2/cc/compile-rule.cxx @@ -4608,7 +4608,7 @@ namespace build2 continue; } else - run_finish (args, pr); // Throws. + run_finish (args, pr); // Throws. @@ DBUF } catch (const process_error& e) { @@ -5008,7 +5008,7 @@ namespace build2 info << "then run failing command to display compiler diagnostics"; } else - run_finish (args, pr); // Throws. + run_finish (args, pr); // Throws. @@ DBUF } catch (const process_error& e) { @@ -7218,7 +7218,7 @@ namespace build2 args.push_back (nullptr); } - dbuf.finish (args, pr); + run_finish (dbuf, args, pr); } catch (const process_error& e) { @@ -7275,8 +7275,7 @@ namespace build2 env.empty () ? nullptr : env.data ()); dbuf.read (); - - dbuf.finish (args, pr); + run_finish (dbuf, args, pr); } catch (const process_error& e) { diff --git a/libbuild2/cc/gcc.cxx b/libbuild2/cc/gcc.cxx index 7999c82..6b24516 100644 --- a/libbuild2/cc/gcc.cxx +++ b/libbuild2/cc/gcc.cxx @@ -132,113 +132,94 @@ namespace build2 if (verb >= 3) print_process (env, args); + // Open pipe to stderr, redirect stdin and stdout to /dev/null. + // + process pr (run_start ( + env, + args, + -2, /* stdin */ + -2, /* stdout */ + {-1, -1} /* stderr */)); try { - //@@ TODO: why don't we use run_start() here? Because it's unable to - // open pipe for stderr and we need to change it first, for example, - // making the err parameter a file descriptor rather than a flag. - // + ifdstream is ( + move (pr.in_efd), fdstream_mode::skip, ifdstream::badbit); - // Open pipe to stderr, redirect stdin and stdout to /dev/null. + // Normally the system header paths appear between the following + // lines: // - process pr (xc, - args.data (), - -2, /* stdin */ - -2, /* stdout */ - -1, /* stderr */ - nullptr /* cwd */, - env.vars); - - try + // #include <...> search starts here: + // End of search list. + // + // The exact text depends on the current locale. What we can rely on + // is the presence of the "#include <...>" substring in the "opening" + // line and the fact that the paths are indented with a single space + // character, unlike the "closing" line. + // + // Note that on Mac OS we will also see some framework paths among + // system header paths, followed with a comment. For example: + // + // /Library/Frameworks (framework directory) + // + // For now we ignore framework paths and to filter them out we will + // only consider valid paths to existing directories, skipping those + // which we fail to normalize or stat. @@ Maybe this is a bit too + // loose, especially compared to gcc_library_search_dirs()? + // + string s; + for (bool found (false); getline (is, s); ) { - ifdstream is ( - move (pr.in_efd), fdstream_mode::skip, ifdstream::badbit); - - // Normally the system header paths appear between the following - // lines: - // - // #include <...> search starts here: - // End of search list. - // - // The exact text depends on the current locale. What we can rely on - // is the presence of the "#include <...>" substring in the - // "opening" line and the fact that the paths are indented with a - // single space character, unlike the "closing" line. - // - // Note that on Mac OS we will also see some framework paths among - // system header paths, followed with a comment. For example: - // - // /Library/Frameworks (framework directory) - // - // For now we ignore framework paths and to filter them out we will - // only consider valid paths to existing directories, skipping those - // which we fail to normalize or stat. @@ Maybe this is a bit too - // loose, especially compared to gcc_library_search_dirs()? - // - string s; - for (bool found (false); getline (is, s); ) + if (!found) + found = s.find ("#include <...>") != string::npos; + else { - if (!found) - found = s.find ("#include <...>") != string::npos; - else - { - if (s[0] != ' ') - break; + if (s[0] != ' ') + break; - dir_path d; - try - { - string ds (s, 1, s.size () - 1); + dir_path d; + try + { + string ds (s, 1, s.size () - 1); #ifdef _WIN32 - if (path_traits::is_separator (ds[0])) - add_current_drive (ds); + if (path_traits::is_separator (ds[0])) + add_current_drive (ds); #endif - d = dir_path (move (ds)); + d = dir_path (move (ds)); - if (d.relative () || !exists (d, true)) - continue; - - d.normalize (); - } - catch (const invalid_path&) - { + if (d.relative () || !exists (d, true)) continue; - } - if (find (r.begin (), r.end (), d) == r.end ()) - r.emplace_back (move (d)); + d.normalize (); } + catch (const invalid_path&) + { + continue; + } + + if (find (r.begin (), r.end (), d) == r.end ()) + r.emplace_back (move (d)); } + } - is.close (); // Don't block. + is.close (); // Don't block. - if (!pr.wait ()) - { - // We have read stderr so better print some diagnostics. - // - diag_record dr (fail); + if (!run_wait (args, pr)) + { + // We have read stderr so better print some diagnostics. + // + diag_record dr (fail); - dr << "failed to extract " << x_lang << " header search paths" << - info << "command line: "; + dr << "failed to extract " << x_lang << " header search paths" << + info << "command line: "; - print_process (dr, args); - } - } - catch (const io_error&) - { - pr.wait (); - fail << "error reading " << x_lang << " compiler -v -E output"; + print_process (dr, args); } } - catch (const process_error& e) + catch (const io_error&) { - error << "unable to execute " << args[0] << ": " << e; - - if (e.child) - exit (1); - - throw failed (); + run_wait (args, pr); + fail << "error reading " << x_lang << " compiler -v -E output"; } // It's highly unlikely not to have any system directories. More likely @@ -302,6 +283,9 @@ namespace build2 // Open pipe to stdout. // + // Note: this function is called in the serial load phase and so no + // diagnostics buffering is needed. + // process pr (run_start (env, args, 0, /* stdin */ diff --git a/libbuild2/cc/guess.cxx b/libbuild2/cc/guess.cxx index e851ea8..5e8dbbc 100644 --- a/libbuild2/cc/guess.cxx +++ b/libbuild2/cc/guess.cxx @@ -181,12 +181,12 @@ namespace build2 // could also be because there is something wrong with the compiler or // options but that we simply leave to blow up later). // - process pr (run_start (3 /* verbosity */, + process pr (run_start (3 /* verbosity */, xp, args, - -1 /* stdin */, - -1 /* stdout */, - false /* error */)); + -1 /* stdin */, + -1 /* stdout */, + {-1, 1} /* stderr (to stdout) */)); string l, r; try { @@ -817,7 +817,8 @@ namespace build2 // Note: allowed to change pre if succeeds. // static guess_result - guess (const char* xm, + guess (context& ctx, + const char* xm, lang xl, const path& xc, const strings& x_mo, @@ -1009,7 +1010,7 @@ namespace build2 #endif string cache; - auto run = [&cs, &env, &args, &cache] ( + auto run = [&ctx, &cs, &env, &args, &cache] ( const char* o, auto&& f, bool checksum = false) -> guess_result @@ -1017,9 +1018,10 @@ namespace build2 args[args.size () - 2] = o; cache.clear (); return build2::run ( + ctx, 3 /* verbosity */, env, - args.data (), + args, forward (f), false /* error */, false /* ignore_exit */, @@ -1318,7 +1320,11 @@ namespace build2 // const char* evars[] = {"CL=", "_CL_=", nullptr}; - r = build2::run (3, process_env (xp, evars), f, false); + r = build2::run (ctx, + 3, + process_env (xp, evars), + f, + false); if (r.empty ()) { @@ -1634,7 +1640,8 @@ namespace build2 "LIB", "LINK", "_LINK_", nullptr}; static compiler_info - guess_msvc (const char* xm, + guess_msvc (context&, + const char* xm, lang xl, const path& xc, const string* xv, @@ -1905,7 +1912,8 @@ namespace build2 "SDKROOT", "MACOSX_DEPLOYMENT_TARGET", nullptr}; static compiler_info - guess_gcc (const char* xm, + guess_gcc (context& ctx, + const char* xm, lang xl, const path& xc, const string* xv, @@ -2018,7 +2026,7 @@ namespace build2 // auto f = [] (string& l, bool) {return move (l);}; - t = run (3, xp, args.data (), f, false); + t = run (ctx, 3, xp, args, f, false); if (t.empty ()) { @@ -2026,7 +2034,7 @@ namespace build2 << "falling back to -dumpmachine";}); args[args.size () - 2] = "-dumpmachine"; - t = run (3, xp, args.data (), f, false); + t = run (ctx, 3, xp, args, f, false); } if (t.empty ()) @@ -2169,9 +2177,9 @@ namespace build2 process pr (run_start (3 /* verbosity */, xp, args, - -2 /* stdin (/dev/null) */, - -1 /* stdout */, - false /* error (2>&1) */)); + -2 /* stdin (to /dev/null) */, + -1 /* stdout */, + {-1, 1} /* stderr (to stdout) */)); clang_msvc_info r; @@ -2357,7 +2365,8 @@ namespace build2 nullptr}; static compiler_info - guess_clang (const char* xm, + guess_clang (context& ctx, + const char* xm, lang xl, const path& xc, const string* xv, @@ -2604,7 +2613,7 @@ namespace build2 // for LC_ALL. // auto f = [] (string& l, bool) {return move (l);}; - t = run (3, xp, args.data (), f, false); + t = run (ctx, 3, xp, args, f, false); if (t.empty ()) fail << "unable to extract target architecture from " << xc @@ -2832,7 +2841,8 @@ namespace build2 } static compiler_info - guess_icc (const char* xm, + guess_icc (context& ctx, + const char* xm, lang xl, const path& xc, const string* xv, @@ -2896,7 +2906,7 @@ namespace build2 // // @@ TODO: running without the mode options. // - s = run (3, env, "-V", f, false); + s = run (ctx, 3, env, "-V", f, false); if (s.empty ()) fail << "unable to extract signature from " << xc << " -V output"; @@ -3022,7 +3032,7 @@ namespace build2 // The -V output is sent to STDERR. // - t = run (3, env, args.data (), f, false); + t = run (ctx, 3, env, args, f, false); if (t.empty ()) fail << "unable to extract target architecture from " << xc @@ -3073,7 +3083,7 @@ namespace build2 // { auto f = [] (string& l, bool) {return move (l);}; - t = run (3, xp, "-dumpmachine", f); + t = run (ctx, 3, xp, "-dumpmachine", f); } if (t.empty ()) @@ -3154,7 +3164,8 @@ namespace build2 static global_cache cache; const compiler_info& - guess (const char* xm, + guess (context& ctx, + const char* xm, lang xl, const string& ec, const path& xc, @@ -3228,7 +3239,7 @@ namespace build2 if (pre.type != invalid_compiler_type) { - gr = guess (xm, xl, xc, x_mo, xi, pre, cs); + gr = guess (ctx, xm, xl, xc, x_mo, xi, pre, cs); if (gr.empty ()) { @@ -3244,13 +3255,14 @@ namespace build2 } if (gr.empty ()) - gr = guess (xm, xl, xc, x_mo, xi, pre, cs); + gr = guess (ctx, xm, xl, xc, x_mo, xi, pre, cs); if (gr.empty ()) fail << "unable to guess " << xl << " compiler type of " << xc << info << "use config." << xm << ".id to specify explicitly"; compiler_info (*gf) ( + context&, const char*, lang, const path&, const string*, const string*, const strings&, const strings*, const strings*, @@ -3270,7 +3282,8 @@ namespace build2 case compiler_type::icc: gf = &guess_icc; break; } - compiler_info r (gf (xm, xl, xc, xv, xt, + compiler_info r (gf (ctx, + xm, xl, xc, xv, xt, x_mo, c_po, x_po, c_co, x_co, c_lo, x_lo, move (gr), cs)); diff --git a/libbuild2/cc/guess.hxx b/libbuild2/cc/guess.hxx index 53acc15..7cbbd87 100644 --- a/libbuild2/cc/guess.hxx +++ b/libbuild2/cc/guess.hxx @@ -253,7 +253,8 @@ namespace build2 // that most of it will be the same, at least for C and C++. // const compiler_info& - guess (const char* xm, // Module (for var names in diagnostics). + guess (context&, + const char* xm, // Module (for var names in diagnostics). lang xl, // Language. const string& ec, // Environment checksum. const path& xc, // Compiler path. diff --git a/libbuild2/cc/link-rule.cxx b/libbuild2/cc/link-rule.cxx index 5d212e6..ff03fd9 100644 --- a/libbuild2/cc/link-rule.cxx +++ b/libbuild2/cc/link-rule.cxx @@ -2970,7 +2970,7 @@ namespace build2 } dbuf.read (); - dbuf.finish (args, pr, 2 /* verbosity */); + run_finish (dbuf, args, pr, 2 /* verbosity */); } catch (const process_error& e) { @@ -4116,23 +4116,10 @@ namespace build2 if (!ctx.dry_run) { -#if 0 - run (rl, + run (dbuf, + rl, args, - dir_path () /* cwd */, env_ptrs.empty () ? nullptr : env_ptrs.data ()); -#else - process pr (rl, - args, - 0 /* stdin */, - 1 /* stdout */, - dbuf.open (args[0]) /* stderr */, - nullptr /* cwd */, - env_ptrs.empty () ? nullptr : env_ptrs.data ()); - - dbuf.read (); - dbuf.finish (args, pr); -#endif } } diff --git a/libbuild2/cc/module.cxx b/libbuild2/cc/module.cxx index 6583001..9063ebc 100644 --- a/libbuild2/cc/module.cxx +++ b/libbuild2/cc/module.cxx @@ -30,6 +30,8 @@ namespace build2 { tracer trace (x, "guess_init"); + context& ctx (rs.ctx); + bool cc_loaded (cast_false (rs["cc.core.guess.loaded"])); // Adjust module priority (compiler). Also order cc module before us @@ -157,6 +159,7 @@ namespace build2 // we are now folding *.std options into mode options. // x_info = &build2::cc::guess ( + ctx, x, x_lang, rs.root_extra->environment_checksum, move (xc), @@ -183,7 +186,8 @@ namespace build2 if (config_sub) { - ct = run (3, + ct = run (ctx, + 3, *config_sub, xi.target.c_str (), [] (string& l, bool) {return move (l);}); diff --git a/libbuild2/cc/msvc.cxx b/libbuild2/cc/msvc.cxx index 765b1c6..3165602 100644 --- a/libbuild2/cc/msvc.cxx +++ b/libbuild2/cc/msvc.cxx @@ -433,9 +433,9 @@ namespace build2 // process pr (run_start (ld, args, - 0 /* stdin */, - -1 /* stdout */, - false /* error */)); + 0 /* stdin */, + -1 /* stdout */, + {-1, 1} /* stderr (to stdout) */)); bool obj (false), dll (false); string s; diff --git a/libbuild2/context.cxx b/libbuild2/context.cxx index 0108d27..4858c4c 100644 --- a/libbuild2/context.cxx +++ b/libbuild2/context.cxx @@ -255,7 +255,8 @@ namespace build2 // Did the user ask us to use config.guess? // string orig (config_guess - ? run (3, + ? run (*this, + 3, *config_guess, [](string& l, bool) {return move (l);}) : BUILD2_HOST_TRIPLET); diff --git a/libbuild2/diagnostics.cxx b/libbuild2/diagnostics.cxx index 629febf..3b0415c 100644 --- a/libbuild2/diagnostics.cxx +++ b/libbuild2/diagnostics.cxx @@ -50,12 +50,10 @@ namespace build2 // diag_buffer // process::pipe diag_buffer:: - open (const char* args0, bool force, bool blocking) + open (const char* args0, bool force, fdstream_mode m) { assert (state_ == state::closed && args0 != nullptr); - assert (blocking); // @@ TODO (also in read() and close () below). - serial = ctx_.sched.serial (); nobuf = !serial && ctx_.no_diag_buffer; @@ -71,7 +69,9 @@ namespace build2 // r = process::pipe (p.in.get (), move (p.out)); - is.open (move (p.in), fdstream_mode::text); + m |= fdstream_mode::text; + + is.open (move (p.in), m); } catch (const io_error& e) { @@ -96,6 +96,26 @@ namespace build2 { try { + // Copy buffers directly. + // + auto copy = [this] (fdstreambuf& sb) + { + 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)); + }; + if (is.blocking ()) { if (serial || nobuf) @@ -139,30 +159,30 @@ namespace build2 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)); - } + copy (sb); } r = false; } else { - // @@ TODO (maybe we can unify the two?) - r = true; + // We do not support finishing off after the custom processing in + // the non-blocking mode (but could probably do if necessary). + // + assert (!(serial || nobuf)); + + fdstreambuf& sb (*static_cast (is.rdbuf ())); + + // Try not to allocate the buffer if there is no diagnostics (the + // common case). + // + // Note that we must read until blocked (0) or EOF (-1). + // + streamsize n; + while ((n = sb.in_avail ()) > 0) + copy (sb); + + r = (n != -1); } if (!r) @@ -216,10 +236,53 @@ namespace build2 } void diag_buffer:: - close (const char* const* args, size_t args_size, + close (const char* const* args, const process_exit& pe, uint16_t v, - const location& loc) + const location& loc, + bool omit_normall) + { + tracer trace ("diag_buffer::close"); + + assert (state_ != state::closed); + + // 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(). + // + if (omit_normall && pe.normal ()) + { + l4 ([&]{trace << "process " << args[0] << " " << pe;}); + } + else + { + // It's unclear whether we should print this only if printing the + // command line (we could also do things differently for + // normal/abnormal exit). Let's print this always and see how it + // wears. + // + // Note: make sure keep the above trace is not printing. + // + dr << error (loc) << "process " << args[0] << " " << pe; + + if (verb >= 1 && verb <= v) + { + dr << info << "command line: "; + print_process (dr, args); + } + } + } + + close (move (dr)); + } + + void diag_buffer:: + close (diag_record&& dr) { assert (state_ != state::closed); @@ -231,9 +294,18 @@ namespace build2 { try { - // @@ TODO: is it ok to call peek() in non-blocking? - // - assert (is.peek () == ifdstream::traits_type::eof ()); + if (is.good ()) + { + if (is.blocking ()) + { + assert (is.peek () == ifdstream::traits_type::eof ()); + } + else + { + assert (is.rdbuf ()->in_avail () == -1); + } + } + is.close (); } catch (const io_error& e) @@ -245,28 +317,6 @@ namespace build2 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; @@ -290,31 +340,6 @@ namespace build2 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 diff --git a/libbuild2/diagnostics.hxx b/libbuild2/diagnostics.hxx index 48dfe36..1659a09 100644 --- a/libbuild2/diagnostics.hxx +++ b/libbuild2/diagnostics.hxx @@ -44,8 +44,9 @@ namespace build2 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. + // 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. // // The args0 argument is the child process program name for diagnostics. // It is expected to remain valid until the call to close() and should @@ -61,8 +62,14 @@ namespace build2 // and throw failed. If an exception is thrown from any of them, then the // instance should not be used any further. // + // Note that when reading from multiple streams in the non-blocking mode, + // 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 - open (const char* args0, bool force = false, bool blocking = true); + open (const char* args0, + bool force = false, + fdstream_mode mode = fdstream_mode::skip); // 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 @@ -82,10 +89,10 @@ namespace build2 // 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. + // reallocation. As a convenience, in the blocking mode only, if the + // stream still contains some diagnostics, then it can be handled by + // calling read(). This is useful when needing to process only the inital + // part of the diagnostics. // bool read (); @@ -93,70 +100,47 @@ namespace build2 // 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 child process exited abnormally or normally with non-0 code, + // then print the error diagnostics to this effect. Additionally, if the + // verbosity level is between 1 and the specified value, then print the + // command line as info after the error. If omit_normall is true, then + // don't print either for the normal exit (usually used when process + // failure can be tolerated). + // + // Normally the specified verbosity will be 1 and the command line args + // represent the verbosity level 2 (logical) command line. Note that args + // should only represent a single command in a pipe (see print_process() + // below for details). // // If the diag_buffer instance is destroyed before calling close(), then // any buffered diagnostics is discarded. // + // Note: see also run_finish(diag_buffer&). + // // @@ 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 = {}) + const location& loc = {}, + bool omit_normall = false) { - close (args.data (), args.size (), pe, verbosity, loc); + close (args.data (), pe, verbosity, loc, omit_normall); } 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 = {}); + const location& loc = {}, + bool omit_normall = false); - - // 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 ()). + // As above but with a custom diag record for the child exit diagnostics, + // if any. // 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& = {}); + close (diag_record&&); // Direct access to the underlying stream and buffer for custom processing // (see read() above for details). diff --git a/libbuild2/dist/operation.cxx b/libbuild2/dist/operation.cxx index 9a662df..f6d00e4 100644 --- a/libbuild2/dist/operation.cxx +++ b/libbuild2/dist/operation.cxx @@ -27,7 +27,7 @@ namespace build2 // install -d // static void - install (const process_path&, const dir_path&); + install (const process_path&, context&, const dir_path&); // install [/] // @@ -504,7 +504,7 @@ namespace build2 fail << "unable to clean target directory " << td; auto_rmdir rm_td (td); // Clean it up if things go bad. - install (dist_cmd, td); + install (dist_cmd, ctx, td); // Copy over all the files. Apply post-processing callbacks. // @@ -571,7 +571,7 @@ namespace build2 dir_path d (td / dl); if (!exists (d)) - install (dist_cmd, d); + install (dist_cmd, ctx, d); path r (install (dist_cmd, t, d, rn)); @@ -727,7 +727,7 @@ namespace build2 // install -d // static void - install (const process_path& cmd, const dir_path& d) + install (const process_path& cmd, context& ctx, const dir_path& d) { path reld (relative (d)); @@ -741,7 +741,7 @@ namespace build2 if (verb >= 2) print_process (args); - run (cmd, args); + run (ctx, cmd, args); } // install [/] @@ -784,7 +784,7 @@ namespace build2 if (verb >= 2) print_process (args); - run (cmd, args); + run (t.ctx, cmd, args); return d / (n.empty () ? relf.leaf () : n); } @@ -939,12 +939,13 @@ namespace build2 // Change the archiver's working directory to dist_root. // - apr = run_start (app, + // Note: this function is called during serial execution and so no + // diagnostics buffering is needed (here and below). + // + apr = run_start (process_env (app, root), args, 0 /* stdin */, - (i != 0 ? -1 : 1) /* stdout */, - true /* error */, - root); + (i != 0 ? -1 : 1) /* stdout */); // Start the compressor if required. // @@ -1022,12 +1023,13 @@ namespace build2 // Note that to only get the archive name (without the directory) in // the output we have to run from the archive's directory. // - process pr (run_start (pp, + // Note: this function is called during serial execution and so no + // diagnostics buffering is needed. + // + process pr (run_start (process_env (pp, ad /* cwd */), args, - 0 /* stdin */, - c_fd.get () /* stdout */, - true /* error */, - ad /* cwd */)); + 0 /* stdin */, + c_fd.get () /* stdout */)); run_finish (args, pr); } else diff --git a/libbuild2/functions-process.cxx b/libbuild2/functions-process.cxx index c4e5c24..28b7a99 100644 --- a/libbuild2/functions-process.cxx +++ b/libbuild2/functions-process.cxx @@ -4,6 +4,8 @@ #include #include +#include +#include #include #include @@ -181,18 +183,32 @@ namespace build2 } static inline value - run_builtin (builtin_function* bf, const strings& args, const string& bn) + run_builtin (const scope* s, + builtin_function* bf, + const strings& args, + const string& bn) { + // See below. + // + if (s != nullptr && s->ctx.phase != run_phase::load) + fail << "process.run() called during " << s->ctx.phase << " phase"; + return run_builtin_impl (bf, args, bn, read); } static inline value - run_builtin_regex (builtin_function* bf, + run_builtin_regex (const scope* s, + builtin_function* bf, const strings& args, const string& bn, const string& pat, const optional& fmt) { + // See below. + // + if (s != nullptr && s->ctx.phase != run_phase::load) + fail << "process.run_regex() called during " << s->ctx.phase << " phase"; + // Note that we rely on the "small function object" optimization here. // return run_builtin_impl (bf, args, bn, @@ -293,6 +309,9 @@ namespace build2 [] (const string& s) {return s.c_str ();}); cargs.push_back (nullptr); + // Note that for now these functions can only be called during the load + // phase (see below) and so no diagnostics buffering is needed. + // return run_start (3 /* verbosity */, pp, cargs, @@ -352,6 +371,15 @@ namespace build2 static inline value run_process (const scope* s, const process_path& pp, const strings& args) { + // The only plausible place where these functions can be called outside + // the load phase are scripts and there it doesn't make much sense to use + // them (the same can be achieved with commands in a uniform manner). Note + // that if there is no scope, then this is most likely (certainly?) the + // load phase (for example, command line). + // + if (s != nullptr && s->ctx.phase != run_phase::load) + fail << "process.run() called during " << s->ctx.phase << " phase"; + return run_process_impl (s, pp, args, read); } @@ -362,6 +390,11 @@ namespace build2 const string& pat, const optional& fmt) { + // See above. + // + if (s != nullptr && s->ctx.phase != run_phase::load) + fail << "process.run_regex() called during " << s->ctx.phase << " phase"; + // Note that we rely on the "small function object" optimization here. // return run_process_impl (s, pp, args, @@ -377,7 +410,7 @@ namespace build2 if (builtin_function* bf = builtin (args)) { pair ba (builtin_args (bf, move (args), "run")); - return run_builtin (bf, ba.second, ba.first); + return run_builtin (s, bf, ba.second, ba.first); } else { @@ -395,7 +428,7 @@ namespace build2 if (builtin_function* bf = builtin (args)) { pair ba (builtin_args (bf, move (args), "run_regex")); - return run_builtin_regex (bf, ba.second, ba.first, pat, fmt); + return run_builtin_regex (s, bf, ba.second, ba.first, pat, fmt); } else { @@ -420,7 +453,8 @@ namespace build2 // result, then such variables should be reported with the // config.environment directive. // - // Note that this function is not pure. + // Note that this function is not pure and can only be called during the + // load phase. // f.insert (".run", false) += [](const scope* s, names args) { @@ -446,7 +480,8 @@ namespace build2 // result, then such variables should be reported with the // config.environment directive. // - // Note that this function is not pure. + // Note that this function is not pure and can only be called during the + // load phase. // { auto e (f.insert (".run_regex", false)); diff --git a/libbuild2/install/rule.cxx b/libbuild2/install/rule.cxx index 9f01578..aa13169 100644 --- a/libbuild2/install/rule.cxx +++ b/libbuild2/install/rule.cxx @@ -848,7 +848,7 @@ namespace build2 text << "install " << chd; } - run (pp, args); + run (ctx, pp, args); } void file_rule:: @@ -903,7 +903,7 @@ namespace build2 } if (!ctx.dry_run) - run (pp, args); + run (ctx, pp, args); } void file_rule:: @@ -946,7 +946,7 @@ namespace build2 } if (!ctx.dry_run) - run (pp, args); + run (ctx, pp, args); #else // The -f part. // @@ -1179,8 +1179,13 @@ namespace build2 text << "uninstall " << reld; } - process pr (run_start (pp, args)); - r = run_finish_code (args, pr); + diag_buffer dbuf (rs.ctx); + process pr (run_start (pp, args, + 0 /* stdin */, + 1 /* stdout */, + dbuf.open (args[0]) /* stderr */)); + dbuf.read (); + r = run_finish_code (dbuf, args, pr); } if (!r) @@ -1279,7 +1284,7 @@ namespace build2 print_process (args); if (!ctx.dry_run) - run (pp, args); + run (ctx, pp, args); } return true; diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx index ed3b144..9b0c7a3 100644 --- a/libbuild2/parser.cxx +++ b/libbuild2/parser.cxx @@ -3126,13 +3126,16 @@ namespace build2 [] (const string& s) {return s.c_str ();}); cargs.push_back (nullptr); + // Note: we are in the serial load phase and so no diagnostics buffering + // is needed. + // process pr (run_start (3 /* verbosity */, cargs, 0 /* stdin */, -1 /* stdout */, - true /* error */, - dir_path () /* cwd */, + {-1, 2} /* stderr */, nullptr /* env */, + dir_path () /* cwd */, l)); try { diff --git a/libbuild2/script/run.cxx b/libbuild2/script/run.cxx index 1971bd1..0d9e472 100644 --- a/libbuild2/script/run.cxx +++ b/libbuild2/script/run.cxx @@ -1015,6 +1015,8 @@ namespace build2 // // Set the empty_ flag to false after the first character is read. // + // @@ PERF: reading one character at a time is not ideal. + // auto get = [this] () -> optional { char r; diff --git a/libbuild2/types.hxx b/libbuild2/types.hxx index d93da6d..bf412a3 100644 --- a/libbuild2/types.hxx +++ b/libbuild2/types.hxx @@ -429,6 +429,7 @@ namespace build2 // // + using butl::nullfd; using butl::auto_fd; using butl::fdpipe; using butl::ifdstream; diff --git a/libbuild2/utility.cxx b/libbuild2/utility.cxx index e73cd61..aa67c92 100644 --- a/libbuild2/utility.cxx +++ b/libbuild2/utility.cxx @@ -218,8 +218,7 @@ namespace build2 const char* args[], int in, int out, - bool err, - const dir_path& cwd, + process::pipe err, const location& l) try { @@ -233,10 +232,8 @@ namespace build2 args, in, out, - (err ? 2 : 1), - (!cwd.empty () - ? cwd.string ().c_str () - : pe.cwd != nullptr ? pe.cwd->string ().c_str () : nullptr), + move (err), + pe.cwd != nullptr ? pe.cwd->string ().c_str () : nullptr, pe.vars); } catch (const process_error& e) @@ -258,7 +255,7 @@ namespace build2 } bool - run_wait (const char* args[], process& pr, const location& loc) + run_wait (const char* const* args, process& pr, const location& loc) try { return pr.wait (); @@ -269,17 +266,23 @@ namespace build2 } bool - run_finish_impl (const char* args[], + run_finish_impl (const char* const* args, process& pr, - bool err, + bool f, const string& l, const location& loc) - try { tracer trace ("run_finish"); - if (pr.wait ()) - return true; + try + { + if (pr.wait ()) + return true; + } + catch (const process_error& e) + { + fail (loc) << "unable to execute " << args[0] << ": " << e << endf; + } // Note: see similar code in diag_buffer::close/finish(). // @@ -290,16 +293,6 @@ namespace build2 // Normall but non-zero exit status. // - if (err) - { - // While we assuming diagnostics has already been issued (to STDERR), if - // that's not the case, it's a real pain to debug. So trace it. - // - l4 ([&]{trace << "process " << args[0] << " " << e;}); - - throw failed (); - } - // Even if the user asked to suppress diagnostiscs, one error that we // want to let through is the inability to execute the program itself. // We cannot reserve a special exit status to signal this so we will @@ -307,19 +300,281 @@ namespace build2 // result in a single error line printed by run_start() above. // if (l.compare (0, 18, "unable to execute ") == 0) - fail (loc) << l; + error (loc) << l; + + if (f) + { + // While we assume diagnostics has already been issued (to stderr), if + // that's not the case, it's a real pain to debug. So trace it. (And + // if you think that doesn't happen in sensible programs, check GCC + // bug #107448). + // + l4 ([&]{trace << "process " << args[0] << " " << e;}); + + throw failed (); + } return false; } - catch (const process_error& e) + + bool + run_finish_impl (diag_buffer& dbuf, + const char* const* args, + process& pr, + bool f, + uint16_t v, + const location& loc) { - fail (loc) << "unable to execute " << args[0] << ": " << e << endf; + try + { + pr.wait (); + } + catch (const process_error& e) + { + fail (loc) << "unable to execute " << args[0] << ": " << e << endf; + } + + const process_exit& pe (*pr.exit); + + dbuf.close (args, pe, v, loc, !f /* omit_normall */); + + if (pe) + return true; + + if (f) + throw failed (); + + return false; + } + + void + run (context& ctx, const process_env& pe, const char* args[]) + { + if (ctx.phase == run_phase::load) + { + process pr (run_start (pe, args)); + run_finish (args, pr); + } + else + { + diag_buffer dbuf (ctx); + run (dbuf, pe, args); + } } void - run_io_error (const char* args[], const io_error& e) + run (diag_buffer& dbuf, const process_env& pe, const char* args[]) { - fail << "io error reading " << args[0] << " output: " << e << endf; + process pr (run_start (pe, + args, + 0 /* stdin */, + 1 /* stdout */, + dbuf.open (args[0]) /* stderr */)); + dbuf.read (); + run_finish (dbuf, args, pr); + } + + bool + run (context& ctx, + uint16_t verbosity, + const process_env& pe, + const char* args[], + const function& f, + bool tr, + bool err, + bool ignore_exit, + sha256* checksum) + { + if (err && ctx.phase != run_phase::load) + { + diag_buffer dbuf (ctx); + return run (dbuf, verbosity, pe, args, f, tr, ignore_exit, checksum); + } + + process pr (run_start (verbosity, + pe, + args, + 0 /* stdin */, + -1 /* stdout */, + {-1, err ? 2 : 1})); + + string l; // Last line of output. + try + { + ifdstream is (move (pr.in_ofd), fdstream_mode::skip); + + bool empty (true); + + // 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 (tr) + trim (l); + + if (checksum != nullptr) + checksum->append (l); + + if (empty) + { + empty = f (l, last); + + if (!empty && checksum == nullptr) + break; + } + } + + is.close (); + } + catch (const io_error& e) + { + 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. + } + + if (!(run_finish_impl (args, pr, err, l) || ignore_exit)) + return false; + + return true; + } + + bool + run (diag_buffer& dbuf, + uint16_t verbosity, + const process_env& pe, + const char* args[], + const function& f, + bool tr, + bool ignore_exit, + 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 */)); + + 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); + + bool empty (true); + + // Read until we reach EOF on all streams. + // + // Note that if dbuf is not opened, then we automatically get 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; + + for (string l; ist.fd != nullfd || dst.fd != nullfd; ) + { + if (ist.fd != nullfd && getline_non_blocking (is, l)) + { + if (eof (is)) + { + if (pl && empty) + f (*pl, true /* last */); + + ist.fd = nullfd; + } + else + { + if (checksum != nullptr || empty) + { + if (tr) + trim (l); + + if (checksum != nullptr) + checksum->append (l); + + if (empty) + { + 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. + } + else + pl = move (l); + } + } + + l.clear (); + } + + continue; + } + + ifdselect (fds); + + if (dst.ready) + { + if (!dbuf.read ()) + dst.fd = nullfd; + } + } + + 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 (ignore_exit) + { + if (!run_finish_code (dbuf, args, pr, verbosity - 1)) + return false; + } + else + run_finish (dbuf, args, pr, verbosity - 1); + + return true; } fdpipe diff --git a/libbuild2/utility.hxx b/libbuild2/utility.hxx index 88ea43d..a2259df 100644 --- a/libbuild2/utility.hxx +++ b/libbuild2/utility.hxx @@ -261,46 +261,13 @@ namespace build2 [[noreturn]] LIBBUILD2_SYMEXPORT void run_search_fail (const path&, const location& = location ()); - // Wait for process termination returning true if the process exited - // normally with a zero code and false otherwise. The latter case is - // normally followed up with a call to run_finish(). - // - LIBBUILD2_SYMEXPORT bool - run_wait (const char* args[], process&, const location& = location ()); - - bool - run_wait (cstrings& args, process&, const location& = location ()); - - // Wait for process termination. Issue diagnostics and throw failed in case - // of abnormal termination. If the process has terminated normally but with - // a non-zero exit status, then assume the diagnostics has already been - // issued and just throw failed. The last argument is used in cooperation - // with run_start() in case STDERR is redirected to STDOUT. - // - void - run_finish (const char* args[], - process&, - const string& = string (), - const location& = location ()); - - void - run_finish (cstrings& args, process& pr, const location& l = location ()); - - // As above but if the process has exited normally with a non-zero code, - // then return false rather than throwing. - // - bool - run_finish_code (const char* args[], - process&, - const string& = string (), - const location& = location ()); - - // Start a process with the specified arguments. If in is -1, then redirect - // STDIN to a pipe (can also be -2 to redirect to /dev/null or equivalent). - // If out is -1, redirect STDOUT to a pipe. If error is false, then - // redirecting STDERR to STDOUT (this can be used to suppress diagnostics - // from the child process). Issue diagnostics and throw failed in case of an - // error. + // Start a process with the specified arguments. Issue diagnostics and throw + // failed in case of an error. If in is -1, then redirect stdin to a pipe + // (can also be -2 to redirect it to /dev/null or equivalent). If out is -1, + // then redirect stdout to a pipe. If stderr is redirected to stdout (can + // be used to analyze diagnostics from the child process), then, in case of + // an error, the last line read from stdout must be passed to run_finish() + // below. // LIBBUILD2_SYMEXPORT process run_start (uint16_t verbosity, @@ -308,9 +275,8 @@ namespace build2 const char* args[], int in = 0, int out = 1, - bool error = true, - const dir_path& cwd = dir_path (), - const location& = location ()); + process::pipe = {-1, 2}, + const location& = {}); inline process run_start (uint16_t verbosity, @@ -318,11 +284,10 @@ namespace build2 cstrings& args, int in = 0, int out = 1, - bool error = true, - const dir_path& cwd = dir_path (), - const location& l = location ()) + process::pipe err = {-1, 2}, + const location& l = {}) { - return run_start (verbosity, pe, args.data (), in, out, error, cwd, l); + return run_start (verbosity, pe, args.data (), in, out, move (err), l); } inline process @@ -330,11 +295,10 @@ namespace build2 const char* args[], int in = 0, int out = 1, - bool error = true, - const dir_path& cwd = dir_path (), - const location& l = location ()) + process::pipe err = {-1, 2}, + const location& l = {}) { - return run_start (verb_never, pe, args, in, out, error, cwd, l); + return run_start (verb_never, pe, args, in, out, move (err), l); } inline process @@ -342,45 +306,10 @@ namespace build2 cstrings& args, int in = 0, int out = 1, - bool error = true, - const dir_path& cwd = dir_path (), - const location& l = location ()) + process::pipe err = {-1, 2}, + const location& l = {}) { - return run_start (pe, args.data (), in, out, error, cwd, l); - } - - inline void - run (const process_env& pe, // Implicit-constructible from process_path. - const char* args[]) - { - process pr (run_start (pe, args)); - run_finish (args, pr); - } - - inline void - run (const process_env& pe, // Implicit-constructible from process_path. - cstrings& args) - { - run (pe, args.data ()); - } - - inline void - run (const process_path& p, - const char* args[], - const dir_path& cwd, - const char* const* env = nullptr) - { - process pr (run_start (process_env (p, env), args, 0, 1, true, cwd)); - run_finish (args, pr); - } - - inline void - run (const process_path& p, - cstrings& args, - const dir_path& cwd, - const char* const* env = nullptr) - { - run (p, args.data (), cwd, env); + return run_start (pe, args.data (), in, out, move (err), l); } // As above, but search for the process (including updating args[0]) and @@ -391,16 +320,16 @@ namespace build2 const char* args[], int in = 0, int out = 1, - bool error = true, - const dir_path& cwd = dir_path (), + process::pipe err = {-1, 2}, const char* const* env = nullptr, - const location& l = location ()) + const dir_path& cwd = {}, + const location& l = {}) { process_path pp (run_search (args[0], l)); return run_start (verbosity, - process_env (pp, env), args, - in, out, error, - cwd, l); + process_env (pp, cwd, env), args, + in, out, move (err), + l); } inline process @@ -408,37 +337,184 @@ namespace build2 cstrings& args, int in = 0, int out = 1, - bool error = true, - const dir_path& cwd = dir_path (), + process::pipe err = {-1, 2}, const char* const* env = nullptr, - const location& l = location ()) + const dir_path& cwd = {}, + const location& l = {}) { - return run_start (verbosity, args.data (), in, out, error, cwd, env, l); + return run_start (verbosity, + args.data (), + in, out, move (err), + env, cwd, l); + } + + // Wait for process termination returning true if the process exited + // normally with a zero code and false otherwise. The latter case is + // normally followed up with a call to run_finish(). + // + LIBBUILD2_SYMEXPORT bool + run_wait (const char* const* args, process&, const location& = location ()); + + bool + run_wait (const cstrings& args, process&, const location& = location ()); + + // Wait for process termination. Issue diagnostics and throw failed in case + // of abnormal termination. If the process has terminated normally but with + // a non-zero exit status, then assume the diagnostics has already been + // issued and just throw failed. The line argument is used in cooperation + // with run_start() in case stderr is redirected to stdout (see the + // implementation for details). + // + void + run_finish (const char* const* args, + process&, + const string& line = string (), + const location& = location ()); + + void + run_finish (const cstrings& args, + process&, + const location& = location ()); + + // As above but if the process has exited normally with a non-zero code, + // then return false rather than throwing. + // + bool + run_finish_code (const char* const* args, + process&, + const string& = string (), + const location& = location ()); + + bool + run_finish_code (const cstrings& args, + process&, + const location& = location ()); + + // As above but with diagnostics buffering. + // + // Specifically, this version first waits for the process termination, then + // calls diag_buffer::close(verbosity), and finally throws failed if the + // process didn't exit with 0 code. Note that what gets printed in case of + // normal termination with non-0 code is different compared to the above + // versions (see diag_buffer::close() for details). + // + class diag_buffer; + + void + run_finish (diag_buffer&, + const char* const* args, + process&, + uint16_t verbosity = 1, + const location& = location ()); + + void + run_finish (diag_buffer&, + const cstrings& args, + process&, + uint16_t verbosity = 1, + const location& = location ()); + + // As above but if the process has exited normally with a non-zero code, + // then return false rather than throwing. Note: diag_buffer::close() is + // called with omit_normall=true assuming appropriate custom diagnostics + // will be issued, if required. + // + bool + run_finish_code (diag_buffer&, + const char* const* args, + process&, + uint16_t verbosity = 1, + const location& = location ()); + + bool + run_finish_code (diag_buffer&, + const cstrings& args, + process&, + uint16_t verbosity = 1, + const location& = location ()); + + // Run the process with the specified arguments by calling the above start + // and finish functions. Buffer diagnostics unless in the load phase. + // + LIBBUILD2_SYMEXPORT void + run (context&, + const process_env& pe, // Implicit-constructible from process_path. + const char* args[]); + + LIBBUILD2_SYMEXPORT void + run (diag_buffer&, + const process_env& pe, + const char* args[]); + + inline void + run (context& ctx, + const process_env& pe, + cstrings& args) + { + run (ctx, pe, args.data ()); } inline void - run (uint16_t verbosity, + run (diag_buffer& dbuf, + const process_env& pe, + cstrings& args) + { + run (dbuf, pe, args.data ()); + } + + // As above but pass cwd/env vars as arguments rather than as part of + // process_env. + // + inline void + run (context& ctx, + const process_path& p, + const char* args[], + const char* const* env, + const dir_path& cwd = {}) + { + run (ctx, process_env (p, cwd, env), args); + } + + inline void + run (diag_buffer& dbuf, + const process_path& p, const char* args[], - const dir_path& cwd = dir_path (), - const char* const* env = nullptr) + const char* const* env, + const dir_path& cwd = {}) { - process pr (run_start (verbosity, args, 0, 1, true, cwd, env)); - run_finish (args, pr); + run (dbuf, process_env (p, cwd, env), args); } inline void - run (uint16_t verbosity, + run (context& ctx, + const process_path& p, cstrings& args, - const dir_path& cwd = dir_path (), - const char* const* env = nullptr) + const char* const* env, + const dir_path& cwd = {}) { - run (verbosity, args.data (), cwd, env); + run (ctx, p, args.data (), env, cwd); + } + + inline void + run (diag_buffer& dbuf, + const process_path& p, + cstrings& args, + const char* const* env, + const dir_path& cwd = {}) + { + run (dbuf, p, args.data (), 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. // + // If verbosity is specified, print the process commands line at that level. + // + // If error is false, then redirecting stderr to stdout (can be used to + // suppress and/or analyze diagnostics from the child process). Otherwise, + // buffer diagnostics unless in the load phase. + // // The predicate can move the value out of the passed string but, if error // is false, only in case of a "content match" (so that any diagnostics // lines are left intact). The function signature should be: @@ -454,7 +530,8 @@ namespace build2 // template T - run (uint16_t verbosity, + run (context&, + uint16_t verbosity, const process_env&, // Implicit-constructible from process_path. const char* args[], F&&, @@ -464,20 +541,117 @@ namespace build2 template inline T - run (const process_env& pe, // Implicit-constructible from process_path. + run (context& ctx, + uint16_t verbosity, + const process_env& pe, + cstrings& args, + F&& f, + bool error = true, + bool ignore_exit = false, + sha256* checksum = nullptr) + { + return run (ctx, + verbosity, + pe, args.data (), + forward (f), + error, ignore_exit, checksum); + } + + template + T + run (diag_buffer&, + uint16_t verbosity, + const process_env&, + const char* args[], + F&&, + bool ignore_exit = false, + sha256* checksum = nullptr); + + template + inline T + run (diag_buffer& dbuf, + uint16_t verbosity, + const process_env& pe, + cstrings& args, + F&& f, + bool ignore_exit = false, + sha256* checksum = nullptr) + { + return run (dbuf, + verbosity, + pe, args.data (), + forward (f), + ignore_exit, checksum); + } + + template + inline T + run (context& ctx, + const process_env& pe, const char* args[], F&& f, bool error = true, bool ignore_exit = false, sha256* checksum = nullptr) { - return run ( - verb_never, pe, args, forward (f), error, ignore_exit, checksum); + return run (ctx, + verb_never, + pe, args, + forward (f), + error, ignore_exit, checksum); + } + + template + inline T + run (context& ctx, + const process_env& pe, + cstrings& args, + F&& f, + bool error = true, + bool ignore_exit = false, + sha256* checksum = nullptr) + { + return run (ctx, + pe, args.data (), + forward (f), + error, ignore_exit, checksum); + } + + template + inline T + run (diag_buffer& dbuf, + const process_env& pe, + const char* args[], + F&& f, + bool ignore_exit = false, + sha256* checksum = nullptr) + { + return run (dbuf, + verb_never, + pe, args, + forward (f), + ignore_exit, checksum); + } + + template + inline T + run (diag_buffer& dbuf, + const process_env& pe, + cstrings& args, + F&& f, + bool ignore_exit = false, + sha256* checksum = nullptr) + { + return run (dbuf, + pe, args.data (), + forward (f), + ignore_exit, checksum); } template inline T - run (uint16_t verbosity, + run (context& ctx, + uint16_t verbosity, const char* args[], F&& f, bool error = true, @@ -485,15 +659,71 @@ namespace build2 sha256* checksum = nullptr) { process_path pp (run_search (args[0])); - return run ( - verbosity, pp, args, forward (f), error, ignore_exit, checksum); + return run (ctx, + verbosity, + pp, args, + forward (f), + error, ignore_exit, checksum); + } + + template + inline T + run (context& ctx, + uint16_t verbosity, + cstrings& args, + F&& f, + bool error = true, + bool ignore_exit = false, + sha256* checksum = nullptr) + { + return run (ctx, + verbosity, + args.data (), + forward (f), + error, ignore_exit, checksum); + } + + template + inline T + run (diag_buffer& dbuf, + uint16_t verbosity, + const char* args[], + F&& f, + bool ignore_exit = false, + sha256* checksum = nullptr) + { + process_path pp (run_search (args[0])); + return run (dbuf, + verbosity, + pp, args, + forward (f), + ignore_exit, checksum); + } + + template + inline T + run (diag_buffer& dbuf, + uint16_t verbosity, + cstrings& args, + F&& f, + bool ignore_exit = false, + sha256* checksum = nullptr) + { + return run (dbuf, + verbosity, + args.data (), + forward (f), + ignore_exit, checksum); } + // As above but run a program without any arguments or with one argument. + // // run // template inline T - run (uint16_t verbosity, + run (context& ctx, + uint16_t verbosity, const path& prog, F&& f, bool error = true, @@ -501,13 +731,17 @@ namespace build2 sha256* checksum = nullptr) { const char* args[] = {prog.string ().c_str (), nullptr}; - return run ( - verbosity, args, forward (f), error, ignore_exit, checksum); + return run (ctx, + verbosity, + args, + forward (f), + error, ignore_exit, checksum); } template inline T - run (uint16_t verbosity, + run (context& ctx, + uint16_t verbosity, const process_env& pe, // Implicit-constructible from process_path. F&& f, bool error = true, @@ -515,15 +749,19 @@ namespace build2 sha256* checksum = nullptr) { const char* args[] = {pe.path->recall_string (), nullptr}; - return run ( - verbosity, pe, args, forward (f), error, ignore_exit, checksum); + return run (ctx, + verbosity, + pe, args, + forward (f), + error, ignore_exit, checksum); } // run // template inline T - run (uint16_t verbosity, + run (context& ctx, + uint16_t verbosity, const path& prog, const char* arg, F&& f, @@ -532,13 +770,17 @@ namespace build2 sha256* checksum = nullptr) { const char* args[] = {prog.string ().c_str (), arg, nullptr}; - return run ( - verbosity, args, forward (f), error, ignore_exit, checksum); + return run (ctx, + verbosity, + args, + forward (f), + error, ignore_exit, checksum); } template inline T - run (uint16_t verbosity, + run (context& ctx, + uint16_t verbosity, const process_env& pe, // Implicit-constructible from process_path. const char* arg, F&& f, @@ -547,10 +789,45 @@ namespace build2 sha256* checksum = nullptr) { const char* args[] = {pe.path->recall_string (), arg, nullptr}; - return run ( - verbosity, pe, args, forward (f), error, ignore_exit, checksum); + return run (ctx, + verbosity, + pe, args, + forward (f), + error, ignore_exit, checksum); } + // As above but a lower-level interface that erases T and F and can also be + // used to suppress trimming. + // + // The passed function should return true if it should be called again + // (i.e., the object is still empty in the T & F interface) and false + // otherwise. + // + // Ruturn true on success and false on failure (only if ignore_exit is + // true). (In the latter case, the T & F interface makes the resulting + // object empty). + // + LIBBUILD2_SYMEXPORT bool + run (context&, + uint16_t verbosity, + const process_env&, + const char* args[], + const function&, + bool trim = true, + bool err = true, + bool ignore_exit = false, + sha256* checksum = nullptr); + + LIBBUILD2_SYMEXPORT bool + run (diag_buffer& dbuf, + uint16_t verbosity, + const process_env&, + const char* args[], + const function&, + bool trim = true, + bool ignore_exit = false, + sha256* checksum = nullptr); + // File descriptor streams. // fdpipe diff --git a/libbuild2/utility.ixx b/libbuild2/utility.ixx index aedfc94..f4b8152 100644 --- a/libbuild2/utility.ixx +++ b/libbuild2/utility.ixx @@ -6,42 +6,152 @@ namespace build2 { inline bool - run_wait (cstrings& args, process& pr, const location& loc) + run_wait (const cstrings& args, process& pr, const location& loc) { return run_wait (args.data (), pr, loc); } - // Note: currently this function is also used in a run() implementations. + // Note: currently this function is also used in run() implementations. // LIBBUILD2_SYMEXPORT bool - run_finish_impl (const char*[], + run_finish_impl (const char* const*, process&, - bool error, + bool fail, const string&, const location& = location ()); + LIBBUILD2_SYMEXPORT bool + run_finish_impl (diag_buffer&, + const char* const*, + process&, + bool fail, + uint16_t, + const location&); + inline void - run_finish (const char* args[], + run_finish (const char* const* args, process& pr, const string& l, const location& loc) { - run_finish_impl (args, pr, true /* error */, l, loc); + run_finish_impl (args, pr, true /* fail */, l, loc); } inline void - run_finish (cstrings& args, process& pr, const location& loc) + run_finish (const cstrings& args, process& pr, const location& loc) { run_finish (args.data (), pr, string (), loc); } inline bool - run_finish_code (const char* args[], + run_finish_code (const char* const* args, process& pr, const string& l, const location& loc) { - return run_finish_impl (args, pr, false /* error */, l, loc); + return run_finish_impl (args, pr, false /* fail */, l, loc); + } + + inline bool + run_finish_code (const cstrings& args, process& pr, const location& loc) + { + return run_finish_code (args.data (), pr, string (), loc); + } + + inline void + run_finish (diag_buffer& dbuf, + const char* const* args, + process& pr, + uint16_t v, + const location& loc) + { + run_finish_impl (dbuf, args, pr, true /* fail */, v, loc); + } + + inline void + run_finish (diag_buffer& dbuf, + const cstrings& args, + process& pr, + uint16_t v, + const location& loc) + { + run_finish_impl (dbuf, args.data (), pr, true /* fail */, v, loc); + } + + inline bool + run_finish_code (diag_buffer& dbuf, + const char* const* args, + process& pr, + uint16_t v, + const location& loc) + { + return run_finish_impl (dbuf, args, pr, false /* fail */, v, loc); + } + + inline bool + run_finish_code (diag_buffer& dbuf, + const cstrings& args, + process& pr, + uint16_t v, + const location& loc) + { + return run_finish_impl (dbuf, args.data (), pr, false /* fail */, v, loc); + } + + template + inline T + run (context& ctx, + uint16_t verbosity, + const process_env& pe, + const char* args[], + F&& f, + bool err, + bool ignore_exit, + sha256* checksum) + { + T r; + if (!run (ctx, + verbosity, + pe, args, + [&r, &f] (string& l, bool last) // Small function optimmization. + { + r = f (l, last); + return r.empty (); + }, + true /* trim */, + err, + ignore_exit, + checksum)) + r = T (); + + return r; + } + + template + inline T + run (diag_buffer& dbuf, + uint16_t verbosity, + const process_env& pe, + const char* args[], + F&& f, + bool ignore_exit, + sha256* checksum) + { + T r; + if (!run (dbuf, + verbosity, + pe, args, + [&r, &f] (string& l, bool last) // Small function optimmization. + { + r = f (l, last); + return r.empty (); + }, + true /* trim */, + ignore_exit, + checksum)) + r = T (); + + return r; } inline void diff --git a/libbuild2/utility.txx b/libbuild2/utility.txx index bb25288..d2fc29c 100644 --- a/libbuild2/utility.txx +++ b/libbuild2/utility.txx @@ -54,68 +54,4 @@ namespace build2 return p; } - - [[noreturn]] LIBBUILD2_SYMEXPORT void - run_io_error (const char*[], const io_error&); - - template - T - run (uint16_t verbosity, - const process_env& pe, - const char* args[], - F&& f, - bool err, - bool ignore_exit, - sha256* checksum) - { - process pr (run_start (verbosity, - pe, - args, - 0 /* stdin */, - -1 /* stdout */, - err)); - T r; - string l; // Last line of output. - - try - { - ifdstream is (move (pr.in_ofd), butl::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 ()); - - trim (l); - - if (checksum != nullptr) - checksum->append (l); - - if (r.empty ()) - { - r = f (l, last); - - if (!r.empty () && checksum == nullptr) - break; - } - } - - is.close (); - } - catch (const io_error& e) - { - if (run_wait (args, pr)) - run_io_error (args, e); - - // If the child process has failed then assume the io error was - // caused by that and let run_finish() deal with it. - } - - if (!(run_finish_impl (args, pr, err, l) || ignore_exit)) - r = T (); - - return r; - } } diff --git a/libbuild2/version/snapshot-git.cxx b/libbuild2/version/snapshot-git.cxx index 2ae3f5b..c4168bf 100644 --- a/libbuild2/version/snapshot-git.cxx +++ b/libbuild2/version/snapshot-git.cxx @@ -21,7 +21,7 @@ namespace build2 static global_cache cache; snapshot - extract_snapshot_git (dir_path rep_root) + extract_snapshot_git (context& ctx, dir_path rep_root) { if (const snapshot* r = cache.find (rep_root)) return *r; @@ -82,7 +82,11 @@ namespace build2 args[args_i + 1] = "--porcelain"; args[args_i + 2] = nullptr; + // @@ PERF: redo with custom stream reading code (then could also + // get rid of context). + // r.committed = run ( + ctx, 3 /* verbosity */, pp, args, @@ -108,7 +112,8 @@ namespace build2 // (reluctantly) assume that the only reason git cat-file fails is if // there is no HEAD (that we equal with the "new repository" condition // which is, strictly speaking, might not be the case either). So we - // suppress any diagnostics, and handle non-zero exit code. + // suppress any diagnostics, and handle non-zero exit code (and so no + // diagnostics buffering is needed, plus we are in the load phase). // string data; @@ -117,12 +122,12 @@ namespace build2 args[args_i + 2] = "HEAD"; args[args_i + 3] = nullptr; - process pr (run_start (3 /* verbosity */, + process pr (run_start (3 /* verbosity */, pp, args, - 0 /* stdin */, - -1 /* stdout */, - false /* error */)); + 0 /* stdin */, + -1 /* stdout */, + {-1, 1} /* stderr (to stdout) */)); string l; try diff --git a/libbuild2/version/snapshot.cxx b/libbuild2/version/snapshot.cxx index d20e633..000bcba 100644 --- a/libbuild2/version/snapshot.cxx +++ b/libbuild2/version/snapshot.cxx @@ -12,7 +12,7 @@ namespace build2 namespace version { snapshot - extract_snapshot_git (dir_path); + extract_snapshot_git (context&, dir_path); static const path git (".git"); @@ -46,7 +46,7 @@ namespace build2 if (butl::entry_exists (d / git, true /* follow_symlinks */, true /* ignore_errors */)) - return extract_snapshot_git (move (d)); + return extract_snapshot_git (rs.ctx, move (d)); } return snapshot (); -- cgit v1.1