From edf83284728f87388405c7c5be88cc52e641207a Mon Sep 17 00:00:00 2001 From: Chaser Huang Date: Thu, 6 Aug 2026 20:46:20 -0400 Subject: [PATCH] fix(fs): remove tombstones when re-creating deleted files, dirs, and symlinks fixes irregularities of branchfs behavior when deleted files are recreated within the same session Assisted-by: pi deepseek-v4-flash-0731 --- src/fs.rs | 8 ++++- src/fs_helpers.rs | 9 +++++ tests/test_file_operations.sh | 64 +++++++++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 1 deletion(-) diff --git a/src/fs.rs b/src/fs.rs index 0dec230..314f001 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -982,6 +982,7 @@ impl Filesystem for BranchFs { use std::os::unix::fs::PermissionsExt; let perm = std::fs::Permissions::from_mode(mode & !umask); let _ = std::fs::set_permissions(&delta, perm); + self.revive_path(&branch, &rel_path); let inode_path = format!("/@{}{}", branch, rel_path); let ino = self.inodes.get_or_create(&inode_path, false); if let Some(attr) = self.make_attr(ino, &delta) { @@ -1016,6 +1017,7 @@ impl Filesystem for BranchFs { use std::os::unix::fs::PermissionsExt; let perm = std::fs::Permissions::from_mode(mode & !umask); let _ = std::fs::set_permissions(&delta, perm); + self.revive_path(&self.get_branch_name(), &path); if self.is_stale() { let _ = std::fs::remove_file(&delta); reply.error(libc::ESTALE); @@ -1282,13 +1284,13 @@ impl Filesystem for BranchFs { if dst_existed { b.add_tombstone(&dst_rel)?; } - b.remove_tombstone(&dst_rel); Ok(()) }); if result.is_err() { reply.error(libc::EIO); return; } + self.revive_path(&branch, &dst_rel); if src_is_root && self.is_stale() { reply.error(libc::ESTALE); @@ -1682,6 +1684,7 @@ impl Filesystem for BranchFs { use std::os::unix::fs::PermissionsExt; let perm = std::fs::Permissions::from_mode(mode & !umask); let _ = std::fs::set_permissions(&delta, perm); + self.revive_path(&branch, &rel_path); let inode_path = format!("/@{}{}", branch, rel_path); let ino = self.inodes.get_or_create(&inode_path, true); if let Some(attr) = self.make_attr(ino, &delta) { @@ -1710,6 +1713,7 @@ impl Filesystem for BranchFs { use std::os::unix::fs::PermissionsExt; let perm = std::fs::Permissions::from_mode(mode & !umask); let _ = std::fs::set_permissions(&delta, perm); + self.revive_path(&self.get_branch_name(), &path); if self.is_stale() { let _ = std::fs::remove_dir_all(&delta); reply.error(libc::ESTALE); @@ -1817,6 +1821,7 @@ impl Filesystem for BranchFs { } match std::os::unix::fs::symlink(target, &delta) { Ok(()) => { + self.revive_path(&branch, &rel_path); let inode_path = format!("/@{}{}", branch, rel_path); let ino = self.inodes.get_or_create(&inode_path, false); match self.make_attr(ino, &delta) { @@ -1844,6 +1849,7 @@ impl Filesystem for BranchFs { } match std::os::unix::fs::symlink(target, &delta) { Ok(()) => { + self.revive_path(&self.get_branch_name(), &path); if self.is_stale() { let _ = std::fs::remove_file(&delta); reply.error(libc::ESTALE); diff --git a/src/fs_helpers.rs b/src/fs_helpers.rs index 8f5ddb7..8563cf2 100644 --- a/src/fs_helpers.rs +++ b/src/fs_helpers.rs @@ -39,6 +39,15 @@ impl BranchFs { .unwrap() } + /// Clear (revive) a tombstone for `rel_path` in the given branch + /// Note that every creation should call this to handle re-creations correctly + pub(crate) fn revive_path(&self, branch: &str, rel_path: &str) { + let _ = self.manager.with_branch(branch, |b| { + b.remove_tombstone(rel_path); + Ok(()) + }); + } + pub(crate) fn ensure_cow(&self, rel_path: &str) -> std::io::Result { self.ensure_cow_for_branch(&self.get_branch_name(), rel_path) } diff --git a/tests/test_file_operations.sh b/tests/test_file_operations.sh index f1f7609..3d80fd9 100755 --- a/tests/test_file_operations.sh +++ b/tests/test_file_operations.sh @@ -234,6 +234,66 @@ test_synthetic_entry_ownership() { } # Run tests +test_recreate_file_after_delete() { + setup + do_mount + echo "first content" > "$TEST_MNT/recreate.txt" + assert_file_contains "$TEST_MNT/recreate.txt" "first content" "File created with first content" + rm "$TEST_MNT/recreate.txt" + assert_file_not_exists "$TEST_MNT/recreate.txt" "File deleted" + echo "second content" > "$TEST_MNT/recreate.txt" + + # readdir forces a fresh resolution through resolve_path (unlike stat, which + # is masked by the kernel's positive dentry cache from create) + assert "ls '$TEST_MNT' | grep -qx 'recreate.txt'" "File recreated with same name visible in listing" + assert_file_contains "$TEST_MNT/recreate.txt" "second content" "Recreated file has second content" + + local delta_dir="$TEST_STORAGE/branches/main/files" + local count + count=$(ls "$delta_dir/recreate.txt" 2>/dev/null | wc -l) + assert_eq "$count" "1" "Exactly one delta entry for recreated file (no orphan)" + do_unmount +} + +test_recreate_dir_after_delete() { + setup + do_mount + mkdir "$TEST_MNT/recreate_dir" + assert "[[ -d '$TEST_MNT/recreate_dir' ]]" "Directory created" + rmdir "$TEST_MNT/recreate_dir" + assert "[[ ! -d '$TEST_MNT/recreate_dir' ]]" "Directory deleted" + mkdir "$TEST_MNT/recreate_dir" + assert "[[ -d '$TEST_MNT/recreate_dir' ]]" "Directory recreated with same name" + do_unmount +} + +test_recreate_symlink_after_delete() { + setup + do_mount + echo "target" > "$TEST_MNT/recreate_target.txt" + ln -s recreate_target.txt "$TEST_MNT/recreate_link" + assert "[[ -L '$TEST_MNT/recreate_link' ]]" "Symlink created" + rm "$TEST_MNT/recreate_link" + assert "[[ ! -L '$TEST_MNT/recreate_link' ]]" "Symlink deleted" + ln -s recreate_target.txt "$TEST_MNT/recreate_link" + assert "[[ -L '$TEST_MNT/recreate_link' ]]" "Symlink recreated with same name" + do_unmount +} + +test_recreate_file_after_delete_in_branch() { + setup + do_mount + do_create "recreate_b" "main" + echo "branch first" > "$TEST_MNT/@recreate_b/recreate.txt" + assert_file_contains "$TEST_MNT/@recreate_b/recreate.txt" "branch first" "Branch file created" + rm "$TEST_MNT/@recreate_b/recreate.txt" + assert_file_not_exists "$TEST_MNT/@recreate_b/recreate.txt" "Branch file deleted" + echo "branch second" > "$TEST_MNT/@recreate_b/recreate.txt" + assert_file_exists "$TEST_MNT/@recreate_b/recreate.txt" "Branch file recreated with same name" + assert_file_contains "$TEST_MNT/@recreate_b/recreate.txt" "branch second" "Branch recreated file has second content" + do_unmount +} + run_test "Read Base Files" test_read_base_files run_test "Write New File in Branch" test_write_new_file_in_branch run_test "Modify Existing File (COW)" test_modify_existing_file_cow @@ -247,5 +307,9 @@ run_test "Chmod File" test_chmod_file run_test "Chmod Directory" test_chmod_directory run_test "Chmod Existing File (COW)" test_chmod_existing_file_cow run_test "Synthetic Entry Ownership" test_synthetic_entry_ownership +run_test "Recreate File After Delete" test_recreate_file_after_delete +run_test "Recreate Dir After Delete" test_recreate_dir_after_delete +run_test "Recreate Symlink After Delete" test_recreate_symlink_after_delete +run_test "Recreate File After Delete in Branch" test_recreate_file_after_delete_in_branch print_summary