git-ents.gitmain
⌘K
foforge
commit bfb7abd
feat: add .schema version marker to git-store trees

Every document tree Store writes now carries a .schema blob entry at its root naming the type’s on-disk version, defaulting to 1 via a blanket-implemented SchemaVersion trait. A missing marker still reads as version 1 (existing data keeps working); a marker naming a version newer than the type supports fails with a clean, typed error instead of a decode failure. The marker is injected/stripped at the tree level around facet_git_tree’s own (de)serialization, and stripped before the structural merge so a concurrent-write retry never sees it as document content.

feat: add SchemaVersion trait with a default VERSION const of 1 feat: add Error::UnsupportedSchema naming both versions test: add roundtrip, missing-marker, and future-version schema tests Assisted-by: Claude:claude-sonnet-5

Joseph D. Carpinelli · 1 month ago

Reviews

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

Start a review

verdict

crates/git-store/src/lib.rs @@ -17,6 +17,15 @@ //! type incompatibly; each type carries a load test against a hand-built //! fixture in the real layout to catch a regression at compile-and-test time //! rather than in production. +//! +//! Every tree-rooted document also carries a one-blob `.schema` entry at its +//! root (see [`SchemaVersion`]), a sibling of the document's own fields +//! naming the shape's on-disk version. A tree missing the entry is version 1 +//! — the pre-marker format, which must keep reading fine — and a tree naming +//! a version newer than the binary's [`SchemaVersion::VERSION`] fails with +//! [`Error::UnsupportedSchema`] rather than however far a mismatched decode +//! happens to get. Migrating is then just a normal write: a new tree +//! committed on the ref's old tip, no separate migration step. use std::cmp::Reverse; use std::collections::BTreeMap; @@ -30,6 +39,9 @@ pub mod component; mod merge; +mod schema; + +pub use schema::SchemaVersion; /// The author and committer identity stamped on every write, fixed so a write /// is self-contained and independent of any ambient git config. @@ -73,6 +85,17 @@ /// rather than the document's own content. #[error("{0}")] Invalid(String), + /// A document's tree names a `.schema` version newer than this binary's + /// [`SchemaVersion::VERSION`] for the type — a future format this binary + /// was never taught to read, reported cleanly rather than surfacing as a + /// decode failure. + #[error("schema {found}, this binary reads {supported}")] + UnsupportedSchema { + /// The version named by the tree's `.schema` marker. + found: u32, + /// The newest version this binary's copy of the type supports. + supported: u32, + }, } /// Whether `segment` is safe as a single ref-path or tree-entry segment: one @@ -153,11 +176,16 @@ } /// Load the document on `refname`, or `None` when the ref is absent. - pub fn load<T: for<'a> Facet<'a>>(&self, refname: &str) -> Result<Option<T>, Error> { + pub fn load<T: for<'a> Facet<'a> + SchemaVersion>( + &self, + refname: &str, + ) -> Result<Option<T>, Error> { let Some(commit) = self.ref_commit(refname)? else { return Ok(None); }; let tree = self.read_commit(&commit)?.tree; + let (tree, version) = schema::strip(&self.odb, tree)?; + schema::check::<T>(version)?; Ok(Some(facet_git_tree::deserialize(&tree, &self.odb)?)) } @@ -172,7 +200,7 @@ /// ## Requirements /// /// @relation(storage.concurrency) - pub fn store<T: for<'a> Facet<'a>>( + pub fn store<T: for<'a> Facet<'a> + SchemaVersion>( &self, refname: &str, value: &T, @@ -187,7 +215,7 @@ /// 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>>( + pub fn store_with_parents<T: for<'a> Facet<'a> + SchemaVersion>( &self, refname: &str, value: &T, @@ -201,7 +229,7 @@ /// (a `(name, email)` pair) while the committer stays the git-ents system /// identity — the way a web edit records the human who made the change while /// the server is the committer. - pub fn store_authored<T: for<'a> Facet<'a>>( + pub fn store_authored<T: for<'a> Facet<'a> + SchemaVersion>( &self, refname: &str, value: &T, @@ -214,7 +242,7 @@ /// 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>>( + pub fn store_authored_with_parents<T: for<'a> Facet<'a> + SchemaVersion>( &self, refname: &str, value: &T, @@ -228,7 +256,7 @@ /// ## Requirements /// /// @relation(storage.concurrency) - fn store_impl<T: for<'a> Facet<'a>>( + fn store_impl<T: for<'a> Facet<'a> + SchemaVersion>( &self, refname: &str, value: &T, @@ -237,13 +265,17 @@ extra_parents: &[ObjectId], ) -> Result<(), Error> { let mut expected = self.ref_commit(refname)?; + // Bare (unmarked) tree throughout: the `.schema` marker is added only + // at the point of writing, so a retry's structural merge never has to + // know about it. let mut tree = facet_git_tree::serialize_into(value, &self.odb)?; for _ in 0..=MAX_MERGE_RETRIES { + let versioned = schema::inject(&self.odb, tree, T::VERSION)?; let parents = expected .into_iter() .chain(extra_parents.iter().copied()) .collect(); - let commit = self.write_commit(tree, parents, expected, message, author)?; + let commit = self.write_commit(versioned, parents, expected, message, author)?; match self.try_set_ref(refname, expected, commit) { Ok(()) => return Ok(()), Err(Error::Conflict) => { @@ -253,8 +285,12 @@ return Err(Error::Conflict); }; let theirs = self.ref_commit(refname)?.ok_or(Error::Conflict)?; - let base_tree = self.read_commit(&base)?.tree; - let theirs_tree = self.read_commit(&theirs)?.tree; + let (base_tree, base_version) = + schema::strip(&self.odb, self.read_commit(&base)?.tree)?; + let (theirs_tree, theirs_version) = + schema::strip(&self.odb, self.read_commit(&theirs)?.tree)?; + schema::check::<T>(base_version)?; + schema::check::<T>(theirs_version)?; tree = merge::three_way_merge::<T>(base_tree, tree, theirs_tree, &self.odb)?; expected = Some(theirs); } @@ -277,13 +313,14 @@ /// ## Requirements /// /// @relation(storage.concurrency) - pub fn amend<T: for<'a> Facet<'a>>( + pub fn amend<T: for<'a> Facet<'a> + SchemaVersion>( &self, refname: &str, value: &T, message: &str, ) -> Result<(), Error> { let tree = facet_git_tree::serialize_into(value, &self.odb)?; + let tree = schema::inject(&self.odb, tree, T::VERSION)?; let expected = self.ref_commit(refname)?; let (parents, chain_parent) = match &expected { Some(tip) => { @@ -483,13 +520,21 @@ /// incompatible format change to `T`: such a commit is unreadable forever, /// not transiently, so returning the readable prefix beats letting one /// stale commit blank out every newer entry that parses fine. A real I/O - /// or repository-corruption error still propagates. - pub fn history<T: for<'a> Facet<'a>>(&self, refname: &str) -> Result<Vec<(u64, T)>, Error> { + /// or repository-corruption error still propagates — including a `.schema` + /// marker newer than `T::VERSION`, which names a real, actionable problem + /// (this binary needs upgrading) rather than the shape drift the + /// stop-on-first-miss rule is built for. + pub fn history<T: for<'a> Facet<'a> + SchemaVersion>( + &self, + refname: &str, + ) -> Result<Vec<(u64, T)>, Error> { let mut out = Vec::new(); let mut cursor = self.ref_commit(refname)?; while let Some(oid) = cursor { let commit = self.read_commit(&oid)?; - match facet_git_tree::deserialize(&commit.tree, &self.odb) { + let (tree, version) = schema::strip(&self.odb, commit.tree)?; + schema::check::<T>(version)?; + match facet_git_tree::deserialize(&tree, &self.odb) { Ok(value) => out.push((commit.seconds, value)), Err(facet_git_tree::Error::Message(_)) => break, Err(error) => return Err(error.into()), @@ -1299,4 +1344,63 @@ let result = store.try_set_ref(refname, stale, commit); assert!(matches!(result, Err(Error::Conflict))); } + + #[test] + fn store_then_load_round_trips_through_the_schema_marker() { + let dir = repo(); + let store = Store::open(dir.path()).unwrap(); + let item = Item { + id: "a".into(), + value: "1".into(), + }; + store.store("refs/meta/doc", &item, "write").unwrap(); + + // The written tree carries a `.schema` marker alongside the + // document's own fields. + let tree = store.ref_tree("refs/meta/doc").unwrap(); + let (_stripped, version) = schema::strip(&store.odb, tree).unwrap(); + assert_eq!(version, 1); + + assert_eq!(store.load::<Item>("refs/meta/doc").unwrap(), Some(item)); + } + + #[test] + fn a_tree_with_no_schema_marker_reads_as_version_1() { + let dir = repo(); + let store = Store::open(dir.path()).unwrap(); + let item = Item { + id: "a".into(), + value: "1".into(), + }; + // Written directly through `facet_git_tree`, bypassing `Store::store` + // and so its `.schema` injection — the on-disk shape of data written + // before the marker existed. + let tree = facet_git_tree::serialize_into(&item, &store.odb).unwrap(); + store.store_tree("refs/meta/legacy", tree, "write").unwrap(); + + assert_eq!(store.load::<Item>("refs/meta/legacy").unwrap(), Some(item)); + } + + #[test] + fn a_schema_version_newer_than_supported_errors_cleanly() { + let dir = repo(); + let store = Store::open(dir.path()).unwrap(); + let item = Item { + id: "a".into(), + value: "1".into(), + }; + let tree = facet_git_tree::serialize_into(&item, &store.odb).unwrap(); + let tree = schema::inject(&store.odb, tree, 2).unwrap(); + store.store_tree("refs/meta/future", tree, "write").unwrap(); + + let error = store.load::<Item>("refs/meta/future").unwrap_err(); + assert!(matches!( + error, + Error::UnsupportedSchema { + found: 2, + supported: 1 + } + )); + assert_eq!(error.to_string(), "schema 2, this binary reads 1"); + } }
crates/git-store/src/schema.rs @@ -1,0 +1,142 @@ +//! The `.schema` version marker every document tree carries at its root. +//! +//! Per `docs/abstractions.adoc`'s "Typed tree" section, a `Facet` shape *is* +//! the storage format, so an incompatible change is normally invisible until +//! it silently fails to decode. The `.schema` marker turns that into a clean, +//! typed error: a tree written by a newer binary names a version this one +//! doesn't support, instead of tripping over fields it doesn't recognize. +//! +//! Only a tree-rooted document (struct, map, list, option, enum) has a +//! sibling slot to hold the marker; a scalar-rooted document (a bare +//! `String`, say) is left untouched. Every real meta-ref document is one of +//! the tree-shaped kinds — a bare scalar only ever shows up in `git-store`'s +//! own plumbing tests — so this is not a gap in practice. + +use facet::Facet; +use gix::ObjectId; +use gix::objs::tree::{Entry as TreeEntry, EntryKind, EntryMode}; +use gix::objs::{Find as _, FindExt as _, Kind, ObjectRef, Write as _}; + +use crate::Error; + +/// The tree-root entry name a document's `.schema` marker is stored under. +/// The leading `.` keeps it out of the way of a struct's field names (never +/// valid Rust identifiers) and of a scalar-keyed map's own keys, which +/// [`crate::ref_segment_ok`] bars from starting with `.` when written +/// through [`crate::Store::store_map`]. +const ENTRY_NAME: &str = ".schema"; + +/// A stored document's on-disk schema version, defaulting to `1` for every +/// `Facet` type. A document overrides [`SchemaVersion::VERSION`] only once +/// its shape changes incompatibly and old readers must be told they can't +/// parse the new tree; until that happens there is nothing to add. Migrating +/// is then just a normal write — a new tree committed on the ref's old tip — +/// not a bespoke migration engine. +pub trait SchemaVersion { + /// This type's current on-disk schema version. + const VERSION: u32 = 1; +} + +impl<T: for<'a> Facet<'a>> SchemaVersion for T {} + +/// Add a `.schema` marker for `version` as a sibling of `tree`'s existing +/// entries, replacing one already there. Returns `tree` unchanged when it +/// names a blob rather than a tree (see the module docs). +pub(crate) fn inject( + odb: &gix::odb::Handle, + tree: ObjectId, + version: u32, +) -> Result<ObjectId, Error> { + let Some(mut entries) = entries_of(odb, &tree)? else { + return Ok(tree); + }; + entries.retain(|entry| entry.filename != ENTRY_NAME); + let marker = odb + .write_buf(Kind::Blob, version.to_string().as_bytes()) + .map_err(|error| Error::Object(error.to_string()))?; + entries.push(TreeEntry { + mode: EntryMode::from(EntryKind::Blob), + filename: ENTRY_NAME.into(), + oid: marker, + }); + entries.sort(); + odb.write(&gix::objs::Tree { entries }) + .map_err(|error| Error::Object(error.to_string())) +} + +/// Remove `tree`'s `.schema` marker, if any, returning the stripped tree and +/// the version it named — `1` when the marker is absent, the pre-marker +/// on-disk format that must keep reading fine. Returns `tree` unchanged (and +/// version `1`) when it names a blob rather than a tree. +pub(crate) fn strip(odb: &gix::odb::Handle, tree: ObjectId) -> Result<(ObjectId, u32), Error> { + let Some(mut entries) = entries_of(odb, &tree)? else { + return Ok((tree, 1)); + }; + let Some(index) = entries + .iter() + .position(|entry| entry.filename == ENTRY_NAME) + else { + return Ok((tree, 1)); + }; + let marker = entries.remove(index); + let version = read_version(odb, &marker.oid)?; + entries.sort(); + let stripped = odb + .write(&gix::objs::Tree { entries }) + .map_err(|error| Error::Object(error.to_string()))?; + Ok((stripped, version)) +} + +/// Fail with a typed, named-versions error when `found` is newer than this +/// binary's `T::VERSION` — a future schema must never be mistaken for a +/// decode failure. +pub(crate) fn check<T: SchemaVersion>(found: u32) -> Result<(), Error> { + if found > T::VERSION { + return Err(Error::UnsupportedSchema { + found, + supported: T::VERSION, + }); + } + Ok(()) +} + +/// `tree`'s entries, or `None` when `tree` names a blob rather than a tree. +fn entries_of(odb: &gix::odb::Handle, tree: &ObjectId) -> Result<Option<Vec<TreeEntry>>, Error> { + let mut buffer = Vec::new(); + let data = odb + .try_find(tree, &mut buffer) + .map_err(|error| Error::Object(error.to_string()))? + .ok_or_else(|| Error::Object(format!("{tree} not found")))?; + if data.kind != Kind::Tree { + return Ok(None); + } + let object = data + .decode() + .map_err(|error| Error::Object(error.to_string()))?; + let ObjectRef::Tree(tree_ref) = object else { + return Ok(None); + }; + Ok(Some( + tree_ref + .entries + .into_iter() + .map(|entry| TreeEntry { + mode: entry.mode, + filename: entry.filename.to_owned(), + oid: entry.oid.to_owned(), + }) + .collect(), + )) +} + +/// The integer named by the blob at `oid`, a `.schema` marker's value. +fn read_version(odb: &gix::odb::Handle, oid: &ObjectId) -> Result<u32, Error> { + let mut buffer = Vec::new(); + let blob = odb + .find_blob(oid, &mut buffer) + .map_err(|error| Error::Object(error.to_string()))?; + let text = std::str::from_utf8(blob.data) + .map_err(|_utf8| Error::Object("`.schema` marker is not valid UTF-8".into()))?; + text.parse() + .map_err(|_parse| Error::Object(format!("`.schema` marker {text:?} is not an integer"))) +}