git ents: feat: retain anchored content in the comment tree instead of a second parent

commit 9170c0e

feat: retain anchored content in the comment tree instead of a second parent

Per docs/abstractions.adoc §3, anchor retention no longer pins the anchored commit as an extra commit parent. Instead the anchored blob (by its existing object id, no copy) and a small window of surrounding source lines are embedded as ordinary tree entries in the comment’s own document tree via facet-git-tree’s RawTree passthrough, keeping both gc-reachable without a gitlink or a second parent. The anchor commit id itself is now best-effort only. Projection is layered: the existing tree-diff path is used while the anchor commit still exists, and falls back to fuzzy line-window matching of the retained context against the target commit’s version of the file once it is gone. This changes the on-disk Comment format (hard cutover, no read fallback, per project precedent) and the git-store Store API (the extra-parents/chain-parent machinery it existed for is gone).

feat: embed the anchored blob and a context window in comment trees feat: fall back to context matching when the anchor commit is gone fix: treat facet-git-tree RawTree fields as atomic in structural merge refactor: drop git-store’s second-parent/chain-parent plumbing docs: describe best-effort anchor retention and context-fallback projection Assisted-by: Claude:claude-sonnet-5

Joseph D. Carpinelli · 1 month ago

tree browse at HEAD · parents: 8cb2e93 · comment on this commit

Reviews

No reviews of this commit yet — record a verdict below.

Start a review

Cargo.lock @@ -1453,6 +1453,7 @@ version = "0.0.0" dependencies = [ "facet", + "facet-git-tree", "git-anchor", "git-store", "gix",

docs/abstractions.adoc @@ -40,12 +40,20 @@ A durable pointer into source: blob, optional line range, specific commit. -* *Reachability invariant:* the meta-ref commit storing an anchor -carries the anchored commit as a second parent. Anchored objects are -reachable from `refs/meta/*` and survive force-push, branch deletion, -and gc — no gc special-casing. +* *Reachability invariant:* the anchored blob and a small window of +surrounding source lines are embedded as ordinary entries in the +consuming document's own stored tree (a reference to the existing +blob's object id, no copy; the surrounding-lines window written fresh) +— not a gitlink, which names a commit without making it reachable. +Both stay reachable from `refs/meta/*` and survive force-push, branch +deletion, and gc — no gc special-casing. The anchored commit's id is +kept only as a best-effort data field, not pinned by this mechanism. * *Projection:* anchors project onto newer commits, so annotations -follow code as it evolves. +follow code as it evolves. When the anchored commit still exists this +is an exact tree diff; once it has been collected, projection falls +back to fuzzy-matching the retained surrounding-lines window against +the target commit's version of the file. Projection never mutates the +stored anchor. Anchors are independent of any consumer; comments use them, but reviews, TODOs, and blame overlays can too.

crates/git-comment/Cargo.toml @@ -7,6 +7,7 @@ [dependencies] facet = { workspace = true } +facet-git-tree = { workspace = true } git-anchor = { workspace = true } git-store = { workspace = true } gix = { workspace = true }

docs/spec/anchor.adoc @@ -16,6 +16,10 @@ revision's actual content. The anchored text is fully derivable from the blob and the line range and MUST be derived at read time, never stored redundantly. +The anchored commit is recorded on a best-effort basis only (see +<<anchor.reachability>>): nothing keeps it from being garbage collected, and +an anchor MUST remain valid — its exact text still derivable, its position +still projectable via a fallback — after it is gone. -- [role="requirement", id="comments.projection"] @@ -28,15 +32,32 @@ new path and range), *outdated* (an edit touched the anchored region, or the entry is no longer a regular file), or *deleted* (the file is gone). Projection MUST follow renames and MUST work between any two commits — -forwards, backwards, or across unrelated history. +forwards, backwards, or across unrelated history, as long as the anchor's own +commit still exists. +Once the anchor commit has been garbage collected, projection MUST fall back +to fuzzy-matching a small retained window of surrounding source lines against +the target commit's version of the same path, reporting *relocated* (with the +matched line range) on a good match, *outdated* on a poor one, and *deleted* +when the path itself is gone — the same four outcomes, recovered +approximately rather than exactly. An outdated or deleted projection MUST NOT lose the comment: the original anchor remains displayable. +Projection is read-only and MUST NEVER mutate the stored anchor, in either +the exact or the fallback path. -- [role="requirement", id="anchor.reachability"] .Anchor Reachability -- -The meta-ref commit storing an anchor MUST carry the anchored commit as an -extra parent, so the anchored commit is reachable from `refs/meta/*` and -survives force-push, branch deletion, and gc without any gc special-casing. +The content an anchor names — the anchored blob and a small window of +surrounding source lines — MUST be embedded as ordinary tree entries in the +consuming document's own stored tree (a reference to the existing blob's +object id, not a copy; the surrounding-lines window written fresh), so both +stay reachable from `refs/meta/*`, and so survive force-push, branch +deletion, and gc, for as long as the consuming document's ref exists. This +MUST NOT use a gitlink (mode `160000`): a gitlink names a commit in another +repository and is not itself a reachability edge, so it would not keep +anything reachable. The anchored commit's id is retained only as a plain data +field (see <<comments.anchor>>) and is NOT pinned by this mechanism; it MAY +be garbage collected once nothing else keeps it reachable. --

docs/spec/conformance.adoc @@ -55,9 +55,9 @@ |`issues.id` |`git-ents/src/issues.rs` (`new_id`, `promote`) |`issues::new_id_hashes_its_own_content_with_no_origin`, `promotion_assigns_a_number_and_advances_the_counter_without_renaming_the_ref` |`comments.ref` |`git-comment/src/lib.rs` (`Comment`, `COMMENTS_NS`, `new_id`) |`git-comment::store_then_load_round_trips_a_comment`, `loads_the_on_disk_comment_format`, `new_id_hashes_its_own_content_with_no_origin` |`comments.authorship` |`git-store/src/lib.rs` (`store_item_authored`, `provenance`); `git-comment/src/lib.rs` (`provenance`) |`git-store::provenance_recovers_the_creating_and_updating_authors`, `git-comment::provenance_comes_from_the_commits_not_the_document` -|`comments.anchor` |`git-anchor/src/lib.rs` (`Anchor`, `capture`, `snippet`) |`git-anchor::capture_records_the_commit_and_blob_and_snippet_derives_the_text`, `capture_rejects_a_missing_path_and_an_oversized_range` -|`comments.projection` |`git-anchor/src/lib.rs` (`project`, `Projection`) |`git-anchor::unchanged_file_projects_as_current`, `a_pure_rename_relocates_with_the_same_lines`, `an_edit_inside_the_range_is_outdated`, `a_deleted_file_projects_as_deleted`, `projection_works_backwards_onto_an_ancestor` -|`anchor.reachability` |`git-store/src/lib.rs` (`store_with_parents`, `store_authored_with_parents`, `CHAIN_PARENT_HEADER`); `git-comment/src/lib.rs` (`store`) |`git-comment::a_stored_comment_carries_its_anchored_commit_as_a_second_parent` +|`comments.anchor` |`git-anchor/src/lib.rs` (`Anchor`, `capture`, `snippet`, `context`) |`git-anchor::capture_records_the_commit_and_blob_and_snippet_derives_the_text`, `capture_rejects_a_missing_path_and_an_oversized_range`, `context_captures_a_margin_around_the_anchored_range`, `context_clamps_to_the_file_when_the_margin_would_overrun_it`, `context_of_a_whole_file_anchor_is_the_whole_file` +|`comments.projection` |`git-anchor/src/lib.rs` (`project`, `project_from_context`, `Projection`) |`git-anchor::unchanged_file_projects_as_current`, `a_pure_rename_relocates_with_the_same_lines`, `an_edit_inside_the_range_is_outdated`, `a_deleted_file_projects_as_deleted`, `projection_works_backwards_onto_an_ancestor`, `project_from_context_relocates_across_an_edit_above_the_range`, `project_from_context_reports_outdated_when_no_window_matches_well`, `project_from_context_reports_file_deleted`, `project_from_context_of_a_whole_file_anchor_survives_any_edit` +|`anchor.reachability` |`git-comment/src/lib.rs` (`StoredComment`, `embed`, `store`); `git-store/src/merge.rs` (`RawTree` handling in `classify`) |`git-comment::the_anchored_blob_survives_branch_deletion_and_gc_pruning_the_anchor_commit` |`web.render-registry` |`git-ents-server/src/render.rs` (`to_html`/`to_text`, `mime_for_name`) | _(passthrough fallback exercised by web/CLI render paths)_ |`web.server-rendered` |`git-ents-server/src/web/*.rs` (Askama/maud templates, no client JS required) | _(manual UI verification)_ |`web.index` |`git-ents-server/src/web/mod.rs` (`index`) | _(manual UI verification)_

docs/spec/meta-ref.adoc @@ -275,6 +275,10 @@ never renamed. One ref per comment keeps comments independently loadable and separately historied; the ref's commit chain is the comment's edit history. +The document's stored tree additionally carries the retained blob and +surrounding-lines window described in <<anchor.reachability>>; this is +storage plumbing, not a `Comment` field, and is invisible to any reader of +the document's public shape. -- [role="requirement", id="comments.authorship"]

crates/git-anchor/src/lib.rs @@ -5,11 +5,18 @@ //! blob at that commit, and an optional line range. The anchored text is never //! stored — the blob is content-addressed, so [`snippet`] derives it exactly //! at read time. The anchor is authoritative at creation and never mutated. -//! [`project`] -//! answers, at read time, where that position sits on any *other* commit — -//! following renames through git's rewrite tracking and shifting line ranges -//! through the blob's diff hunks, the way git itself re-derives positions when -//! replaying diffs on rebase. +//! +//! `commit` is recorded on a best-effort basis: nothing pins it against +//! garbage collection, so it may no longer exist by the time the anchor is +//! read back. [`project`] answers, at read time, where the anchor's position +//! sits on any *other* commit — following renames through git's rewrite +//! tracking and shifting line ranges through the blob's diff hunks, the way +//! git itself re-derives positions when replaying diffs on rebase — but it +//! needs `commit` to still exist to do so. [`context`] captures a small, +//! independently-retainable window of surrounding lines at capture time; +//! [`project_from_context`] fuzzy-matches that window against a target +//! commit's version of the same path when `commit` is gone, giving projection +//! a fallback that survives the anchor commit's own collection. //! //! Projection is a two-point tree diff, not a history walk: it compares the //! anchor commit's tree directly against the target commit's tree, so it works @@ -95,6 +102,11 @@ /// How many lines the file actually has. len: u64, }, + /// [`project`]'s anchor commit is no longer present in the repository + /// (garbage collected) — the trigger for [`project_from_context`]'s + /// fuzzy-matching fallback, which needs no commit at all. + #[error("the anchor commit {0} no longer exists")] + AnchorCommitMissing(ObjectId), } /// A 1-based inclusive range of lines within an anchored file. @@ -115,7 +127,10 @@ /// @relation(comments.anchor) #[derive(Debug, Clone, PartialEq, Eq, Facet)] pub struct Anchor { - /// The commit the anchor was created against. + /// The commit the anchor was created against, recorded on a best-effort + /// basis: nothing keeps it reachable, so it may be gone (garbage + /// collected) by the time the anchor is read back. [`project`] needs it + /// to still exist; [`project_from_context`] does not. pub commit: Oid, /// The repository-relative path of the anchored file at that commit. pub path: String, @@ -242,6 +257,11 @@ /// range is mapped through the blob diff's hunks — shifted past edits that /// land entirely outside it, [`Projection::Outdated`] when an edit touches it. /// +/// Fails with [`Error::AnchorCommitMissing`] when `anchor.commit` no longer +/// exists (it is retained on a best-effort basis only); a caller that also +/// holds the anchor's [`context`] should retry with [`project_from_context`] +/// in that case. +/// /// ## Requirements /// /// @relation(comments.projection) @@ -265,6 +285,9 @@ return Ok(Projection::Current); } + if !repo.has_object(anchor_commit_id) { + return Err(Error::AnchorCommitMissing(anchor_commit_id)); + } let anchor_commit = commit_at(&repo, anchor_commit_id)?; let anchor_tree = anchor_commit .tree() @@ -346,6 +369,146 @@ Ok(Projection::Relocated { path, lines }) } +/// How many lines of surrounding source [`context`] captures on each side of +/// an anchored range — enough for [`project_from_context`]'s line-window scan +/// to recognize the anchored lines' neighborhood even after they themselves +/// moved a little, without dragging in unrelated parts of a large file. +const CONTEXT_MARGIN: u64 = 3; + +/// The anchored range (or, for a whole-file anchor, the whole file) plus up to +/// [`CONTEXT_MARGIN`] lines on either side, read from `anchor.blob` — a small, +/// independently-retainable snapshot of the anchor's surroundings for +/// [`project_from_context`] to fuzzy-match once `anchor.commit` itself is +/// gone. The caller decides how (or whether) to retain the result; this +/// function only derives it. +/// +/// ## Requirements +/// +/// @relation(comments.anchor) +pub fn context(repo: &Path, anchor: &Anchor) -> Result<String, Error> { + let repo = gix::open(repo).map_err(|error| Error::Open(Box::new(error)))?; + let blob = ObjectId::try_from(&anchor.blob) + .map_err(|_error| Error::Resolve(anchor.blob.to_string()))?; + let data = read_blob(&repo, blob)?; + let Some(range) = anchor.lines else { + return Ok(String::from_utf8_lossy(&data).into_owned()); + }; + let all: Vec<&[u8]> = data.lines_with_terminator().collect(); + let len = u64::try_from(all.len()).unwrap_or(u64::MAX); + let start0 = range.start.saturating_sub(1); + let margin_before = CONTEXT_MARGIN.min(start0); + let ctx_start = start0.saturating_sub(margin_before); + let margin_after = CONTEXT_MARGIN.min(len.saturating_sub(range.end)); + let ctx_end = range.end.saturating_add(margin_after).min(len); + let (Ok(ctx_start), Ok(ctx_end)) = (usize::try_from(ctx_start), usize::try_from(ctx_end)) + else { + return Ok(String::new()); + }; + let bytes = all.get(ctx_start..ctx_end).unwrap_or_default().concat(); + Ok(String::from_utf8_lossy(&bytes).into_owned()) +} + +/// Project `anchor` onto `target` by fuzzy-matching `context` (as produced by +/// [`context`] at capture time) against `target`'s version of `anchor.path`, +/// for use once `anchor.commit` no longer exists and [`project`] can no +/// longer diff against its tree. +/// +/// Looks up `anchor.path` in `target`'s tree directly (no rename tracking is +/// possible without the anchor commit's tree, so a genuine rename reports +/// [`Projection::FileDeleted`] here, same as a real deletion); a whole-file +/// anchor (`anchor.lines` is `None`) survives any edit at that path, same as +/// [`project`]. For a line-range anchor, every contiguous window of the +/// target file's lines the same length as `context` is scored by how many +/// lines match `context`'s exactly; the best-scoring window (at least half +/// its lines matching) is accepted and the anchored sub-range is mapped back +/// through the same margin [`context`] used to build it. No match clears that +/// bar reports [`Projection::Outdated`], the same as an unrecoverable edit +/// would under [`project`]. +/// +/// ## Requirements +/// +/// @relation(comments.projection) +pub fn project_from_context( + repo: &Path, + anchor: &Anchor, + target: &str, + context: &str, +) -> Result<Projection, Error> { + let repo = gix::open(repo).map_err(|error| Error::Open(Box::new(error)))?; + let target_commit = resolve_commit(&repo, target)?; + let target_tree = target_commit + .tree() + .map_err(|error| Error::Object(error.to_string()))?; + let Some(entry) = target_tree + .lookup_entry_by_path(&anchor.path) + .map_err(|error| Error::Object(error.to_string()))? + else { + return Ok(Projection::FileDeleted); + }; + if !entry.mode().is_blob() { + return Ok(Projection::Outdated { + path: anchor.path.clone(), + }); + } + let Some(range) = anchor.lines else { + return Ok(Projection::Relocated { + path: anchor.path.clone(), + lines: None, + }); + }; + + let data = read_blob(&repo, entry.object_id())?; + let target_lines: Vec<&[u8]> = data.lines_with_terminator().collect(); + let context_lines: Vec<&[u8]> = context.as_bytes().lines_with_terminator().collect(); + let window = context_lines.len(); + if window == 0 || window > target_lines.len() { + return Ok(Projection::Outdated { + path: anchor.path.clone(), + }); + } + + let mut best: Option<(usize, usize)> = None; + for (start, slice) in target_lines.windows(window).enumerate() { + let score = slice + .iter() + .zip(context_lines.iter()) + .filter(|(have, want)| have == want) + .count(); + if best.is_none_or(|(_start, best_score)| score > best_score) { + best = Some((start, score)); + } + } + // Require at least half the window's lines to match exactly, so an + // unrelated coincidence of blank or near-empty lines is not mistaken for + // the anchored region having relocated there. + let Some((start, _score)) = best.filter(|(_start, score)| { + score + .checked_mul(2) + .is_some_and(|doubled| doubled >= window) + }) else { + return Ok(Projection::Outdated { + path: anchor.path.clone(), + }); + }; + + let margin_before = CONTEXT_MARGIN.min(range.start.saturating_sub(1)); + let range_len = range.end.saturating_sub(range.start).saturating_add(1); + let Ok(start) = u64::try_from(start) else { + return Ok(Projection::Outdated { + path: anchor.path.clone(), + }); + }; + let mapped_start = start.saturating_add(margin_before).saturating_add(1); + let mapped_end = mapped_start.saturating_add(range_len).saturating_sub(1); + Ok(Projection::Relocated { + path: anchor.path.clone(), + lines: Some(LineRange { + start: mapped_start, + end: mapped_end, + }), + }) +} + /// Resolve `revision` (a hex id, ref name, or revspec) to the commit it names. fn resolve_commit<'repo>( repo: &'repo gix::Repository, @@ -661,4 +824,150 @@ // A range past the end of the old file cannot map. assert_eq!(map_range(old, old, LineRange { start: 4, end: 9 }), None); } + + // @relation(comments.projection, role=Verifies) + #[test] + fn project_reports_the_anchor_commit_as_missing_once_it_is_gone() { + let dir = repo(); + std::fs::write(dir.path().join("file.txt"), numbered(1..=10)).unwrap(); + commit_all(dir.path(), "one"); + let mut anchor = capture(dir.path(), "HEAD", "file.txt", range(3, 4)).unwrap(); + + let edited = numbered(1..=10).replace("line 5\n", "line five\n"); + std::fs::write(dir.path().join("file.txt"), edited).unwrap(); + commit_all(dir.path(), "two"); + + // A made-up commit id that was never written to this repository + // stands in for "gc'd away" without actually having to run gc in a + // unit test — `has_object` answers `false` either way. + anchor.commit = "0123456789abcdef0123456789abcdef01234567".into(); + assert!(matches!( + project(dir.path(), &anchor, "HEAD"), + Err(Error::AnchorCommitMissing(_)) + )); + } + + // @relation(comments.anchor, role=Verifies) + #[test] + fn context_captures_a_margin_around_the_anchored_range() { + let dir = repo(); + std::fs::write(dir.path().join("file.txt"), numbered(1..=10)).unwrap(); + commit_all(dir.path(), "one"); + let anchor = capture(dir.path(), "HEAD", "file.txt", range(5, 6)).unwrap(); + + // 3 lines of margin on each side of a 2-line range: lines 2..=9. + let expected: String = (2..=9).map(|n| format!("line {n}\n")).collect(); + assert_eq!(context(dir.path(), &anchor).unwrap(), expected); + } + + // @relation(comments.anchor, role=Verifies) + #[test] + fn context_clamps_to_the_file_when_the_margin_would_overrun_it() { + let dir = repo(); + std::fs::write(dir.path().join("file.txt"), numbered(1..=4)).unwrap(); + commit_all(dir.path(), "one"); + let anchor = capture(dir.path(), "HEAD", "file.txt", range(1, 2)).unwrap(); + + assert_eq!(context(dir.path(), &anchor).unwrap(), numbered(1..=4)); + } + + // @relation(comments.anchor, role=Verifies) + #[test] + fn context_of_a_whole_file_anchor_is_the_whole_file() { + let dir = repo(); + std::fs::write(dir.path().join("file.txt"), numbered(1..=5)).unwrap(); + commit_all(dir.path(), "one"); + let anchor = capture(dir.path(), "HEAD", "file.txt", None).unwrap(); + + assert_eq!(context(dir.path(), &anchor).unwrap(), numbered(1..=5)); + } + + // @relation(comments.projection, role=Verifies) + #[test] + fn project_from_context_relocates_across_an_edit_above_the_range() { + let dir = repo(); + std::fs::write(dir.path().join("file.txt"), numbered(1..=10)).unwrap(); + commit_all(dir.path(), "one"); + let anchor = capture(dir.path(), "HEAD", "file.txt", range(5, 6)).unwrap(); + let context = context(dir.path(), &anchor).unwrap(); + + let edited = format!("added a\nadded b\n{}", numbered(1..=10)); + std::fs::write(dir.path().join("file.txt"), edited).unwrap(); + commit_all(dir.path(), "two"); + + // Same answer `project` itself would give, but derived with no + // reference at all to the anchor's own (still very much present) + // commit — exercising the exact code path that stands in once it is + // gone. + assert_eq!( + project_from_context(dir.path(), &anchor, "HEAD", &context).unwrap(), + Projection::Relocated { + path: "file.txt".to_owned(), + lines: range(7, 8), + } + ); + } + + // @relation(comments.projection, role=Verifies) + #[test] + fn project_from_context_reports_outdated_when_no_window_matches_well() { + let dir = repo(); + std::fs::write(dir.path().join("file.txt"), numbered(1..=10)).unwrap(); + commit_all(dir.path(), "one"); + let anchor = capture(dir.path(), "HEAD", "file.txt", range(5, 6)).unwrap(); + let context = context(dir.path(), &anchor).unwrap(); + + // A wholesale rewrite leaves nothing resembling the captured + // neighborhood anywhere in the file. + std::fs::write(dir.path().join("file.txt"), "totally\nunrelated\ncontent\n").unwrap(); + commit_all(dir.path(), "two"); + + assert_eq!( + project_from_context(dir.path(), &anchor, "HEAD", &context).unwrap(), + Projection::Outdated { + path: "file.txt".to_owned(), + } + ); + } + + // @relation(comments.projection, role=Verifies) + #[test] + fn project_from_context_reports_file_deleted() { + let dir = repo(); + std::fs::write(dir.path().join("file.txt"), numbered(1..=10)).unwrap(); + commit_all(dir.path(), "one"); + let anchor = capture(dir.path(), "HEAD", "file.txt", range(5, 6)).unwrap(); + let context = context(dir.path(), &anchor).unwrap(); + + std::fs::remove_file(dir.path().join("file.txt")).unwrap(); + std::fs::write(dir.path().join("unrelated.txt"), "different\n").unwrap(); + commit_all(dir.path(), "two"); + + assert_eq!( + project_from_context(dir.path(), &anchor, "HEAD", &context).unwrap(), + Projection::FileDeleted + ); + } + + // @relation(comments.projection, role=Verifies) + #[test] + fn project_from_context_of_a_whole_file_anchor_survives_any_edit() { + let dir = repo(); + std::fs::write(dir.path().join("file.txt"), numbered(1..=10)).unwrap(); + commit_all(dir.path(), "one"); + let anchor = capture(dir.path(), "HEAD", "file.txt", None).unwrap(); + let context = context(dir.path(), &anchor).unwrap(); + + let edited = numbered(1..=10).replace("line 5\n", "line five\n"); + std::fs::write(dir.path().join("file.txt"), edited).unwrap(); + commit_all(dir.path(), "two"); + + assert_eq!( + project_from_context(dir.path(), &anchor, "HEAD", &context).unwrap(), + Projection::Relocated { + path: "file.txt".to_owned(), + lines: None, + } + ); + } }

crates/git-comment/src/lib.rs @@ -6,14 +6,30 @@ //! authoritative at creation and never mutated, and [`project`] re-derives at //! read time where the comment sits on any other commit. //! +//! # Retention +//! +//! Nothing pins the anchored commit against garbage collection any more — its +//! id on [`Anchor::commit`] is best-effort. What actually survives is the +//! anchored *content*: [`store`] embeds the anchored blob directly (a tree +//! entry pointing at its existing object id, no copy — content addressing +//! makes this free) alongside a small `context` blob of the surrounding source +//! lines ([`git_anchor::context`]), both as ordinary entries in the comment's +//! own document tree. That makes them reachable — and so un-collectable — for +//! as long as the comment's ref exists, with no gitlink and no second commit +//! parent involved. [`project`] uses the context blob to fuzzy-match the +//! anchor's location back onto a target commit once the anchor commit itself +//! is gone (see [`git_anchor::project_from_context`]). +//! //! # The comment is the commit //! //! The document tree holds only the body, the anchor, and an optional issue -//! cross-reference. Who wrote the comment and when are *not* fields: they are -//! recovered from the ref's commit chain ([`provenance`]) — the genesis -//! commit's author created the comment, the tip commit's author last edited -//! it — exactly as git itself carries authorship. [`store`] therefore takes -//! the author and stamps it on the commit it writes. +//! cross-reference (plus the retained blob and context, invisible to the +//! public [`Comment`] type — see [`StoredComment`]). Who wrote the comment and +//! when are *not* fields: they are recovered from the ref's commit chain +//! ([`provenance`]) — the genesis commit's author created the comment, the +//! tip commit's author last edited it — exactly as git itself carries +//! authorship. [`store`] therefore takes the author and stamps it on the +//! commit it writes. //! //! # Identity //! @@ -25,9 +41,12 @@ use std::path::Path; use facet::Facet; +use facet_git_tree::RawTree; use git_anchor::{Anchor, Projection}; use git_store::Provenance; use gix::ObjectId; +use gix::objs::tree::{Entry as TreeEntry, EntryKind, EntryMode}; +use gix::objs::{Blob, FindExt as _, Tree, Write as _}; // @relation(comments.ref) /// The namespace under which comments are recorded: one ref, @@ -52,6 +71,41 @@ pub issue: Option<String>, } +/// The document actually written to and read from a comment's ref: [`Comment`] +/// plus `retained`, a passthrough tree ([`facet_git_tree::RawTree`]) holding +/// two entries invisible to [`Comment`] itself — `blob`, the anchored file at +/// its own object id (a reference, not a copy: content addressing makes this +/// free), and `context`, [`git_anchor::context`]'s snapshot of the +/// surrounding lines. Both ride along in the comment's own document tree +/// purely so they stay reachable from `refs/meta/comments/<id>` — and so +/// survive force-push, branch deletion, and gc — for as long as the comment's +/// ref exists, with no gitlink and no second commit parent involved. +/// +/// `retained` is deliberately absent from the public [`Comment`]: it is +/// storage plumbing a caller never needs to see, read, or set — [`store`] +/// derives it fresh from `comment.anchor` every write. +/// +/// ## Requirements +/// +/// @relation(anchor.reachability) +#[derive(Debug, Clone, PartialEq, Eq, Facet)] +struct StoredComment { + body: String, + anchor: Anchor, + issue: Option<String>, + retained: RawTree, +} + +impl From<StoredComment> for Comment { + fn from(stored: StoredComment) -> Self { + Self { + body: stored.body, + anchor: stored.anchor, + issue: stored.issue, + } + } +} + /// Derive a comment's stable genesis key: `origin`'s object id (hex) when the /// comment derives from one, otherwise the hash of the comment's own initial /// content — every comment is a git object, so it always has one. @@ -62,15 +116,17 @@ /// Load the comment recorded at `refs/meta/comments/<id>` in `repo`, or `None` /// when no such comment exists. pub fn load(repo: &Path, id: &str) -> Result<Option<Comment>, git_store::Error> { - git_store::Store::open(repo)?.load_item(COMMENTS_NS, id) + Ok(git_store::Store::open(repo)? + .load_item::<StoredComment>(COMMENTS_NS, id)? + .map(Into::into)) } /// Write `comment` to `refs/meta/comments/<id>` in `repo` as a new commit /// authored by `author` (a `(name, email)` pair), so the ref's commit chain is -/// the comment's edit history and carries its authorship. The commit also -/// carries the anchored commit as a second parent, so the commit the comment -/// annotates stays reachable — and so cannot be garbage-collected — for as -/// long as the comment's ref exists. +/// the comment's edit history and carries its authorship. Also embeds the +/// anchored blob and a context snapshot in the written document tree (see +/// [`StoredComment`]), so the content the comment is anchored to stays +/// reachable independently of whether `comment.anchor.commit` itself survives. /// /// ## Requirements /// @@ -81,21 +137,80 @@ comment: &Comment, author: (&str, &str), ) -> Result<(), git_store::Error> { - let anchored = ObjectId::try_from(&comment.anchor.commit) + let context = git_anchor::context(repo, &comment.anchor) .map_err(|error| git_store::Error::Invalid(error.to_string()))?; - git_store::Store::open(repo)?.store_item_authored_with_parents( + let odb = odb_at(repo)?; + let retained = embed(&odb, &comment.anchor, &context)?; + let stored = StoredComment { + body: comment.body.clone(), + anchor: comment.anchor.clone(), + issue: comment.issue.clone(), + retained, + }; + git_store::Store::open(repo)?.store_item_authored( COMMENTS_NS, id, - comment, + &stored, "Update comment", author, - &[anchored], ) } +/// Write the anchored blob (by its existing object id, no copy) and a fresh +/// `context` blob into a small tree, wrapped as a [`RawTree`] ready to embed +/// in a [`StoredComment`] — the retention mechanism [`store`] relies on. +/// +/// ## Requirements +/// +/// @relation(anchor.reachability) +fn embed( + odb: &gix::odb::Handle, + anchor: &Anchor, + context: &str, +) -> Result<RawTree, git_store::Error> { + let blob_oid = ObjectId::try_from(&anchor.blob) + .map_err(|error| git_store::Error::Invalid(error.to_string()))?; + let context_oid = odb + .write(&Blob { + data: context.as_bytes().to_vec(), + }) + .map_err(|error| git_store::Error::Object(error.to_string()))?; + let mut entries = vec![ + TreeEntry { + mode: EntryMode::from(EntryKind::Blob), + filename: "blob".into(), + oid: blob_oid, + }, + TreeEntry { + mode: EntryMode::from(EntryKind::Blob), + filename: "context".into(), + oid: context_oid, + }, + ]; + entries.sort(); + let tree_oid = odb + .write(&Tree { entries }) + .map_err(|error| git_store::Error::Object(error.to_string()))?; + Ok(RawTree::new(tree_oid)) +} + +/// Open a raw object database on `repo`'s common git directory, the same one +/// [`git_store::Store`] uses internally — opened again here since writing the +/// retained blob and context tree directly is this crate's own concern (see +/// [`embed`]), the same reasoning `git-toolchain` documents for its own +/// direct object writes. +fn odb_at(repo: &Path) -> Result<gix::odb::Handle, git_store::Error> { + let opened = gix::open(repo).map_err(|error| git_store::Error::Open(Box::new(error)))?; + gix::odb::at(opened.common_dir().join("objects")).map_err(|_io| git_store::Error::Odb) +} + /// List every comment in `repo` as `(id, comment)` pairs, newest ref first. pub fn list(repo: &Path) -> Result<Vec<(String, Comment)>, git_store::Error> { - git_store::Store::open(repo)?.list_items(COMMENTS_NS) + Ok(git_store::Store::open(repo)? + .list_items::<StoredComment>(COMMENTS_NS)? + .into_iter() + .map(|(id, stored)| (id, stored.into())) + .collect()) } /// Who created and who last updated the comment at `id`, recovered from its @@ -108,19 +223,56 @@ git_store::Store::open(repo)?.item_provenance(COMMENTS_NS, id) } -/// Where `comment`'s anchor sits on `target` (a revision in `repo`): still -/// [`Projection::Current`], relocated to a new path or shifted lines, outdated -/// because the anchored region was edited, or gone with its file. +/// Where the comment `id`'s anchor sits on `target` (a revision in `repo`): +/// still [`Projection::Current`], relocated to a new path or shifted lines, +/// outdated because the anchored region was edited, or gone with its file. +/// +/// Tries [`git_anchor::project`] first; if `comment.anchor.commit` has been +/// garbage collected, falls back to [`git_anchor::project_from_context`] +/// against the comment's retained `context` blob, read directly off `id`'s +/// ref rather than recomputed — recomputing would need the very commit that +/// is gone. /// /// ## Requirements /// /// @relation(comments.projection) pub fn project( repo: &Path, + id: &str, comment: &Comment, target: &str, ) -> Result<Projection, git_anchor::Error> { - git_anchor::project(repo, &comment.anchor, target) + match git_anchor::project(repo, &comment.anchor, target) { + Err(git_anchor::Error::AnchorCommitMissing(_)) => { + let context = retained_context(repo, id) + .map_err(|error| git_anchor::Error::Object(error.to_string()))?; + git_anchor::project_from_context(repo, &comment.anchor, target, &context) + } + other => other, + } +} + +/// Read the `context` blob out of comment `id`'s retained tree (see +/// [`StoredComment`]) directly off its ref, for [`project`]'s fallback path. +fn retained_context(repo: &Path, id: &str) -> Result<String, git_store::Error> { + let stored: StoredComment = git_store::Store::open(repo)? + .load_item(COMMENTS_NS, id)? + .ok_or_else(|| git_store::Error::Ref(format!("{COMMENTS_NS}/{id} does not exist")))?; + let odb = odb_at(repo)?; + let mut tree_buf = Vec::new(); + let tree = odb + .find_tree(&stored.retained.oid(), &mut tree_buf) + .map_err(|error| git_store::Error::Object(error.to_string()))?; + let entry = tree + .entries + .iter() + .find(|entry| entry.filename == "context") + .ok_or_else(|| git_store::Error::Object("retained tree has no context entry".to_owned()))?; + let mut blob_buf = Vec::new(); + let blob = odb + .find_blob(entry.oid, &mut blob_buf) + .map_err(|error| git_store::Error::Object(error.to_string()))?; + Ok(String::from_utf8_lossy(blob.data).into_owned()) } #[cfg(test)] @@ -131,6 +283,7 @@ reason = "unit test" )] + use std::path::Path; use std::process::Command; use git_anchor::LineRange; @@ -138,13 +291,23 @@ use super::*; - fn comment(body: &str, issue: Option<&str>) -> Comment { + /// A comment whose anchor points at a real (if otherwise arbitrary) blob + /// in `dir` — `store` now has to read that blob to derive `context`, so a + /// fixture anchored to a made-up oid would fail before ever reaching the + /// assertions these tests care about. The anchor's `commit` stays a + /// made-up hex string: nothing reads it back except as an opaque field. + fn comment(dir: &Path, body: &str, issue: Option<&str>) -> Comment { + let blob = git_with_stdin( + dir, + &["hash-object", "-w", "--stdin"], + "one\ntwo\nthree\nfour\n", + ); Comment { body: body.to_owned(), anchor: Anchor { commit: "0123456789abcdef0123456789abcdef01234567".into(), path: "src/lib.rs".to_owned(), - blob: "89abcdef0123456789abcdef0123456789abcdef".into(), + blob: blob.as_str().into(), lines: Some(LineRange { start: 3, end: 4 }), }, issue: issue.map(str::to_owned), @@ -157,7 +320,7 @@ #[test] fn store_then_load_round_trips_a_comment() { let dir = repo(); - let written = comment("Why is this 1?", Some("deadbeef")); + let written = comment(dir.path(), "Why is this 1?", Some("deadbeef")); store(dir.path(), "1", &written, AUTHOR).unwrap(); assert_eq!(load(dir.path(), "1").unwrap(), Some(written)); } @@ -172,8 +335,14 @@ #[test] fn lists_comments_keyed_by_id() { let dir = repo(); - store(dir.path(), "1", &comment("first", None), AUTHOR).unwrap(); - store(dir.path(), "2", &comment("second", None), AUTHOR).unwrap(); + store(dir.path(), "1", &comment(dir.path(), "first", None), AUTHOR).unwrap(); + store( + dir.path(), + "2", + &comment(dir.path(), "second", None), + AUTHOR, + ) + .unwrap(); let mut ids: Vec<String> = list(dir.path()) .unwrap() .into_iter() @@ -185,14 +354,16 @@ #[test] fn new_id_uses_the_origin_when_one_is_given() { - let content = comment("a comment", None); + let dir = repo(); + let content = comment(dir.path(), "a comment", None); assert_eq!(new_id(Some("deadbeef"), &content).unwrap(), "deadbeef"); } #[test] fn new_id_hashes_its_own_content_with_no_origin() { - let a = comment("a comment", None); - let b = comment("a different comment", None); + let dir = repo(); + let a = comment(dir.path(), "a comment", None); + let b = comment(dir.path(), "a different comment", None); let a_id = new_id(None, &a).unwrap(); assert_eq!(a_id, new_id(None, &a).unwrap()); assert_ne!(a_id, new_id(None, &b).unwrap()); @@ -202,11 +373,11 @@ #[test] fn provenance_comes_from_the_commits_not_the_document() { let dir = repo(); - store(dir.path(), "1", &comment("first", None), AUTHOR).unwrap(); + store(dir.path(), "1", &comment(dir.path(), "first", None), AUTHOR).unwrap(); store( dir.path(), "1", - &comment("edited", None), + &comment(dir.path(), "edited", None), ("bob", "bob@example.com"), ) .unwrap(); @@ -217,40 +388,120 @@ assert!(provenance.created.seconds > 0); } + /// The current branch's short name, so a test that force-moves the + /// branch ref does not have to guess `init.defaultBranch`. + fn current_branch(dir: &Path) -> String { + let output = Command::new("git") + .arg("-C") + .arg(dir) + .args(["symbolic-ref", "--short", "HEAD"]) + .output() + .unwrap(); + assert!(output.status.success()); + String::from_utf8(output.stdout).unwrap().trim().to_owned() + } + + /// Whether `oid` still exists as an object in `dir`'s repository. + fn object_exists(dir: &Path, oid: &str) -> bool { + Command::new("git") + .arg("-C") + .arg(dir) + .args(["cat-file", "-e", oid]) + .status() + .unwrap() + .success() + } + // @relation(anchor.reachability, role=Verifies) #[test] - fn a_stored_comment_carries_its_anchored_commit_as_a_second_parent() { + fn the_anchored_blob_survives_branch_deletion_and_gc_pruning_the_anchor_commit() { let dir = repo(); - std::fs::write(dir.path().join("file.txt"), "one\ntwo\nthree\n").unwrap(); - commit_all(dir.path(), "one"); + let repo_path = dir.path(); + std::fs::write(repo_path.join("file.txt"), "one\ntwo\nthree\nfour\nfive\n").unwrap(); + commit_all(repo_path, "one"); let anchor = git_anchor::capture( - dir.path(), + repo_path, "HEAD", "file.txt", Some(LineRange { start: 2, end: 2 }), ) .unwrap(); - let anchored_commit = anchor.commit.to_string(); + let anchor_commit = anchor.commit.to_string(); let written = Comment { - body: "Why two?".to_owned(), + body: "why two?".to_owned(), anchor, issue: None, }; let id = new_id(None, &written).unwrap(); - store(dir.path(), &id, &written, AUTHOR).unwrap(); + store(repo_path, &id, &written, AUTHOR).unwrap(); - let is_ancestor = Command::new("git") - .current_dir(dir.path()) - .args([ - "merge-base", - "--is-ancestor", - &anchored_commit, - &format!("{COMMENTS_NS}/{id}"), - ]) + // Rewrite the branch onto a brand-new parentless commit holding an + // edited file, so the original commit is no longer anyone's + // ancestor. An ordinary edit could never detach history like this, + // but a rebase, a `filter-repo` pass, or a force-push can, and that + // is exactly the scenario retention has to survive. + let edited_blob = git_with_stdin( + repo_path, + &["hash-object", "-w", "--stdin"], + "zero\none\ntwo\nthree\nfour\nfive\n", + ); + let edited_tree = git_with_stdin( + repo_path, + &["mktree"], + &format!("100644 blob {edited_blob}\tfile.txt\n"), + ); + let replacement = + git_with_stdin(repo_path, &["commit-tree", &edited_tree, "-m", "two"], ""); + let branch = current_branch(repo_path); + let status = Command::new("git") + .arg("-C") + .arg(repo_path) + .args(["update-ref", &format!("refs/heads/{branch}"), &replacement]) .status() .unwrap(); - assert!(is_ancestor.success()); + assert!(status.success()); + assert!( + Command::new("git") + .arg("-C") + .arg(repo_path) + .args(["reflog", "expire", "--expire=now", "--all"]) + .status() + .unwrap() + .success() + ); + assert!( + Command::new("git") + .arg("-C") + .arg(repo_path) + .args(["gc", "--prune=now", "--quiet"]) + .status() + .unwrap() + .success() + ); + + assert!( + !object_exists(repo_path, &anchor_commit), + "the anchor commit should have been pruned" + ); + + // The anchored blob, embedded in the comment's own tree, is still + // readable straight off the ref... + let loaded = load(repo_path, &id).unwrap().unwrap(); + assert_eq!( + git_anchor::snippet(repo_path, &loaded.anchor).unwrap(), + "two\n" + ); + // ...and still projects onto the rewritten branch tip, via the + // context fallback `project` reaches for once the anchor commit is + // gone. + assert_eq!( + project(repo_path, &id, &loaded, &replacement).unwrap(), + Projection::Relocated { + path: "file.txt".to_owned(), + lines: Some(LineRange { start: 3, end: 3 }), + } + ); } // @relation(comments.anchor, comments.projection, role=Verifies) @@ -281,7 +532,7 @@ "two\n" ); assert_eq!( - project(dir.path(), &loaded, "HEAD").unwrap(), + project(dir.path(), &id, &loaded, "HEAD").unwrap(), Projection::Current ); } @@ -291,14 +542,15 @@ fn loads_the_on_disk_comment_format() { // A fixture written as the real on-disk layout — a `body` blob, an // `anchor/` subtree of `commit`/`path`/`blob` blobs with a - // `lines/some/{start,end}` Option subtree, and an `issue/some` Option - // blob — must keep loading, guarding the Comment document's shape - // against an incompatible change to data already on a ref. + // `lines/some/{start,end}` Option subtree, an `issue/some` Option + // blob, and a `retained/{blob,context}` passthrough tree — must keep + // loading, guarding the document's shape against an incompatible + // change to data already on a ref. let dir = repo(); let repo = dir.path(); let blob = |value: &str| git_with_stdin(repo, &["hash-object", "-w", "--stdin"], value); - let expected = comment("Why is this 1?", Some("deadbeef")); + let expected = comment(repo, "Why is this 1?", Some("deadbeef")); let range = expected.anchor.lines.unwrap(); let range_tree = git_with_stdin( repo, @@ -335,13 +587,23 @@ blob(expected.issue.as_deref().unwrap()) ), ); + let retained_tree = git_with_stdin( + repo, + &["mktree"], + &format!( + "100644 blob {}\tblob\n100644 blob {}\tcontext\n", + expected.anchor.blob, + blob("one\ntwo\nthree\nfour\n"), + ), + ); let root = git_with_stdin( repo, &["mktree"], &format!( "100644 blob {}\tbody\n\ 040000 tree {anchor_tree}\tanchor\n\ - 040000 tree {issue_tree}\tissue\n", + 040000 tree {issue_tree}\tissue\n\ + 040000 tree {retained_tree}\tretained\n", blob(&expected.body), ), );

crates/git-ents/src/main.rs @@ -845,7 +845,7 @@ let author = git_comment::provenance(&repo, &id) .map_err(|error| error.to_string())? .map_or_else(|| "?".to_owned(), |provenance| provenance.created.name); - let place = describe_projection(&repo, &comment, rev); + let place = describe_projection(&repo, &id, &comment, rev); let title = comment.body.lines().next().unwrap_or_default(); println!("{} {author} {place} {title}", short_id(&id)); } @@ -883,7 +883,10 @@ location(&comment.anchor.path, comment.anchor.lines), short_id(&comment.anchor.commit.to_string()) ); - println!("on {rev}: {}", describe_projection(&repo, &comment, rev)); + println!( + "on {rev}: {}", + describe_projection(&repo, &id, &comment, rev) + ); if let Some(issue) = &comment.issue { println!("issue {issue}"); } @@ -969,9 +972,9 @@ } } -/// One-line description of where `comment` sits on `rev`. -fn describe_projection(repo: &Path, comment: &Comment, rev: &str) -> String { - match git_comment::project(repo, comment, rev) { +/// One-line description of where `comment` (`id`'s document) sits on `rev`. +fn describe_projection(repo: &Path, id: &str, comment: &Comment, rev: &str) -> String { + match git_comment::project(repo, id, comment, rev) { Ok(Projection::Current) => location(&comment.anchor.path, comment.anchor.lines), Ok(Projection::Relocated { path, lines }) => location(&path, lines), Ok(Projection::Outdated { path }) => format!("{path} [outdated]"),

crates/git-store/src/lib.rs @@ -178,23 +178,7 @@ value: &T, message: &str, ) -> Result<(), Error> { - self.store_impl(refname, value, message, None, &[]) - } - - /// Like [`store`](Self::store), but also parenting the written commit on - /// each of `extra_parents` (beyond the ref's own prior tip), so the - /// commit stays reachability-anchored to other history it depends on — - /// e.g. a comment's commit carries the commit it annotates as a second - /// parent, so the annotated commit can never be garbage-collected out - /// from under it. - pub fn store_with_parents<T: for<'a> Facet<'a>>( - &self, - refname: &str, - value: &T, - message: &str, - extra_parents: &[ObjectId], - ) -> Result<(), Error> { - self.store_impl(refname, value, message, None, extra_parents) + self.store_impl(refname, value, message, None) } /// Like [`store`](Self::store), but attributing authorship to `author` @@ -208,21 +192,7 @@ message: &str, author: (&str, &str), ) -> Result<(), Error> { - self.store_impl(refname, value, message, Some(author), &[]) - } - - /// Like [`store_authored`](Self::store_authored), but also parenting the - /// written commit on each of `extra_parents`, per - /// [`store_with_parents`](Self::store_with_parents). - pub fn store_authored_with_parents<T: for<'a> Facet<'a>>( - &self, - refname: &str, - value: &T, - message: &str, - author: (&str, &str), - extra_parents: &[ObjectId], - ) -> Result<(), Error> { - self.store_impl(refname, value, message, Some(author), extra_parents) + self.store_impl(refname, value, message, Some(author)) } /// ## Requirements @@ -234,16 +204,12 @@ value: &T, message: &str, author: Option<(&str, &str)>, - extra_parents: &[ObjectId], ) -> Result<(), Error> { let mut expected = self.ref_commit(refname)?; let mut tree = facet_git_tree::serialize_into(value, &self.odb)?; for _ in 0..=MAX_MERGE_RETRIES { - let parents = expected - .into_iter() - .chain(extra_parents.iter().copied()) - .collect(); - let commit = self.write_commit(tree, parents, expected, message, author)?; + let parents = expected.into_iter().collect(); + let commit = self.write_commit(tree, parents, message, author)?; match self.try_set_ref(refname, expected, commit) { Ok(()) => return Ok(()), Err(Error::Conflict) => { @@ -285,14 +251,11 @@ ) -> Result<(), Error> { let tree = facet_git_tree::serialize_into(value, &self.odb)?; let expected = self.ref_commit(refname)?; - let (parents, chain_parent) = match &expected { - Some(tip) => { - let tip = self.read_commit(tip)?; - (tip.parents, tip.chain_parent) - } - None => (Vec::new(), None), + let parents = match &expected { + Some(tip) => self.read_commit(tip)?.parents, + None => Vec::new(), }; - let commit = self.write_commit(tree, parents, chain_parent, message, None)?; + let commit = self.write_commit(tree, parents, message, None)?; self.try_set_ref(refname, expected, commit) } @@ -304,7 +267,7 @@ pub fn store_tree(&self, refname: &str, tree: ObjectId, message: &str) -> Result<(), Error> { let expected = self.ref_commit(refname)?; let parents = expected.into_iter().collect(); - let commit = self.write_commit(tree, parents, expected, message, None)?; + let commit = self.write_commit(tree, parents, message, None)?; self.try_set_ref(refname, expected, commit) } @@ -322,7 +285,7 @@ ) -> Result<(), Error> { let expected = self.ref_commit(refname)?; let parents = Vec::new(); - let commit = self.write_commit(tree, parents, expected, message, None)?; + let commit = self.write_commit(tree, parents, message, None)?; self.try_set_ref(refname, expected, commit) } @@ -388,27 +351,6 @@ self.store_authored(&item_ref(prefix, id)?, value, message, author) } - /// Like [`store_item_authored`](Self::store_item_authored), but also - /// parenting the written commit on each of `extra_parents`, per - /// [`store_with_parents`](Self::store_with_parents). - pub fn store_item_authored_with_parents<T: for<'a> Facet<'a>>( - &self, - prefix: &str, - id: &str, - value: &T, - message: &str, - author: (&str, &str), - extra_parents: &[ObjectId], - ) -> Result<(), Error> { - self.store_authored_with_parents( - &item_ref(prefix, id)?, - value, - message, - author, - extra_parents, - ) - } - /// Like [`store_item`](Self::store_item), but for a [`HasId`] value that /// carries its own collection key, so the caller does not pass it twice. pub fn store_keyed<T: for<'a> Facet<'a> + HasId>( @@ -494,7 +436,7 @@ Err(facet_git_tree::Error::Message(_)) => break, Err(error) => return Err(error.into()), } - cursor = commit.chain_parent; + cursor = commit.parents.first().copied(); } Ok(out) } @@ -512,7 +454,7 @@ }; let mut commit = self.read_commit(&tip)?; let updated = commit.author.clone(); - while let Some(parent) = commit.chain_parent { + while let Some(parent) = commit.parents.first().copied() { commit = self.read_commit(&parent)?; } Ok(Some(Provenance { @@ -567,8 +509,8 @@ } } - /// Read `oid`'s tree, parents, chain parent, author, and committer date - /// from the durable store. + /// Read `oid`'s tree, parents, author, and committer date from the + /// durable store. fn read_commit(&self, oid: &ObjectId) -> Result<CommitFacts, Error> { let mut buffer = Vec::new(); let commit = self @@ -582,24 +524,9 @@ let author = commit .author() .map_err(|error| Error::Object(error.to_string()))?; - let chain_parent = commit - .extra_headers() - .find(CHAIN_PARENT_HEADER) - .map(|value| { - if value.is_empty() { - Ok(None) - } else { - ObjectId::from_hex(value) - .map(Some) - .map_err(|error| Error::Object(error.to_string())) - } - }) - .transpose()? - .unwrap_or_else(|| commit.parents().next()); Ok(CommitFacts { tree: commit.tree(), parents: commit.parents().collect(), - chain_parent, seconds: u64::try_from(seconds).unwrap_or(0), author: Authorship { name: author.name.to_string(), @@ -610,18 +537,15 @@ } /// Wrap `tree` in a commit over `parents` and write it to the durable - /// store, recording `chain_parent` (the document's own prior state, as - /// opposed to any other parent riding along for reachability, e.g. an - /// anchored commit) in a header when `parents` holds more than just it — - /// otherwise the chain and the parent list agree and no header is needed. - /// The committer is always the git-ents system identity; `author` - /// overrides the authorship when set, otherwise it too is the system - /// identity. + /// store. Every write through this `Store` produces a single-parent (or + /// parentless) commit, so `parents.first()` is always the document's own + /// prior state — no other parent ever rides along. The committer is + /// always the git-ents system identity; `author` overrides the + /// authorship when set, otherwise it too is the system identity. fn write_commit( &self, tree: ObjectId, parents: Vec<ObjectId>, - chain_parent: Option<ObjectId>, message: &str, author: Option<(&str, &str)>, ) -> Result<ObjectId, Error> { @@ -639,16 +563,6 @@ }, None => committer.clone(), }; - // The chain-parent header is only needed when the plain "first - // parent is the chain" convention would recover the wrong thing — - // i.e. a genesis commit (no chain parent) that still carries an - // extra parent, which would otherwise occupy the first slot. - let extra_headers = if parents.first().copied() == chain_parent { - Vec::new() - } else { - let value = chain_parent.map(|oid| oid.to_string()).unwrap_or_default(); - vec![(CHAIN_PARENT_HEADER.into(), value.into())] - }; let commit = Commit { tree, parents: parents.into(), @@ -656,7 +570,7 @@ committer, encoding: None, message: message.into(), - extra_headers, + extra_headers: Vec::new(), }; self.odb .write(&commit) @@ -697,14 +611,6 @@ /// contention; ordinary racing writers resolve within one or two rounds. const MAX_MERGE_RETRIES: usize = 5; -/// The commit header recording a document's chain parent explicitly, written -/// only when the plain "first parent is the chain" convention would recover -/// the wrong thing: a genesis commit (no prior document state) that still -/// carries an extra parent for reachability (<<anchor.reachability>>), which -/// would otherwise occupy the first — and only — parent slot. An empty value -/// means the chain has no parent at all (this commit is the genesis). -const CHAIN_PARENT_HEADER: &str = "chain-parent"; - /// The ref name for item `id` under the collection namespace `prefix` /// (`{prefix}/{id}`), rejecting an `id` that fails [`ref_segment_ok`] since it /// becomes the ref's last path segment. @@ -736,13 +642,11 @@ pub updated: Authorship, } -/// The facts read off a commit: its tree, its parents, its chain parent (the -/// document's own prior state, distinct from any other parent riding along -/// for reachability), its author, and its committer date. +/// The facts read off a commit: its tree, its parents, its author, and its +/// committer date. struct CommitFacts { tree: ObjectId, parents: Vec<ObjectId>, - chain_parent: Option<ObjectId>, seconds: u64, author: Authorship, } @@ -1242,7 +1146,7 @@ // A write built from the now-stale snapshot loses the CAS race. let tree = facet_git_tree::serialize_into(&"third".to_string(), &store.odb).unwrap(); let commit = store - .write_commit(tree, stale.into_iter().collect(), stale, "write", None) + .write_commit(tree, stale.into_iter().collect(), "write", None) .unwrap(); let result = store.try_set_ref(refname, stale, commit); assert!(matches!(result, Err(Error::Conflict))); @@ -1263,9 +1167,7 @@ // Our write, built assuming the ref was still absent, has no common // ancestor with theirs and so cannot be merged. let tree = facet_git_tree::serialize_into(&"ours".to_string(), &store.odb).unwrap(); - let commit = store - .write_commit(tree, Vec::new(), None, "ours", None) - .unwrap(); + let commit = store.write_commit(tree, Vec::new(), "ours", None).unwrap(); let result = store.try_set_ref(refname, None, commit); assert!(matches!(result, Err(Error::Conflict))); } @@ -1294,7 +1196,7 @@ }; let tree = facet_git_tree::serialize_into(&"pass".to_string(), &store.odb).unwrap(); let commit = store - .write_commit(tree, parents, stale, "advance to pass", None) + .write_commit(tree, parents, "advance to pass", None) .unwrap(); let result = store.try_set_ref(refname, stale, commit); assert!(matches!(result, Err(Error::Conflict)));

crates/git-store/src/merge.rs @@ -59,6 +59,15 @@ } fn classify(shape: &'static Shape) -> Classify { + // A `RawTree` field's tree entry is the wrapped subtree's own object id, + // not a struct encoding of `RawTree` itself (see `facet_git_tree::RawTree`), + // so walking it field-by-field would look for a `hash` entry that was + // never written and fail. Treat it as a leaf instead: identical oids are + // untouched, one side changed is taken as-is, both sides changed conflicts + // — the same fallback every other opaque leaf gets. + if shape.is_type::<facet_git_tree::RawTree>() { + return Classify::Atomic; + } if let Type::User(UserType::Struct(st)) = shape.ty && !matches!(st.kind, StructKind::Unit) {

crates/git-ents-server/src/web/pages.rs @@ -685,7 +685,7 @@ }; let mut out = Vec::new(); for (id, comment) in comments { - let Ok(projection) = git_comment::project(&repo, &comment, "HEAD") else { + let Ok(projection) = git_comment::project(&repo, &id, &comment, "HEAD") else { continue; }; let (landed, lines, outdated) = match projection {