Make rmdir_r() to call entry_exists(path) rather than exists(dir_path)

4 files changed, 31 insertions, 14 deletions

diff --git a/build2/filesystem b/build2/filesystem
index 325d28d..6a730c5 100644
--- a/build2/filesystem
+++ b/build2/filesystem
@@ -83,8 +83,8 @@ namespace build2
   fs_status<rmdir_status>
   rmdir_r (const dir_path&, bool dir = true, uint16_t verbosity = 1);
 
-  // Check for a file or directory existence. Print the diagnostics and fail on
-  // system error.
+  // Check for a file, directory or filesystem entry existence. Print the
+  // diagnostics and fail on system error.
   //
   bool
   exists (const path&, bool follow_symlinks = true);
@@ -93,6 +93,12 @@ namespace build2
   exists (const dir_path&);
 
   bool
+  entry_exists (const path&, bool follow_symlinks = false);
+
+  // Check for a directory emptiness. Print the diagnostics and fail on system
+  // error.
+  //
+  bool
   empty (const dir_path&);
 }
 
diff --git a/build2/filesystem.cxx b/build2/filesystem.cxx
index be4cad8..c71d2da 100644
--- a/build2/filesystem.cxx
+++ b/build2/filesystem.cxx
@@ -79,7 +79,7 @@ namespace build2
     if (work.sub (d)) // Don't try to remove working directory.
       return rmdir_status::not_empty;
 
-    if (!exists (d))
+    if (!build2::entry_exists (d))
       return rmdir_status::not_exist;
 
     if (verb >= v)
@@ -126,6 +126,20 @@ namespace build2
   }
 
   bool
+  entry_exists (const path& p, bool fs)
+  {
+    try
+    {
+      return butl::entry_exists (p, fs);
+    }
+    catch (const system_error& e)
+    {
+      error << "unable to stat path " << p << ": " << e.what ();
+      throw failed ();
+    }
+  }
+
+  bool
   empty (const dir_path& d)
   {
     try
diff --git a/build2/test/script/runner.cxx b/build2/test/script/runner.cxx
index 2af5865..79201b2 100644
--- a/build2/test/script/runner.cxx
+++ b/build2/test/script/runner.cxx
@@ -254,17 +254,6 @@ namespace build2
           // Recursive removal of non-existing directory is not an error for
           // 'maybe' cleanup type.
           //
-          // Note that if some file system entry of non-directory type exists
-          // with such a name it is not removed but the operation still
-          // succeeds for 'maybe' cleanup type. The removal of this entry can
-          // be handled at the time of the containing directory cleanup.
-          //
-          // @@ The behavior in the situation described differes for &?a/***
-          //    and &?a/ due to build2::rmdir_r() implementation details which
-          //    checks for directory existence before trying to remove it.
-          //    Shouldn't rmdir_r() behave the same way as rmdir() in regards
-          //    to non-directory removal?
-          //
           if (p.leaf ().string () == "***")
           {
             verify (p.directory (), p, "wildcard");
diff --git a/tests/test/script/runner/cleanup.test b/tests/test/script/runner/cleanup.test
index 599bb5c..bb65c27 100644
--- a/tests/test/script/runner/cleanup.test
+++ b/tests/test/script/runner/cleanup.test
@@ -133,6 +133,14 @@ $b 2>>EOE != 0
 testscript:1: error: registered for cleanup wildcard test/a/*** is out of working directory test/1/
 EOE
 
+: wildcard-not-dir
+: Test cleanup of file as a wildcard
+:
+$c <"$* -f a &a/***";
+$b 2>>EOE != 0
+error: unable to remove directory test/1/a/: Not a directory
+EOE
+
 : implicit-overwrite
 : Test implicit cleanup being overwritten with an explicit one
 :