From 2ecd127f96d3ddd3aaa2c2a8e5dd9d74936dc2f5 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Wed, 10 Dec 2025 21:32:05 +0100 Subject: [PATCH] Fix errors with fs::RecursiveDelete() on paths that don't exist --- src/filesys.cpp | 27 ++++++++++++++------------- src/porting.cpp | 3 +-- src/unittest/test_filesys.cpp | 3 +++ 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/filesys.cpp b/src/filesys.cpp index 1391e346d7..cf22af8e09 100644 --- a/src/filesys.cpp +++ b/src/filesys.cpp @@ -142,32 +142,33 @@ bool IsExecutable(const std::string &path) bool RecursiveDelete(const std::string &path) { - infostream << "Recursively deleting \"" << path << "\"" << std::endl; assert(IsPathAbsolute(path)); - if (!IsDir(path)) { - infostream << "RecursiveDelete: Deleting file " << path << std::endl; + if (!PathExists(path)) + return true; + + bool is_file = !IsDir(path); + infostream << "Recursively deleting " << (is_file ? "file" : "directory") + << " \"" << path << "\"" << std::endl; + if (is_file) { if (!DeleteFile(path.c_str())) { - errorstream << "RecursiveDelete: Failed to delete file " - << path << std::endl; + errorstream << "RecursiveDelete: Failed to delete file \"" + << path << "\": " << LAST_OS_ERROR() << std::endl; return false; } return true; } - infostream << "RecursiveDelete: Deleting content of directory " - << path << std::endl; std::vector content = GetDirListing(path); - for (const DirListNode &n: content) { + for (const auto &n : content) { std::string fullpath = path + DIR_DELIM + n.name; if (!RecursiveDelete(fullpath)) { - errorstream << "RecursiveDelete: Failed to recurse to " - << fullpath << std::endl; + errorstream << "RecursiveDelete: Failed to recurse to \"" + << fullpath << "\"" << std::endl; return false; } } - infostream << "RecursiveDelete: Deleting directory " << path << std::endl; if (!RemoveDirectory(path.c_str())) { - errorstream << "Failed to recursively delete directory " - << path << std::endl; + errorstream << "RecursiveDelete: Failed to delete directory \"" + << path << "\": " << LAST_OS_ERROR() << std::endl; return false; } return true; diff --git a/src/porting.cpp b/src/porting.cpp index 1df84b5c62..e7f3dd3258 100644 --- a/src/porting.cpp +++ b/src/porting.cpp @@ -599,8 +599,7 @@ bool setSystemPaths() // Delete tmp folder if it exists (it only ever contained // a temporary ogg file, which is no longer used). - if (fs::PathExists(local_cache_path + DIR_DELIM + "tmp")) - fs::RecursiveDelete(local_cache_path + DIR_DELIM + "tmp"); + fs::RecursiveDelete(local_cache_path + DIR_DELIM + "tmp"); // Bail if migration impossible if (path_cache == local_cache_path || !fs::PathExists(local_cache_path) diff --git a/src/unittest/test_filesys.cpp b/src/unittest/test_filesys.cpp index 2da27e829e..6795ec01b7 100644 --- a/src/unittest/test_filesys.cpp +++ b/src/unittest/test_filesys.cpp @@ -415,6 +415,9 @@ void TestFileSys::testRecursiveDelete() UASSERT(!fs::IsDir(it)); for (auto &it : files) UASSERT(!fs::IsFile(it)); + + // Deleting something that doesn't exist is *not* an error + UASSERT(fs::RecursiveDelete(dirs[0])); } void TestFileSys::testGetRecursiveSubPaths()