From db05f8ead4c2b5ef8a27c3ffc6b20c291b0e7c8c Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Tue, 19 May 2020 14:50:16 +0300 Subject: Set proper build script command default redirects Also add printing of set and exit pseudo-builtin command arguments for verb >= 2 as we do for normal builtins. --- libbuild2/build/script/script.cxx | 5 +- libbuild2/build/script/script.hxx | 12 ++++- libbuild2/script/parser.cxx | 100 ++++++++++++++++++++++++------------- libbuild2/script/run.cxx | 62 +++++++++++++---------- libbuild2/script/script.cxx | 102 +++++++++++++++++++++++++++++--------- libbuild2/script/script.hxx | 68 +++++++++++++++++++++---- libbuild2/test/script/script.cxx | 5 +- libbuild2/test/script/script.hxx | 2 + 8 files changed, 256 insertions(+), 100 deletions(-) diff --git a/libbuild2/build/script/script.cxx b/libbuild2/build/script/script.cxx index 5dbd24c..2a796ab 100644 --- a/libbuild2/build/script/script.cxx +++ b/libbuild2/build/script/script.cxx @@ -25,7 +25,10 @@ namespace build2 pt.ctx, cast (pt.ctx.global_scope["build.host"]), work, - wd_name), + wd_name, + redirect (redirect_type::none), + redirect (redirect_type::merge, 2), + redirect (redirect_type::pass)), script (s), primary_target (pt), vars (context, false /* global */) diff --git a/libbuild2/build/script/script.hxx b/libbuild2/build/script/script.hxx index dfd725b..8569a1f 100644 --- a/libbuild2/build/script/script.hxx +++ b/libbuild2/build/script/script.hxx @@ -20,10 +20,18 @@ namespace build2 { using build2::script::line; using build2::script::line_type; + using build2::script::redirect; + using build2::script::redirect_type; using build2::script::command_expr; - // Once parsed, the script can be executed in multiple threads with the - // state (variable values, etc) maintained in the environment object. + // Notes: + // + // - Once parsed, the script can be executed in multiple threads with + // the state (variable values, etc) maintained in the environment + // object. + // + // - The default script command redirects semantics is none for stdin, + // merge into stderr for stdout, and pass for stderr. // class script { diff --git a/libbuild2/script/parser.cxx b/libbuild2/script/parser.cxx index 160374d..6c651a6 100644 --- a/libbuild2/script/parser.cxx +++ b/libbuild2/script/parser.cxx @@ -114,11 +114,11 @@ namespace build2 // auto check_command = [&c, this] (const location& l, bool last) { - if (c.out.type == redirect_type::merge && - c.err.type == redirect_type::merge) + if (c.out && c.out->type == redirect_type::merge && + c.err && c.err->type == redirect_type::merge) fail (l) << "stdout and stderr redirected to each other"; - if (!last && c.out.type != redirect_type::none) + if (!last && c.out) fail (l) << "stdout is both redirected and piped"; }; @@ -176,33 +176,41 @@ namespace build2 auto add_word = [&c, &p, &mod, &check_regex_mod, this] ( string&& w, const location& l) { - auto add_merge = [&l, this] (redirect& r, const string& w, int fd) + auto add_merge = [&l, this] (optional& r, + const string& w, + int fd) { + assert (r); // Must already be present. + try { size_t n; if (stoi (w, &n) == fd && n == w.size ()) { - r.fd = fd; + r->fd = fd; return; } } catch (const exception&) {} // Fall through. fail (l) << (fd == 1 ? "stderr" : "stdout") << " merge redirect " - << "file descriptor must be " << fd; + << "file descriptor must be " << fd; }; - auto add_here_str = [] (redirect& r, string&& w) + auto add_here_str = [] (optional& r, string&& w) { - if (r.modifiers.find (':') == string::npos) + assert (r); // Must already be present. + + if (r->modifiers.find (':') == string::npos) w += '\n'; - r.str = move (w); + r->str = move (w); }; auto add_here_str_regex = [&l, &check_regex_mod] ( - redirect& r, int fd, string&& w) + optional& r, int fd, string&& w) { + assert (r); // Must already be present. + const char* what (nullptr); switch (fd) { @@ -210,11 +218,11 @@ namespace build2 case 2: what = "stderr regex redirect"; break; } - check_regex_mod (r.modifiers, w, l, what); + check_regex_mod (r->modifiers, w, l, what); regex_parts rp (parse_regex (w, l, what)); - regex_lines& re (r.regex); + regex_lines& re (r->regex); re.intro = rp.intro; re.lines.emplace_back ( @@ -225,7 +233,7 @@ namespace build2 // Note that the position is synthetic, but that's ok as we don't // expect any diagnostics to refer this line. // - if (r.modifiers.find (':') == string::npos) + if (r->modifiers.find (':') == string::npos) re.lines.emplace_back (l.line, l.column, string (), false); }; @@ -249,8 +257,12 @@ namespace build2 } }; - auto add_file = [&parse_path] (redirect& r, int fd, string&& w) + auto add_file = [&parse_path] (optional& r, + int fd, + string&& w) { + assert (r); // Must already be present. + const char* what (nullptr); switch (fd) { @@ -259,7 +271,7 @@ namespace build2 case 2: what = "stderr redirect path"; break; } - r.file.path = parse_path (move (w), what); + r->file.path = parse_path (move (w), what); }; switch (p) @@ -442,6 +454,9 @@ namespace build2 mod = move (t.value); + // Handle the none redirect (no data allowed) in the switch construct + // if/when the respective syntax is invented. + // redirect_type rt (redirect_type::none); switch (tt) { @@ -487,19 +502,30 @@ namespace build2 case type::out_file_app: rt = redirect_type::file; break; } - redirect& r (fd == 0 ? c.in : fd == 1 ? c.out : c.err); - redirect_type overriden (r.type); + optional& r (fd == 0 ? c.in : + fd == 1 ? c.out : + c.err); + + optional overriden; + + if (r) + overriden = r->type; r = redirect (rt); // Don't move as still may be used for pending here-document end // marker processing. // - r.modifiers = mod; + r->modifiers = mod; switch (rt) { case redirect_type::none: + // Remove the assertion if/when the none redirect syntax is + // invented. + // + assert (false); + // Fall through. case redirect_type::pass: case redirect_type::null: case redirect_type::trace: @@ -554,11 +580,9 @@ namespace build2 // Also sets for stdin, but this is harmless. // - r.file.mode = tt == type::out_file_ovr - ? redirect_fmode::overwrite - : (tt == type::out_file_app - ? redirect_fmode::append - : redirect_fmode::compare); + r->file.mode = tt == type::out_file_ovr ? redirect_fmode::overwrite : + tt == type::out_file_app ? redirect_fmode::append : + redirect_fmode::compare; break; @@ -569,8 +593,9 @@ namespace build2 // to this command redirect from the corresponding here_doc object. // if (!pre_parse_ && - (overriden == redirect_type::here_doc_literal || - overriden == redirect_type::here_doc_regex)) + overriden && + (*overriden == redirect_type::here_doc_literal || + *overriden == redirect_type::here_doc_regex)) { size_t e (expr.size () - 1); size_t p (expr.back ().pipe.size ()); @@ -1251,25 +1276,30 @@ namespace build2 auto i (h.redirects.cbegin ()); command& c (p.first[i->expr].pipe[i->pipe]); - redirect& r (i->fd == 0 ? c.in : i->fd == 1 ? c.out : c.err); + + optional& r (i->fd == 0 ? c.in : + i->fd == 1 ? c.out : + c.err); + + assert (r); // Must be present since it is referred. if (v.re) { - assert (r.type == redirect_type::here_doc_regex); + assert (r->type == redirect_type::here_doc_regex); - r.regex = move (v.regex); - r.regex.flags = move (h.regex_flags); + r->regex = move (v.regex); + r->regex.flags = move (h.regex_flags); } else { - assert (r.type == redirect_type::here_doc_literal); + assert (r->type == redirect_type::here_doc_literal); - r.str = move (v.str); + r->str = move (v.str); } - r.end = move (h.end); - r.end_line = v.end_line; - r.end_column = v.end_column; + r->end = move (h.end); + r->end_line = v.end_line; + r->end_column = v.end_column; // Note that our references cannot be invalidated because the // command_expr/command-pipe vectors already contain all their @@ -1280,7 +1310,7 @@ namespace build2 command& c (p.first[i->expr].pipe[i->pipe]); (i->fd == 0 ? c.in : i->fd == 1 ? c.out : c.err) = - redirect (redirect_type::here_doc_ref, r); + redirect (redirect_type::here_doc_ref, *r); } } diff --git a/libbuild2/script/run.cxx b/libbuild2/script/run.cxx index a1cd733..b53fbc5 100644 --- a/libbuild2/script/run.cxx +++ b/libbuild2/script/run.cxx @@ -873,9 +873,6 @@ namespace build2 env.clean ({cl.type, move (np)}, false); } - const redirect& in (c.in.effective ()); - const redirect& out (c.out.effective ()); - const redirect& err (c.err.effective ()); bool eq (c.exit.comparison == exit_comparison::eq); // If stdin file descriptor is not open then this is the first pipeline @@ -888,6 +885,25 @@ namespace build2 const string& program (c.program.string ()); + const redirect& in ((c.in ? *c.in : env.in).effective ()); + + const redirect* out (!last + ? nullptr // stdout is piped. + : &(c.out ? *c.out : env.out).effective ()); + + const redirect& err ((c.err ? *c.err : env.err).effective ()); + + auto process_args = [&c] () -> cstrings + { + cstrings args {c.program.string ().c_str ()}; + + for (const auto& a: c.arguments) + args.push_back (a.c_str ()); + + args.push_back (nullptr); + return args; + }; + // Prior to opening file descriptors for command input/output // redirects let's check if the command is the exit builtin. Being a // builtin syntactically it differs from the regular ones in a number @@ -914,13 +930,13 @@ namespace build2 if (!first || !last) fail (ll) << "exit builtin must be the only pipe command"; - if (in.type != redirect_type::none) + if (c.in) fail (ll) << "exit builtin stdin cannot be redirected"; - if (out.type != redirect_type::none) + if (c.out) fail (ll) << "exit builtin stdout cannot be redirected"; - if (err.type != redirect_type::none) + 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 @@ -929,6 +945,9 @@ namespace build2 if (eq != (c.exit.code == 0)) fail (ll) << "exit builtin exit code cannot be non-zero"; + if (verb >= 2) + print_process (process_args ()); + exit_builtin (c.arguments, ll); // Throws exit exception. } @@ -963,7 +982,7 @@ namespace build2 path isp; if (!first) - assert (in.type == redirect_type::none); // No redirect expected. + assert (!c.in); // No redirect expected. else { // Open a file for passing to the command stdin. @@ -997,7 +1016,6 @@ 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 @@ -1020,7 +1038,6 @@ namespace build2 ifd = open_null (); break; } - case redirect_type::file: { isp = normalize (in.file.path, env, ll); @@ -1028,7 +1045,6 @@ namespace build2 open_stdin (); break; } - case redirect_type::here_str_literal: case redirect_type::here_doc_literal: { @@ -1071,15 +1087,18 @@ namespace build2 if (!last) fail (ll) << "set builtin must be the last pipe command"; - if (out.type != redirect_type::none) + if (c.out) fail (ll) << "set builtin stdout cannot be redirected"; - if (err.type != redirect_type::none) + 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 (verb >= 2) + print_process (process_args ()); + set_builtin (env, c.arguments, move (ifd), ll); return true; } @@ -1200,10 +1219,10 @@ namespace build2 // "tightening". // if (last) - ofd.out = open (out, 1, osp); + ofd.out = open (*out, 1, osp); else { - assert (out.type == redirect_type::none); // No redirect expected. + assert (!c.out); // No redirect expected. ofd = open_pipe (); } @@ -1212,7 +1231,7 @@ namespace build2 // Merge standard streams. // - bool mo (out.type == redirect_type::merge); + bool mo (out != nullptr && out->type == redirect_type::merge); if (mo || err.type == redirect_type::merge) { auto_fd& self (mo ? ofd.out : efd); @@ -1239,17 +1258,6 @@ namespace build2 bool success; - auto process_args = [&c] () -> cstrings - { - cstrings args {c.program.string ().c_str ()}; - - for (const auto& a: c.arguments) - args.push_back (a.c_str ()); - - args.push_back (nullptr); - return args; - }; - if (bf != nullptr) { // Execute the builtin. @@ -1654,7 +1662,7 @@ namespace build2 success = check_output (pr, esp, isp, err, ll, env, diag, "stderr") && (!last || - check_output (pr, osp, isp, out, ll, env, diag, "stdout")); + check_output (pr, osp, isp, *out, ll, env, diag, "stdout")); return success; } diff --git a/libbuild2/script/script.cxx b/libbuild2/script/script.cxx index db34084..349b05e 100644 --- a/libbuild2/script/script.cxx +++ b/libbuild2/script/script.cxx @@ -272,14 +272,17 @@ namespace build2 // Redirects. // - if (c.in.effective ().type != redirect_type::none) - print_redirect (c.in.effective (), "<"); + // Print the none redirect (no data allowed) if/when the respective + // syntax is invened. + // + if (c.in && c.in->effective ().type != redirect_type::none) + print_redirect (c.in->effective (), "<"); - if (c.out.effective ().type != redirect_type::none) - print_redirect (c.out.effective (), ">"); + if (c.out && c.out->effective ().type != redirect_type::none) + print_redirect (c.out->effective (), ">"); - if (c.err.effective ().type != redirect_type::none) - print_redirect (c.err.effective (), "2>"); + if (c.err && c.err->effective ().type != redirect_type::none) + print_redirect (c.err->effective (), "2>"); for (const auto& p: c.cleanups) { @@ -307,17 +310,20 @@ namespace build2 { // Here-documents. // - if (c.in.type == redirect_type::here_doc_literal || - c.in.type == redirect_type::here_doc_regex) - print_doc (c.in); - - if (c.out.type == redirect_type::here_doc_literal || - c.out.type == redirect_type::here_doc_regex) - print_doc (c.out); - - if (c.err.type == redirect_type::here_doc_literal || - c.err.type == redirect_type::here_doc_regex) - print_doc (c.err); + if (c.in && + (c.in->type == redirect_type::here_doc_literal || + c.in->type == redirect_type::here_doc_regex)) + print_doc (*c.in); + + if (c.out && + (c.out->type == redirect_type::here_doc_literal || + c.out->type == redirect_type::here_doc_regex)) + print_doc (*c.out); + + if (c.err && + (c.err->type == redirect_type::here_doc_literal || + c.err->type == redirect_type::here_doc_regex)) + print_doc (*c.err); } } @@ -400,7 +406,7 @@ namespace build2 } redirect:: - redirect (redirect&& r) + redirect (redirect&& r) noexcept : type (r.type), modifiers (move (r.modifiers)), end (move (r.end)), @@ -441,6 +447,17 @@ namespace build2 } } + redirect& redirect:: + operator= (redirect&& r) noexcept + { + if (this != &r) + { + this->~redirect (); + new (this) redirect (move (r)); // Assume noexcept move-constructor. + } + return *this; + } + redirect:: ~redirect () { @@ -468,14 +485,53 @@ namespace build2 } } - redirect& redirect:: - operator= (redirect&& r) + redirect:: + redirect (const redirect& r) + : type (r.type), + modifiers (r.modifiers), + end (r.end), + end_line (r.end_line), + end_column (r.end_column) { - if (this != &r) + switch (type) { - this->~redirect (); - new (this) redirect (move (r)); // Assume noexcept move-constructor. + case redirect_type::none: + case redirect_type::pass: + case redirect_type::null: + case redirect_type::trace: break; + + case redirect_type::merge: fd = r.fd; break; + + case redirect_type::here_str_literal: + case redirect_type::here_doc_literal: + { + new (&str) string (r.str); + break; + } + case redirect_type::here_str_regex: + case redirect_type::here_doc_regex: + { + new (®ex) regex_lines (r.regex); + break; + } + case redirect_type::file: + { + new (&file) file_type (r.file); + break; + } + case redirect_type::here_doc_ref: + { + new (&ref) reference_wrapper (r.ref); + break; + } } + } + + redirect& redirect:: + operator= (const redirect& r) + { + if (this != &r) + *this = redirect (r); // Reduce to move-assignment. return *this; } diff --git a/libbuild2/script/script.hxx b/libbuild2/script/script.hxx index ac9383c..7d3fdd0 100644 --- a/libbuild2/script/script.hxx +++ b/libbuild2/script/script.hxx @@ -58,7 +58,14 @@ namespace build2 // enum class redirect_type { + // No data is allowed to be read or written. + // + // Note that redirect of this type cannot be currently specified on the + // script command line and can only be set via the environment object + // as a default redirect (see below). + // none, + pass, null, trace, @@ -162,7 +169,7 @@ namespace build2 // Create redirect of a type other than reference. // explicit - redirect (redirect_type = redirect_type::none); + redirect (redirect_type); // Create redirect of the reference type. // @@ -175,10 +182,27 @@ namespace build2 r.type != redirect_type::here_doc_ref); } - // Move constuctible/assignable-only type. + // Create redirect of the merge type. + // + // Note that it's the caller's responsibility to make sure that the file + // descriptor is valid for this redirect (2 for stdout, etc). // - redirect (redirect&&); - redirect& operator= (redirect&&); + redirect (redirect_type t, int f) + : type (redirect_type::merge), fd (f) + { + assert (t == redirect_type::merge && (f == 1 || f == 2)); + } + + redirect (redirect&&) noexcept; + redirect& operator= (redirect&&) noexcept; + + // Note that defining optional movable-only redirects in the command + // class make the older C++ compilers (GCC 4.9, Clang 4, VC 15) fail to + // compile the command vector manipulating code. Thus, we make the + // redirect class copyable to workaround the issue. + // + redirect (const redirect&); + redirect& operator= (const redirect&); ~redirect (); @@ -256,9 +280,9 @@ namespace build2 path program; strings arguments; - redirect in; - redirect out; - redirect err; + optional in; + optional out; + optional err; script::cleanups cleanups; @@ -336,16 +360,31 @@ namespace build2 const string& work_dir_name; const string& sandbox_dir_name; + // Process streams default redirects. + // + // If a stream redirect is not specified on the script command line, + // then the respective redirect data member will be used. + // + const redirect in; + const redirect out; + const redirect err; + environment (build2::context& ctx, const target_triplet& pt, const dir_path& wd, const string& wn, - const dir_path& sd, const string& sn) + const dir_path& sd, const string& sn, + redirect&& i = redirect (redirect_type::pass), + redirect&& o = redirect (redirect_type::pass), + redirect&& e = redirect (redirect_type::pass)) : context (ctx), platform (pt), work_dir (wd), sandbox_dir (sd), work_dir_name (wn), - sandbox_dir_name (sn) + sandbox_dir_name (sn), + in (move (i)), + out (move (o)), + err (move (e)) { } @@ -353,8 +392,15 @@ namespace build2 // environment (build2::context& ctx, const target_triplet& pt, - const dir_path& wd, const string& wn) - : environment (ctx, pt, wd, wn, empty_dir_path, empty_string) + const dir_path& wd, const string& wn, + redirect&& i = redirect (redirect_type::pass), + redirect&& o = redirect (redirect_type::pass), + redirect&& e = redirect (redirect_type::pass)) + : environment (ctx, + pt, + wd, wn, + empty_dir_path, empty_string, + move (i), move (o), move (e)) { } diff --git a/libbuild2/test/script/script.cxx b/libbuild2/test/script/script.cxx index 2b99fc2..ed9f2e7 100644 --- a/libbuild2/test/script/script.cxx +++ b/libbuild2/test/script/script.cxx @@ -63,7 +63,10 @@ namespace build2 environment (root.test_target.ctx, test_tt (), wd_path (), wd_name, - p != nullptr ? root.work_dir : wd_path (), sd_name), + p != nullptr ? root.work_dir : wd_path (), sd_name, + redirect (redirect_type::none), + redirect (redirect_type::none), + redirect (redirect_type::none)), parent (p), id_path (cast (assign (root.id_var) = path ())) { diff --git a/libbuild2/test/script/script.hxx b/libbuild2/test/script/script.hxx index e31bf09..163ad65 100644 --- a/libbuild2/test/script/script.hxx +++ b/libbuild2/test/script/script.hxx @@ -24,6 +24,8 @@ namespace build2 { using build2::script::line; using build2::script::lines; + using build2::script::redirect; + using build2::script::redirect_type; using build2::script::line_type; using build2::script::command_expr; -- cgit v1.1