diff options
| -rw-r--r-- | libbuild2/build/script/parser.cxx | 2 | ||||
| -rw-r--r-- | libbuild2/build/script/runner.cxx | 8 | ||||
| -rw-r--r-- | libbuild2/cc/pkgconfig.cxx | 1 | ||||
| -rw-r--r-- | libbuild2/script/run.cxx | 190 | ||||
| -rw-r--r-- | libbuild2/script/run.hxx | 6 | ||||
| -rw-r--r-- | libbuild2/test/script/runner.cxx | 6 |
6 files changed, 155 insertions, 58 deletions
diff --git a/libbuild2/build/script/parser.cxx b/libbuild2/build/script/parser.cxx index c362776..95b835e 100644 --- a/libbuild2/build/script/parser.cxx +++ b/libbuild2/build/script/parser.cxx @@ -2826,7 +2826,7 @@ namespace build2 cmd, nullptr /* iteration_index */, li, ll, - cf, false /* last_cmd */); + cf); iss.exceptions (istream::badbit); } diff --git a/libbuild2/build/script/runner.cxx b/libbuild2/build/script/runner.cxx index 5d9764b..fc0fc05 100644 --- a/libbuild2/build/script/runner.cxx +++ b/libbuild2/build/script/runner.cxx @@ -143,7 +143,13 @@ namespace build2 (cf != nullptr && p.recall.string () == "for")); }) != expr.end ()) - build2::script::run (env, expr, ii, li, ll, cf); + { + build2::script::run (env, + expr, + ii, li, + ll, + cf, (cf != nullptr) /* replace_last_cmd */); + } else if (verb >= 2) text << expr; } diff --git a/libbuild2/cc/pkgconfig.cxx b/libbuild2/cc/pkgconfig.cxx index 79a38ea..d4be03b 100644 --- a/libbuild2/cc/pkgconfig.cxx +++ b/libbuild2/cc/pkgconfig.cxx @@ -693,6 +693,7 @@ namespace build2 cmp ("msxml", 5) || // msxml* cmp ("netapi32") || cmp ("normaliz") || + cmp ("ntdll") || cmp ("odbc32") || cmp ("ole32") || cmp ("oleaut32") || diff --git a/libbuild2/script/run.cxx b/libbuild2/script/run.cxx index 99f4c97..528f42f 100644 --- a/libbuild2/script/run.cxx +++ b/libbuild2/script/run.cxx @@ -1711,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) { @@ -1723,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. // @@ -1798,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"; @@ -1833,12 +1834,42 @@ namespace build2 // 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); @@ -1910,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) @@ -1985,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. // @@ -2023,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 (); @@ -2078,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 @@ -2113,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) @@ -2132,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. // @@ -2173,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, @@ -2297,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 @@ -2311,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. // @@ -3041,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); @@ -3166,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); @@ -3219,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. @@ -3264,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 (); @@ -3279,9 +3357,12 @@ namespace build2 const iteration_index* ii, size_t li, const location& ll, const function<command_function>& cf, - bool last_cmd) + bool replace_last_cmd) { - assert (last_cmd || cf != nullptr); + // 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 @@ -3291,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. } @@ -3300,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 aa11def..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 (last_cmd=false). + // 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/test/script/runner.cxx b/libbuild2/test/script/runner.cxx index 98d6868..54009f6 100644 --- a/libbuild2/test/script/runner.cxx +++ b/libbuild2/test/script/runner.cxx @@ -182,7 +182,11 @@ namespace build2 }); ++sp.exec_level; - build2::script::run (sp, expr, ii, li, ll, cf); + build2::script::run (sp, + expr, + ii, li, + ll, + cf, (cf != nullptr) /* replace_last_cmd */); --sp.exec_level; } |