aboutsummaryrefslogtreecommitdiff
path: root/libbuild2/utility.cxx
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2022-11-08 10:34:22 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2022-11-08 11:08:03 +0200
commit3bc0fc4c4496c345c79734dcd6dc56d44119aebf (patch)
treed058018aaf35641e461e51c2c20d374fd0d1684c /libbuild2/utility.cxx
parent84e6c7e62c9d1613af3cad81787b3f277d276140 (diff)
Make process exit diagnostics consistent
In particular, we now always print error message on non-0 exit except in cases where such exit is ignored.
Diffstat (limited to 'libbuild2/utility.cxx')
-rw-r--r--libbuild2/utility.cxx103
1 files changed, 65 insertions, 38 deletions
diff --git a/libbuild2/utility.cxx b/libbuild2/utility.cxx
index 556c4a3..d4d632c 100644
--- a/libbuild2/utility.cxx
+++ b/libbuild2/utility.cxx
@@ -240,7 +240,7 @@ namespace build2
{
if (e.child)
{
- // Note: run_finish() expects this exact message.
+ // Note: run_finish_impl() below expects this exact message.
//
cerr << "unable to execute " << args[0] << ": " << e << endl;
@@ -270,6 +270,8 @@ namespace build2
process& pr,
bool f,
const string& l,
+ uint16_t v,
+ bool omit_normal,
const location& loc)
{
tracer trace ("run_finish");
@@ -284,36 +286,55 @@ namespace build2
fail (loc) << "unable to execute " << args[0] << ": " << e << endf;
}
- // Note: see similar code in diag_buffer::close/finish().
- //
- const process_exit& e (*pr.exit);
-
- if (!e.normal ())
- fail (loc) << "process " << args[0] << " " << e;
-
- // Normall but non-zero exit status.
+ // Note: see similar code in diag_buffer::close().
//
- // 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
- // just have to compare the output. This particular situation will
- // result in a single error line printed by run_start() above.
- //
- if (l.compare (0, 18, "unable to execute ") == 0)
- error (loc) << l;
+ const process_exit& pe (*pr.exit);
+ bool ne (pe.normal ());
- if (f)
+ if (omit_normal && ne)
{
// 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;});
+ l4 ([&]{trace << "process " << args[0] << " " << pe;});
+ }
+ else
+ {
+ // Even if the user redirected the 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 just
+ // have to compare the output. This particular situation will result in
+ // a single error line printed by run_start() above.
+ //
+ // Shouldn't we treat this as abnormal termination? But if we didn't
+ // redirect stderr to stdout, then this will be handled as normal exit.
+ // So let's be consistent even if wrong.
+ //
+ if (ne && l.compare (0, 18, "unable to execute ") == 0)
+ error (loc) << l;
- throw failed ();
+ // 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 that we
+ // now rely on this in, for example, process_finish().
+ //
+ // Note: make sure keep the above trace if decide not to print.
+ //
+ diag_record dr;
+ dr << error (loc) << "process " << args[0] << " " << pe;
+
+ if (verb >= 1 && verb <= v)
+ {
+ dr << info << "command line: ";
+ print_process (dr, args);
+ }
}
+ if (f || !ne)
+ throw failed ();
+
return false;
}
@@ -323,6 +344,7 @@ namespace build2
process& pr,
bool f,
uint16_t v,
+ bool on,
const location& loc)
{
try
@@ -336,34 +358,40 @@ namespace build2
const process_exit& pe (*pr.exit);
- dbuf.close (args, pe, v, loc, !f /* omit_normall */);
+ dbuf.close (args, pe, v, on, loc);
if (pe)
return true;
- if (f)
+ if (f || !pe.normal ())
throw failed ();
return false;
}
void
- run (context& ctx, const process_env& pe, const char* const* args)
+ run (context& ctx,
+ const process_env& pe,
+ const char* const* args,
+ uint16_t v)
{
if (ctx.phase == run_phase::load)
{
process pr (run_start (pe, args));
- run_finish (args, pr);
+ run_finish (args, pr, v);
}
else
{
diag_buffer dbuf (ctx);
- run (dbuf, pe, args);
+ run (dbuf, pe, args, v);
}
}
void
- run (diag_buffer& dbuf, const process_env& pe, const char* const* args)
+ run (diag_buffer& dbuf,
+ const process_env& pe,
+ const char* const* args,
+ uint16_t v)
{
process pr (run_start (pe,
args,
@@ -371,7 +399,7 @@ namespace build2
1 /* stdout */,
dbuf.open (args[0]) /* stderr */));
dbuf.read ();
- run_finish (dbuf, args, pr);
+ run_finish (dbuf, args, pr, v);
}
bool
@@ -379,6 +407,7 @@ namespace build2
uint16_t verbosity,
const process_env& pe,
const char* const* args,
+ uint16_t finish_verbosity,
const function<bool (string&, bool)>& f,
bool tr,
bool err,
@@ -388,7 +417,12 @@ namespace build2
if (err && ctx.phase != run_phase::load)
{
diag_buffer dbuf (ctx);
- return run (dbuf, verbosity, pe, args, f, tr, ignore_exit, checksum);
+ return run (dbuf,
+ verbosity,
+ pe, args,
+ finish_verbosity,
+ f,
+ tr, ignore_exit, checksum);
}
process pr (run_start (verbosity,
@@ -438,7 +472,7 @@ namespace build2
// caused by that and let run_finish() deal with it.
}
- if (!(run_finish_impl (args, pr, err, l) || ignore_exit))
+ if (!(run_finish_impl (args, pr, err, l, finish_verbosity) || ignore_exit))
return false;
return true;
@@ -449,6 +483,7 @@ namespace build2
uint16_t verbosity,
const process_env& pe,
const char* const* args,
+ uint16_t finish_verbosity,
const function<bool (string&, bool)>& f,
bool tr,
bool ignore_exit,
@@ -566,15 +601,7 @@ namespace build2
// 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;
+ return run_finish_impl (dbuf, args, pr, !ignore_exit, finish_verbosity);
}
fdpipe