Fix recursive directory removal across platforms

Add FileSystem::dirRemoveAll() to normalize trailing separators and use the non-throwing std::filesystem::remove_all overload.

Migrate ecode and unit-test cleanup call sites to the centralized helper, preventing Haiku from crashing when removing directories with trailing slashes during shutdown.
This commit is contained in:
Martín Lucas Golini
2026-08-18 13:37:56 -03:00
parent 1b2927e129
commit 01ebd7a82d
8 changed files with 43 additions and 43 deletions

View File

@@ -81,6 +81,11 @@ class EE_API FileSystem {
/** Deletes a file from the file system. */
static bool fileRemove( const std::string& filepath );
/** Recursively deletes a directory and all of its contents.
* @return True if the directory was removed or did not exist, false on error.
*/
static bool dirRemoveAll( const std::string& path );
/** Moves a file or folder to the destination path
* @param fromPath The path of the file or folder to move
* @param toPath The folder / directory destination path

View File

@@ -202,6 +202,19 @@ bool FileSystem::fileRemove( const std::string& filepath ) {
#endif
}
bool FileSystem::dirRemoveAll( const std::string& path ) {
#if EE_PLATFORM == EE_PLATFORM_WIN
std::filesystem::path normalizedPath( String( path ).toWideString() );
#else
std::filesystem::path normalizedPath( path );
#endif
if ( normalizedPath.has_relative_path() && !normalizedPath.has_filename() )
normalizedPath = normalizedPath.parent_path();
std::error_code error;
std::filesystem::remove_all( normalizedPath, error );
return !error;
}
bool FileSystem::fileHide( const std::string& filepath ) {
#if EE_PLATFORM == EE_PLATFORM_WIN
return SetFileAttributesW( (LPCWSTR)String( filepath ).toWideString().c_str(),

View File

@@ -26,13 +26,26 @@ struct TempDirectory {
std::filesystem::create_directories( path );
}
~TempDirectory() { std::filesystem::remove_all( path ); }
~TempDirectory() { FileSystem::dirRemoveAll( path.string() ); }
std::filesystem::path path;
};
} // namespace
UTEST( FileSystem, dirRemoveAllHandlesTrailingSlash ) {
TempDirectory temp;
const std::filesystem::path nested = temp.path / "nested";
std::filesystem::create_directories( nested );
ASSERT_TRUE( FileSystem::fileWrite( ( nested / "file.txt" ).string(), "contents" ) );
std::string path( temp.path.string() );
FileSystem::dirAddSlashAtEnd( path );
EXPECT_TRUE( FileSystem::dirRemoveAll( path ) );
EXPECT_FALSE( std::filesystem::exists( temp.path ) );
EXPECT_TRUE( FileSystem::dirRemoveAll( path ) );
}
UTEST( FileInfo, sameInodeUsesDeviceAndRejectsInvalidIdentity ) {
FileInfoIdentity first;
FileInfoIdentity same;

View File

@@ -6,6 +6,7 @@
#include <filesystem>
using namespace EE::UI::Models;
using namespace EE::System;
namespace {
@@ -131,11 +132,11 @@ struct TempTree {
static unsigned long long id = 0;
path = std::filesystem::temp_directory_path() /
( "eepp-filesystem-model-move-" + std::to_string( ++id ) );
std::filesystem::remove_all( path );
FileSystem::dirRemoveAll( path.string() );
std::filesystem::create_directories( path );
}
~TempTree() { std::filesystem::remove_all( path ); }
~TempTree() { FileSystem::dirRemoveAll( path.string() ); }
std::filesystem::path path;
};

View File

@@ -55,10 +55,7 @@ class ScopedTestDirectory {
public:
explicit ScopedTestDirectory( std::filesystem::path path ) : mPath( std::move( path ) ) {}
~ScopedTestDirectory() {
std::error_code error;
std::filesystem::remove_all( mPath, error );
}
~ScopedTestDirectory() { FileSystem::dirRemoveAll( mPath.string() ); }
private:
std::filesystem::path mPath;

View File

@@ -1046,14 +1046,6 @@ App::App( const size_t& jobs, const std::vector<std::string>& args ) :
mFontPickerController( std::make_unique<FontPickerController>( this ) ),
mSettingsActions( std::make_unique<SettingsActions>( this ) ) {}
static void fsRemoveAll( const std::string& fpath ) {
#if EE_PLATFORM == EE_PLATFORM_WIN
fs::remove_all( std::filesystem::path( String( fpath ).toWideString() ) );
#else
fs::remove_all( fpath );
#endif
}
App::~App() {
appInstance = nullptr;
mDestroyingApp = true;
@@ -1082,7 +1074,8 @@ App::~App() {
}
mDirTree.reset();
fsRemoveAll( mPidPath );
if ( !FileSystem::dirRemoveAll( mPidPath ) )
Log::warning( "Failed to remove directory \"%s\"", mPidPath );
if ( mFirstInstance )
FileSystem::fileRemove( firstInstanceIndicatorPath() );

View File

@@ -1,9 +1,6 @@
#include "settingsmenu.hpp"
#include "uitreeviewfs.hpp"
#include <filesystem>
namespace fs = std::filesystem;
namespace ecode {
String SettingsMenu::i18n( const std::string& key, const String& def ) {
@@ -2866,14 +2863,6 @@ void SettingsMenu::createProjectTreeMenu() {
showProjectTreeMenu();
}
static void fsRemoveAll( const std::string& fpath ) {
#if EE_PLATFORM == EE_PLATFORM_WIN
fs::remove_all( std::filesystem::path( String( fpath ).toWideString() ) );
#else
fs::remove_all( fpath );
#endif
}
void SettingsMenu::createProjectTreeMenu( const std::vector<FileInfo>& files ) {
if ( mProjectTreeMenu && mProjectTreeMenu->isVisible() )
mProjectTreeMenu->close();
@@ -3152,13 +3141,8 @@ void SettingsMenu::deleteFileDialog( const FileInfo& file ) {
};
if ( file.isDirectory() ) {
try {
std::string fpath( file.getFilepath() );
FileSystem::dirRemoveSlashAtEnd( fpath );
fsRemoveAll( fpath );
} catch ( const fs::filesystem_error& ) {
if ( !FileSystem::dirRemoveAll( file.getFilepath() ) )
errFn();
}
} else if ( !FileSystem::fileRemove( file.getFilepath() ) ) {
errFn();
}

View File

@@ -8,8 +8,6 @@
#include <eepp/ui/uimessagebox.hpp>
#include <eepp/window/cursormanager.hpp>
#include <filesystem>
namespace ecode {
static const std::map<KeyBindings::Shortcut, std::string> getDefaultKeybindings() {
@@ -405,14 +403,10 @@ void UITreeViewFS::deleteItems( const std::vector<std::string>& paths ) {
msgBox->on( Event::OnConfirm, [paths]( const Event* ) {
for ( const auto& path : paths ) {
FileInfo info( path );
try {
if ( info.isDirectory() ) {
std::filesystem::remove_all( std::filesystem::path( path ) );
} else {
FileSystem::fileRemove( path );
}
} catch ( const std::filesystem::filesystem_error& ) {
}
if ( info.isDirectory() )
FileSystem::dirRemoveAll( path );
else
FileSystem::fileRemove( path );
}
} );
}