diff --git a/core/crates/layerfs-content/src/merge/build.rs b/core/crates/layerfs-content/src/merge/build.rs index 38e391875..30da6be5c 100644 --- a/core/crates/layerfs-content/src/merge/build.rs +++ b/core/crates/layerfs-content/src/merge/build.rs @@ -4,9 +4,11 @@ //! in the merge's records; the merged root is one backed update of the //! `into` side over them, so it is built by the same canonical producer, on //! the same route, as a Workspace Commit. A file value written at a name -//! keeps its source serial when the serial is new to that side, already -//! holds that value there, or is changed in place at its only binding; -//! otherwise it gets a fresh serial, so one serial never carries two values. +//! keeps its source serial when the serial is new to that side and `from` +//! binds it at no other name, already holds that value there, or is changed +//! in place at its only binding; otherwise it gets a fresh serial, so one +//! serial never carries two values and a new file `from` also binds at other +//! names keeps its own value there. use layerfs_telemetry::timer::{Active, TimingScope}; @@ -168,16 +170,17 @@ impl Plan<'_, '_> { } } - /// The serial that carries `value` for the source serial `source`. + /// The serial that carries `value` for the entry `like`. pub(super) fn assign( &mut self, resolver: &Resolver<'_>, into: usize, writer: &mut Writer<'_>, - source: u64, + like: EntryFacts, value: InodeValue, here: Option, ) -> ContentResult { + let source = like.serial; let stored = if self.assigned.may_hold(source) { self.spill.get(ASSIGNED, single(source))? } else { @@ -194,6 +197,9 @@ impl Plan<'_, '_> { let first = given.is_empty(); let serial = match resolver.value(into, source)? { _ if !first => None, + // `from` binds the file at other names too, which keep its own + // value: the value written here is another file. + None if like.value.namespace_ref_count > 1 => None, None => { self.fresh(source)?; Some(source) @@ -233,11 +239,6 @@ impl Plan<'_, '_> { Ok(self.spill.get(VALUE, single(serial))?.is_some()) } - /// True when a decision gave `source`'s entry a value. - pub(super) fn assigned(&self, source: u64) -> ContentResult { - Ok(self.assigned.may_hold(source) && self.spill.get(ASSIGNED, single(source))?.is_some()) - } - /// Adds one assignment to a source serial's record. fn record( &mut self, diff --git a/core/crates/layerfs-content/src/merge/serial.rs b/core/crates/layerfs-content/src/merge/serial.rs index 3517200c7..fb8ed8f17 100644 --- a/core/crates/layerfs-content/src/merge/serial.rs +++ b/core/crates/layerfs-content/src/merge/serial.rs @@ -407,8 +407,9 @@ impl Ctx<'_, '_> { let mut listing = self.listing_of(c, PathSlot::Name(self.from, s)); self.names(&mut listing, Mode::Emit)?; } - // A new file decided at a name already carries its result. - (None, Some(f), None) if !self.plan.assigned(s)? => { + // A new file keeps `from`'s value, unless a decision at a name + // gave this serial its result. + (None, Some(f), None) if !self.plan.valued(s)? => { self.plan.fresh(s)?; self.plan.value(s, f)?; } @@ -547,7 +548,7 @@ impl Ctx<'_, '_> { &self.resolver, self.into, &mut self.writer, - like.serial, + like, value, here, )?), diff --git a/core/docs/architecture/81-compare-and-merge.md b/core/docs/architecture/81-compare-and-merge.md index 0ef90d612..bf182ae3b 100644 --- a/core/docs/architecture/81-compare-and-merge.md +++ b/core/docs/architecture/81-compare-and-merge.md @@ -150,6 +150,7 @@ text) go through `construct_bytes` into the same consumer first. | `from` changed a serial `into` deleted | decided as a deletion against a change; a kept file is placed at `from`'s path, its missing parents revived | | one side changed a directory's names, the other replaced it at the same path | decided at the path in either direction: a new directory there takes the changed directory's names; otherwise the entries decide (step 3), so a file or symlink there wins; a lost directory leaves its new serials unbound, a kept one stays whole | | a serial new on `from` that loses a collision | left unbound, with the new subtree below it; a file `from` hard-linked at other names only once every link is lost, else it stays at the names the merge keeps | + | a file new on `from`, hard-linked at other names, whose name is decided with written bytes (merged text or markers) | the bytes are a new file at that name; the other names keep the file and its own bytes, in a Branch merge and in the root a conflicted Commit installs | 5. **Paths.** Only a serial that needs a path is named: a decision (for its conflict report and `/.lww` matching), a deletion against a change below @@ -159,9 +160,13 @@ text) go through `construct_bytes` into the same consumer first. rewritten; link counts are derived from the changed bindings, and a serial left with none is released. A directory new to `into` is built from its bindings. A file value keeps its serial in three cases: the serial is new to - `into`, `into` already holds that value, or the value changes in place at - the serial's only binding. Otherwise the caller's `fresh` allocator supplies - a new serial, so one serial never carries two values. + `into` and `from` binds it at no other name, `into` already holds that + value, or the value changes in place at the serial's only binding. + Otherwise the caller's `fresh` allocator supplies a new serial, so one + serial never carries two values: a new file `from` also binds at other + names stays at those names with its own value, and the written value is a + fresh file at the decided name. Link counts follow the bindings the merge + keeps, for the file and for the fresh serial alike. **Working state (#355 (d), M1).** Memory does not grow with the merge. The serials deferred until moves are known are one run of 60 KiB chunks read once; diff --git a/engine/crates/layerfs-api/tests/merge_by_serial.rs b/engine/crates/layerfs-api/tests/merge_by_serial.rs index b18f47021..251ccec3d 100644 --- a/engine/crates/layerfs-api/tests/merge_by_serial.rs +++ b/engine/crates/layerfs-api/tests/merge_by_serial.rs @@ -67,18 +67,25 @@ fn merge( (left_root.to_owned(), merged) } -fn view(fixture: &Fixture, root: &str) -> Vec { +fn viewer(fixture: &Fixture) -> view::Viewer { view::Viewer::open( &fixture.host.config.path, installed::BINDING, installed::CURSOR, ) - .view(root, false) } -fn merged_view(fixture: &Fixture, merged: Result) -> Vec { +fn view(fixture: &Fixture, root: &str) -> Vec { + viewer(fixture).view(root, false) +} + +fn merged_root(merged: Result) -> String { let reply = merged.unwrap_or_else(|refusal| panic!("merge refused: {refusal:?}")); - view(fixture, reply.get_text("root").unwrap()) + reply.get_text("root").unwrap().to_owned() +} + +fn merged_view(fixture: &Fixture, merged: Result) -> Vec { + view(fixture, &merged_root(merged)) } /// The view's line for `path`, if the path exists. @@ -89,8 +96,16 @@ fn line<'v>(view: &'v [String], path: &str) -> Option<&'v str> { } fn read(fixture: &Fixture, path: &str) -> Option { + read_at(fixture, "m", path) +} + +/// The text at `path` on `branch`, if the path exists. +fn read_at(fixture: &Fixture, branch: &str, path: &str) -> Option { fixture - .try_call("fs.read", vec![("branch", text("m")), ("path", text(path))]) + .try_call( + "fs.read", + vec![("branch", text(branch)), ("path", text(path))], + ) .ok() .map(|reply| reply.get_text("text").unwrap().to_owned()) } @@ -875,6 +890,201 @@ fn a_new_link_over_a_base_name_in_a_merged_directory_is_lost_once() { } } +/// `l` replaces `/shared.txt` with a new file that it also links at +/// `/uncontested.txt`; `r` edits `/shared.txt` and never touches the other +/// name. Merging `branch` with `with` gives `/shared.txt` the merged text, a +/// file of its own, and leaves `/uncontested.txt` the bytes `l` wrote, as one +/// link. +fn merge_beside_a_new_link(branch: &str, with: &str) { + let fixture = Fixture::new(&format!("serial-link-beside-{branch}"), |source| { + fs::write(source.join("shared.txt"), b"line1\nline2\nline3\n").unwrap(); + 1 + }); + fixture.fork("l"); + fixture.fork("r"); + fixture.apply("l", vec![rm("/shared.txt")]); + fixture.workspace_commit( + "l", + &[ + Change::Create("/uncontested.txt", b"OUR_LINE1\nline2\nline3\n"), + Change::Link("/uncontested.txt", "/shared.txt"), + ], + 1, + || {}, + ); + fixture.apply( + "r", + vec![write("/shared.txt", "line1\nline2\nTHEIR_LINE3\n")], + ); + let merged = fixture.try_call( + "branch.merge", + vec![ + ("branch", text(branch)), + ("with", text(with)), + ("name", text("m")), + ], + ); + let root = merged_root(merged); + let merged_view = view(&fixture, &root); + assert_eq!( + read(&fixture, "/shared.txt").as_deref(), + Some("OUR_LINE1\nline2\nTHEIR_LINE3\n") + ); + assert_eq!( + read(&fixture, "/uncontested.txt").as_deref(), + Some("OUR_LINE1\nline2\nline3\n") + ); + for path in ["/shared.txt", "/uncontested.txt"] { + let found = line(&merged_view, path).expect("both names stay"); + assert!(!found.contains(" link="), "{found}"); + assert_eq!(viewer(&fixture).links(&root, path), 1, "{path}"); + } + fixture.cleanup(); +} + +/// `l` is merged with `r`: the merge reads the changes of the branch that +/// made the new file. +#[test] +fn text_merged_at_one_link_of_a_new_file_leaves_its_other_link_alone() { + merge_beside_a_new_link("l", "r"); +} + +/// `r` is merged with `l`: the merge reads the changes of the branch that +/// only edited. +#[test] +fn text_merged_at_one_link_of_a_new_file_leaves_its_other_link_alone_in_the_other_order() { + merge_beside_a_new_link("r", "l"); +} + +/// The new file linked at `/a.txt` and `/b.txt`, both edited by `r`: each +/// name holds its own merged text and the file keeps none of its own bytes. +/// Two merged texts that differ are two files; two that are equal stay one +/// file with two links. +#[test] +fn a_new_file_with_every_name_merged_keeps_none_of_its_own_bytes() { + for (label, edits, merged_texts, links) in [ + ( + "serial-links-merged-apart", + ["1\n2\nA3\n", "1\n2\nB3\n"], + ["O1\n2\nA3\n", "O1\n2\nB3\n"], + 1, + ), + ( + "serial-links-merged-alike", + ["1\n2\nT3\n", "1\n2\nT3\n"], + ["O1\n2\nT3\n", "O1\n2\nT3\n"], + 2, + ), + ] { + let fixture = Fixture::new(label, |source| { + fs::write(source.join("a.txt"), b"1\n2\n3\n").unwrap(); + fs::write(source.join("b.txt"), b"1\n2\n3\n").unwrap(); + 2 + }); + fixture.fork("l"); + fixture.fork("r"); + fixture.apply("l", vec![rm("/a.txt"), rm("/b.txt")]); + fixture.workspace_commit( + "l", + &[ + Change::Create("/a.txt", b"O1\n2\n3\n"), + Change::Link("/a.txt", "/b.txt"), + ], + 1, + || {}, + ); + fixture.apply( + "r", + vec![write("/a.txt", edits[0]), write("/b.txt", edits[1])], + ); + let merged = fixture.try_call( + "branch.merge", + vec![ + ("branch", text("l")), + ("with", text("r")), + ("name", text("m")), + ], + ); + let root = merged_root(merged); + let merged_view = view(&fixture, &root); + for (path, merged_text) in ["/a.txt", "/b.txt"].into_iter().zip(merged_texts) { + let case = format!("{label} {path}"); + assert_eq!(read(&fixture, path).as_deref(), Some(merged_text), "{case}"); + let found = line(&merged_view, path).expect("both names stay"); + assert_eq!(found.contains(" link="), links == 2, "{case}: {found}"); + assert_eq!(viewer(&fixture).links(&root, path), links, "{case}"); + } + fixture.cleanup(); + } +} + +/// A Workspace creates `/conflict.txt` and links it at `/clean_alias.txt` +/// while another writer adds a different `/conflict.txt` to the Branch. The +/// Commit conflicts at `/conflict.txt` alone and publishes nothing. The root +/// it installs in the Workspace holds the markers at `/conflict.txt` and +/// leaves `/clean_alias.txt` the bytes the Workspace wrote, as one link. +#[test] +fn a_conflict_at_one_link_of_a_new_file_leaves_its_other_link_alone() { + let fixture = Fixture::new("serial-link-conflict", |source| { + fs::write(source.join("readme.txt"), b"base\n").unwrap(); + 1 + }); + fixture.fork("work"); + let committed = fixture.workspace_commit( + "work", + &[ + Change::Create("/conflict.txt", b"our new file\n"), + Change::Link("/conflict.txt", "/clean_alias.txt"), + ], + 1, + || { + fixture.apply("work", vec![write("/conflict.txt", "their new file\n")]); + }, + ); + let report = committed + .merge + .as_ref() + .expect("the head moved: a merge ran"); + let conflicts: Vec<_> = report.conflicts.iter().map(|c| c.path.as_str()).collect(); + assert_eq!(conflicts, ["/conflict.txt"]); + // Nothing is published: the Branch keeps only the other writer's file. + assert!(committed.history.is_none(), "{:?}", committed.history); + assert_eq!( + read_at(&fixture, "work", "/conflict.txt").as_deref(), + Some("their new file\n") + ); + assert_eq!(read_at(&fixture, "work", "/clean_alias.txt"), None); + // The Workspace's own root binds one file at both names; the root the + // merge installed in its place binds a file of its own at each. + let written = report.written.clone(); + let (shown, local) = committed + .replaced + .expect("a conflicted merge installs a root"); + let (shown, local) = (view::hex(shown.as_bytes()), view::hex(local.as_bytes())); + let store = viewer(&fixture); + let shown_view = store.view(&shown, false); + for path in ["/conflict.txt", "/clean_alias.txt"] { + let found = line(&shown_view, path).expect("both names bound"); + assert!(found.ends_with(" link=/clean_alias.txt"), "{found}"); + } + let local_view = store.view(&local, false); + assert_eq!(store.text(&local, "/clean_alias.txt"), "our new file\n"); + let marked = store.text(&local, "/conflict.txt"); + assert!( + marked + .starts_with("<<<<<<< workspace\nour new file\n=======\ntheir new file\n>>>>>>> work@"), + "{marked}" + ); + for path in ["/conflict.txt", "/clean_alias.txt"] { + let found = line(&local_view, path).expect("both names stay"); + assert!(!found.contains(" link="), "{found}"); + assert_eq!(store.links(&local, path), 1, "{path}"); + } + // Only the conflicted name is reported as written into the Workspace. + assert_eq!(written, ["/conflict.txt"]); + fixture.cleanup(); +} + #[test] fn a_one_sided_mode_change_is_kept() { let fixture = seeded("serial-chmod"); diff --git a/engine/crates/layerfs-api/tests/support/view.rs b/engine/crates/layerfs-api/tests/support/view.rs index 4f29d87b4..69a491b5e 100644 --- a/engine/crates/layerfs-api/tests/support/view.rs +++ b/engine/crates/layerfs-api/tests/support/view.rs @@ -5,16 +5,18 @@ use layerfs_content::compare::tree::{inode_changes, join, list_directory, TreeTable}; use layerfs_content::compare::DifferenceWork; +use layerfs_content::file::read_all_bounded; use layerfs_content::filesystem::attributes::{read_portable, AttributeReadWork}; -use layerfs_content::filesystem::FilesystemRootId; +use layerfs_content::filesystem::{FilesystemRead, FilesystemRootId, LogicalPath, Resolved}; use layerfs_content::object::inode_leaf::InodeKind; use layerfs_content::object::{AuthenticatedObjects, ObjectId}; use layerfs_persistence::{Handles, PersistenceConfig, SqlitePersistenceProfile}; use layerfs_storage::Storage; +use layerfs_telemetry::timer::Timing; use std::collections::BTreeMap; use std::path::Path; -fn hex(bytes: &[u8]) -> String { +pub fn hex(bytes: &[u8]) -> String { bytes.iter().fold(String::new(), |mut text, byte| { text.push_str(&format!("{byte:02x}")); text @@ -50,6 +52,44 @@ impl Viewer { view(&reader, root, mtime) } + /// The inode bound at `path` in the root named by `root` (hex). + fn resolve(&self, root: &str, path: &str) -> Resolved { + let reader = self.storage.reader().unwrap(); + let root = FilesystemRootId(ObjectId::from_bytes(&unhex(root)).unwrap()); + let path = LogicalPath::new(path.trim_start_matches('/')).unwrap(); + FilesystemRead::new(&reader, root) + .unwrap() + .resolve(&path) + .unwrap() + } + + /// The link count stored for the inode bound at `path` in the root + /// named by `root` (hex). + pub fn links(&self, root: &str, path: &str) -> u64 { + self.resolve(root, path).value.namespace_ref_count + } + + /// The bytes of the regular file bound at `path` in the root named by + /// `root` (hex), as text. + pub fn text(&self, root: &str, path: &str) -> String { + let reader = self.storage.reader().unwrap(); + let file = self.resolve(root, path).value; + assert_eq!(file.kind, InodeKind::RegularFile, "{path}"); + let mut bytes = Vec::new(); + Timing::disabled("view.text", |timing| { + read_all_bounded( + &reader, + file.content_root, + u64::MAX, + &mut bytes, + timing.child("read"), + ) + }) + .0 + .unwrap(); + String::from_utf8(bytes).unwrap() + } + /// Serials whose value differs between two roots (hex): what a merge by /// serial decides for that side. pub fn changed(&self, from: &str, to: &str) -> u64 {