chore: merge .schema version marker work
commit
5c3fe7dchore: merge .schema version marker work
Merges worktree-agent-ad48835e6251d5822.
Assisted-by: Claude:claude-fable-5
Reviews
No reviews of this commit yet — record a verdict below.
Start a review
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")))
+}