From 7fdc55efe423f58e5ce03e7f758ace6443800b7a Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Fri, 5 Jun 2020 20:06:06 +0300 Subject: Cleanup script command failure diagnostics --- libbuild2/script/run.cxx | 31 ++++++++++++++++-------------- libbuild2/script/script.cxx | 6 +++--- libbuild2/script/script.hxx | 4 +++- tests/test/script/runner/exit.testscript | 2 +- tests/test/script/runner/pipe.testscript | 2 +- tests/test/script/runner/set.testscript | 2 +- tests/test/script/runner/status.testscript | 9 +++++++++ 7 files changed, 35 insertions(+), 21 deletions(-) diff --git a/libbuild2/script/run.cxx b/libbuild2/script/run.cxx index 5629a15..46c061c 100644 --- a/libbuild2/script/run.cxx +++ b/libbuild2/script/run.cxx @@ -939,8 +939,6 @@ namespace build2 env.clean ({cl.type, move (np)}, false); } - bool eq (c.exit.comparison == exit_comparison::eq); - // If stdin file descriptor is not open then this is the first pipeline // command. // @@ -1012,11 +1010,8 @@ namespace build2 if (c.err) fail (ll) << "exit builtin stderr cannot be redirected"; - // We can't make sure that there is no exit code check. Let's, at - // least, check that non-zero code is not expected. - // - if (eq != (c.exit.code == 0)) - fail (ll) << "exit builtin exit code cannot be non-zero"; + if (c.exit) + fail (ll) << "exit builtin exit code cannot be checked"; if (verb >= 2) print_process (process_args ()); @@ -1168,8 +1163,8 @@ namespace build2 if (c.err) fail (ll) << "set builtin stderr cannot be redirected"; - if (eq != (c.exit.code == 0)) - fail (ll) << "set builtin exit code cannot be non-zero"; + if (c.exit) + fail (ll) << "set builtin exit code cannot be checked"; if (verb >= 2) print_process (process_args ()); @@ -1677,8 +1672,7 @@ namespace build2 // If there is no valid exit code available by whatever reason then we // print the proper diagnostics, dump stderr (if cached and not too // large) and fail the whole script. Otherwise if the exit code is not - // correct then we print diagnostics if requested and fail the - // pipeline. + // correct then we print diagnostics if requested and fail the pipeline. // bool valid (exit->normal ()); @@ -1690,7 +1684,11 @@ namespace build2 valid = exit->code () < 256; #endif - success = valid && eq == (exit->code () == c.exit.code); + exit_comparison cmp (c.exit ? c.exit->comparison : exit_comparison::eq); + uint16_t exc (c.exit ? c.exit->code : 0); + + success = valid && + (cmp == exit_comparison::eq) == (exc == exit->code ()); if (!valid || (!success && diag)) { @@ -1710,8 +1708,13 @@ namespace build2 else if (!success) { if (diag) - d << pr << " exit code " << ec << (eq ? " != " : " == ") - << static_cast (c.exit.code); + { + if (c.exit) + d << pr << " exit code " << ec + << (cmp == exit_comparison::eq ? " != " : " == ") << exc; + else + d << pr << " exited with code " << ec; + } } else assert (false); diff --git a/libbuild2/script/script.cxx b/libbuild2/script/script.cxx index c9b3a5e..d0d3304 100644 --- a/libbuild2/script/script.cxx +++ b/libbuild2/script/script.cxx @@ -406,15 +406,15 @@ namespace build2 print_path (p.path); } - if (c.exit.comparison != exit_comparison::eq || c.exit.code != 0) + if (c.exit) { - switch (c.exit.comparison) + switch (c.exit->comparison) { case exit_comparison::eq: o << " == "; break; case exit_comparison::ne: o << " != "; break; } - o << static_cast (c.exit.code); + o << static_cast (c.exit->code); } } diff --git a/libbuild2/script/script.hxx b/libbuild2/script/script.hxx index 31527a0..d751169 100644 --- a/libbuild2/script/script.hxx +++ b/libbuild2/script/script.hxx @@ -314,7 +314,9 @@ namespace build2 script::cleanups cleanups; - command_exit exit {exit_comparison::eq, 0}; + // If nullopt, then the command is expected to succeed (0 exit code). + // + optional exit; }; enum class command_to_stream: uint16_t diff --git a/tests/test/script/runner/exit.testscript b/tests/test/script/runner/exit.testscript index 200b963..24e51fa 100644 --- a/tests/test/script/runner/exit.testscript +++ b/tests/test/script/runner/exit.testscript @@ -54,7 +54,7 @@ empty_id = '' : exit-code : $c <'exit != 0' && $b 2>>EOE != 0 - testscript:1:1: error: exit builtin exit code cannot be non-zero + testscript:1:1: error: exit builtin exit code cannot be checked info: test id: 1 EOE } diff --git a/tests/test/script/runner/pipe.testscript b/tests/test/script/runner/pipe.testscript index 8fecbfc..205fd55 100644 --- a/tests/test/script/runner/pipe.testscript +++ b/tests/test/script/runner/pipe.testscript @@ -16,7 +16,7 @@ $c <'$* -o foo | cat >foo' && $b : process-to-builtin : exit-code : $c <'$* -o foo -s 1 | $* -i 1 >foo -s 2' && $b 2>>/~%EOE% != 0 - %testscript:1:1: error: .+ exit code 2 != 0% + %testscript:1:1: error: .+ exited with code 2% info: stdout: test/1/stdout-2 info: test id: 1 EOE diff --git a/tests/test/script/runner/set.testscript b/tests/test/script/runner/set.testscript index dcf428d..9219cbb 100644 --- a/tests/test/script/runner/set.testscript +++ b/tests/test/script/runner/set.testscript @@ -34,7 +34,7 @@ : status : $c <'set foo == 1' && $b 2>>EOE != 0 - testscript:1:1: error: set builtin exit code cannot be non-zero + testscript:1:1: error: set builtin exit code cannot be checked info: test id: 1 EOE } diff --git a/tests/test/script/runner/status.testscript b/tests/test/script/runner/status.testscript index d4759e3..e4586d9 100644 --- a/tests/test/script/runner/status.testscript +++ b/tests/test/script/runner/status.testscript @@ -38,6 +38,15 @@ b += --no-column : error : $c <'$* -s 1 -e "Error"' && $b 2>>/~%EOE% != 0 +%testscript:1: error: ../../../driver(.exe)? exited with code 1% + info: stderr: test/1/stderr +Error + info: test id: 1 +EOE + +: error-check +: +$c <'$* -s 1 -e "Error" == 0' && $b 2>>/~%EOE% != 0 %testscript:1: error: ../../../driver(.exe)? exit code 1 != 0% info: stderr: test/1/stderr Error -- cgit v1.1