summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/buildtool/execution_api/local/local_action.cpp4
-rw-r--r--src/buildtool/file_system/file_system_manager.hpp15
-rw-r--r--src/buildtool/storage/compactifier.cpp3
-rw-r--r--src/buildtool/storage/garbage_collector.cpp3
-rw-r--r--src/buildtool/storage/repository_garbage_collector.cpp6
-rw-r--r--src/utils/cpp/tmp_dir.cpp2
-rw-r--r--test/buildtool/execution_engine/executor/executor_api.test.hpp8
-rw-r--r--test/buildtool/graph_traverser/graph_traverser_remote.test.cpp2
-rw-r--r--test/utils/archive/archive_usage.test.cpp13
9 files changed, 23 insertions, 33 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_);
}
diff --git a/test/buildtool/execution_engine/executor/executor_api.test.hpp b/test/buildtool/execution_engine/executor/executor_api.test.hpp
index 2043da88..ff4c4999 100644
--- a/test/buildtool/execution_engine/executor/executor_api.test.hpp
+++ b/test/buildtool/execution_engine/executor/executor_api.test.hpp
@@ -479,7 +479,7 @@ static inline void TestUploadAndDownloadTrees(
CHECK(FileSystemManager::IsNonUpwardsSymlink(tmpdir / "b"));
CHECK(*FileSystemManager::ReadFile(tmpdir / "a") == "foo");
CHECK(*FileSystemManager::ReadSymlink(tmpdir / "b") == "bar");
- REQUIRE(FileSystemManager::RemoveDirectory(tmpdir, true));
+ REQUIRE(FileSystemManager::RemoveDirectory(tmpdir));
}
SECTION("Subdir in tree path") {
@@ -496,7 +496,7 @@ static inline void TestUploadAndDownloadTrees(
CHECK(FileSystemManager::IsNonUpwardsSymlink(tmpdir / "b" / "a"));
CHECK(*FileSystemManager::ReadFile(tmpdir / "a") == "foo");
CHECK(*FileSystemManager::ReadSymlink(tmpdir / "b" / "a") == "bar");
- REQUIRE(FileSystemManager::RemoveDirectory(tmpdir, true));
+ REQUIRE(FileSystemManager::RemoveDirectory(tmpdir));
}
SECTION("Nested trees") {
@@ -517,7 +517,7 @@ static inline void TestUploadAndDownloadTrees(
CHECK(FileSystemManager::IsNonUpwardsSymlink(tmpdir / "b" / "a"));
CHECK(*FileSystemManager::ReadFile(tmpdir / "a") == "foo");
CHECK(*FileSystemManager::ReadSymlink(tmpdir / "b" / "a") == "bar");
- REQUIRE(FileSystemManager::RemoveDirectory(tmpdir, true));
+ REQUIRE(FileSystemManager::RemoveDirectory(tmpdir));
}
SECTION("Dot-path tree as action input") {
@@ -550,7 +550,7 @@ static inline void TestUploadAndDownloadTrees(
CHECK(FileSystemManager::IsNonUpwardsSymlink(tmpdir / "b" / "a"));
CHECK(*FileSystemManager::ReadFile(tmpdir / "a") == "foo");
CHECK(*FileSystemManager::ReadSymlink(tmpdir / "b" / "a") == "bar");
- REQUIRE(FileSystemManager::RemoveDirectory(tmpdir, true));
+ REQUIRE(FileSystemManager::RemoveDirectory(tmpdir));
}
SECTION("Dot-path non-tree as action input") {
diff --git a/test/buildtool/graph_traverser/graph_traverser_remote.test.cpp b/test/buildtool/graph_traverser/graph_traverser_remote.test.cpp
index f2b2d852..a611caab 100644
--- a/test/buildtool/graph_traverser/graph_traverser_remote.test.cpp
+++ b/test/buildtool/graph_traverser/graph_traverser_remote.test.cpp
@@ -35,7 +35,7 @@
[[nodiscard]] static auto CreateStorageConfig(
RemoteExecutionConfig const& remote_config) -> StorageConfig {
auto cache_dir = FileSystemManager::GetCurrentDirectory() / "cache";
- if (not FileSystemManager::RemoveDirectory(cache_dir, true) or
+ if (not FileSystemManager::RemoveDirectory(cache_dir) or
not FileSystemManager::CreateDirectoryExclusive(cache_dir)) {
Logger::Log(LogLevel::Error,
"failed to create a test-local cache dir {}",
diff --git a/test/utils/archive/archive_usage.test.cpp b/test/utils/archive/archive_usage.test.cpp
index 030cd223..ddf71353 100644
--- a/test/utils/archive/archive_usage.test.cpp
+++ b/test/utils/archive/archive_usage.test.cpp
@@ -339,8 +339,7 @@ TEST_CASE("Read-write archives", "[archive_read_write]") {
auto const& scenario = kTestScenarios[test_index];
// perform the test
- REQUIRE(FileSystemManager::RemoveDirectory(scenario.test_dir,
- /*recursively=*/true));
+ REQUIRE(FileSystemManager::RemoveDirectory(scenario.test_dir));
REQUIRE(FileSystemManager::CreateDirectory(scenario.test_dir));
auto anchor = FileSystemManager::ChangeDirectory(scenario.test_dir);
@@ -395,8 +394,7 @@ TEST_CASE("ArchiveOps", "[archive_ops]") {
std::optional<std::string> res{std::nullopt};
SECTION(std::string("Write ") + scenario.test_name) {
- REQUIRE(FileSystemManager::RemoveDirectory(scenario.test_dir,
- /*recursively=*/true));
+ REQUIRE(FileSystemManager::RemoveDirectory(scenario.test_dir));
REQUIRE(FileSystemManager::CreateDirectory(scenario.test_dir));
create_files(scenario.test_dir);
@@ -408,8 +406,7 @@ TEST_CASE("ArchiveOps", "[archive_ops]") {
}
SECTION(std::string("Extract ") + scenario.test_name + " to disk") {
- REQUIRE(FileSystemManager::RemoveDirectory(scenario.test_dir,
- /*recursively=*/true));
+ REQUIRE(FileSystemManager::RemoveDirectory(scenario.test_dir));
REQUIRE(FileSystemManager::CreateDirectory(scenario.test_dir));
res = ArchiveOps::ExtractArchive(
scenario.type, scenario.filename, ".");
@@ -430,9 +427,7 @@ TEST_CASE("ArchiveOps", "[archive_ops]") {
path = std::string{env_path} + ":" + path;
}
SECTION("Extract via system tools") {
- REQUIRE(
- FileSystemManager::RemoveDirectory(scenario.test_dir,
- /*recursively=*/true));
+ REQUIRE(FileSystemManager::RemoveDirectory(scenario.test_dir));
REQUIRE(FileSystemManager::CreateDirectory(scenario.test_dir));
REQUIRE(system(("export PATH=" + path + " && " + scenario.cmd +
" " + scenario.filename)
generated by cgit v1.2.3 (git 2.39.1) at 2025-07-03 14:41:33 +0000