FileSystemManager: Always remove directories recursively

6 files changed, 14 insertions, 19 deletions

diff --git a/src/buildtool/execution_api/local/local_action.cpp b/src/buildtool/execution_api/local/local_action.cpp
index 9884a5ec..7ea0ce34 100644
--- a/src/buildtool/execution_api/local/local_action.cpp
+++ b/src/buildtool/execution_api/local/local_action.cpp
@@ -55,7 +55,7 @@ class BuildCleanupAnchor {
     auto operator=(BuildCleanupAnchor const&) -> BuildCleanupAnchor& = delete;
     auto operator=(BuildCleanupAnchor&&) -> BuildCleanupAnchor& = delete;
     ~BuildCleanupAnchor() {
-        if (not FileSystemManager::RemoveDirectory(build_path_, true)) {
+        if (not FileSystemManager::RemoveDirectory(build_path_)) {
             Logger::Log(LogLevel::Error,
                         "Could not cleanup build directory {}",
                         build_path_.string());
@@ -347,7 +347,7 @@ auto LocalAction::StageInputs(
 auto LocalAction::CreateDirectoryStructure(
     std::filesystem::path const& exec_path) const noexcept -> bool {
     // clean execution directory
-    if (not FileSystemManager::RemoveDirectory(exec_path, true)) {
+    if (not FileSystemManager::RemoveDirectory(exec_path)) {
         logger_.Emit(LogLevel::Error, "failed to clean exec_path");
         return false;
     }
diff --git a/src/buildtool/file_system/file_system_manager.hpp b/src/buildtool/file_system/file_system_manager.hpp
index fe316451..a6d15c84 100644
--- a/src/buildtool/file_system/file_system_manager.hpp
+++ b/src/buildtool/file_system/file_system_manager.hpp
@@ -44,6 +44,7 @@
 #include <string>
 #include <string_view>
 #include <system_error>
+#include <tuple>
 #include <unordered_set>
 #include <utility>
 #include <variant>
@@ -510,9 +511,9 @@ class FileSystemManager {
         }
     }
 
-    [[nodiscard]] static auto RemoveDirectory(std::filesystem::path const& dir,
-                                              bool recursively = false) noexcept
-        -> bool {
+    /// \brief Remove directory recursively.
+    [[nodiscard]] static auto RemoveDirectory(
+        std::filesystem::path const& dir) noexcept -> bool {
         try {
             auto status = std::filesystem::symlink_status(dir);
             if (not std::filesystem::exists(status)) {
@@ -521,11 +522,9 @@ class FileSystemManager {
             if (not std::filesystem::is_directory(status)) {
                 return false;
             }
-            if (recursively) {
-                return (std::filesystem::remove_all(dir) !=
-                        static_cast<uintmax_t>(-1));
-            }
-            return std::filesystem::remove(dir);
+            // If it doesn't throw, it succeeds:
+            std::ignore = std::filesystem::remove_all(dir);
+            return true;
         } catch (std::exception const& e) {
             Logger::Log(LogLevel::Error,
                         "removing directory {}:\n{}",
diff --git a/src/buildtool/storage/compactifier.cpp b/src/buildtool/storage/compactifier.cpp
index 66fa6035..ab1f557e 100644
--- a/src/buildtool/storage/compactifier.cpp
+++ b/src/buildtool/storage/compactifier.cpp
@@ -163,8 +163,7 @@ template <ObjectType kType>
     std::string const d_name = directory.filename();
     if (d_name.size() != FileStorageData::kDirectoryNameLength or
         not FromHexString(d_name)) {
-        static constexpr bool kRecursively = true;
-        if (FileSystemManager::RemoveDirectory(directory, kRecursively)) {
+        if (FileSystemManager::RemoveDirectory(directory)) {
             return true;
         }
 
diff --git a/src/buildtool/storage/garbage_collector.cpp b/src/buildtool/storage/garbage_collector.cpp
index f1254eed..1ac1bf93 100644
--- a/src/buildtool/storage/garbage_collector.cpp
+++ b/src/buildtool/storage/garbage_collector.cpp
@@ -39,8 +39,7 @@ auto RemoveDirs(const std::vector<std::filesystem::path>& directories) -> bool {
     bool success = true;
     for (auto const& d : directories) {
         if (FileSystemManager::IsDirectory(d)) {
-            if (not FileSystemManager::RemoveDirectory(d,
-                                                       /*recursively=*/true)) {
+            if (not FileSystemManager::RemoveDirectory(d)) {
                 Logger::Log(LogLevel::Warning,
                             "Failed to remove directory {}",
                             d.string());
diff --git a/src/buildtool/storage/repository_garbage_collector.cpp b/src/buildtool/storage/repository_garbage_collector.cpp
index be3d7857..95b90f9d 100644
--- a/src/buildtool/storage/repository_garbage_collector.cpp
+++ b/src/buildtool/storage/repository_garbage_collector.cpp
@@ -60,8 +60,7 @@ auto RepositoryGarbageCollector::TriggerGarbageCollection(
             return false;
         }
         if (FileSystemManager::IsDirectory(remove_me)) {
-            if (not FileSystemManager::RemoveDirectory(remove_me,
-                                                       /*recursively=*/true)) {
+            if (not FileSystemManager::RemoveDirectory(remove_me)) {
                 Logger::Log(LogLevel::Error,
                             "Failed to remove directory {}",
                             remove_me.string());
@@ -136,8 +135,7 @@ auto RepositoryGarbageCollector::TriggerGarbageCollection(
             return false;
         }
         if (FileSystemManager::IsDirectory(remove_me)) {
-            if (not FileSystemManager::RemoveDirectory(remove_me,
-                                                       /*recursively=*/true)) {
+            if (not FileSystemManager::RemoveDirectory(remove_me)) {
                 Logger::Log(LogLevel::Error,
                             "Failed to remove directory {}",
                             remove_me.string());
diff --git a/src/utils/cpp/tmp_dir.cpp b/src/utils/cpp/tmp_dir.cpp
index 8d6e0724..9a38aca2 100644
--- a/src/utils/cpp/tmp_dir.cpp
+++ b/src/utils/cpp/tmp_dir.cpp
@@ -93,5 +93,5 @@ auto TmpDir::CreateImpl(TmpDir::Ptr parent,
 
 TmpDir::~TmpDir() noexcept {
     // try to remove the tmp dir and all its content
-    std::ignore = FileSystemManager::RemoveDirectory(tmp_dir_, true);
+    std::ignore = FileSystemManager::RemoveDirectory(tmp_dir_);
 }