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.

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<target_triplet> (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<redirect>& 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<redirect>& 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<redirect>& 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<redirect>& 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<redirect>& r (fd == 0 ? c.in  :
+                               fd == 1 ? c.out :
+                                         c.err);
+
+        optional<redirect_type> 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<redirect>& 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 (&regex) 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<const redirect> (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<redirect> in;
+      optional<redirect> out;
+      optional<redirect> 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<path> (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;