Make build script to create special files in temporary directory

6 files changed, 200 insertions, 47 deletions

diff --git a/libbuild2/build/script/runner.cxx b/libbuild2/build/script/runner.cxx
index 5535bbb..c4a93fd 100644
--- a/libbuild2/build/script/runner.cxx
+++ b/libbuild2/build/script/runner.cxx
@@ -3,8 +3,12 @@
 
 #include <libbuild2/build/script/runner.hxx>
 
+#include <libbutl/filesystem.mxx>
+
 #include <libbuild2/script/run.hxx>
 
+using namespace butl;
+
 namespace build2
 {
   namespace build
@@ -12,15 +16,104 @@ namespace build2
     namespace script
     {
       void default_runner::
-      enter (environment&, const location&)
+      enter (environment& env, const location& ll)
       {
-        // Noop.
+        // Create the temporary directory for this run regardless of the
+        // dry-run mode, since some commands still can be executed (see run()
+        // for details). This also a reason for not using the build2
+        // filesystem API that considers the dry-run mode.
+        //
+        // Note that the directory auto-removal is active.
+        //
+        dir_path& td (env.temp_dir.path);
+
+        try
+        {
+          td = dir_path::temp_path ("build2-build-script");
+        }
+        catch (const system_error& e)
+        {
+          // While there can be no fault of the script being currently
+          // executed let's add the location anyway to ease the
+          // troubleshooting. And let's stick to that principle down the road.
+          //
+          fail (ll) << "unable to obtain temporary directory for buildscript "
+                    << "execution" << e;
+        }
+
+        mkdir_status r;
+
+        try
+        {
+          r = try_mkdir (td);
+        }
+        catch (const system_error& e)
+        {
+          fail(ll) << "unable to create temporary directory '" << td << "': "
+                   << e << endf;
+        }
+
+        // Note that the temporary directory can potentially stay after some
+        // abnormally terminated script run. Clean it up and reuse if that's
+        // the case.
+        //
+        if (r == mkdir_status::already_exists)
+        try
+        {
+          butl::rmdir_r (td, false /* dir */);
+        }
+        catch (const system_error& e)
+        {
+          fail (ll) << "unable to cleanup temporary directory '" << td
+                    << "': " << e;
+        }
+
+        if (verb >= 3)
+          text << "mkdir " << td;
       }
 
       void default_runner::
       leave (environment& env, const location& ll)
       {
         clean (env, ll);
+
+        // Note that since the temporary directory may only contain special
+        // files that are created and registered for cleanup by the script
+        // running machinery and should all be removed by the above clean()
+        // function call, its removal failure may not be the script fault but
+        // potentially a bug or a filesystem problem. Thus, we don't ignore
+        // the errors and report them.
+        //
+        env.temp_dir.cancel ();
+
+        const dir_path& td (env.temp_dir.path);
+
+        try
+        {
+          // Note that the temporary directory must be empty to date.
+          //
+          rmdir_status r (try_rmdir (td));
+
+          if (r != rmdir_status::success)
+          {
+            diag_record dr (fail (ll));
+            dr << "temporary directory '" << td
+               << (r == rmdir_status::not_exist
+                   ? "' does not exist"
+                   : "' is not empty");
+
+            if (r == rmdir_status::not_empty)
+              build2::script::print_dir (dr, td, ll);
+          }
+        }
+        catch (const system_error& e)
+        {
+          fail (ll) << "unable to remove temporary directory '" << td << "': "
+                    << e;
+        }
+
+        if (verb >= 3)
+          text << "rmdir " << td;
       }
 
       void default_runner::
diff --git a/libbuild2/build/script/script.cxx b/libbuild2/build/script/script.cxx
index cbd41c7..b43203c 100644
--- a/libbuild2/build/script/script.cxx
+++ b/libbuild2/build/script/script.cxx
@@ -24,8 +24,8 @@ namespace build2
           : build2::script::environment (
               pt.ctx,
               cast<target_triplet> (pt.ctx.global_scope["build.host"]),
-              work,
-              wd_name,
+              work, wd_name,
+              temp_dir.path, false /* temp_dir_keep */,
               redirect (redirect_type::none),
               redirect (redirect_type::merge, 2),
               redirect (redirect_type::pass)),
diff --git a/libbuild2/build/script/script.hxx b/libbuild2/build/script/script.hxx
index 29d62aa..f8306e1 100644
--- a/libbuild2/build/script/script.hxx
+++ b/libbuild2/build/script/script.hxx
@@ -9,6 +9,7 @@
 #include <libbuild2/utility.hxx>
 
 #include <libbuild2/variable.hxx>
+#include <libbuild2/filesystem.hxx> // auto_rmdir
 
 #include <libbuild2/script/script.hxx>
 
@@ -58,9 +59,6 @@ namespace build2
         location end_loc;
       };
 
-      //@@ Does environment need script? Can't we just pass it to parser along
-      //   with environment.
-      //
       class environment: public build2::script::environment
       {
       public:
@@ -83,6 +81,16 @@ namespace build2
         //
         variable_map vars;
 
+        // Temporary directory for the script run (see build2::script::
+        // environment::temp_dir for details).
+        //
+        // Currently this directory is removed regardless of the script
+        // execution success or failure. Later, to ease the troubleshooting,
+        // we may invent the build2 option suppressing the directory removal
+        // on failure.
+        //
+        auto_rmdir temp_dir;
+
         virtual void
         set_variable (string&& name, names&&, const string& attrs) override;
 
diff --git a/libbuild2/script/run.cxx b/libbuild2/script/run.cxx
index b53fbc5..3474ccb 100644
--- a/libbuild2/script/run.cxx
+++ b/libbuild2/script/run.cxx
@@ -25,12 +25,12 @@ namespace build2
   namespace script
   {
     // Normalize a path. Also make the relative path absolute using the
-    // environment's working directory unless it is already absolute.
+    // specified directory unless it is already absolute.
     //
     static path
-    normalize (path p, environment& env, const location& l)
+    normalize (path p, const dir_path& d, const location& l)
     {
-      path r (p.absolute () ? move (p) : env.work_dir / move (p));
+      path r (p.absolute () ? move (p) : d / move (p));
 
       try
       {
@@ -178,6 +178,15 @@ namespace build2
       return r;
     }
 
+    // Return true if a path is not under the script temporary directory or
+    // this directory will not be removed on failure.
+    //
+    static inline bool
+    avail_on_failure (const path& p, const environment& env)
+    {
+      return env.temp_dir_keep || !p.sub (env.temp_dir);
+    }
+
     // Check if the script command output matches the expected result
     // (redirect value). Noop for redirect types other than none, here_*.
     //
@@ -191,19 +200,22 @@ namespace build2
                   bool diag,
                   const char* what)
     {
-      auto input_info = [&ip, &ll] (diag_record& d)
+      auto input_info = [&ip, &ll, &env] (diag_record& d)
       {
-        if (non_empty (ip, ll))
+        if (non_empty (ip, ll) && avail_on_failure (ip, env))
           d << info << "stdin: " << ip;
       };
 
-      auto output_info = [&what, &ll] (diag_record& d,
-                                       const path& p,
-                                       const char* prefix = "",
-                                       const char* suffix = "")
+      auto output_info = [&what, &ll, &env] (diag_record& d,
+                                             const path& p,
+                                             const char* prefix = "",
+                                             const char* suffix = "")
       {
         if (non_empty (p, ll))
-          d << info << prefix << what << suffix << ": " << p;
+        {
+          if (avail_on_failure (p, env))
+            d << info << prefix << what << suffix << ": " << p;
+        }
         else
           d << info << prefix << what << suffix << " is empty";
       };
@@ -220,8 +232,10 @@ namespace build2
         if (diag)
         {
           diag_record d (error (ll));
-          d << pr << " unexpectedly writes to " << what <<
-            info << what << ": " << op;
+          d << pr << " unexpectedly writes to " << what;
+
+          if (avail_on_failure (op, env))
+            d << info << what << ": " << op;
 
           input_info (d);
 
@@ -246,7 +260,7 @@ namespace build2
         path eop;
 
         if (rd.type == redirect_type::file)
-          eop = normalize (rd.file.path, env, ll);
+          eop = normalize (rd.file.path, env.work_dir, ll);
         else
         {
           eop = path (op + ".orig");
@@ -263,7 +277,17 @@ namespace build2
         path dp ("diff");
         process_path pp (run_search (dp, true));
 
-        cstrings args {pp.recall_string (), "-u"};
+        cstrings args {pp.recall_string ()};
+
+        // If both files being compared won't be available on failure, then
+        // instruct diff not to print the file paths. It seems that the only
+        // way to achieve this is to abandon the output unified format in the
+        // favor of the minimal output, which normally is still informative
+        // enough for the troubleshooting (contains the difference line
+        // numbers, etc).
+        //
+        if (avail_on_failure (eop, env) || avail_on_failure (op, env))
+          args.push_back ("-u");
 
         // Ignore Windows newline fluff if that's what we are running on.
         //
@@ -317,6 +341,8 @@ namespace build2
             diag_record d (fail (ll));
             print_process (d, args);
             d << " " << pe;
+
+            print_file (d, ep, ll);
           }
 
           // Output doesn't match the expected result.
@@ -568,7 +594,11 @@ namespace build2
           //
           d << "invalid " << what << " regex redirect" << e;
 
-          output_info (d, save_regex (), "", " regex");
+          // It would be a waste to save the regex into the file just to
+          // remove it.
+          //
+          if (env.temp_dir_keep)
+            output_info (d, save_regex (), "", " regex");
         }
 
         // Parse the output into the literal line string.
@@ -621,13 +651,17 @@ namespace build2
         if (regex_match (ls, regex)) // Doesn't throw.
           return true;
 
-        // Output doesn't match the regex. We save the regex to file for
-        // troubleshooting regardless of whether we print the diagnostics or
-        // not. We, however, register it for cleanup in the later case (the
-        // expression may still succeed, we can be evaluating the if
-        // condition, etc).
+        // Output doesn't match the regex.
         //
-        path rp (save_regex ());
+        // Unless the temporary directory is removed on failure, we save the
+        // regex to file for troubleshooting regardless of whether we print
+        // the diagnostics or not. We, however, register it for cleanup in the
+        // later case (the expression may still succeed, we can be evaluating
+        // the if condition, etc).
+        //
+        optional<path> rp;
+        if (env.temp_dir_keep)
+          rp = save_regex ();
 
         if (diag)
         {
@@ -635,15 +669,18 @@ namespace build2
           d << pr << " " << what << " doesn't match regex";
 
           output_info (d, op);
-          output_info (d, rp, "", " regex");
+
+          if (rp)
+            output_info (d, *rp, "", " regex");
+
           input_info  (d);
 
           // Print cached output.
           //
           print_file (d, op, ll);
         }
-        else
-          env.clean_special (rp);
+        else if (rp)
+          env.clean_special (*rp);
 
         // Fall through (to return false).
         //
@@ -856,7 +893,7 @@ namespace build2
       for (const auto& cl: c.cleanups)
       {
         const path& p (cl.path);
-        path np (normalize (p, env, ll));
+        path np (normalize (p, env.work_dir, ll));
 
         const string& ls (np.leaf ().string ());
         bool wc (ls == "*" || ls == "**" || ls == "***");
@@ -973,7 +1010,7 @@ namespace build2
         if (ci > 0)
           p += "-" + to_string (ci);
 
-        return normalize (move (p), env, ll);
+        return normalize (move (p), env.temp_dir, ll);
       };
 
       // If this is the first pipeline command, then open stdin descriptor
@@ -1040,7 +1077,7 @@ namespace build2
           }
         case redirect_type::file:
           {
-            isp = normalize (in.file.path, env, ll);
+            isp = normalize (in.file.path, env.work_dir, ll);
 
             open_stdin ();
             break;
@@ -1155,7 +1192,7 @@ namespace build2
             //
             p = r.file.mode == redirect_fmode::compare
               ? std_path (what)
-              : normalize (r.file.path, env, ll);
+              : normalize (r.file.path, env.work_dir, ll);
 
             m |= r.file.mode == redirect_fmode::append
               ? fdopen_mode::at_end
@@ -1520,10 +1557,10 @@ namespace build2
           if (p.relative ())
           {
             auto program = [&p, &args] (path pp)
-              {
-                p = move (pp);
-                args[0] = p.string ().c_str ();
-              };
+            {
+              p = move (pp);
+              args[0] = p.string ().c_str ();
+            };
 
             if (p.simple ())
             {
@@ -1636,13 +1673,13 @@ namespace build2
             assert (false);
         }
 
-        if (non_empty (esp, ll))
+        if (non_empty (esp, ll) && avail_on_failure (esp, env))
           d << info << "stderr: " << esp;
 
-        if (non_empty (osp, ll))
+        if (non_empty (osp, ll) && avail_on_failure (osp, env))
           d << info << "stdout: " << osp;
 
-        if (non_empty (isp, ll))
+        if (non_empty (isp, ll) && avail_on_failure (isp, env))
           d << info << "stdin: " << isp;
 
         // Print cached stderr.
diff --git a/libbuild2/script/script.hxx b/libbuild2/script/script.hxx
index 120cd1d..abb2fd7 100644
--- a/libbuild2/script/script.hxx
+++ b/libbuild2/script/script.hxx
@@ -353,6 +353,7 @@ namespace build2
       // an absolute path.
       //
       const dir_path& work_dir;
+      const string&   work_dir_name; // Directory name for diagnostics.
 
       // If non-empty, then any attempt to remove or move a filesystem entry
       // outside this directory using an explicit cleanup or the rm/mv
@@ -360,11 +361,18 @@ namespace build2
       // specified for the builtin. Must be an absolute path, unless is empty.
       //
       const dir_path& sandbox_dir;
+      const string&   sandbox_dir_name; // Directory name for diagnostics.
 
-      // Directory names for diagnostics.
+      // Used by the script running machinery to create special files in it.
+      // Must be an absolute path.
       //
-      const string& work_dir_name;
-      const string& sandbox_dir_name;
+      const dir_path& temp_dir;
+
+      // The temporary directory will not be removed on the script failure,
+      // which allows the script running machinery to refer to the special
+      // files in the diagnostics.
+      //
+      const bool temp_dir_keep;
 
       // Process streams default redirects.
       //
@@ -379,15 +387,18 @@ namespace build2
                    const target_triplet& pt,
                    const dir_path& wd, const string& wn,
                    const dir_path& sd, const string& sn,
+                   const dir_path& td, bool tk,
                    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 (sd),
             sandbox_dir_name (sn),
+            temp_dir (td),
+            temp_dir_keep (tk),
             in (move (i)),
             out (move (o)),
             err (move (e))
@@ -399,6 +410,7 @@ namespace build2
       environment (build2::context& ctx,
                    const target_triplet& pt,
                    const dir_path& wd, const string& wn,
+                   const dir_path& td, bool tk,
                    redirect&& i = redirect (redirect_type::pass),
                    redirect&& o = redirect (redirect_type::pass),
                    redirect&& e = redirect (redirect_type::pass))
@@ -406,6 +418,7 @@ namespace build2
                          pt,
                          wd, wn,
                          empty_dir_path, empty_string,
+                         td, tk,
                          move (i), move (o), move (e))
       {
       }
@@ -425,7 +438,8 @@ namespace build2
 
       // Register cleanup of a special file. Such files are created to
       // maintain the script running machinery and must be removed first, not
-      // to interfere with the user-defined wildcard cleanups.
+      // to interfere with the user-defined wildcard cleanups if the working
+      // and temporary directories are the same.
       //
       void
       clean_special (path);
diff --git a/libbuild2/test/script/script.cxx b/libbuild2/test/script/script.cxx
index ed9f2e7..9f8cb0b 100644
--- a/libbuild2/test/script/script.cxx
+++ b/libbuild2/test/script/script.cxx
@@ -64,6 +64,7 @@ namespace build2
                          test_tt (),
                          wd_path (), wd_name,
                          p != nullptr ? root.work_dir : wd_path (), sd_name,
+                         wd_path (), true /* temp_dir_keep */,
                          redirect (redirect_type::none),
                          redirect (redirect_type::none),
                          redirect (redirect_type::none)),