Implement new testscript cleanup wildcards semantics

3 files changed, 168 insertions, 122 deletions

diff --git a/build2/test/script/runner.cxx b/build2/test/script/runner.cxx
index f4c54a9..c1bf2d4 100644
--- a/build2/test/script/runner.cxx
+++ b/build2/test/script/runner.cxx
@@ -746,99 +746,108 @@ namespace build2
             if (t == cleanup_type::never)
               continue;
 
-            const path& p (c.path);
-            const path& l (p.leaf ());
-            const string& ls (l.string ());
+            const path& cp (c.path);
 
-            // Remove the directory recursively if not current. Fail otherwise.
-            // Recursive removal of non-existing directory is not an error for
-            // 'maybe' cleanup type.
+            // Wildcard with the last component being '***' (without trailing
+            // separator) matches all files and sub-directories recursively as
+            // well as the start directories itself. So we will recursively
+            // remove the directories that match the parent (for the original
+            // path) directory wildcard.
             //
-            if (ls == "***")
-            {
-              // 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 (d, d != sp.wd_path, static_cast<uint16_t> (3)));
-
-              if (r == rmdir_status::success ||
-                  (r == rmdir_status::not_exist && t == cleanup_type::maybe))
-                continue;
+            bool recursive (cp.leaf ().representation () == "***");
+            const path& p (!recursive ? cp : cp.directory ());
 
-              // The directory is unlikely to be current but let's keep for
-              // completeness.
-              //
-              fail (ll) << "registered for cleanup wildcard " << p
-                        << (r == rmdir_status::not_empty
-                            ? " matches the current directory"
-                            : " doesn't match a directory");
-            }
-
-            // Remove files or directories using wildcard. Removal of
-            // sub-entries of non-existing directory is not an error for
-            // 'maybe' cleanup type.
+            // Remove files or directories using wildcard.
             //
-            // Note that only the leaf part of the cleanup wildcard is a
-            // pattern in terms of libbutl::path_search().
-            //
-            if (ls == "*" || ls == "**")
+            if (p.string ().find_first_of ("?*") != string::npos)
             {
-              dir_path d (p.directory ());
-
-              if (t == cleanup_type::always && !dir_exists (d))
-                fail (ll) << "registered for cleanup wildcard " << p
-                          << " doesn't match a directory";
+              bool removed (false);
 
-              if (l.to_directory ())
+              auto rm = [&cp, recursive, &removed, &sp, &ll] (path&& pe,
+                                                              const string&,
+                                                              bool interm)
               {
-                auto rm =
-                  [&p, &d, &ll] (path&& de, const string&, bool interm) -> bool
+                if (!interm)
                 {
-                  if (!interm)
-                  {
-                    dir_path sd (path_cast<dir_path> (d / de));
-
-                    // We can get not_exist here due to racing conditions, but
-                    // that's ok if somebody did our job.
-                    //
-                    rmdir_status r (rmdir (sd, 3));
-
-                    if (r != rmdir_status::not_empty)
-                      return true;
-
-                    diag_record dr (fail (ll));
-                    dr << "registered for cleanup directory " << sd
-                       << " is not empty";
+                  // While removing the entry we can get not_exist due to
+                  // racing conditions, but that's ok if somebody did our job.
+                  // Note that we still set the removed flag to true in this
+                  // case.
+                  //
+                  removed = true; // Will be meaningless on failure.
 
-                    print_dir (dr, sd, ll);
-                    dr << info << "wildcard: '" << p << "'";
+                  if (pe.to_directory ())
+                  {
+                    dir_path d (path_cast<dir_path> (pe));
+
+                    if (!recursive)
+                    {
+                      rmdir_status r (rmdir (d, 3));
+
+                      if (r != rmdir_status::not_empty)
+                        return true;
+
+                      diag_record dr (fail (ll));
+                      dr << "registered for cleanup directory " << d
+                         << " is not empty";
+
+                      print_dir (dr, d, ll);
+                      dr << info << "wildcard: '" << cp << "'";
+                    }
+                    else
+                    {
+                      // Don't remove the working directory (it will be removed
+                      // by the dedicated cleanup).
+                      //
+                      // Cast to uint16_t to avoid ambiguity with
+                      // libbutl::rmdir_r().
+                      //
+                      rmdir_status r (rmdir_r (d,
+                                               d != sp.wd_path,
+                                               static_cast<uint16_t> (3)));
+
+                      if (r != rmdir_status::not_empty)
+                        return true;
+
+                      // The directory is unlikely to be current but let's keep
+                      // for completeness.
+                      //
+                      fail (ll) << "registered for cleanup wildcard " << cp
+                                << " matches the current directory";
+                    }
                   }
+                  else
+                    rmfile (pe, 3);
+                }
 
-                  return true;
-                };
+                return true;
+              };
 
-                path_search (l, rm, d);
+              // Note that here we rely on the fact that recursive iterating
+              // goes depth-first (which make sense for the cleanup).
+              //
+              try
+              {
+                path_search (p, rm);
               }
-              else
+              catch (const system_error& e)
               {
-                auto rm = [&d] (path&& p, const string&, bool interm) -> bool
-                {
-                  if (!interm)
-                    rmfile (d / p, 3); // That's ok if not exists.
-
-                  return true;
-                };
-
-                path_search (l, rm, d);
+                fail (ll) << "unable to cleanup wildcard " << cp << ": " << e;
               }
 
-              continue;
+              // Removal of no filesystem entries is not an error for 'maybe'
+              // cleanup type.
+              //
+              if (removed || t == cleanup_type::maybe)
+                continue;
+
+              fail (ll) << "registered for cleanup wildcard " << cp
+                        << " doesn't match any "
+                        << (recursive
+                            ? "path"
+                            : p.to_directory ()
+                              ? "directory"
+                              : "file");
             }
 
             // Remove the directory if exists and empty. Fail otherwise.
@@ -853,13 +862,22 @@ namespace build2
               // level 2 (that was used for its creation). For other
               // directories use level 3 (as for other cleanups).
               //
+              int v (d == sp.wd_path ? 2 : 3);
+
+              // Don't remove the working directory for the recursive cleanup
+              // (it will be removed by the dedicated one).
+              //
               // @@ If 'd' is a file then will fail with a diagnostics having
               //    no location info. Probably need to add an optional location
               //    parameter to rmdir() function. The same problem exists for
               //    a file cleanup when try to rmfile() directory instead of
               //    file.
               //
-              rmdir_status r (rmdir (d, d == sp.wd_path ? 2 : 3));
+              rmdir_status r (!recursive
+                              ? rmdir (d, v)
+                              : rmdir_r (d,
+                                         d != sp.wd_path,
+                                         static_cast <uint16_t> (v)));
 
               if (r == rmdir_status::success ||
                   (r == rmdir_status::not_exist && t == cleanup_type::maybe))
@@ -867,9 +885,11 @@ namespace build2
 
               diag_record dr (fail (ll));
               dr << "registered for cleanup directory " << d
-                 << (r == rmdir_status::not_empty
-                     ? " is not empty"
-                     : " does not exist");
+                 << (r == rmdir_status::not_exist
+                     ? " does not exist"
+                     : !recursive
+                       ? " is not empty"
+                       : " is current");
 
               if (r == rmdir_status::not_empty)
                 print_dir (dr, d, ll);
diff --git a/tests/test/config-test/testscript b/tests/test/config-test/testscript
index d918a3b..0d08eb0 100644
--- a/tests/test/config-test/testscript
+++ b/tests/test/config-test/testscript
@@ -7,7 +7,7 @@
 
 test.options   = --serial-stop --quiet
 test.arguments = 'test(../proj/@./)' # Test out-of-src (for parallel).
-test.cleanups  = &**/                # Cleanup out directory structure.
+test.cleanups  = &?**/               # Cleanup out directory structure.
 
 +mkdir proj
 +mkdir proj/build
diff --git a/tests/test/script/runner/cleanup.test b/tests/test/script/runner/cleanup.test
index a26c1af..68d2014 100644
--- a/tests/test/script/runner/cleanup.test
+++ b/tests/test/script/runner/cleanup.test
@@ -163,47 +163,73 @@ b += --no-column
   : self
   :
   {
-    : always
+    : dirs
     :
-    $c <'$* -d a/b -f a/b/c &a/***' && $b
+    {
+      : always
+      :
+      $c <'$* -d a/b -d a/b/c &a/***/' && $b
 
-    : maybe
-    :
-    $c <'$* &?a/***' && $b
+      : maybe
+      :
+      $c <'$* &?a/***/' && $b
 
-    : not-exists
-    :
-    : Test cleanup of a wildcard not matching any directory.
-    :
-    $c <'$* &a/***' && $b 2>>/EOE != 0
-    testscript:1: error: registered for cleanup wildcard test/1/a/*** doesn't match a directory
-    EOE
+      : not-empty
+      :
+      $c <'$* -d a/b -d a/b/c -f a/c &a/***/' && $b 2>>/EOE != 0
+      testscript:1: error: registered for cleanup directory test/1/a/ is not empty
+      c
+        info: wildcard: 'test/1/a/***/'
+      EOE
+    }
 
-    : out-wd
+    : all-entries
     :
-    : Test cleanup of a wildcard out of the testscript working directory.
+    : Test the trailing triple-star special case.
     :
-    $c <'$* &../../a/***' && $b 2>>/EOE != 0
-    testscript:1: error: wildcard cleanup ../../a/*** is out of working directory test/
-    EOE
+    {
+      : always
+      :
+      $c <'$* -d a1/b -f a1/b/c -d a2/b -f a2/b/c &a?/***' && $b
 
-    : in-wd
-    :
-    : Test cleanup registration of a wildcard matching the directory that being
-    : outside the test working directory is inside the testscript working
-    : directory.
-    :
-    $c <'$* &../a/***' && $b 2>>/EOE != 0
-    testscript:1: error: registered for cleanup wildcard test/a/*** doesn't match a directory
-    EOE
+      : maybe
+      :
+      $c <'$* &?a/***' && $b
 
-    : not-dir
-    :
-    : Test cleanup of a file as a wildcard.
-    :
-    $c <'$* -f a &a/***' && $b 2>>/~%EOE% != 0
-    %error: unable to remove directory test/1/a/: .+%
-    EOE
+      : not-exists
+      :
+      : Test cleanup of a wildcard not matching any directory.
+      :
+      $c <'$* &a/***' && $b 2>>/EOE != 0
+      testscript:1: error: registered for cleanup directory test/1/a/ does not exist
+      EOE
+
+      : out-wd
+      :
+      : 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/
+      EOE
+
+      : in-wd
+      :
+      : Test cleanup registration of a wildcard matching the directory that being
+      : outside the test working directory is inside the testscript working
+      : directory.
+      :
+      $c <'$* &../a/***' && $b 2>>/EOE != 0
+      testscript:1: error: registered for cleanup directory test/a/ does not exist
+      EOE
+
+      : not-dir
+      :
+      : Test cleanup of a file as a wildcard.
+      :
+      $c <'$* -f a &a/***' && $b 2>>/~%EOE% != 0
+      %error: unable to remove directory test/1/a/: .*%
+      EOE
+    }
   }
 
   : dir
@@ -214,11 +240,11 @@ b += --no-column
     {
       : immediate
       :
-      $c <'$* -d a/b &a/ &a/*/' && $b
+      $c <'$* -d aa/b &aa/ &a*/*/' && $b
 
       : recursive
       :
-      $c <'$* -d a/b/c &a/ &a/**/' && $b
+      $c <'$* -d aa/b/c &aa/ &a?/**/' && $b
     }
 
     : maybe
@@ -230,7 +256,7 @@ b += --no-column
     : Test cleanup of a wildcard that doesn't match any directory.
     :
     $c <'$* &a/**/' && $b 2>>/EOE != 0
-    testscript:1: error: registered for cleanup wildcard test/1/a/**/ doesn't match a directory
+    testscript:1: error: registered for cleanup wildcard test/1/a/**/ doesn't match any directory
     EOE
 
     : not-dir
@@ -238,7 +264,7 @@ b += --no-column
     : Test cleanup of a file as a directory wildcard.
     :
     $c <'$* -f a &a/**/' && $b 2>>/EOE != 0
-    testscript:1: error: registered for cleanup wildcard test/1/a/**/ doesn't match a directory
+    testscript:1: error: registered for cleanup wildcard test/1/a/**/ doesn't match any directory
     EOE
 
     : not-empty
@@ -260,11 +286,11 @@ b += --no-column
     {
       : immediate
       :
-      $c <'$* -d a -f a/c &a/ &a/*' && $b
+      $c <'$* -d aa -f aa/c &aa/ &a?/*' && $b
 
       : recursive
       :
-      $c <'$* -d a/b -f a/c -f a/b/e &a/ &a/b/ &a/**' && $b
+      $c <'$* -d aa/b -f aa/c -f aa/b/e &aa/ &aa/b/ &a*/**' && $b
     }
 
     : maybe
@@ -286,7 +312,7 @@ $c <'$* -d a/b &a/ &a/b/' && $b
 :
 $c <<EOI && $b
 $* -o foo >'foo';
-true &*
+$* -f bar &*
 EOI
 
 : wd-wildcard