From 192e0a3f190102393d4a3f3f35ed09ba92a942ca Mon Sep 17 00:00:00 2001 From: Malcolm Greaves Date: Tue, 31 Mar 2026 13:02:56 -0700 Subject: [PATCH 01/11] initial new TMerkleTreeNode trait --- crates/lib/src/model/merkle_tree/node_type.rs | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/crates/lib/src/model/merkle_tree/node_type.rs b/crates/lib/src/model/merkle_tree/node_type.rs index a2269b0af..d4275a4d7 100644 --- a/crates/lib/src/model/merkle_tree/node_type.rs +++ b/crates/lib/src/model/merkle_tree/node_type.rs @@ -58,4 +58,23 @@ pub trait MerkleTreeNodeIdType { fn hash(&self) -> MerkleHash; } -pub trait TMerkleTreeNode: MerkleTreeNodeIdType + Serialize + Debug + Display {} +/// Trait for things that are Merkle tree nodes. +/// +/// Critical functionality is to be able to serialize & deserialize nodes. The Debug & Display +/// constraints are for convenience. +/// +/// Since this trait is used as a parameter for generic functions, it must be object-safe. This means +/// that it cannot extend `Serialize` since that has a `` parameter on the +/// `serialize` method, which causes the type to not be implementable as a vtable lookup. +/// +/// This is why the trait contains manual serialization (`to_msgpack_bytes`) and deserialization +/// (`from_msgpack_bytes`) methods. For types that do implement `Serialize` and `Deserialize`, +/// there's a blanket implementation that delegates the appropriate `serialize` & `deserialze` +/// calls to these trait methods. +pub trait TMerkleTreeNode: MerkleTreeNodeIdType + Debug + Display { + /// Serialize this node to a MsgPack byte array. + fn to_msgpack_bytes(&self) -> Vec; + + /// Deserialize a node from a MsgPack byte array. + fn from_msgpack_bytes(bytes: &[u8]) -> Self; +} From af8353dec4057e3cb0940c1111c91c09e4432e20 Mon Sep 17 00:00:00 2001 From: Malcolm Greaves Date: Tue, 31 Mar 2026 16:42:14 -0700 Subject: [PATCH 02/11] implemented trait final + blanket impl - modified trait to use associated type for error - to_msgpack_bytes returns result w/ that error or vec - blanket impl uses something that also has serialize as to_msgpack_bytes implmentation - made `from_u8` return error & added it to the OxenError heirarchy WIP propigate changes --- .../src/core/db/merkle_node/merkle_node_db.rs | 11 +- crates/lib/src/error.rs | 9 ++ .../src/model/merkle_tree/node/commit_node.rs | 2 - .../src/model/merkle_tree/node/dir_node.rs | 2 - .../model/merkle_tree/node/file_chunk_node.rs | 2 - .../src/model/merkle_tree/node/file_node.rs | 2 - .../lib/src/model/merkle_tree/node/vnode.rs | 2 - crates/lib/src/model/merkle_tree/node_type.rs | 129 +++++++++++++++--- 8 files changed, 127 insertions(+), 32 deletions(-) diff --git a/crates/lib/src/core/db/merkle_node/merkle_node_db.rs b/crates/lib/src/core/db/merkle_node/merkle_node_db.rs index 1af44a3d1..65e057541 100644 --- a/crates/lib/src/core/db/merkle_node/merkle_node_db.rs +++ b/crates/lib/src/core/db/merkle_node/merkle_node_db.rs @@ -226,7 +226,10 @@ impl MerkleNodeDB { Self::to_node(self.dtype, &self.data()) } - pub fn to_node(dtype: MerkleTreeNodeType, data: &[u8]) -> Result { + pub fn to_node( + dtype: MerkleTreeNodeType, + data: &[u8], + ) -> Result { match dtype { MerkleTreeNodeType::Commit => { Ok(EMerkleTreeNode::Commit(CommitNode::deserialize(data)?)) @@ -324,7 +327,7 @@ impl MerkleNodeDB { let dtype = lookup .as_ref() - .map(|l| MerkleTreeNodeType::from_u8(l.data_type)) + .map(|l| MerkleTreeNodeType::from_u8_unwrap(l.data_type)) .unwrap_or(MerkleTreeNodeType::Commit); let parent_id = lookup.as_ref().map(|l| l.parent_id); Ok(Self { @@ -498,7 +501,7 @@ impl MerkleNodeDB { }; // Parse the node parent id - let data_type = MerkleTreeNodeType::from_u8(lookup.data_type); + let data_type = MerkleTreeNodeType::from_u8_unwrap(lookup.data_type); let parent_id = MerkleTreeNode::deserialize_id(&lookup.data, data_type)?; let mut file_data = Vec::new(); @@ -517,7 +520,7 @@ impl MerkleNodeDB { cursor.seek(SeekFrom::Start(*offset))?; let mut data = vec![0; *len as usize]; cursor.read_exact(&mut data)?; - let dtype = MerkleTreeNodeType::from_u8(*dtype); + let dtype = MerkleTreeNodeType::from_u8_unwrap(*dtype); let node = MerkleTreeNode { parent_id: Some(parent_id), hash: MerkleHash::new(*hash), diff --git a/crates/lib/src/error.rs b/crates/lib/src/error.rs index ca3de92a3..a0ea5ef23 100644 --- a/crates/lib/src/error.rs +++ b/crates/lib/src/error.rs @@ -14,6 +14,7 @@ use crate::model::ParsedResource; use crate::model::RepoNew; use crate::model::Schema; use crate::model::Workspace; +use crate::model::merkle_tree::node_type::InvalidMerkleTreeNodeType; pub mod path_buf_error; pub mod string_error; @@ -143,6 +144,14 @@ pub enum OxenError { #[error("{0}")] CommitEntryNotFound(StringError), + // + // Merkle Tree Operations + // + /// A failure during serialization or deserialization of a merkle tree node: it has an unknown + /// u8 marker for its node type. + #[error("{0}")] + MerkleTreeError(#[from] InvalidMerkleTreeNodeType), + // // Schema (dataframes) // diff --git a/crates/lib/src/model/merkle_tree/node/commit_node.rs b/crates/lib/src/model/merkle_tree/node/commit_node.rs index 8b0918e27..f63274640 100644 --- a/crates/lib/src/model/merkle_tree/node/commit_node.rs +++ b/crates/lib/src/model/merkle_tree/node/commit_node.rs @@ -204,8 +204,6 @@ impl MerkleTreeNodeIdType for CommitNode { } } -impl TMerkleTreeNode for CommitNode {} - /// Debug is used for verbose multi-line output with println!("{:?}", node) impl fmt::Debug for CommitNode { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { diff --git a/crates/lib/src/model/merkle_tree/node/dir_node.rs b/crates/lib/src/model/merkle_tree/node/dir_node.rs index 4f95b87f1..079a7deed 100644 --- a/crates/lib/src/model/merkle_tree/node/dir_node.rs +++ b/crates/lib/src/model/merkle_tree/node/dir_node.rs @@ -262,8 +262,6 @@ impl MerkleTreeNodeIdType for DirNode { } } -impl TMerkleTreeNode for DirNode {} - /// Debug is used for verbose multi-line output with println!("{:?}", node) impl fmt::Debug for DirNode { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { diff --git a/crates/lib/src/model/merkle_tree/node/file_chunk_node.rs b/crates/lib/src/model/merkle_tree/node/file_chunk_node.rs index 6fd0c6c6c..da1e194cc 100644 --- a/crates/lib/src/model/merkle_tree/node/file_chunk_node.rs +++ b/crates/lib/src/model/merkle_tree/node/file_chunk_node.rs @@ -43,8 +43,6 @@ impl MerkleTreeNodeIdType for FileChunkNode { } } -impl TMerkleTreeNode for FileChunkNode {} - /// Debug is used for verbose multi-line output with println!("{:?}", node) impl fmt::Debug for FileChunkNode { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { diff --git a/crates/lib/src/model/merkle_tree/node/file_node.rs b/crates/lib/src/model/merkle_tree/node/file_node.rs index 1f484f920..6041433b0 100644 --- a/crates/lib/src/model/merkle_tree/node/file_node.rs +++ b/crates/lib/src/model/merkle_tree/node/file_node.rs @@ -263,8 +263,6 @@ impl Hash for FileNode { } } -impl TMerkleTreeNode for FileNode {} - /// Debug is used for verbose multi-line output with println!("{:?}", node) impl fmt::Debug for FileNode { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { diff --git a/crates/lib/src/model/merkle_tree/node/vnode.rs b/crates/lib/src/model/merkle_tree/node/vnode.rs index ca2f12193..897be1866 100644 --- a/crates/lib/src/model/merkle_tree/node/vnode.rs +++ b/crates/lib/src/model/merkle_tree/node/vnode.rs @@ -153,5 +153,3 @@ impl fmt::Display for VNode { write!(f, "") } } - -impl TMerkleTreeNode for VNode {} diff --git a/crates/lib/src/model/merkle_tree/node_type.rs b/crates/lib/src/model/merkle_tree/node_type.rs index d4275a4d7..7e5b78f1e 100644 --- a/crates/lib/src/model/merkle_tree/node_type.rs +++ b/crates/lib/src/model/merkle_tree/node_type.rs @@ -14,13 +14,28 @@ use std::{ fmt::{Debug, Display}, hash::{Hash, Hasher}, }; +use thiserror::Error; +/// The type of node that we are storing in the merkle tree. #[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize, Copy)] pub enum MerkleTreeNodeType { + /// A commit in the repository. From this commit node, we can traverse the tree to recover the + /// entire repository state at the commit. Commit, + + /// A file in the repository. File, + + /// A directory in the repository. Dir, + + /// Directories can be very large, so we split their contents into (potentially) multiple + /// virtual directory nodes: a `VNode`. A `VNode` is like a directory: it contains some + /// number of files and directories. It is always a subset of a real directory. If a + /// directory has multiple `VNode` children, these children form a strict paritioning. VNode, + + /// A chunk of a file in the repository. (TODO: unused at this point, but planned) FileChunk, } @@ -30,7 +45,39 @@ impl Hash for MerkleTreeNodeType { } } +#[derive(Debug, Error)] +#[error("Deserialization failure: Invalid MerkleTreeNodeType: {0}")] +/// Failed to deserialize a `MerkleTreeNodeType` due to unknown `u8` value. +pub struct InvalidMerkleTreeNodeType(u8); + impl MerkleTreeNodeType { + /* + ********************************************************************************************** + + NOTE: The mapping of u8 -> node type is important! DO NOT MODIFY THIS!!!! + + It is stored on disk / in a database as a `u8` and this value is used to deserialize nodes into + their correct type. Changing this will **BREAK REPOSITORIES**. + + IF it is absolutely necessary to change it, then a migration **MUST** be implemented and + operations **MUST** be gated behind a breaking version update to force this migration on + outdated repositories. + + That being said, it's difficult to imagine any valid reason why this mapping would need to be + changed for existing `MerkleTreeNode` variants. + + **NEW** variants can be added without a migration. New variants must have an incremented `u8` + value to not conflcit with any existing variants. Older repositories will still be able to be + read by newer variants: they will simply not contain nodes of the new variant. + + If there is some **OTHER** backwards-incompatible change with how the repository data is stored + that requires a new variant, then a migration **MUST** be implemented and operations **MUST** + be gated behind a breaking version update to force this migration on outdated repositories. + + ********************************************************************************************** + */ + + /// Serialize the node type into a stable `u8` value. pub fn to_u8(&self) -> u8 { match self { MerkleTreeNodeType::Commit => 0u8, @@ -41,40 +88,86 @@ impl MerkleTreeNodeType { } } - pub fn from_u8(val: u8) -> MerkleTreeNodeType { + /// Deserialize a `u8` value into a `MerkleTreeNodeType`. + /// Panics if the `u8` value is not a valid `MerkleTreeNodeType`. + pub fn from_u8_unwrap(val: u8) -> MerkleTreeNodeType { + from_u8(val).expect("Invalid MerkleTreeNodeType: {val}") + } + + /// Deserialize a `u8` value into a `MerkleTreeNodeType`. + pub fn from_u8(val: u8) -> Result { match val { - 0u8 => MerkleTreeNodeType::Commit, - 1u8 => MerkleTreeNodeType::Dir, - 2u8 => MerkleTreeNodeType::VNode, - 3u8 => MerkleTreeNodeType::File, - 4u8 => MerkleTreeNodeType::FileChunk, - _ => panic!("Invalid MerkleTreeNodeType: {val}"), + 0u8 => Ok(MerkleTreeNodeType::Commit), + 1u8 => Ok(MerkleTreeNodeType::Dir), + 2u8 => Ok(MerkleTreeNodeType::VNode), + 3u8 => Ok(MerkleTreeNodeType::File), + 4u8 => Ok(MerkleTreeNodeType::FileChunk), + _ => Err(InvalidMerkleTreeNodeType(val)), } } } +/// Allows for types to identify themselves as a specific kind of merkle tree node via the +/// `MerkleTreeNodeType` enum. This is ciritical for defining the node's stable on-disk +/// byte representation. pub trait MerkleTreeNodeIdType { + /// The type of merkle tree node this node can serialize & deserialize into. fn node_type(&self) -> MerkleTreeNodeType; + + /// The stable merkle hash of this node. fn hash(&self) -> MerkleHash; } /// Trait for things that are Merkle tree nodes. /// -/// Critical functionality is to be able to serialize & deserialize nodes. The Debug & Display -/// constraints are for convenience. +/// Critical functionality is to be able to serialize node, compute a stable merkle hash, and +/// identify the node as a `MerkleTreeNodeType`. The Debug & Display constraints are for +/// convenience. /// -/// Since this trait is used as a parameter for generic functions, it must be object-safe. This means -/// that it cannot extend `Serialize` since that has a `` parameter on the +/// Since this trait is used as a parameter for generic functions, it must be object-safe. This +/// means that it cannot extend `Serialize` since that has a `` parameter on the /// `serialize` method, which causes the type to not be implementable as a vtable lookup. /// -/// This is why the trait contains manual serialization (`to_msgpack_bytes`) and deserialization -/// (`from_msgpack_bytes`) methods. For types that do implement `Serialize` and `Deserialize`, -/// there's a blanket implementation that delegates the appropriate `serialize` & `deserialze` -/// calls to these trait methods. +/// This is why the trait contains a manual serialization (`to_msgpack_bytes`) method. For types +/// that do implement `Serialize`, there's a blanket implementation that delegates the `serialize` +/// call to the `to_msgpack_bytes` method. pub trait TMerkleTreeNode: MerkleTreeNodeIdType + Debug + Display { + /// The error type that can occur during serialization. + type SerializationError: std::error::Error + Send + Sync + 'static; + /// Serialize this node to a MsgPack byte array. - fn to_msgpack_bytes(&self) -> Vec; + fn to_msgpack_bytes(&self) -> Result, Self::SerializationError>; +} + +/// Blanket implementation for Merkle tree nodes that implement serde's `Serialize` trait. +impl TMerkleTreeNode for T { + /// Serialization errors come from serde. + type SerializationError = rmp_serde::encode::Error; - /// Deserialize a node from a MsgPack byte array. - fn from_msgpack_bytes(bytes: &[u8]) -> Self; + /// Uses serde to serialize this node. + fn to_msgpack_bytes(&self) -> Result, Self::SerializationError> { + let mut buf = Vec::new(); + let x = self.serialize(&mut rmp_serde::Serializer::new(&mut buf)); + x.map(|_| buf) + } +} + +#[cfg(test)] +mod tests { + use crate::model::merkle_tree::node::{CommitNode, DirNode, FileChunkNode, FileNode, VNode}; + + use super::*; + + #[test] + fn test_nodes_implement_trait() { + /// this only exists so we can check that all node types implement `TMerkleTreeNode` + /// it will be a compile-time error if a node type does not implement the trait + fn is_tmerkletreenode(x: T) {} + + is_tmerkletreenode(CommitNode::default()); + is_tmerkletreenode(DirNode::default()); + is_tmerkletreenode(VNode::default()); + is_tmerkletreenode(FileNode::default()); + is_tmerkletreenode(FileChunkNode::default()); + } } From 738c8c23fe126804a58373cb6e2f272b6a8d6666 Mon Sep 17 00:00:00 2001 From: Malcolm Greaves Date: Tue, 31 Mar 2026 16:54:48 -0700 Subject: [PATCH 03/11] fixing --- .../src/core/db/merkle_node/merkle_node_db.rs | 43 ++++++++++++------- crates/lib/src/error.rs | 4 ++ .../src/model/merkle_tree/node/commit_node.rs | 2 +- .../src/model/merkle_tree/node/dir_node.rs | 2 +- .../model/merkle_tree/node/file_chunk_node.rs | 3 +- .../src/model/merkle_tree/node/file_node.rs | 2 +- .../merkle_tree/node/merkle_tree_node.rs | 5 ++- .../lib/src/model/merkle_tree/node/vnode.rs | 2 +- crates/lib/src/model/merkle_tree/node_type.rs | 15 +++++-- crates/lib/src/repositories/tree.rs | 9 ++-- 10 files changed, 58 insertions(+), 29 deletions(-) diff --git a/crates/lib/src/core/db/merkle_node/merkle_node_db.rs b/crates/lib/src/core/db/merkle_node/merkle_node_db.rs index 65e057541..ef97132a4 100644 --- a/crates/lib/src/core/db/merkle_node/merkle_node_db.rs +++ b/crates/lib/src/core/db/merkle_node/merkle_node_db.rs @@ -61,6 +61,7 @@ use crate::constants; use crate::error::OxenError; use crate::model::LocalRepository; use crate::model::MerkleHash; +use crate::model::merkle_tree::node_type::InvalidMerkleTreeNodeType; use crate::util; use crate::model::merkle_tree::node::{ @@ -223,13 +224,14 @@ impl MerkleNodeDB { } pub fn node(&self) -> Result { - Self::to_node(self.dtype, &self.data()) + let node = Self::to_node(self.dtype, &self.data())?; + Ok(node) } pub fn to_node( dtype: MerkleTreeNodeType, data: &[u8], - ) -> Result { + ) -> Result { match dtype { MerkleTreeNodeType::Commit => { Ok(EMerkleTreeNode::Commit(CommitNode::deserialize(data)?)) @@ -257,11 +259,14 @@ impl MerkleNodeDB { Self::open(path, true) } - pub fn open_read_write_if_not_exists( + pub fn open_read_write_if_not_exists( repo: &LocalRepository, - node: &impl TMerkleTreeNode, + node: &N, parent_id: Option, - ) -> Result, OxenError> { + ) -> Result, OxenError> + where + OxenError: From, + { if Self::exists(repo, &node.hash()) { let db_path = node_db_path(repo, &node.hash()); log::debug!( @@ -274,11 +279,14 @@ impl MerkleNodeDB { } } - pub fn open_read_write( + pub fn open_read_write( repo: &LocalRepository, - node: &impl TMerkleTreeNode, + node: &N, parent_id: Option, - ) -> Result { + ) -> Result + where + OxenError: From, + { let path = node_db_path(repo, &node.hash()); if !path.exists() { util::fs::create_dir_all(&path)?; @@ -367,11 +375,14 @@ impl MerkleNodeDB { } /// Write the base node info. - fn write_node( + fn write_node( &mut self, node: &N, parent_id: Option, - ) -> Result<(), OxenError> { + ) -> Result<(), OxenError> + where + OxenError: From, + { if self.read_only { return Err(OxenError::basic_str("Cannot write to read-only db")); } @@ -396,8 +407,7 @@ impl MerkleNodeDB { } // Write data length - let mut buf = Vec::new(); - node.serialize(&mut Serializer::new(&mut buf)).unwrap(); + let buf = node.to_msgpack_bytes()?; let data_len = buf.len() as u32; node_file.write_all(&data_len.to_le_bytes())?; // log::debug!("write_node Wrote data length {}", data_len); @@ -416,7 +426,10 @@ impl MerkleNodeDB { Ok(()) } - pub fn add_child(&mut self, item: &N) -> Result<(), OxenError> { + pub fn add_child(&mut self, item: &N) -> Result<(), OxenError> + where + OxenError: From, + { if self.read_only { return Err(OxenError::basic_str("Cannot write to read-only db")); } @@ -428,9 +441,7 @@ impl MerkleNodeDB { return Err(OxenError::basic_str("Must call open() before writing")); }; - // TODO: Abstract and re-use in write_all - let mut buf = Vec::new(); - item.serialize(&mut Serializer::new(&mut buf)).unwrap(); + let buf = item.to_msgpack_bytes()?; let data_len = buf.len() as u64; // log::debug!("--add_child-- node_file {:?}", node_file); // log::debug!("--add_child-- dtype {:?}", item.dtype()); diff --git a/crates/lib/src/error.rs b/crates/lib/src/error.rs index a0ea5ef23..79133b41a 100644 --- a/crates/lib/src/error.rs +++ b/crates/lib/src/error.rs @@ -312,6 +312,10 @@ pub enum OxenError { #[error("Invalid integer: {0}")] ParseIntError(#[from] ParseIntError), + /// Wraps any error that we get from encoding message pack data. + #[error("Encode error: {0}")] + RmpEncodeError(#[from] rmp_serde::encode::Error), + /// Wraps any error that we get from decoding message pack data. #[error("Decode error: {0}")] RmpDecodeError(#[from] rmp_serde::decode::Error), diff --git a/crates/lib/src/model/merkle_tree/node/commit_node.rs b/crates/lib/src/model/merkle_tree/node/commit_node.rs index f63274640..2c4dddbbf 100644 --- a/crates/lib/src/model/merkle_tree/node/commit_node.rs +++ b/crates/lib/src/model/merkle_tree/node/commit_node.rs @@ -124,7 +124,7 @@ impl CommitNode { } } - pub fn deserialize(data: &[u8]) -> Result { + pub fn deserialize(data: &[u8]) -> Result { // In order to support versions that didn't have the enum, // if it fails we will fall back to the old struct, then populate the enum let commit: CommitNode = match rmp_serde::from_slice(data) { diff --git a/crates/lib/src/model/merkle_tree/node/dir_node.rs b/crates/lib/src/model/merkle_tree/node/dir_node.rs index 079a7deed..62bf0d3be 100644 --- a/crates/lib/src/model/merkle_tree/node/dir_node.rs +++ b/crates/lib/src/model/merkle_tree/node/dir_node.rs @@ -120,7 +120,7 @@ impl DirNode { } } - pub fn deserialize(data: &[u8]) -> Result { + pub fn deserialize(data: &[u8]) -> Result { // In order to support versions that didn't have the enum, // if it fails we will fall back to the old struct, then populate the enum let dir_node: DirNode = match rmp_serde::from_slice(data) { diff --git a/crates/lib/src/model/merkle_tree/node/file_chunk_node.rs b/crates/lib/src/model/merkle_tree/node/file_chunk_node.rs index da1e194cc..c7e6d9568 100644 --- a/crates/lib/src/model/merkle_tree/node/file_chunk_node.rs +++ b/crates/lib/src/model/merkle_tree/node/file_chunk_node.rs @@ -17,9 +17,8 @@ pub struct FileChunkNode { } impl FileChunkNode { - pub fn deserialize(data: &[u8]) -> Result { + pub fn deserialize(data: &[u8]) -> Result { rmp_serde::from_slice(data) - .map_err(|e| OxenError::basic_str(format!("Error deserializing file chunk node: {e}"))) } } diff --git a/crates/lib/src/model/merkle_tree/node/file_node.rs b/crates/lib/src/model/merkle_tree/node/file_node.rs index 6041433b0..f30b1750b 100644 --- a/crates/lib/src/model/merkle_tree/node/file_node.rs +++ b/crates/lib/src/model/merkle_tree/node/file_node.rs @@ -95,7 +95,7 @@ impl FileNode { } } - pub fn deserialize(data: &[u8]) -> Result { + pub fn deserialize(data: &[u8]) -> Result { let file_node: FileNode = match rmp_serde::from_slice(data) { Ok(file_node) => file_node, Err(_) => { diff --git a/crates/lib/src/model/merkle_tree/node/merkle_tree_node.rs b/crates/lib/src/model/merkle_tree/node/merkle_tree_node.rs index d036064a4..57f65a70f 100644 --- a/crates/lib/src/model/merkle_tree/node/merkle_tree_node.rs +++ b/crates/lib/src/model/merkle_tree/node/merkle_tree_node.rs @@ -684,7 +684,10 @@ impl MerkleTreeNode { self.node.clone() } - pub fn deserialize_id(data: &[u8], dtype: MerkleTreeNodeType) -> Result { + pub fn deserialize_id( + data: &[u8], + dtype: MerkleTreeNodeType, + ) -> Result { match dtype { MerkleTreeNodeType::Commit => { CommitNode::deserialize(data).map(|commit| *commit.hash()) diff --git a/crates/lib/src/model/merkle_tree/node/vnode.rs b/crates/lib/src/model/merkle_tree/node/vnode.rs index 897be1866..0f4172b2d 100644 --- a/crates/lib/src/model/merkle_tree/node/vnode.rs +++ b/crates/lib/src/model/merkle_tree/node/vnode.rs @@ -57,7 +57,7 @@ impl VNode { } } - pub fn deserialize(data: &[u8]) -> Result { + pub fn deserialize(data: &[u8]) -> Result { // In order to support versions that didn't have the enum, // if it fails we will fall back to the old struct, then populate the enum let vnode: VNode = match rmp_serde::from_slice(data) { diff --git a/crates/lib/src/model/merkle_tree/node_type.rs b/crates/lib/src/model/merkle_tree/node_type.rs index 7e5b78f1e..e626e7487 100644 --- a/crates/lib/src/model/merkle_tree/node_type.rs +++ b/crates/lib/src/model/merkle_tree/node_type.rs @@ -78,6 +78,7 @@ impl MerkleTreeNodeType { */ /// Serialize the node type into a stable `u8` value. + /// This function is 1:1 with `from_u8`. pub fn to_u8(&self) -> u8 { match self { MerkleTreeNodeType::Commit => 0u8, @@ -91,10 +92,11 @@ impl MerkleTreeNodeType { /// Deserialize a `u8` value into a `MerkleTreeNodeType`. /// Panics if the `u8` value is not a valid `MerkleTreeNodeType`. pub fn from_u8_unwrap(val: u8) -> MerkleTreeNodeType { - from_u8(val).expect("Invalid MerkleTreeNodeType: {val}") + Self::from_u8(val).expect("Invalid MerkleTreeNodeType: {val}") } /// Deserialize a `u8` value into a `MerkleTreeNodeType`. + /// This function is 1:1 with `to_u8`: all outputs from `to_u8` result in an `Ok`. pub fn from_u8(val: u8) -> Result { match val { 0u8 => Ok(MerkleTreeNodeType::Commit), @@ -154,7 +156,10 @@ impl TMerkleTreeNode for #[cfg(test)] mod tests { - use crate::model::merkle_tree::node::{CommitNode, DirNode, FileChunkNode, FileNode, VNode}; + use crate::{ + error::OxenError, + model::merkle_tree::node::{CommitNode, DirNode, FileChunkNode, FileNode, VNode}, + }; use super::*; @@ -162,7 +167,11 @@ mod tests { fn test_nodes_implement_trait() { /// this only exists so we can check that all node types implement `TMerkleTreeNode` /// it will be a compile-time error if a node type does not implement the trait - fn is_tmerkletreenode(x: T) {} + fn is_tmerkletreenode(_: T) + where + OxenError: From, + { + } is_tmerkletreenode(CommitNode::default()); is_tmerkletreenode(DirNode::default()); diff --git a/crates/lib/src/repositories/tree.rs b/crates/lib/src/repositories/tree.rs index 6a1e4480a..e0f9c14d4 100644 --- a/crates/lib/src/repositories/tree.rs +++ b/crates/lib/src/repositories/tree.rs @@ -1077,11 +1077,14 @@ pub fn write_tree(repo: &LocalRepository, node: &MerkleTreeNode) -> Result<(), O Ok(()) } -fn p_write_tree( +fn p_write_tree( repo: &LocalRepository, node: &MerkleTreeNode, - node_impl: &impl TMerkleTreeNode, -) -> Result<(), OxenError> { + node_impl: &N, +) -> Result<(), OxenError> +where + OxenError: From, +{ let parent_id = node.parent_id; let mut db = MerkleNodeDB::open_read_write(repo, node_impl, parent_id)?; From e94839b7856b30e3c2016517089816d74a758755 Mon Sep 17 00:00:00 2001 From: Malcolm Greaves Date: Tue, 31 Mar 2026 17:21:41 -0700 Subject: [PATCH 04/11] fix by constraining SerializationError to match the types on all of the impls --- crates/lib/src/repositories/tree.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/lib/src/repositories/tree.rs b/crates/lib/src/repositories/tree.rs index e0f9c14d4..792162242 100644 --- a/crates/lib/src/repositories/tree.rs +++ b/crates/lib/src/repositories/tree.rs @@ -1077,13 +1077,13 @@ pub fn write_tree(repo: &LocalRepository, node: &MerkleTreeNode) -> Result<(), O Ok(()) } -fn p_write_tree( +fn p_write_tree( repo: &LocalRepository, node: &MerkleTreeNode, node_impl: &N, ) -> Result<(), OxenError> where - OxenError: From, + N: TMerkleTreeNode, { let parent_id = node.parent_id; From 4656c795cf6188ac4110eb5abb0650da759380de Mon Sep 17 00:00:00 2001 From: Malcolm Greaves Date: Tue, 31 Mar 2026 17:23:10 -0700 Subject: [PATCH 05/11] clippy fix --- crates/lib/src/core/db/merkle_node/merkle_node_db.rs | 5 ----- crates/lib/src/model/merkle_tree/node/commit_node.rs | 2 +- crates/lib/src/model/merkle_tree/node/dir_node.rs | 4 +--- crates/lib/src/model/merkle_tree/node/file_chunk_node.rs | 3 +-- crates/lib/src/model/merkle_tree/node/file_node.rs | 1 - crates/lib/src/model/merkle_tree/node/vnode.rs | 4 +--- 6 files changed, 4 insertions(+), 15 deletions(-) diff --git a/crates/lib/src/core/db/merkle_node/merkle_node_db.rs b/crates/lib/src/core/db/merkle_node/merkle_node_db.rs index ef97132a4..0de493e4a 100644 --- a/crates/lib/src/core/db/merkle_node/merkle_node_db.rs +++ b/crates/lib/src/core/db/merkle_node/merkle_node_db.rs @@ -46,10 +46,6 @@ For example, data for a vnode of hash 1234 with two children: {dir data node} */ -use rmp_serde::Serializer; -use serde::Serialize; -use std::fmt::Debug; -use std::fmt::Display; use std::fs::File; use std::io::Read; use std::io::Seek; @@ -61,7 +57,6 @@ use crate::constants; use crate::error::OxenError; use crate::model::LocalRepository; use crate::model::MerkleHash; -use crate::model::merkle_tree::node_type::InvalidMerkleTreeNodeType; use crate::util; use crate::model::merkle_tree::node::{ diff --git a/crates/lib/src/model/merkle_tree/node/commit_node.rs b/crates/lib/src/model/merkle_tree/node/commit_node.rs index 2c4dddbbf..d9f18d66c 100644 --- a/crates/lib/src/model/merkle_tree/node/commit_node.rs +++ b/crates/lib/src/model/merkle_tree/node/commit_node.rs @@ -9,7 +9,7 @@ use crate::core::v_old::v0_19_0::model::merkle_tree::node::commit_node::CommitNo use crate::core::versions::MinOxenVersion; use crate::error::OxenError; use crate::model::{Commit, LocalRepository}; -use crate::model::{MerkleHash, MerkleTreeNodeIdType, MerkleTreeNodeType, TMerkleTreeNode}; +use crate::model::{MerkleHash, MerkleTreeNodeIdType, MerkleTreeNodeType}; pub trait TCommitNode { fn node_type(&self) -> &MerkleTreeNodeType; diff --git a/crates/lib/src/model/merkle_tree/node/dir_node.rs b/crates/lib/src/model/merkle_tree/node/dir_node.rs index 62bf0d3be..d026190ec 100644 --- a/crates/lib/src/model/merkle_tree/node/dir_node.rs +++ b/crates/lib/src/model/merkle_tree/node/dir_node.rs @@ -8,9 +8,7 @@ use crate::core::v_latest::model::merkle_tree::node::dir_node::DirNodeData as Di use crate::core::v_old::v0_19_0::model::merkle_tree::node::dir_node::DirNodeData as DirNodeDataV0_19_0; use crate::core::versions::MinOxenVersion; use crate::error::OxenError; -use crate::model::{ - LocalRepository, MerkleHash, MerkleTreeNodeIdType, MerkleTreeNodeType, TMerkleTreeNode, -}; +use crate::model::{LocalRepository, MerkleHash, MerkleTreeNodeIdType, MerkleTreeNodeType}; use crate::view::DataTypeCount; pub trait TDirNode { diff --git a/crates/lib/src/model/merkle_tree/node/file_chunk_node.rs b/crates/lib/src/model/merkle_tree/node/file_chunk_node.rs index c7e6d9568..b0cd92a4b 100644 --- a/crates/lib/src/model/merkle_tree/node/file_chunk_node.rs +++ b/crates/lib/src/model/merkle_tree/node/file_chunk_node.rs @@ -4,8 +4,7 @@ use serde::{Deserialize, Serialize}; -use crate::error::OxenError; -use crate::model::{MerkleHash, MerkleTreeNodeIdType, MerkleTreeNodeType, TMerkleTreeNode}; +use crate::model::{MerkleHash, MerkleTreeNodeIdType, MerkleTreeNodeType}; use std::fmt; diff --git a/crates/lib/src/model/merkle_tree/node/file_node.rs b/crates/lib/src/model/merkle_tree/node/file_node.rs index f30b1750b..5c4aa01ba 100644 --- a/crates/lib/src/model/merkle_tree/node/file_node.rs +++ b/crates/lib/src/model/merkle_tree/node/file_node.rs @@ -7,7 +7,6 @@ use crate::model::merkle_tree::node::file_node_types::{FileChunkType, FileStorag use crate::model::metadata::generic_metadata::GenericMetadata; use crate::model::{ EntryDataType, LocalRepository, MerkleHash, MerkleTreeNodeIdType, MerkleTreeNodeType, - TMerkleTreeNode, }; use serde::{Deserialize, Serialize}; use std::fmt; diff --git a/crates/lib/src/model/merkle_tree/node/vnode.rs b/crates/lib/src/model/merkle_tree/node/vnode.rs index 0f4172b2d..300286b08 100644 --- a/crates/lib/src/model/merkle_tree/node/vnode.rs +++ b/crates/lib/src/model/merkle_tree/node/vnode.rs @@ -9,9 +9,7 @@ use crate::core::v_latest::model::merkle_tree::node::vnode::VNodeData as VNodeIm use crate::core::v_old::v0_19_0::model::merkle_tree::node::vnode::VNodeData as VNodeImplV0_19_0; use crate::core::versions::MinOxenVersion; use crate::error::OxenError; -use crate::model::{ - LocalRepository, MerkleHash, MerkleTreeNodeIdType, MerkleTreeNodeType, TMerkleTreeNode, -}; +use crate::model::{LocalRepository, MerkleHash, MerkleTreeNodeIdType, MerkleTreeNodeType}; pub trait TVNode { fn node_type(&self) -> &MerkleTreeNodeType; From 430dea432c5ea3dc2ecac0bb208b72761030f02e Mon Sep 17 00:00:00 2001 From: Malcolm Greaves Date: Tue, 31 Mar 2026 17:30:58 -0700 Subject: [PATCH 06/11] wip --- crates/lib/src/repositories/tree.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/crates/lib/src/repositories/tree.rs b/crates/lib/src/repositories/tree.rs index 792162242..8aa366026 100644 --- a/crates/lib/src/repositories/tree.rs +++ b/crates/lib/src/repositories/tree.rs @@ -1077,6 +1077,19 @@ pub fn write_tree(repo: &LocalRepository, node: &MerkleTreeNode) -> Result<(), O Ok(()) } +/// Write the entire merkle tree, starting from the node (`node_impl`) to the local repository. +/// +/// Recursively writes the node and all its children to disk. To write a full tree, the node +/// (`node_impl`) **MUST** be the root of the tree -- i.e. a `Commit` node. +/// +/// This implementation requires all of the serialization errors for the node types to be the same. +/// Unfortunately, we cannot a where clause to make this equality constraint [1]. Since this +/// function is recursive and the `N`s are changing +/// Instead, since +/// all of the node implementations use serde for deserialization, we constrain the serialization +/// error type to `rmp_serde::encode::Error`. +/// +/// [1] https://github.com/rust-lang/rust/issues/20041) fn p_write_tree( repo: &LocalRepository, node: &MerkleTreeNode, From ce6c05fa788c6208ffe94095eebb0aca450ec0b0 Mon Sep 17 00:00:00 2001 From: Malcolm Greaves Date: Tue, 31 Mar 2026 17:40:58 -0700 Subject: [PATCH 07/11] fixed! express serialization error type equality constraint --- crates/lib/src/repositories/tree.rs | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/crates/lib/src/repositories/tree.rs b/crates/lib/src/repositories/tree.rs index 8aa366026..6daa65ccc 100644 --- a/crates/lib/src/repositories/tree.rs +++ b/crates/lib/src/repositories/tree.rs @@ -18,7 +18,8 @@ use crate::core::v_old::v0_19_0::index::CommitMerkleTree as CommitMerkleTreeV0_1 use crate::core::versions::MinOxenVersion; use crate::error::OxenError; use crate::model::merkle_tree::node::{ - CommitNode, DirNodeWithPath, EMerkleTreeNode, FileNode, FileNodeWithDir, MerkleTreeNode, + CommitNode, DirNode, DirNodeWithPath, EMerkleTreeNode, FileNode, FileNodeWithDir, + MerkleTreeNode, VNode, }; use crate::model::{ Commit, EntryDataType, LocalRepository, MerkleHash, MerkleTreeNodeType, PartialNode, @@ -1082,21 +1083,29 @@ pub fn write_tree(repo: &LocalRepository, node: &MerkleTreeNode) -> Result<(), O /// Recursively writes the node and all its children to disk. To write a full tree, the node /// (`node_impl`) **MUST** be the root of the tree -- i.e. a `Commit` node. /// -/// This implementation requires all of the serialization errors for the node types to be the same. -/// Unfortunately, we cannot a where clause to make this equality constraint [1]. Since this -/// function is recursive and the `N`s are changing -/// Instead, since -/// all of the node implementations use serde for deserialization, we constrain the serialization -/// error type to `rmp_serde::encode::Error`. +/// This function requires all of the serialization errors (`S`) for the node types to be the same. /// /// [1] https://github.com/rust-lang/rust/issues/20041) -fn p_write_tree( +fn p_write_tree( repo: &LocalRepository, node: &MerkleTreeNode, node_impl: &N, ) -> Result<(), OxenError> where - N: TMerkleTreeNode, + // we make sure we can convert the node's serialization error into an OxenError + OxenError: From, + // we make sure that the node type implements TMerkleTreeNode + N: TMerkleTreeNode, + // and we make sure that the nodes we're serializing all have the same serialization error type + VNode: TMerkleTreeNode, + DirNode: TMerkleTreeNode, + FileNode: TMerkleTreeNode, + // NOTE: + // We could have dropped everything here, have no `S` generic, and have this in the where: + // N: TMerkleTreeNode, + // But, by doing it this way, we make it so that we can change the actual SerializationError + // type without needing to change this function's type. It works so long as the error type + // for all of the node's trait implementations align. { let parent_id = node.parent_id; From 4b3d9a45fc4c8f36cffa0908e211cdb2348e56d4 Mon Sep 17 00:00:00 2001 From: Malcolm Greaves Date: Wed, 1 Apr 2026 16:25:46 -0700 Subject: [PATCH 08/11] address review feedback --- crates/lib/src/model/merkle_tree/node_type.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/lib/src/model/merkle_tree/node_type.rs b/crates/lib/src/model/merkle_tree/node_type.rs index e626e7487..5da2dc64c 100644 --- a/crates/lib/src/model/merkle_tree/node_type.rs +++ b/crates/lib/src/model/merkle_tree/node_type.rs @@ -110,7 +110,7 @@ impl MerkleTreeNodeType { } /// Allows for types to identify themselves as a specific kind of merkle tree node via the -/// `MerkleTreeNodeType` enum. This is ciritical for defining the node's stable on-disk +/// `MerkleTreeNodeType` enum. This is critical for defining the node's stable on-disk /// byte representation. pub trait MerkleTreeNodeIdType { /// The type of merkle tree node this node can serialize & deserialize into. From 79b5e0ab45183463611122b9588929131bb9e7a5 Mon Sep 17 00:00:00 2001 From: Malcolm Greaves Date: Wed, 1 Apr 2026 17:29:38 -0700 Subject: [PATCH 09/11] Drop SerializationError associated type: hard code to serde serialization error --- .../src/core/db/merkle_node/merkle_node_db.rs | 20 ++++------------ crates/lib/src/model/merkle_tree/node_type.rs | 21 ++++------------ crates/lib/src/repositories/tree.rs | 24 ++++--------------- 3 files changed, 12 insertions(+), 53 deletions(-) diff --git a/crates/lib/src/core/db/merkle_node/merkle_node_db.rs b/crates/lib/src/core/db/merkle_node/merkle_node_db.rs index 0de493e4a..2dee528dc 100644 --- a/crates/lib/src/core/db/merkle_node/merkle_node_db.rs +++ b/crates/lib/src/core/db/merkle_node/merkle_node_db.rs @@ -258,10 +258,7 @@ impl MerkleNodeDB { repo: &LocalRepository, node: &N, parent_id: Option, - ) -> Result, OxenError> - where - OxenError: From, - { + ) -> Result, OxenError> { if Self::exists(repo, &node.hash()) { let db_path = node_db_path(repo, &node.hash()); log::debug!( @@ -278,10 +275,7 @@ impl MerkleNodeDB { repo: &LocalRepository, node: &N, parent_id: Option, - ) -> Result - where - OxenError: From, - { + ) -> Result { let path = node_db_path(repo, &node.hash()); if !path.exists() { util::fs::create_dir_all(&path)?; @@ -374,10 +368,7 @@ impl MerkleNodeDB { &mut self, node: &N, parent_id: Option, - ) -> Result<(), OxenError> - where - OxenError: From, - { + ) -> Result<(), OxenError> { if self.read_only { return Err(OxenError::basic_str("Cannot write to read-only db")); } @@ -421,10 +412,7 @@ impl MerkleNodeDB { Ok(()) } - pub fn add_child(&mut self, item: &N) -> Result<(), OxenError> - where - OxenError: From, - { + pub fn add_child(&mut self, item: &N) -> Result<(), OxenError> { if self.read_only { return Err(OxenError::basic_str("Cannot write to read-only db")); } diff --git a/crates/lib/src/model/merkle_tree/node_type.rs b/crates/lib/src/model/merkle_tree/node_type.rs index 5da2dc64c..921fc3fad 100644 --- a/crates/lib/src/model/merkle_tree/node_type.rs +++ b/crates/lib/src/model/merkle_tree/node_type.rs @@ -134,20 +134,14 @@ pub trait MerkleTreeNodeIdType { /// that do implement `Serialize`, there's a blanket implementation that delegates the `serialize` /// call to the `to_msgpack_bytes` method. pub trait TMerkleTreeNode: MerkleTreeNodeIdType + Debug + Display { - /// The error type that can occur during serialization. - type SerializationError: std::error::Error + Send + Sync + 'static; - /// Serialize this node to a MsgPack byte array. - fn to_msgpack_bytes(&self) -> Result, Self::SerializationError>; + fn to_msgpack_bytes(&self) -> Result, rmp_serde::encode::Error>; } /// Blanket implementation for Merkle tree nodes that implement serde's `Serialize` trait. impl TMerkleTreeNode for T { - /// Serialization errors come from serde. - type SerializationError = rmp_serde::encode::Error; - /// Uses serde to serialize this node. - fn to_msgpack_bytes(&self) -> Result, Self::SerializationError> { + fn to_msgpack_bytes(&self) -> Result, rmp_serde::encode::Error> { let mut buf = Vec::new(); let x = self.serialize(&mut rmp_serde::Serializer::new(&mut buf)); x.map(|_| buf) @@ -156,10 +150,7 @@ impl TMerkleTreeNode for #[cfg(test)] mod tests { - use crate::{ - error::OxenError, - model::merkle_tree::node::{CommitNode, DirNode, FileChunkNode, FileNode, VNode}, - }; + use crate::model::merkle_tree::node::{CommitNode, DirNode, FileChunkNode, FileNode, VNode}; use super::*; @@ -167,11 +158,7 @@ mod tests { fn test_nodes_implement_trait() { /// this only exists so we can check that all node types implement `TMerkleTreeNode` /// it will be a compile-time error if a node type does not implement the trait - fn is_tmerkletreenode(_: T) - where - OxenError: From, - { - } + fn is_tmerkletreenode(_: T) {} is_tmerkletreenode(CommitNode::default()); is_tmerkletreenode(DirNode::default()); diff --git a/crates/lib/src/repositories/tree.rs b/crates/lib/src/repositories/tree.rs index 6daa65ccc..8aedbfbe1 100644 --- a/crates/lib/src/repositories/tree.rs +++ b/crates/lib/src/repositories/tree.rs @@ -18,8 +18,8 @@ use crate::core::v_old::v0_19_0::index::CommitMerkleTree as CommitMerkleTreeV0_1 use crate::core::versions::MinOxenVersion; use crate::error::OxenError; use crate::model::merkle_tree::node::{ - CommitNode, DirNode, DirNodeWithPath, EMerkleTreeNode, FileNode, FileNodeWithDir, - MerkleTreeNode, VNode, + CommitNode, DirNodeWithPath, EMerkleTreeNode, FileNode, FileNodeWithDir, + MerkleTreeNode, }; use crate::model::{ Commit, EntryDataType, LocalRepository, MerkleHash, MerkleTreeNodeType, PartialNode, @@ -1086,27 +1086,11 @@ pub fn write_tree(repo: &LocalRepository, node: &MerkleTreeNode) -> Result<(), O /// This function requires all of the serialization errors (`S`) for the node types to be the same. /// /// [1] https://github.com/rust-lang/rust/issues/20041) -fn p_write_tree( +fn p_write_tree( repo: &LocalRepository, node: &MerkleTreeNode, node_impl: &N, -) -> Result<(), OxenError> -where - // we make sure we can convert the node's serialization error into an OxenError - OxenError: From, - // we make sure that the node type implements TMerkleTreeNode - N: TMerkleTreeNode, - // and we make sure that the nodes we're serializing all have the same serialization error type - VNode: TMerkleTreeNode, - DirNode: TMerkleTreeNode, - FileNode: TMerkleTreeNode, - // NOTE: - // We could have dropped everything here, have no `S` generic, and have this in the where: - // N: TMerkleTreeNode, - // But, by doing it this way, we make it so that we can change the actual SerializationError - // type without needing to change this function's type. It works so long as the error type - // for all of the node's trait implementations align. -{ +) -> Result<(), OxenError> { let parent_id = node.parent_id; let mut db = MerkleNodeDB::open_read_write(repo, node_impl, parent_id)?; From 0ed49f7b43de571bf341b5a29f9a196f261b1012 Mon Sep 17 00:00:00 2001 From: Malcolm Greaves Date: Wed, 1 Apr 2026 17:38:20 -0700 Subject: [PATCH 10/11] Use from_u8() everywhere and propigate the error up on failiure --- .../lib/src/core/db/merkle_node/merkle_node_db.rs | 13 +++++++------ crates/lib/src/model/merkle_tree/node_type.rs | 6 ------ crates/lib/src/repositories/tree.rs | 3 +-- 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/crates/lib/src/core/db/merkle_node/merkle_node_db.rs b/crates/lib/src/core/db/merkle_node/merkle_node_db.rs index 2dee528dc..e64187744 100644 --- a/crates/lib/src/core/db/merkle_node/merkle_node_db.rs +++ b/crates/lib/src/core/db/merkle_node/merkle_node_db.rs @@ -322,10 +322,11 @@ impl MerkleNodeDB { (None, Some(node_file), Some(children_file)) }; - let dtype = lookup - .as_ref() - .map(|l| MerkleTreeNodeType::from_u8_unwrap(l.data_type)) - .unwrap_or(MerkleTreeNodeType::Commit); + let dtype = match lookup.as_ref() { + Some(l) => MerkleTreeNodeType::from_u8(l.data_type)?, + None => MerkleTreeNodeType::Commit, + }; + let parent_id = lookup.as_ref().map(|l| l.parent_id); Ok(Self { read_only, @@ -495,7 +496,7 @@ impl MerkleNodeDB { }; // Parse the node parent id - let data_type = MerkleTreeNodeType::from_u8_unwrap(lookup.data_type); + let data_type = MerkleTreeNodeType::from_u8(lookup.data_type)?; let parent_id = MerkleTreeNode::deserialize_id(&lookup.data, data_type)?; let mut file_data = Vec::new(); @@ -514,7 +515,7 @@ impl MerkleNodeDB { cursor.seek(SeekFrom::Start(*offset))?; let mut data = vec![0; *len as usize]; cursor.read_exact(&mut data)?; - let dtype = MerkleTreeNodeType::from_u8_unwrap(*dtype); + let dtype = MerkleTreeNodeType::from_u8(*dtype)?; let node = MerkleTreeNode { parent_id: Some(parent_id), hash: MerkleHash::new(*hash), diff --git a/crates/lib/src/model/merkle_tree/node_type.rs b/crates/lib/src/model/merkle_tree/node_type.rs index 921fc3fad..6dc35163a 100644 --- a/crates/lib/src/model/merkle_tree/node_type.rs +++ b/crates/lib/src/model/merkle_tree/node_type.rs @@ -89,12 +89,6 @@ impl MerkleTreeNodeType { } } - /// Deserialize a `u8` value into a `MerkleTreeNodeType`. - /// Panics if the `u8` value is not a valid `MerkleTreeNodeType`. - pub fn from_u8_unwrap(val: u8) -> MerkleTreeNodeType { - Self::from_u8(val).expect("Invalid MerkleTreeNodeType: {val}") - } - /// Deserialize a `u8` value into a `MerkleTreeNodeType`. /// This function is 1:1 with `to_u8`: all outputs from `to_u8` result in an `Ok`. pub fn from_u8(val: u8) -> Result { diff --git a/crates/lib/src/repositories/tree.rs b/crates/lib/src/repositories/tree.rs index 8aedbfbe1..6f88ac057 100644 --- a/crates/lib/src/repositories/tree.rs +++ b/crates/lib/src/repositories/tree.rs @@ -18,8 +18,7 @@ use crate::core::v_old::v0_19_0::index::CommitMerkleTree as CommitMerkleTreeV0_1 use crate::core::versions::MinOxenVersion; use crate::error::OxenError; use crate::model::merkle_tree::node::{ - CommitNode, DirNodeWithPath, EMerkleTreeNode, FileNode, FileNodeWithDir, - MerkleTreeNode, + CommitNode, DirNodeWithPath, EMerkleTreeNode, FileNode, FileNodeWithDir, MerkleTreeNode, }; use crate::model::{ Commit, EntryDataType, LocalRepository, MerkleHash, MerkleTreeNodeType, PartialNode, From 66a3d1500340b4eb63754f6e3fd5cdde8ac51338 Mon Sep 17 00:00:00 2001 From: Malcolm Greaves Date: Wed, 1 Apr 2026 17:53:11 -0700 Subject: [PATCH 11/11] update doc comment --- crates/lib/src/repositories/tree.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/lib/src/repositories/tree.rs b/crates/lib/src/repositories/tree.rs index 6f88ac057..c0498abd7 100644 --- a/crates/lib/src/repositories/tree.rs +++ b/crates/lib/src/repositories/tree.rs @@ -1082,8 +1082,6 @@ pub fn write_tree(repo: &LocalRepository, node: &MerkleTreeNode) -> Result<(), O /// Recursively writes the node and all its children to disk. To write a full tree, the node /// (`node_impl`) **MUST** be the root of the tree -- i.e. a `Commit` node. /// -/// This function requires all of the serialization errors (`S`) for the node types to be the same. -/// /// [1] https://github.com/rust-lang/rust/issues/20041) fn p_write_tree( repo: &LocalRepository,