diff options
Diffstat (limited to 'libbuild2/script')
| -rw-r--r-- | libbuild2/script/parser.cxx | 8 | ||||
| -rw-r--r-- | libbuild2/script/run.cxx | 245 | ||||
| -rw-r--r-- | libbuild2/script/run.hxx | 6 | ||||
| -rw-r--r-- | libbuild2/script/script.hxx | 5 |
4 files changed, 189 insertions, 75 deletions
diff --git a/libbuild2/script/parser.cxx b/libbuild2/script/parser.cxx index a82ccb8..30c7ff6 100644 --- a/libbuild2/script/parser.cxx +++ b/libbuild2/script/parser.cxx @@ -1668,9 +1668,9 @@ namespace build2 if (es > 255) { - diag_record dr; + diag_record dr (fail (l)); - dr << fail (l) << "expected exit status instead of "; + dr << "expected exit status instead of "; to_stream (dr.os, ns, quote_mode::normal); dr << info << "exit status is an unsigned integer less than 256"; @@ -2218,7 +2218,7 @@ namespace build2 // Copy the tokens and start playing. // - replay_data (replay_tokens (ln.tokens)); + replay_data (ln.tokens); // We don't really need to change the mode since we already know // the line type. @@ -2433,7 +2433,7 @@ namespace build2 // Prepare for the condition reevaluation. // - replay_data (replay_tokens (ln.tokens)); + replay_data (ln.tokens); next (t, tt); li = wli; } diff --git a/libbuild2/script/run.cxx b/libbuild2/script/run.cxx index f8f98c1..e669b29 100644 --- a/libbuild2/script/run.cxx +++ b/libbuild2/script/run.cxx @@ -1301,7 +1301,6 @@ namespace build2 // Terminate processes gracefully and set the terminate flag for the // pipe commands. // - diag_record dr; for (pipe_command* c (pc); c != nullptr; c = c->prev) { if (process* p = c->proc) @@ -1324,6 +1323,10 @@ namespace build2 c->terminated = true; } + // Delay any failure until we process the entire pipeline. + // + maybe_diag_record dr; + // Wait a bit for the processes to terminate and kill the remaining // ones. // @@ -1708,7 +1711,7 @@ namespace build2 const iteration_index* ii, size_t li, size_t ci, const location& ll, bool diag, - const function<command_function>& cf, bool last_cmd, + const function<command_function>& cf, bool replace_last_cmd, optional<deadline> dl = nullopt, pipe_command* prev_cmd = nullptr) { @@ -1720,7 +1723,7 @@ namespace build2 { if (cf != nullptr) { - assert (!last_cmd); // Otherwise we wouldn't be here. + assert (!replace_last_cmd); // Otherwise we wouldn't be here. // The pipeline can't be empty. // @@ -1795,8 +1798,9 @@ namespace build2 command_pipe::const_iterator nc (bc + 1); bool last (nc == ec); - // Make sure that stdout is not redirected if meant to be read (last_cmd - // is false) or cannot not be produced (last_cmd is true). + // Make sure that stdout is not redirected if meant to be read + // (replace_last_cmd is false) or cannot not be produced + // (replace_last_cmd is true). // if (last && c.out && cf != nullptr) fail (ll) << "stdout cannot be redirected"; @@ -1810,14 +1814,62 @@ namespace build2 // const string& program (c.program.recall.string ()); + // Optimize standalone true/false builtins (used in conditions, etc). We + // know they don't read/write stdin/stdout/stderr so none of the below + // complications are necessary. + // + if (resolve && (program == "true" || program == "false") && + first && last && cf == nullptr && + !c.in && !c.out && !c.err && !c.exit) + { + // Note: we can safely ignore agruments, env vars, and timeout. + // + // Don't print the true and false builtins, since they are normally + // used for the commands execution flow control (also below). + // + return program == "true"; + } + + // Standard streams. + // const redirect& in ((c.in ? *c.in : env.in).effective ()); - const redirect* out (!last || (cf != nullptr && !last_cmd) + const redirect* out (!last || (cf != nullptr && !replace_last_cmd) ? nullptr // stdout is piped. : &(c.out ? *c.out : env.out).effective ()); const redirect& err ((c.err ? *c.err : env.err).effective ()); + // If the output redirect is `none` (default for testscript) or `null` + // and this is a builtin which is known not to write to stdout, then + // redirect its stdout to stderr, unless stderr itself is redirected to + // stdout. This way we replace the expensive fdopen(/dev/null) call with + // the cheap fddup(...) call. + // + // Note that we ignore the pseudo builtins since they are handled prior + // to opening any file descriptors for stdout. + // + const redirect out_merge (redirect_type::merge, 2); + if (out != nullptr && + (out->type == redirect_type::none || + out->type == redirect_type::null) && + err.type != redirect_type::merge && + resolve && + (program == "cp" || + program == "false" || + program == "ln" || + program == "mkdir" || + program == "mv" || + program == "rm" || + program == "rmdir" || + program == "sleep" || + program == "test" || + program == "touch" || + program == "true")) + { + out = &out_merge; + } + auto process_args = [&c] () -> cstrings { return build2::process_args (c.program.recall_string (), c.arguments); @@ -1889,7 +1941,7 @@ namespace build2 if (c.out) fail (ll) << program << " builtin stdout cannot be redirected"; - if (cf != nullptr && !last_cmd) + if (cf != nullptr && !replace_last_cmd) fail (ll) << program << " builtin stdout cannot be read"; if (c.err) @@ -1964,11 +2016,14 @@ namespace build2 // If this is the first pipeline command, then open stdin descriptor // according to the redirect specified. // + // Specifically, save into ifd the descriptor of a newly created file, + // existing file, stdin descriptor duplicate, or null-device descriptor + // for the here_*_literal, file, pass, or null/none redirects, + // respectively. + // path isp; - if (!first) - assert (!c.in); // No redirect expected. - else + if (first) { // Open a file for passing to the command stdin. // @@ -2002,25 +2057,58 @@ namespace build2 break; } case redirect_type::none: - // Somehow need to make sure that the child process doesn't read - // from stdin. That is tricky to do in a portable way. Here we - // suppose that the program which (erroneously) tries to read some - // data from stdin being redirected to /dev/null fails not being - // able to read the expected data, and so the command doesn't pass - // through. - // - // @@ Obviously doesn't cover the case when the process reads - // whatever available. - // @@ Another approach could be not to redirect stdin and let the - // process to hang which can be interpreted as a command failure. - // @@ Both ways are quite ugly. Is there some better way to do - // this? - // @@ Maybe we can create a pipe, write a byte into it, close the - // writing end, and after the process terminates make sure we can - // still read this byte out? - // + { + // Somehow need to make sure that the child process doesn't read + // from stdin. That is tricky to do in a portable way. Here we + // suppose that the program which (erroneously) tries to read some + // data from stdin being redirected to /dev/null fails not being + // able to read the expected data, and so the command doesn't pass + // through. + // + // @@ Obviously doesn't cover the case when the process reads + // whatever available. + // @@ Another approach could be not to redirect stdin and let the + // process to hang which can be interpreted as a command + // failure. + // @@ Both ways are quite ugly. Is there some better way to do + // this? + // @@ Maybe we can create a pipe, write a byte into it, close the + // writing end, and after the process terminates make sure we + // can still read this byte out? + // + // Let's optimize for the builtins, which are known not to read + // any input, by not opening /dev/null but duplicating the stdin + // file descriptor instead. + // + if (resolve && + (program == "cp" || + program == "date" || + program == "echo" || + program == "false" || + program == "find" || + program == "ln" || + program == "mkdir" || + program == "mv" || + program == "rm" || + program == "rmdir" || + program == "sleep" || + program == "test" || + program == "touch" || + program == "true")) + { + try + { + ifd = fddup (0); + } + catch (const io_error& e) + { + fail (ll) << "unable to duplicate stdin: " << e; + } + + break; + } + } // Fall through. - // case redirect_type::null: { ifd = open_null (); @@ -2057,8 +2145,10 @@ namespace build2 case redirect_type::here_doc_ref: assert (false); break; } } + else + assert (!c.in); // No redirect expected. - assert (ifd.get () != -1); + assert (ifd != nullfd); // Calculate the process/builtin execution deadline. Note that we should // also consider the left-hand side processes deadlines, not to keep @@ -2092,7 +2182,7 @@ namespace build2 if (c.out) fail (ll) << "set builtin stdout cannot be redirected"; - if (cf != nullptr && !last_cmd) + if (cf != nullptr && !replace_last_cmd) fail (ll) << "set builtin stdout cannot be read"; if (c.err) @@ -2111,7 +2201,7 @@ namespace build2 // If this is the last command in the pipe and the command function is // specified for it, then call it. // - if (last && cf != nullptr && last_cmd) + if (last && cf != nullptr && replace_last_cmd) { // Must be enforced by the caller. // @@ -2152,14 +2242,16 @@ namespace build2 else pc.next = &pc; // Points to itself. - // Open a file for command output redirect if requested explicitly - // (file overwrite/append redirects) or for the purpose of the output + // Open a file for command output redirect if requested explicitly (file + // overwrite/append redirects) or for the purpose of the output // validation (none, here_*, file comparison redirects), register the // file for cleanup, return the file descriptor. Interpret trace // redirect according to the verbosity level (as null if below 2, as - // pass otherwise). Return nullfd, standard stream descriptor duplicate - // or null-device descriptor for merge, pass or null redirects - // respectively (not opening any file). + // pass otherwise). Return nullfd, standard stream descriptor duplicate, + // or null-device descriptor for merge, pass (except for the buffered + // stderr), or null redirects respectively (not opening/creating any + // file/pipe). Create the pipe and return its write end for the pass + // redirect of the buffered stderr. // auto open = [&env, &wdir, &ll, &std_path, &c, &pc] (const redirect& r, int dfd, @@ -2276,10 +2368,12 @@ namespace build2 path osp; fdpipe ofd; - // If this is the last command in the pipeline than redirect the - // command process stdout to a file. Otherwise create a pipe and - // redirect the stdout to the write-end of the pipe. The read-end will - // be passed as stdin for the next command in the pipeline. + // If this is either not the last command in the pipeline or the + // command's output needs to be read by the specified function, then + // create a pipe and redirect the stdout to the write-end of the + // pipe. The read-end will be passed as stdin for the next command in + // the pipeline or the function. Otherwise, proceed according to the + // specified redirect (see open() lambda for details). // // @@ Shouldn't we allow the here-* and file output redirects for a // command with pipelined output? Say if such redirect is present @@ -2290,16 +2384,21 @@ namespace build2 // script failures investigation and, for example, for validation // "tightening". // - if (last && out != nullptr) - ofd.out = open (*out, 1, osp); - else + if (!last || out == nullptr) { assert (!c.out); // No redirect expected. + + // Otherwise we wouldn't be here. + // + assert (!last || (cf != nullptr && !replace_last_cmd)); + ofd = open_pipe (); } + else + ofd.out = open (*out, 1, osp); // Note: may or may not open file. path esp; - auto_fd efd (open (err, 2, esp)); + auto_fd efd (open (err, 2, esp)); // Note: may or may not open file/pipe. // Merge standard streams. // @@ -2547,7 +2646,7 @@ namespace build2 // Verify the exit status and issue the diagnostics on failure. // - diag_record dr; + maybe_diag_record dr; path pr (cmd_path (cmd)); @@ -2563,16 +2662,16 @@ namespace build2 if (c->unread_stdout) { - dr << "stdout "; + *dr << "stdout "; if (c->unread_stderr) - dr << "and "; + *dr << "and "; } if (c->unread_stderr) - dr << "stderr "; + *dr << "stderr "; - dr << "not closed after exit"; + *dr << "not closed after exit"; }; // Fail if the process is terminated due to reaching the deadline. @@ -2588,7 +2687,7 @@ namespace build2 if (verb == 1) { dr << info << "command line: "; - print_process (dr, *c->args); + print_process (*dr, *c->args); } fail = true; @@ -2643,23 +2742,23 @@ namespace build2 dr << error (ll) << w << ' ' << pr << ' '; if (!exit->normal ()) - dr << *exit; + *dr << *exit; else { uint16_t ec (exit->code ()); // Make sure printed as integer. if (!valid) { - dr << "exit code " << ec << " out of 0-255 range"; + *dr << "exit code " << ec << " out of 0-255 range"; } else { if (cmd.exit) - dr << "exit code " << ec - << (cmp == exit_comparison::eq ? " != " : " == ") - << exc; + *dr << "exit code " << ec + << (cmp == exit_comparison::eq ? " != " : " == ") + << exc; else - dr << "exited with code " << ec; + *dr << "exited with code " << ec; } } @@ -2669,7 +2768,7 @@ namespace build2 if (verb == 1) { dr << info << "command line: "; - print_process (dr, *c->args); + print_process (*dr, *c->args); } if (non_empty (*c->esp, ll) && avail_on_failure (*c->esp, env)) @@ -2683,7 +2782,7 @@ namespace build2 // Print cached stderr. // - print_file (dr, *c->esp, ll); + print_file (*dr, *c->esp, ll); } else if (c->unread_stdout || c->unread_stderr) unread_output_diag (true /* main_error */); @@ -2754,7 +2853,7 @@ namespace build2 // Execute the builtin. // // Don't print the true and false builtins, since they are normally - // used for the commands execution flow control. + // used for the commands execution flow control (also above). // if (verb >= 2 && program != "true" && program != "false") print_process (args); @@ -2984,7 +3083,7 @@ namespace build2 // If/when required we could probably support the precise sleep // mode (e.g., via an option). // - env.context.sched->sleep (t); + env.sleep (t); } }; @@ -3020,7 +3119,7 @@ namespace build2 nc, ec, move (ofd.in), ii, li, ci + 1, ll, diag, - cf, last_cmd, + cf, replace_last_cmd, dl, &pc); @@ -3145,7 +3244,7 @@ namespace build2 nc, ec, move (ofd.in), ii, li, ci + 1, ll, diag, - cf, last_cmd, + cf, replace_last_cmd, dl, &pc); @@ -3198,7 +3297,7 @@ namespace build2 const iteration_index* ii, size_t li, const location& ll, bool diag, - const function<command_function>& cf, bool last_cmd) + const function<command_function>& cf, bool replace_last_cmd) { // Commands are numbered sequentially throughout the expression // starting with 1. Number 0 means the command is a single one. @@ -3243,7 +3342,7 @@ namespace build2 p.begin (), p.end (), auto_fd (), ii, li, ci, ll, print, - cf, last_cmd); + cf, replace_last_cmd); } ci += p.size (); @@ -3258,8 +3357,13 @@ namespace build2 const iteration_index* ii, size_t li, const location& ll, const function<command_function>& cf, - bool last_cmd) + bool replace_last_cmd) { + // If the last command in the pipeline is ought to be replaced, then the + // replacement function must be specified. + // + assert (!replace_last_cmd || cf != nullptr); + // Note that we don't print the expression at any verbosity level // assuming that the caller does this, potentially providing some // additional information (command type, etc). @@ -3268,7 +3372,7 @@ namespace build2 expr, ii, li, ll, true /* diag */, - cf, last_cmd)) + cf, replace_last_cmd)) throw failed (); // Assume diagnostics is already printed. } @@ -3277,15 +3381,20 @@ namespace build2 const command_expr& expr, const iteration_index* ii, size_t li, const location& ll, - const function<command_function>& cf, bool last_cmd) + const function<command_function>& cf, bool replace_last_cmd) { + // If the last command in the pipeline is ought to be replaced, then the + // replacement function must be specified. + // + assert (!replace_last_cmd || cf != nullptr); + // Note that we don't print the expression here (see above). // return run_expr (env, expr, ii, li, ll, false /* diag */, - cf, last_cmd); + cf, replace_last_cmd); } void diff --git a/libbuild2/script/run.hxx b/libbuild2/script/run.hxx index c4c2aa2..f41a3d0 100644 --- a/libbuild2/script/run.hxx +++ b/libbuild2/script/run.hxx @@ -39,7 +39,7 @@ namespace build2 // can be used in diagnostics. // // Optionally, execute the specified function at the end of the pipe, - // either after the last command or instead of it. + // potentially instead of the last command (replace_last_cmd). // void run (environment&, @@ -47,7 +47,7 @@ namespace build2 const iteration_index*, size_t index, const location&, const function<command_function>& = nullptr, - bool last_cmd = true); + bool replace_last_cmd = false); bool run_cond (environment&, @@ -55,7 +55,7 @@ namespace build2 const iteration_index*, size_t index, const location&, const function<command_function>& = nullptr, - bool last_cmd = true); + bool replace_last_cmd = false); // Perform the registered special file cleanups in the direct order and // then the regular cleanups in the reverse order. diff --git a/libbuild2/script/script.hxx b/libbuild2/script/script.hxx index f5bd69a..76c4010 100644 --- a/libbuild2/script/script.hxx +++ b/libbuild2/script/script.hxx @@ -588,6 +588,11 @@ namespace build2 virtual void create_temp_dir () = 0; + // Used as an implementation of the sleep builtin. + // + virtual void + sleep (const duration&) = 0; + public: virtual ~environment () = default; |