tree_builder: ignore tombstone overrides on nested trees

scm-record yields a selected change with `FileMode::Absent` in one of
two cases:
- when an existing file is deleted and this change is selected,
- when a new file is created and this change is not selected.

From the information provided by `File::get_selected_contents`, it is
not possible to distinguish these two cases. In the first case, the
tree definitely needs to change to reflect the deletion, so a "tombstone"
override is set on the tree builder. In the second case, that tombstone
is usually ignored when `TreeBuilder::write_tree()` processes the
overrides because the associated file does not exist.

However, when the tombstone happens to coindice with a deleted directory
and a new file has been created in its place, but neither change is
selected, then the tombstone had the effect of deleting the directory
from the tree.

This is fixed now by ignoring tombstones on trees. When a directory is
actually intentionally deleted, this will result in a bunch of tombstones
on the files therein; after processing the file overrides, `write_tree()`
will remove any now-empty directory trees anyway.

Perhaps, a better solution would see scm-record provide information to
distinguish a selected deletion from an unselected creation to avoid
placing the tombstone altogether. For now, this fixes the test case
`test_edit_diff_builtin_replace_directory_with_file` and one failure
source of the property-based tests.
This commit is contained in:
Jonas Greitemann 2025-04-29 14:28:37 +02:00
parent e000275ba5
commit 21c63fbdf8

View File

@ -89,9 +89,13 @@ impl TreeBuilder {
Override::Replace(value) => { Override::Replace(value) => {
tree.set(basename.to_owned(), value); tree.set(basename.to_owned(), value);
} }
Override::Tombstone => { Override::Tombstone => match tree.value(basename) {
tree.remove(basename); Some(TreeValue::Tree(_)) => {}
} Some(_) => {
tree.remove(basename);
}
None => {}
},
} }
} }