diff --git a/sources/menus/language_selection/package_downloader.gd b/sources/menus/language_selection/package_downloader.gd index c28fde97..028f5b94 100644 --- a/sources/menus/language_selection/package_downloader.gd +++ b/sources/menus/language_selection/package_downloader.gd @@ -201,6 +201,11 @@ func _copy_data(this: PackageDownloader) -> void: if DirAccess.dir_exists_absolute(current_language_path): Log.trace("PackageDownloader: Removing previous language directory") Utils.delete_directory_recursive(ProjectSettings.globalize_path(current_language_path)) + if DirAccess.dir_exists_absolute(current_language_path): + # Keep the temporary directory so the new pack is not lost + Log.error("PackageDownloader: Cannot remove the previous language directory, aborting swap") + this.call_thread_safe("_show_error", 2) # Error downloading + return var error: Error = DirAccess.rename_absolute(new_pack_path, current_language_path) if error != OK: diff --git a/sources/utils/autoloads/utils.gd b/sources/utils/autoloads/utils.gd index 877639f1..a4c292df 100644 --- a/sources/utils/autoloads/utils.gd +++ b/sources/utils/autoloads/utils.gd @@ -94,14 +94,22 @@ func clean_dir(path: String) -> Error: return error if dir == null: return ERR_FILE_BAD_PATH + # Hidden files (e.g. .DS_Store created by the macOS Finder) must be removed + # too, otherwise the directory is never empty and cannot be deleted + dir.include_hidden = true + # Best effort: try to remove everything, report the first error encountered + var first_error: Error = OK for file: String in dir.get_files(): - dir.remove(file) + error = dir.remove(file) + if error != OK and first_error == OK: + first_error = error for subfolder: String in dir.get_directories(): error = clean_dir(path.path_join(subfolder)) - if error != OK: - return error - dir.remove(subfolder) - return OK + if error == OK: + error = dir.remove(subfolder) + if error != OK and first_error == OK: + first_error = error + return first_error func delete_directory_recursive(path: String) -> void: diff --git a/tests/unit_tests/test_utils.gd b/tests/unit_tests/test_utils.gd new file mode 100644 index 00000000..a3dc04e8 --- /dev/null +++ b/tests/unit_tests/test_utils.gd @@ -0,0 +1,63 @@ +extends GutTest + +const TEST_ROOT: String = "user://test_clean_dir" + + +func after_each() -> void: + _force_delete_directory(TEST_ROOT) + + +func test_delete_directory_recursive_removes_hidden_files() -> void: + # Regression test: the macOS Finder drops .DS_Store files in browsed + # folders; deletion used to fail silently on them, breaking the + # language pack swap in PackageDownloader + _create_file(TEST_ROOT.path_join("visible.txt")) + _create_file(TEST_ROOT.path_join(".hidden")) + _create_file(TEST_ROOT.path_join("sub").path_join(".hidden_nested")) + + Utils.delete_directory_recursive(TEST_ROOT) + + assert_false(DirAccess.dir_exists_absolute(TEST_ROOT), "directory with hidden files should be fully deleted") + + +func test_clean_dir_keeps_root_and_removes_all_content() -> void: + _create_file(TEST_ROOT.path_join("visible.txt")) + _create_file(TEST_ROOT.path_join("sub").path_join("nested.txt")) + + var error: Error = Utils.clean_dir(TEST_ROOT) + + assert_eq(error, OK) + assert_true(DirAccess.dir_exists_absolute(TEST_ROOT), "root directory should be kept") + var dir: DirAccess = DirAccess.open(TEST_ROOT) + dir.include_hidden = true + assert_eq(dir.get_files().size(), 0, "all files should be removed") + assert_eq(dir.get_directories().size(), 0, "all subdirectories should be removed") + + +func test_clean_dir_returns_error_on_missing_directory() -> void: + var error: Error = Utils.clean_dir("user://test_clean_dir_does_not_exist") + + assert_ne(error, OK) + + +func _create_file(path: String) -> void: + DirAccess.make_dir_recursive_absolute(path.get_base_dir()) + var file: FileAccess = FileAccess.open(path, FileAccess.WRITE) + assert_not_null(file, "test setup should be able to create %s" % path) + file.store_string("test") + file.close() + + +# Cleanup helper independent from the code under test +func _force_delete_directory(path: String) -> void: + if not DirAccess.dir_exists_absolute(path): + return + var dir: DirAccess = DirAccess.open(path) + if dir == null: + return + dir.include_hidden = true + for file: String in dir.get_files(): + dir.remove(file) + for subfolder: String in dir.get_directories(): + _force_delete_directory(path.path_join(subfolder)) + DirAccess.remove_absolute(path)