From a4bcb9f98b00885823c8c9b2b5fc53bc52c1edc9 Mon Sep 17 00:00:00 2001
From: Karen Arutyunov <karen@codesynthesis.com>
Date: Wed, 31 May 2017 14:19:18 +0300
Subject: Fix testscript runner not to remove special files with wildcard
 cleanups

---
 build2/test/script/runner.cxx         | 37 ++++++++++++++++++++++++++++-------
 build2/test/script/script.cxx         |  6 ++++++
 build2/test/script/script.hxx         |  8 ++++++++
 tests/test/script/runner/cleanup.test | 19 ++++++++++++++++++
 4 files changed, 63 insertions(+), 7 deletions(-)

diff --git a/build2/test/script/runner.cxx b/build2/test/script/runner.cxx
index 2453672..f4c54a9 100644
--- a/build2/test/script/runner.cxx
+++ b/build2/test/script/runner.cxx
@@ -282,7 +282,7 @@ namespace build2
           {
             eop = path (op + ".orig");
             save (eop, transform (rd.str, false, rd.modifiers, *sp.root), ll);
-            sp.clean ({cleanup_type::always, eop}, true);
+            sp.clean_special (eop);
           }
 
           // Use diff utility for the comparison.
@@ -317,7 +317,7 @@ namespace build2
             try
             {
               efd = fdopen (ep, fdopen_mode::out | fdopen_mode::create);
-              sp.clean ({cleanup_type::always, ep}, true);
+              sp.clean_special (ep);
             }
             catch (const io_error& e)
             {
@@ -720,11 +720,23 @@ namespace build2
         //
         if (common_.after == output_after::clean)
         {
+          // Note that we operate with normalized paths here.
+          //
+          // Remove special files. The order is not important as we don't
+          // expect directories here.
+          //
+          for (const auto& p: sp.special_cleanups)
+          {
+            // Remove the file if exists. Fail otherwise.
+            //
+            if (rmfile (p, 3) == rmfile_status::not_exist)
+              fail (ll) << "registered for cleanup special file " << p
+                        << " does not exist";
+          }
+
           // Remove files and directories in the order opposite to the order of
           // cleanup registration.
           //
-          // Note that we operate with normalized paths here.
-          //
           for (const auto& c: reverse_iterate (sp.cleanups))
           {
             cleanup_type t (c.type);
@@ -746,8 +758,14 @@ namespace build2
             {
               // Cast to uint16_t to avoid ambiguity with libbutl::rmdir_r().
               //
+
+              dir_path d (p.directory ());
+
+              // Don't remove the working directory (it will be removed by the
+              // dedicated cleanup).
+              //
               rmdir_status r (
-                rmdir_r (p.directory (), true, static_cast<uint16_t> (3)));
+                rmdir_r (d, d != sp.wd_path, static_cast<uint16_t> (3)));
 
               if (r == rmdir_status::success ||
                   (r == rmdir_status::not_exist && t == cleanup_type::maybe))
@@ -1223,7 +1241,7 @@ namespace build2
               save (
                 isp, transform (in.str, false, in.modifiers, *sp.root), ll);
 
-              sp.clean ({cleanup_type::always, isp}, true);
+              sp.clean_special (isp);
 
               open_stdin ();
               break;
@@ -1369,7 +1387,12 @@ namespace build2
             fd = fdopen (p, m);
 
             if ((m & fdopen_mode::at_end) != fdopen_mode::at_end)
-              sp.clean ({cleanup_type::always, p}, true);
+            {
+              if (rt == redirect_type::file)
+                sp.clean ({cleanup_type::always, p}, true);
+              else
+                sp.clean_special (p);
+            }
           }
           catch (const io_error& e)
           {
diff --git a/build2/test/script/script.cxx b/build2/test/script/script.cxx
index cb114a6..7ddd63a 100644
--- a/build2/test/script/script.cxx
+++ b/build2/test/script/script.cxx
@@ -489,6 +489,12 @@ namespace build2
           i->type = c.type;
       }
 
+      void scope::
+      clean_special (path p)
+      {
+        special_cleanups.emplace_back (move (p));
+      }
+
       // script_base
       //
       script_base::
diff --git a/build2/test/script/script.hxx b/build2/test/script/script.hxx
index 9aadf06..acf2b3c 100644
--- a/build2/test/script/script.hxx
+++ b/build2/test/script/script.hxx
@@ -364,6 +364,7 @@ namespace build2
 
         scope_state state = scope_state::unknown;
         test::script::cleanups cleanups;
+        paths special_cleanups;
 
         // Variables.
         //
@@ -414,6 +415,13 @@ namespace build2
         void
         clean (cleanup, bool implicit);
 
+        // Register cleanup of a special file. Such files are created to
+        // maintain testscript machinery and must be removed first, not to
+        // interfere with the user-defined wildcard cleanups.
+        //
+        void
+        clean_special (path p);
+
       public:
         virtual
         ~scope () = default;
diff --git a/tests/test/script/runner/cleanup.test b/tests/test/script/runner/cleanup.test
index fe72807..a26c1af 100644
--- a/tests/test/script/runner/cleanup.test
+++ b/tests/test/script/runner/cleanup.test
@@ -279,6 +279,25 @@ b += --no-column
 :
 $c <'$* -d a/b &a/ &a/b/' && $b
 
+: special-order
+:
+: Test that special files are cleaned before others, and there is no attempt
+: to remove them twice (normally and with wildcard cleanup).
+:
+$c <<EOI && $b
+$* -o foo >'foo';
+true &*
+EOI
+
+: wd-wildcard
+:
+: Test that there is no attempt to remove working directory twice (normally and
+: with wildcard cleanup).
+:
+$c <<EOI && $b
+true &***
+EOI
+
 : implicit-overwrite
 :
 : Test an implicit cleanup being overwritten with the explicit one,
-- 
cgit v1.1