Use dir_name_view for script environment working and sandbox directories

14 files changed, 129 insertions, 85 deletions

diff --git a/doc/testscript.cli b/doc/testscript.cli
index 5b36c5e..4c78e18 100644
--- a/doc/testscript.cli
+++ b/doc/testscript.cli
@@ -545,7 +545,7 @@ complete picture:
 \
 $* 'World' >'Hello, World!' : command-name
 
-$* 'John' 'Jane' >>EOO       : command-names
+$* 'John' 'Jane' >>EOO      : command-names
 Hello, Jane!
 Hello, John!
 EOO
@@ -943,7 +943,7 @@ Alternatively, we can use an absolute path:
 \
 
 Inside the scope working directory filesystem names that start with \c{stdin},
-\c{stdout}, and \c{stderr}.
+\c{stdout}, and \c{stderr} are reserved.
 
 To execute a test scope its commands (including variable assignments) are
 executed sequentially and in the order specified. If any of the commands
diff --git a/libbuild2/build/script/script.cxx b/libbuild2/build/script/script.cxx
index c258c4c..d27a30a 100644
--- a/libbuild2/build/script/script.cxx
+++ b/libbuild2/build/script/script.cxx
@@ -17,14 +17,14 @@ namespace build2
     {
       // environment
       //
-      static const string wd_name ("current directory");
+      static const optional<string> wd_name ("current directory");
 
       environment::
       environment (action a, const target_type& t)
           : build2::script::environment (
               t.ctx,
               cast<target_triplet> (t.ctx.global_scope["build.host"]),
-              work, wd_name,
+              dir_name_view (&work, &wd_name),
               temp_dir.path, false /* temp_dir_keep */,
               redirect (redirect_type::none),
               redirect (redirect_type::merge, 2),
diff --git a/libbuild2/script/run.cxx b/libbuild2/script/run.cxx
index 870a70f..a48421c 100644
--- a/libbuild2/script/run.cxx
+++ b/libbuild2/script/run.cxx
@@ -24,6 +24,35 @@ namespace build2
 {
   namespace script
   {
+    string
+    diag_path (const path& d)
+    {
+      string r ("'");
+
+      r += stream_verb_map ().path < 1
+           ? diag_relative (d)
+           : d.representation ();
+
+      r += '\'';
+      return r;
+    }
+
+    string
+    diag_path (const dir_name_view& dn)
+    {
+      string r;
+      if (dn.name != nullptr && *dn.name)
+      {
+        r += **dn.name;
+        r += ' ';
+      }
+
+      assert (dn.path != nullptr);
+
+      r += diag_path (*dn.path);
+      return r;
+    }
+
     // Normalize a path. Also make the relative path absolute using the
     // specified directory unless it is already absolute.
     //
@@ -260,7 +289,7 @@ namespace build2
         path eop;
 
         if (rd.type == redirect_type::file)
-          eop = normalize (rd.file.path, env.work_dir, ll);
+          eop = normalize (rd.file.path, *env.work_dir.path, ll);
         else
         {
           eop = path (op + ".orig");
@@ -894,19 +923,19 @@ namespace build2
       for (const auto& cl: c.cleanups)
       {
         const path& p (cl.path);
-        path np (normalize (p, env.work_dir, ll));
+        path np (normalize (p, *env.work_dir.path, ll));
 
         const string& ls (np.leaf ().string ());
         bool wc (ls == "*" || ls == "**" || ls == "***");
         const path& cp (wc ? np.directory () : np);
-        const dir_path& sd (env.sandbox_dir);
+        const dir_path* sd (env.sandbox_dir.path);
 
-        if (!sd.empty () && !cp.sub (sd))
+        if (sd != nullptr && !cp.sub (*sd))
           fail (ll) << (wc                ? "wildcard"  :
                         p.to_directory () ? "directory" :
                                             "file")
                     << " cleanup " << p << " is out of "
-                    << env.sandbox_dir_name << " " << sd;
+                    << diag_path (env.sandbox_dir);
 
         env.clean ({cl.type, move (np)}, false);
       }
@@ -993,6 +1022,8 @@ namespace build2
       //
       auto std_path = [&env, &ci, &li, &ll] (const char* n) -> path
       {
+        using std::to_string;
+
         path p (n);
 
         // 0 if belongs to a single-line script, otherwise is the command line
@@ -1078,7 +1109,7 @@ namespace build2
           }
         case redirect_type::file:
           {
-            isp = normalize (in.file.path, env.work_dir, ll);
+            isp = normalize (in.file.path, *env.work_dir.path, ll);
 
             open_stdin ();
             break;
@@ -1193,7 +1224,7 @@ namespace build2
             //
             p = r.file.mode == redirect_fmode::compare
               ? std_path (what)
-              : normalize (r.file.path, env.work_dir, ll);
+              : normalize (r.file.path, *env.work_dir.path, ll);
 
             m |= r.file.mode == redirect_fmode::append
               ? fdopen_mode::at_end
@@ -1378,21 +1409,20 @@ namespace build2
 
             if (pre)
             {
-              const dir_path& wd (env.work_dir);
-              const dir_path& sd (env.sandbox_dir);
+              const dir_path& wd (*env.work_dir.path);
+              const dir_path* sd (env.sandbox_dir.path);
 
               auto fail = [] (const string& d) {throw runtime_error (d);};
 
-              if (!sd.empty () && !from.sub (sd) && !force)
-                fail ("'" + from.representation () +
-                      "' is out of " + env.sandbox_dir_name + " '" +
-                      sd.string () + "'");
+              if (sd != nullptr && !from.sub (*sd) && !force)
+                fail (diag_path (from) + " is out of " +
+                      diag_path (env.sandbox_dir));
 
               auto check_wd = [&wd, &env, fail] (const path& p)
               {
                 if (wd.sub (path_cast<dir_path> (p)))
-                  fail ("'" + p.string () + "' contains " +
-                        env.work_dir_name + " '" + wd.string () + "'");
+                  fail (diag_path (p) + " contains " +
+                        diag_path (env.work_dir));
               };
 
               check_wd (from);
@@ -1404,7 +1434,7 @@ namespace build2
               //
               if (cln->enabled)
                 cln->move = !butl::entry_exists (to) &&
-                            (sd.empty () || to.sub (sd));
+                            (sd == nullptr || to.sub (*sd));
             }
             else if (cln->enabled)
             {
@@ -1469,20 +1499,18 @@ namespace build2
           {
             if (pre)
             {
-              const dir_path& wd (env.work_dir);
-              const dir_path& sd (env.sandbox_dir);
+              const dir_path& wd (*env.work_dir.path);
+              const dir_path* sd (env.sandbox_dir.path);
 
               auto fail = [] (const string& d) {throw runtime_error (d);};
 
-              if (!sd.empty () && !p.sub (sd) && !force)
-                fail ("'" + p.representation () +
-                      "' is out of " + env.sandbox_dir_name + " '" +
-                      sd.string () + "'");
+              if (sd != nullptr && !p.sub (*sd) && !force)
+                fail (diag_path (p) + " is out of " +
+                      diag_path (env.sandbox_dir));
 
               if (wd.sub (path_cast<dir_path> (p)))
-                fail ("'" + p.string () +
-                      "' contains " + env.work_dir_name +
-                      " '" + wd.string () + "'");
+                fail (diag_path (p) + " contains " +
+                      diag_path (env.work_dir));
             }
           },
 
@@ -1520,7 +1548,7 @@ namespace build2
           builtin b (bf (r,
                          c.arguments,
                          move (ifd), move (ofd.out), move (efd),
-                         env.work_dir,
+                         *env.work_dir.path,
                          bcs));
 
           success = run_pipe (env,
@@ -1573,7 +1601,7 @@ namespace build2
                 program (path (s, 1, s.size () - 1));
             }
             else
-              program (env.work_dir / p);
+              program (*env.work_dir.path / p);
           }
         }
         catch (const invalid_path& e)
@@ -1594,7 +1622,7 @@ namespace build2
             pp,
             args.data (),
             {ifd.get (), -1}, process::pipe (ofd), {-1, efd.get ()},
-            env.work_dir.string ().c_str ());
+            env.work_dir.path->string ().c_str ());
 
           ifd.reset ();
           ofd.out.reset ();
@@ -1783,7 +1811,7 @@ namespace build2
     clean (environment& env, const location& ll)
     {
       context& ctx (env.context);
-      const dir_path& wdir (env.work_dir);
+      const dir_path& wdir (*env.work_dir.path);
 
       // Note that we operate with normalized paths here.
       //
diff --git a/libbuild2/script/run.hxx b/libbuild2/script/run.hxx
index 3f73eed..477dd88 100644
--- a/libbuild2/script/run.hxx
+++ b/libbuild2/script/run.hxx
@@ -56,6 +56,19 @@ namespace build2
     //
     void
     print_dir (diag_record&, const dir_path&, const location&);
+
+    // Return the quoted path representation with the preserved trailing
+    // directory separator. The path is relative if the verbosity level is
+    // less than 3.
+    //
+    string
+    diag_path (const path&);
+
+    // Same as above, but prepends the path with a name, if present. The path
+    // must be not NULL.
+    //
+    string
+    diag_path (const dir_name_view&);
   }
 }
 
diff --git a/libbuild2/script/script.cxx b/libbuild2/script/script.cxx
index 9e8780e..c85bfd3 100644
--- a/libbuild2/script/script.cxx
+++ b/libbuild2/script/script.cxx
@@ -633,7 +633,7 @@ namespace build2
 
       const path& p (c.path);
 
-      if (!sandbox_dir.empty () && !p.sub (sandbox_dir))
+      if (sandbox_dir.path != nullptr && !p.sub (*sandbox_dir.path))
       {
         if (implicit)
           return;
diff --git a/libbuild2/script/script.hxx b/libbuild2/script/script.hxx
index 359bb36..b887df6 100644
--- a/libbuild2/script/script.hxx
+++ b/libbuild2/script/script.hxx
@@ -363,16 +363,14 @@ namespace build2
       // using the rm or mv builtins will fail the script execution. Must be
       // an absolute path.
       //
-      const dir_path& work_dir; // @@ dir_path_name
-      const string&   work_dir_name; // Directory name for diagnostics.
+      const dir_name_view work_dir;
 
-      // If non-empty, then any attempt to remove or move a filesystem entry
-      // outside this directory using an explicit cleanup or the rm/mv
+      // If path is not NULL, then any attempt to remove or move a filesystem
+      // entry outside this directory using an explicit cleanup or the rm/mv
       // builtins will fail the script execution, unless the --force option is
-      // specified for the builtin. Must be an absolute path, unless is empty.
+      // specified for the builtin. Must be an absolute path, unless is NULL.
       //
-      const dir_path& sandbox_dir; // @@ dir_path_name
-      const string&   sandbox_dir_name; // Directory name for diagnostics.
+      const dir_name_view sandbox_dir;
 
       // Used by the script running machinery to create special files in it.
       // Must be an absolute path.
@@ -396,8 +394,8 @@ namespace build2
 
       environment (build2::context& ctx,
                    const target_triplet& pt,
-                   const dir_path& wd, const string& wn,
-                   const dir_path& sd, const string& sn,
+                   const dir_name_view& wd,
+                   const dir_name_view& sd,
                    const dir_path& td, bool tk,
                    redirect&& i = redirect (redirect_type::pass),
                    redirect&& o = redirect (redirect_type::pass),
@@ -405,9 +403,7 @@ namespace build2
           : context (ctx),
             platform (pt),
             work_dir (wd),
-            work_dir_name (wn),
             sandbox_dir (sd),
-            sandbox_dir_name (sn),
             temp_dir (td),
             temp_dir_keep (tk),
             in (move (i)),
@@ -420,15 +416,15 @@ namespace build2
       //
       environment (build2::context& ctx,
                    const target_triplet& pt,
-                   const dir_path& wd, const string& wn,
+                   const dir_name_view& wd,
                    const dir_path& td, bool tk,
                    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,
+                         wd,
+                         dir_name_view (),
                          td, tk,
                          move (i), move (o), move (e))
       {
diff --git a/libbuild2/test/script/runner.cxx b/libbuild2/test/script/runner.cxx
index 6e722de..03a1f0e 100644
--- a/libbuild2/test/script/runner.cxx
+++ b/libbuild2/test/script/runner.cxx
@@ -13,6 +13,8 @@ namespace build2
   {
     namespace script
     {
+      using namespace build2::script;
+
       bool default_runner::
       test (scope& s) const
       {
@@ -49,20 +51,20 @@ namespace build2
           sp.parent == nullptr
           ? mkdir_buildignore (
             ctx,
-            sp.work_dir,
+            *sp.work_dir.path,
             sp.root.target_scope.root_scope ()->root_extra->buildignore_file,
             2)
-          : mkdir (sp.work_dir, 2));
+          : mkdir (*sp.work_dir.path, 2));
 
         if (r == mkdir_status::already_exists)
-          fail << "working directory " << sp.work_dir << " already exists" <<
+          fail << diag_path (sp.work_dir) << " already exists" <<
             info << "are tests stomping on each other's feet?";
 
         // We don't change the current directory here but indicate that the
         // scope test commands will be executed in that directory.
         //
         if (verb >= 2)
-          text << "cd " << sp.work_dir;
+          text << "cd " << *sp.work_dir.path;
       }
 
       void default_runner::
@@ -88,22 +90,23 @@ namespace build2
           rmdir_status r (
             sp.parent == nullptr
             ?  rmdir_buildignore (ctx,
-                                  sp.work_dir,
+                                  *sp.work_dir.path,
                                   sp.root.target_scope.root_scope ()->
                                     root_extra->buildignore_file,
                                   2)
-            : rmdir (ctx, sp.work_dir, 2));
+            : rmdir (ctx, *sp.work_dir.path, 2));
 
           if (r != rmdir_status::success)
           {
             diag_record dr (fail (ll));
-            dr << "working directory " << sp.work_dir
+
+            dr << diag_path (sp.work_dir)
                << (r == rmdir_status::not_exist
                    ? " does not exist"
                    : " is not empty");
 
             if (r == rmdir_status::not_empty)
-              build2::script::print_dir (dr, sp.work_dir, ll);
+              print_dir (dr, *sp.work_dir.path, ll);
           }
         }
 
@@ -112,8 +115,8 @@ namespace build2
         //
         if (verb >= 2)
           text << "cd " << (sp.parent != nullptr
-                            ? sp.parent->work_dir
-                            : sp.work_dir.directory ());
+                            ? *sp.parent->work_dir.path
+                            : sp.work_dir.path->directory ());
       }
 
       void default_runner::
diff --git a/libbuild2/test/script/script.cxx b/libbuild2/test/script/script.cxx
index 9f8cb0b..b56da1b 100644
--- a/libbuild2/test/script/script.cxx
+++ b/libbuild2/test/script/script.cxx
@@ -28,10 +28,10 @@ namespace build2
         vars.assign (root.wd_var) = dir_path ();
       }
 
-      const dir_path& scope_base::
+      const dir_path* scope_base::
       wd_path () const
       {
-        return cast<dir_path> (vars [root.wd_var]);
+        return &cast<dir_path> (vars[root.wd_var]);
       }
 
       const target_triplet& scope_base::
@@ -49,8 +49,8 @@ namespace build2
 
       // scope
       //
-      static const string wd_name ("test working directory");
-      static const string sd_name ("working directory");
+      static const optional<string> wd_name ("test working directory");
+      static const optional<string> sd_name ("working directory");
 
       scope::
       scope (const string& id, scope* p, script& r)
@@ -62,9 +62,11 @@ namespace build2
             //
             environment (root.test_target.ctx,
                          test_tt (),
-                         wd_path (), wd_name,
-                         p != nullptr ? root.work_dir : wd_path (), sd_name,
-                         wd_path (), true /* temp_dir_keep */,
+                         dir_name_view (wd_path (), &wd_name),
+                         dir_name_view (
+                           p != nullptr ? root.work_dir.path : wd_path (),
+                           &sd_name),
+                         *wd_path (), true /* temp_dir_keep */,
                          redirect (redirect_type::none),
                          redirect (redirect_type::none),
                          redirect (redirect_type::none)),
@@ -89,7 +91,8 @@ namespace build2
         // (handled in an ad hoc way).
         //
         if (p != nullptr)
-          const_cast<dir_path&> (work_dir) = dir_path (p->work_dir) /= id;
+          const_cast<dir_path&> (*work_dir.path) =
+            dir_path (*p->work_dir.path) /= id;
       }
 
       void scope::
@@ -171,7 +174,8 @@ namespace build2
         // Set the script working dir ($~) to $out_base/test/<id> (id_path
         // for root is just the id which is empty if st is 'testscript').
         //
-        const_cast<dir_path&> (work_dir) = dir_path (rwd) /= id_path.string ();
+        const_cast<dir_path&> (*work_dir.path) =
+          dir_path (rwd) /= id_path.string ();
 
         // Set the test variable at the script level. We do it even if it's
         // set in the buildfile since they use different types.
diff --git a/libbuild2/test/script/script.hxx b/libbuild2/test/script/script.hxx
index 93846df..7bdb2ac 100644
--- a/libbuild2/test/script/script.hxx
+++ b/libbuild2/test/script/script.hxx
@@ -67,7 +67,7 @@ namespace build2
       protected:
         scope_base (script&);
 
-        const dir_path&
+        const dir_path*
         wd_path () const;
 
         const target_triplet&
diff --git a/libbuild2/types.hxx b/libbuild2/types.hxx
index d9f222b..6582c3a 100644
--- a/libbuild2/types.hxx
+++ b/libbuild2/types.hxx
@@ -230,6 +230,7 @@ namespace build2
   using butl::path_name_view;
   using butl::path_name_value;
   using butl::dir_path;
+  using butl::dir_name_view;
   using butl::path_cast;
   using butl::basic_path;
   using butl::invalid_path;
@@ -377,7 +378,6 @@ namespace build2
 
   LIBBUILD2_SYMEXPORT ostream&
   operator<< (ostream&, run_phase); // utility.cxx
-
 }
 
 // In order to be found (via ADL) these have to be either in std:: or in
diff --git a/tests/test/script/builtin/mv.testscript b/tests/test/script/builtin/mv.testscript
index 31e2603..764c1e0 100644
--- a/tests/test/script/builtin/mv.testscript
+++ b/tests/test/script/builtin/mv.testscript
@@ -21,8 +21,8 @@
       : Moving path outside the testscript working directory fails.
       :
       $c <<EOI && $b
-        mv ../../a/b/c ./c 2>>/~%EOE% == 1
-        %mv: '.+/fail/a/b/c' is out of working directory '.+/fail/test'%
+        mv ../../a/b/c ./c 2>>/EOE == 1
+        mv: 'a/b/c' is out of working directory 'test/'
         EOE
         EOI
 
@@ -84,14 +84,14 @@
       : src
       :
       $c <<EOI && $b
-        mv $~ b 2>"mv: '$~' contains test working directory '$~'" != 0
+        mv $~ b 2>/"mv: 'test/1/' contains test working directory 'test/1/'" != 0
         EOI
 
       : dst
       :
       $c <<EOI && $b
         mkdir a;
-        mv a "$~" 2>"mv: '$~' contains test working directory '$~'" != 0
+        mv a "$~" 2>/"mv: 'test/1' contains test working directory 'test/1/'" != 0
         EOI
     }
 
diff --git a/tests/test/script/builtin/rm.testscript b/tests/test/script/builtin/rm.testscript
index 5b00042..21ec2a9 100644
--- a/tests/test/script/builtin/rm.testscript
+++ b/tests/test/script/builtin/rm.testscript
@@ -11,7 +11,7 @@
   : Removing scope directory fails.
   :
   $c <<EOI && $b
-    rm -r ./ 2>"rm: '$~' contains test working directory '$~'" == 1
+    rm -r ./ 2>/"rm: 'test/1/' contains test working directory 'test/1/'" == 1
     EOI
 }
 
@@ -28,8 +28,8 @@
     : Removing path outside the testscript working directory fails.
     :
     $c <<EOI && $b
-      rm ../../a/b/c 2>>/~%EOE% == 1
-      %rm: '.+/path/outside-scope/fail/a/b/c' is out of working directory '.+/path/outside-scope/fail/test'%
+      rm ../../a/b/c 2>>/EOE == 1
+      rm: 'a/b/c' is out of working directory 'test/'
       EOE
       EOI
 
diff --git a/tests/test/script/builtin/rmdir.testscript b/tests/test/script/builtin/rmdir.testscript
index 269dd58..a63a5dd 100644
--- a/tests/test/script/builtin/rmdir.testscript
+++ b/tests/test/script/builtin/rmdir.testscript
@@ -11,7 +11,7 @@
   : Removing scope directory fails.
   :
   $c <<EOI && $b
-  rmdir ./ 2>"rmdir: '$~' contains test working directory '$~'" == 1
+  rmdir ./ 2>/"rmdir: 'test/1/' contains test working directory 'test/1/'" == 1
   EOI
 
   : outside-scope
@@ -24,8 +24,8 @@
     : Removing directory outside the testscript working directory fails.
     :
     $c <<EOI && $b
-    rmdir ../../a/b/c 2>>/~%EOE% == 1
-    %rmdir: '.+/dir/outside-scope/fail/a/b/c/' is out of working directory '.+/dir/outside-scope/fail/test'%
+    rmdir ../../a/b/c 2>>/EOE == 1
+    rmdir: 'a/b/c/' is out of working directory 'test/'
     EOE
     EOI
 
diff --git a/tests/test/script/runner/cleanup.testscript b/tests/test/script/runner/cleanup.testscript
index fcbdfc2..03153e4 100644
--- a/tests/test/script/runner/cleanup.testscript
+++ b/tests/test/script/runner/cleanup.testscript
@@ -56,7 +56,7 @@ b += --no-column
   : Test explicit cleanup of a file out of the testscript working directory.
   :
   $c <'$* &../../a' && $b 2>>/EOE != 0
-  testscript:1: error: file cleanup ../../a is out of working directory test/
+  testscript:1: error: file cleanup ../../a is out of working directory 'test/'
     info: test id: 1
   EOE
 
@@ -116,7 +116,7 @@ b += --no-column
   : Test cleanup of a directory out of the testscript working directory.
   :
   $c <'$* &../../a/' && $b 2>>/EOE != 0
-  testscript:1: error: directory cleanup ../../a/ is out of working directory test/
+  testscript:1: error: directory cleanup ../../a/ is out of working directory 'test/'
     info: test id: 1
   EOE
 
@@ -150,7 +150,7 @@ b += --no-column
     : dir
     :
     $c <'$* -d a/b' && $b 2>>/EOE != 0
-    testscript:1: error: working directory test/1/ is not empty
+    testscript:1: error: test working directory 'test/1/' is not empty
     a/
       info: test id: 1
     EOE
@@ -220,7 +220,7 @@ b += --no-column
       : Test cleanup of a wildcard out of the testscript working directory.
       :
       $c <'$* &../../a/***' && $b 2>>/EOE != 0
-      testscript:1: error: wildcard cleanup ../../a/*** is out of working directory test/
+      testscript:1: error: wildcard cleanup ../../a/*** is out of working directory 'test/'
         info: test id: 1
       EOE
 
@@ -346,7 +346,7 @@ EOI
 : Test an implicit cleanup being overwritten with the explicit one,
 :
 $c <'$* -o foo >=a &!a' && $b 2>>/EOE != 0
-testscript:1: error: working directory test/1/ is not empty
+testscript:1: error: test working directory 'test/1/' is not empty
 a
   info: test id: 1
 EOE
@@ -359,7 +359,7 @@ $c <<EOO && $b 2>>/EOE != 0
 $* &!a;
 $* -o foo >=a
 EOO
-testscript:2: error: working directory test/1/ is not empty
+testscript:2: error: test working directory 'test/1/' is not empty
 a
   info: test id: 1
 EOE