revert: drop the .schema version marker from git-store trees
commit
f04d0d5revert: drop the .schema version marker from git-store trees
Trees stay pure struct representations of their Facet type; the marker polluted every tree and forced inject/strip around store, load, and the structural merge. If explicit version detection is ever needed it will be a Schema-Version commit-message trailer, not a tree entry. Reverts commit bfb7abd9; abstractions.adoc updated to match.
reverts: .schema version marker (bfb7abd9) Assisted-by: Claude:claude-fable-5
Reviews
No reviews of this commit yet — record a verdict below.
Start a review
docs/abstractions.adoc
@@ -35,9 +35,11 @@
Changing a struct is a storage migration, not a refactor — and a
migration is itself a signed push: rewrite the tree under the new
struct, commit on the ref’s old tip. History keeps the old encoding as
-archive. Every tree carries a one-blob `.schema` version marker (absent
-means version 1) so a mismatched binary fails with “schema 2, I speak
-1” instead of misreading silently.
+archive. Trees stay pure struct representations — no version marker
+entry. If explicit version detection is ever needed, it belongs in a
+`Schema-Version:` commit-message trailer: the commit is already the
+storage unit, so versioning the encoding there pollutes neither the
+tree nor the merge path.
*Tip invariant:* the tip of a meta-ref is always readable by the
current binary; history is archival.
crates/git-store/src/lib.rs
@@ -17,15 +17,6 @@
//! 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;
@@ -39,9 +30,6 @@
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.
@@ -85,17 +73,6 @@
/// 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
@@ -176,16 +153,11 @@
}
/// Load the document on `refname`, or `None` when the ref is absent.
- pub fn load<T: for<'a> Facet<'a> + SchemaVersion>(
- &self,
- refname: &str,
- ) -> Result<Option<T>, Error> {
+ pub fn load<T: for<'a> Facet<'a>>(&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)?))
}
@@ -200,7 +172,7 @@
/// ## Requirements
///
/// @relation(storage.concurrency)
- pub fn store<T: for<'a> Facet<'a> + SchemaVersion>(
+ pub fn store<T: for<'a> Facet<'a>>(
&self,
refname: &str,
value: &T,
@@ -213,7 +185,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> + SchemaVersion>(
+ pub fn store_authored<T: for<'a> Facet<'a>>(
&self,
refname: &str,
value: &T,
@@ -226,7 +198,7 @@
/// ## Requirements
///
/// @relation(storage.concurrency)
- fn store_impl<T: for<'a> Facet<'a> + SchemaVersion>(
+ fn store_impl<T: for<'a> Facet<'a>>(
&self,
refname: &str,
value: &T,
@@ -234,14 +206,10 @@
author: Option<(&str, &str)>,
) -> 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().collect();
- let commit = self.write_commit(versioned, parents, message, author)?;
+ let commit = self.write_commit(tree, parents, message, author)?;
match self.try_set_ref(refname, expected, commit) {
Ok(()) => return Ok(()),
Err(Error::Conflict) => {
@@ -251,12 +219,8 @@
return Err(Error::Conflict);
};
let theirs = self.ref_commit(refname)?.ok_or(Error::Conflict)?;
- 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)?;
+ let base_tree = self.read_commit(&base)?.tree;
+ let theirs_tree = self.read_commit(&theirs)?.tree;
tree = merge::three_way_merge::<T>(base_tree, tree, theirs_tree, &self.odb)?;
expected = Some(theirs);
}
@@ -279,14 +243,13 @@
/// ## Requirements
///
/// @relation(storage.concurrency)
- pub fn amend<T: for<'a> Facet<'a> + SchemaVersion>(
+ pub fn amend<T: for<'a> Facet<'a>>(
&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 = match &expected {
Some(tip) => self.read_commit(tip)?.parents,
@@ -462,21 +425,13 @@
/// 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 — 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> {
+ /// or repository-corruption error still propagates.
+ pub fn history<T: for<'a> Facet<'a>>(&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)?;
- let (tree, version) = schema::strip(&self.odb, commit.tree)?;
- schema::check::<T>(version)?;
- match facet_git_tree::deserialize(&tree, &self.odb) {
+ match facet_git_tree::deserialize(&commit.tree, &self.odb) {
Ok(value) => out.push((commit.seconds, value)),
Err(facet_git_tree::Error::Message(_)) => break,
Err(error) => return Err(error.into()),
@@ -1246,63 +1201,4 @@
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,142 +1,0 @@
-//! 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")))
-}