From 1abc10223b37d9ead3454a06e176b4b65370a2be Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Mon, 27 Jan 2020 08:37:56 +0200 Subject: Improve process run_*() API --- libbuild2/cc/gcc.cxx | 11 +++++---- libbuild2/cc/guess.cxx | 4 ++-- libbuild2/cc/link-rule.cxx | 5 +++- libbuild2/cc/msvc.cxx | 2 +- libbuild2/functions-process.cxx | 49 ++++++++++++++++---------------------- libbuild2/parser.cxx | 15 +++++------- libbuild2/utility.cxx | 28 ++++++++++++++++++---- libbuild2/utility.hxx | 36 +++++++++++++++++++--------- libbuild2/utility.ixx | 39 ++++++++++++++++++++++++++++++ libbuild2/utility.txx | 13 +++++++--- libbuild2/version/snapshot-git.cxx | 2 +- 11 files changed, 138 insertions(+), 66 deletions(-) diff --git a/libbuild2/cc/gcc.cxx b/libbuild2/cc/gcc.cxx index 5857709..91a04f6 100644 --- a/libbuild2/cc/gcc.cxx +++ b/libbuild2/cc/gcc.cxx @@ -219,11 +219,14 @@ namespace build2 is.close (); // Don't block. } - catch (const io_error&) + catch (const io_error& e) { - pr.wait (); - fail << "error reading " << x_lang << " compiler -print-search-dirs " - << "output"; + if (run_wait (args, pr)) + fail << "io error reading " << args[0] << " -print-search-dirs " + << "output: " << e; + + // If the child process has failed then assume the io error was caused + // by that and let run_finish() deal with it. } run_finish (args, pr); diff --git a/libbuild2/cc/guess.cxx b/libbuild2/cc/guess.cxx index 053a4cb..d5fc884 100644 --- a/libbuild2/cc/guess.cxx +++ b/libbuild2/cc/guess.cxx @@ -224,7 +224,7 @@ namespace build2 // that. } - if (!run_finish (args.data (), pr, false /* error */, l)) + if (!run_finish_code (args.data (), pr, l)) r = "none"; if (r.empty ()) @@ -2077,7 +2077,7 @@ namespace build2 // that. } - if (!run_finish (args.data (), pr, false /* error */, l)) + if (!run_finish_code (args.data (), pr, l)) fail << "unable to extract MSVC information from " << xp; if (const char* w = ( diff --git a/libbuild2/cc/link-rule.cxx b/libbuild2/cc/link-rule.cxx index ccb8dc6..6969ef2 100644 --- a/libbuild2/cc/link-rule.cxx +++ b/libbuild2/cc/link-rule.cxx @@ -2127,9 +2127,12 @@ namespace build2 } catch (const io_error& e) { - if (pr.wait ()) // Ignore if child failed. + if (run_wait (args, pr)) fail << "unable to pipe resource file to " << args[0] << ": " << e; + + // If the child process has failed then assume the io error + // was caused by that and let run_finish() deal with it. } run_finish (args, pr); diff --git a/libbuild2/cc/msvc.cxx b/libbuild2/cc/msvc.cxx index c7022fc..1bdd2bb 100644 --- a/libbuild2/cc/msvc.cxx +++ b/libbuild2/cc/msvc.cxx @@ -356,7 +356,7 @@ namespace build2 // that. } - if (!run_finish (args, pr, false, s)) + if (!run_finish_code (args, pr, s)) return otype::e; if (obj && dll) diff --git a/libbuild2/functions-process.cxx b/libbuild2/functions-process.cxx index 2cc3385..63bdf61 100644 --- a/libbuild2/functions-process.cxx +++ b/libbuild2/functions-process.cxx @@ -65,10 +65,10 @@ namespace build2 } static process - start (const scope*, - const process_path& pp, - const strings& args, - cstrings& cargs) + run_start (const scope*, + const process_path& pp, + const strings& args, + cstrings& cargs) { cargs.reserve (args.size () + 2); cargs.push_back (pp.recall_string ()); @@ -85,23 +85,13 @@ namespace build2 -1 /* stdout */); } - static void - finish (cstrings& args, process& pr, bool io) - { - run_finish (args, pr); - - if (io) - fail << "error reading " << args[0] << " output"; - } - static value run (const scope* s, const process_path& pp, const strings& args) { cstrings cargs; - process pr (start (s, pp, args, cargs)); + process pr (run_start (s, pp, args, cargs)); string v; - bool io (false); try { ifdstream is (move (pr.in_ofd)); @@ -113,15 +103,16 @@ namespace build2 is.close (); // Detect errors. } - catch (const io_error&) + catch (const io_error& e) { - // Presumably the child process failed and issued diagnostics so let - // finish() try to deal with that first. - // - io = true; + if (run_wait (cargs, pr)) + fail << "io error reading " << cargs[0] << " output: " << e; + + // If the child process has failed then assume the io error was + // caused by that and let run_finish() deal with it. } - finish (cargs, pr, io); + run_finish (cargs, pr); names r; r.push_back (to_name (move (trim (v)))); @@ -141,10 +132,9 @@ namespace build2 regex re (parse_regex (pat, regex::ECMAScript)); cstrings cargs; - process pr (start (s, pp, args, cargs)); + process pr (run_start (s, pp, args, cargs)); names r; - bool io (false); try { ifdstream is (move (pr.in_ofd), ifdstream::badbit); @@ -167,15 +157,16 @@ namespace build2 is.close (); // Detect errors. } - catch (const io_error&) + catch (const io_error& e) { - // Presumably the child process failed and issued diagnostics so let - // finish() try to deal with that first. - // - io = true; + if (run_wait (cargs, pr)) + fail << "io error reading " << cargs[0] << " output: " << e; + + // If the child process has failed then assume the io error was + // caused by that and let run_finish() deal with it. } - finish (cargs, pr, io); + run_finish (cargs, pr); return value (move (r)); } diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx index f79f80e..59eeafd 100644 --- a/libbuild2/parser.cxx +++ b/libbuild2/parser.cxx @@ -1597,7 +1597,6 @@ namespace build2 dir_path () /* cwd */, nullptr /* env */, l)); - bool bad (false); try { // While a failing process could write garbage to stdout, for simplicity @@ -1627,19 +1626,17 @@ namespace build2 is.close (); // Detect errors. } - catch (const io_error&) + catch (const io_error& e) { - // Presumably the child process failed and issued diagnostics so let - // run_finish() try to deal with that first. - // - bad = true; + if (run_wait (cargs, pr, l)) + fail (l) << "io error reading " << cargs[0] << " output: " << e; + + // If the child process has failed then assume the io error was + // caused by that and let run_finish() deal with it. } run_finish (cargs, pr, l); - if (bad) - fail (l) << "error reading " << args[0] << " output"; - next_after_newline (t, tt); } diff --git a/libbuild2/utility.cxx b/libbuild2/utility.cxx index 20f0c22..0c44a59 100644 --- a/libbuild2/utility.cxx +++ b/libbuild2/utility.cxx @@ -252,11 +252,22 @@ namespace build2 } bool - run_finish (const char* args[], - process& pr, - bool err, - const string& l, - const location& loc) + run_wait (const char* args[], process& pr, const location& loc) + try + { + return pr.wait (); + } + catch (const process_error& e) + { + fail (loc) << "unable to execute " << args[0] << ": " << e << endf; + } + + bool + run_finish_impl (const char* args[], + process& pr, + bool err, + const string& l, + const location& loc) try { tracer trace ("run_finish"); @@ -297,6 +308,13 @@ namespace build2 fail (loc) << "unable to execute " << args[0] << ": " << e << endf; } + void + run_io_error (const char* args[], const io_error& e) + { + fail << "io error reading " << args[0] << " output: " << e << endf; + } + + const string empty_string; const path empty_path; const dir_path empty_dir_path; diff --git a/libbuild2/utility.hxx b/libbuild2/utility.hxx index 5c6e984..a6ed8e0 100644 --- a/libbuild2/utility.hxx +++ b/libbuild2/utility.hxx @@ -242,25 +242,39 @@ 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, if error is true, assume the diagnostics - // has already been issued and throw failed as well. Otherwise (error is - // false), return false. The last argument is used in cooperation with - // run_start() in case STDERR is redirected to STDOUT. + // 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. // - LIBBUILD2_SYMEXPORT bool + void run_finish (const char* args[], process&, - bool error = true, const string& = string (), const location& = location ()); - inline void - run_finish (cstrings& args, process& pr, const location& l = location ()) - { - run_finish (args.data (), pr, true, string (), l); - } + 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). diff --git a/libbuild2/utility.ixx b/libbuild2/utility.ixx index 514d4ee..4b841f7 100644 --- a/libbuild2/utility.ixx +++ b/libbuild2/utility.ixx @@ -6,6 +6,45 @@ namespace build2 { + inline bool + run_wait (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. + // + LIBBUILD2_SYMEXPORT bool + run_finish_impl (const char*[], + process&, + bool error, + const string&, + const location& = location ()); + + inline void + run_finish (const char* args[], + process& pr, + const string& l, + const location& loc) + { + run_finish_impl (args, pr, true /* error */, l, loc); + } + + inline void + run_finish (cstrings& args, process& pr, const location& loc) + { + run_finish (args.data (), pr, string (), loc); + } + + inline bool + run_finish_code (const char* args[], + process& pr, + const string& l, + const location& loc) + { + return run_finish_impl (args, pr, false /* error */, l, loc); + } + inline void hash_path (sha256& cs, const path& p, const dir_path& prefix) { diff --git a/libbuild2/utility.txx b/libbuild2/utility.txx index c183930..aadb181 100644 --- a/libbuild2/utility.txx +++ b/libbuild2/utility.txx @@ -56,6 +56,9 @@ namespace build2 return p; } + [[noreturn]] LIBBUILD2_SYMEXPORT void + run_io_error (const char*[], const io_error&); + template T run (uint16_t verbosity, @@ -102,12 +105,16 @@ namespace build2 is.close (); } - catch (const io_error&) + catch (const io_error& e) { - // Presumably the child process failed. Let run_finish() deal with that. + 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 (args, pr, err, l) || ignore_exit)) + 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 7dcb1f3..30dea6d 100644 --- a/libbuild2/version/snapshot-git.cxx +++ b/libbuild2/version/snapshot-git.cxx @@ -187,7 +187,7 @@ namespace build2 // that. } - if (!run_finish (args, pr, false /* error */, l)) + if (!run_finish_code (args, pr, l)) { // Presumably new repository without HEAD. Return uncommitted snapshot // with UNIX epoch as timestamp. -- cgit v1.1