Skip to content

Commit 1cfed6b

Browse files
authored
Fix the staging issue with git-prolly (#51)
* Fix the staging issue with git-prolly Problem Identified The staging area (HashMap<Vec<u8>, Option<Vec<u8>>>) was being created fresh on each command invocation, causing staged changes to be lost between commands. Solution Implemented 1. Added staging area persistence: Created save_staging_area() and load_staging_area() methods that serialize/deserialize the staging area to/from .git/PROLLY_STAGING file 2. Fixed HEAD reference updates: Implemented proper update_head() method that writes branch references and HEAD file 3. Updated all staging operations: Modified insert(), update(), delete(), and checkout() to persist staging area changes 4. Fixed commit flow: Ensured staging area is cleared after successful commits * fix save and load of the storage * fix preserving prolly tree root issue The issue was that the original implementation was creating a Git tree with a null ObjectId placeholder, which resulted in the prolly_tree_root file being deleted during staging. Now it properly creates a real Git blob containing the serialized ProllyTree root and reconstructs the tree correctly when loading from HEAD. * fixed the git-prolly issue where prolly list was showing "No keys found" after committing data The problem had three root causes: 1. Missing config save after commit - Issue: The commit() function wasn't calling save_config() after persisting the tree - Fix: Added self.tree.save_config() call in /Users/feng/github/prollytree/src/git/versioned_store.rs:216 2. Hash mappings not loaded when opening store - Issue: GitNodeStorage::new() wasn't calling load_hash_mappings() - Fix: Added storage.load_hash_mappings() call in /Users/feng/github/prollytree/src/git/storage.rs:67 3. ValueDigest deserialization incompatible with JSON - Issue: ValueDigest::deserialize() expected binary bytes but JSON config contained array format [188,78,66,...] - Fix: Updated deserialization in /Users/feng/github/prollytree/src/digest.rs:134 to handle Vec<u8> from JSON
1 parent a42a978 commit 1cfed6b

6 files changed

Lines changed: 564 additions & 76 deletions

File tree

src/digest.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,11 @@ impl<'de, const N: usize> Deserialize<'de> for ValueDigest<N> {
130130
where
131131
D: serde::Deserializer<'de>,
132132
{
133-
let bytes: &[u8] = serde::de::Deserialize::deserialize(deserializer)?;
134-
let array = <[u8; N]>::try_from(bytes)
135-
.map_err(|_| serde::de::Error::invalid_length(bytes.len(), &stringify!(N)))?;
133+
// Try to deserialize as a sequence of bytes (for JSON format)
134+
let bytes: Vec<u8> = serde::de::Deserialize::deserialize(deserializer)?;
135+
let array = <[u8; N]>::try_from(bytes.as_slice()).map_err(|_| {
136+
serde::de::Error::invalid_length(bytes.len(), &format!("array of length {N}").as_str())
137+
})?;
136138
Ok(ValueDigest(array))
137139
}
138140
}

src/git/operations.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -397,14 +397,24 @@ mod tests {
397397
#[test]
398398
fn test_git_operations_creation() {
399399
let temp_dir = TempDir::new().unwrap();
400-
let store = VersionedKvStore::<32>::init(temp_dir.path()).unwrap();
400+
// Initialize git repository (regular, not bare)
401+
gix::init(temp_dir.path()).unwrap();
402+
// Create subdirectory for dataset
403+
let dataset_dir = temp_dir.path().join("dataset");
404+
std::fs::create_dir_all(&dataset_dir).unwrap();
405+
let store = VersionedKvStore::<32>::init(&dataset_dir).unwrap();
401406
let _ops = GitOperations::new(store);
402407
}
403408

404409
#[test]
405410
fn test_parse_commit_id() {
406411
let temp_dir = TempDir::new().unwrap();
407-
let store = VersionedKvStore::<32>::init(temp_dir.path()).unwrap();
412+
// Initialize git repository (regular, not bare)
413+
gix::init(temp_dir.path()).unwrap();
414+
// Create subdirectory for dataset
415+
let dataset_dir = temp_dir.path().join("dataset");
416+
std::fs::create_dir_all(&dataset_dir).unwrap();
417+
let store = VersionedKvStore::<32>::init(&dataset_dir).unwrap();
408418
let ops = GitOperations::new(store);
409419

410420
// Test HEAD parsing

src/git/storage.rs

Lines changed: 133 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -33,34 +33,69 @@ pub struct GitNodeStorage<const N: usize> {
3333
configs: Mutex<HashMap<String, Vec<u8>>>,
3434
// Maps ProllyTree hashes to Git object IDs
3535
hash_to_object_id: Mutex<HashMap<ValueDigest<N>, gix::ObjectId>>,
36+
// Directory where this dataset's config and mapping files are stored
37+
dataset_dir: std::path::PathBuf,
38+
}
39+
40+
impl<const N: usize> Clone for GitNodeStorage<N> {
41+
fn clone(&self) -> Self {
42+
let cloned = Self {
43+
_repository: self._repository.clone(),
44+
cache: Mutex::new(LruCache::new(NonZeroUsize::new(1000).unwrap())),
45+
configs: Mutex::new(HashMap::new()),
46+
hash_to_object_id: Mutex::new(HashMap::new()),
47+
dataset_dir: self.dataset_dir.clone(),
48+
};
49+
50+
// Load the hash mappings for the cloned instance
51+
cloned.load_hash_mappings();
52+
53+
cloned
54+
}
3655
}
3756

3857
impl<const N: usize> GitNodeStorage<N> {
3958
/// Create a new GitNodeStorage instance
40-
pub fn new(repository: gix::Repository) -> Result<Self, GitKvError> {
59+
pub fn new(
60+
repository: gix::Repository,
61+
dataset_dir: std::path::PathBuf,
62+
) -> Result<Self, GitKvError> {
4163
let cache_size = NonZeroUsize::new(1000).unwrap(); // Default cache size
4264

43-
Ok(GitNodeStorage {
65+
let storage = GitNodeStorage {
4466
_repository: Arc::new(Mutex::new(repository)),
4567
cache: Mutex::new(LruCache::new(cache_size)),
4668
configs: Mutex::new(HashMap::new()),
4769
hash_to_object_id: Mutex::new(HashMap::new()),
48-
})
70+
dataset_dir,
71+
};
72+
73+
// Load existing hash mappings
74+
storage.load_hash_mappings();
75+
76+
Ok(storage)
4977
}
5078

5179
/// Create GitNodeStorage with custom cache size
5280
pub fn with_cache_size(
5381
repository: gix::Repository,
82+
dataset_dir: std::path::PathBuf,
5483
cache_size: usize,
5584
) -> Result<Self, GitKvError> {
5685
let cache_size = NonZeroUsize::new(cache_size).unwrap_or(NonZeroUsize::new(1000).unwrap());
5786

58-
Ok(GitNodeStorage {
87+
let storage = GitNodeStorage {
5988
_repository: Arc::new(Mutex::new(repository)),
6089
cache: Mutex::new(LruCache::new(cache_size)),
6190
configs: Mutex::new(HashMap::new()),
6291
hash_to_object_id: Mutex::new(HashMap::new()),
63-
})
92+
dataset_dir,
93+
};
94+
95+
// Load existing hash mappings
96+
storage.load_hash_mappings();
97+
98+
Ok(storage)
6499
}
65100

66101
/// Store a node as a Git blob
@@ -118,7 +153,14 @@ impl<const N: usize> NodeStorage<N> for GitNodeStorage<N> {
118153
match self.store_node_as_blob(&node) {
119154
Ok(blob_id) => {
120155
// Store the mapping between ProllyTree hash and Git object ID
121-
self.hash_to_object_id.lock().unwrap().insert(hash, blob_id);
156+
self.hash_to_object_id
157+
.lock()
158+
.unwrap()
159+
.insert(hash.clone(), blob_id);
160+
161+
// Persist the mapping to filesystem
162+
self.save_hash_mapping(&hash, &blob_id);
163+
122164
Some(())
123165
}
124166
Err(_) => None,
@@ -139,14 +181,90 @@ impl<const N: usize> NodeStorage<N> for GitNodeStorage<N> {
139181
}
140182

141183
fn save_config(&self, key: &str, config: &[u8]) {
142-
// Store config in memory for now
143-
// In a real implementation, we'd store this as a Git blob or in a config file
184+
// Store config in memory
144185
let mut configs = self.configs.lock().unwrap();
145186
configs.insert(key.to_string(), config.to_vec());
187+
188+
// Also persist to filesystem for durability in the dataset directory
189+
let config_path = self.dataset_dir.join(format!("prolly_config_{key}"));
190+
let _ = std::fs::write(config_path, config);
146191
}
147192

148193
fn get_config(&self, key: &str) -> Option<Vec<u8>> {
149-
self.configs.lock().unwrap().get(key).cloned()
194+
// First try to get from memory
195+
if let Some(config) = self.configs.lock().unwrap().get(key).cloned() {
196+
return Some(config);
197+
}
198+
199+
// If not in memory, try to load from filesystem
200+
let config_path = self.dataset_dir.join(format!("prolly_config_{key}"));
201+
if let Ok(config) = std::fs::read(config_path) {
202+
// Cache in memory for future use
203+
self.configs
204+
.lock()
205+
.unwrap()
206+
.insert(key.to_string(), config.clone());
207+
return Some(config);
208+
}
209+
210+
None
211+
}
212+
}
213+
214+
impl<const N: usize> GitNodeStorage<N> {
215+
/// Save hash mapping to filesystem
216+
fn save_hash_mapping(&self, hash: &ValueDigest<N>, object_id: &gix::ObjectId) {
217+
let mapping_path = self.dataset_dir.join("prolly_hash_mappings");
218+
219+
// Read existing mappings
220+
let mut mappings = if mapping_path.exists() {
221+
std::fs::read_to_string(&mapping_path).unwrap_or_default()
222+
} else {
223+
String::new()
224+
};
225+
226+
// Add new mapping - use simple format for now without hex dependency
227+
let hash_bytes: Vec<String> = hash.0.iter().map(|b| format!("{b:02x}")).collect();
228+
let hash_hex = hash_bytes.join("");
229+
let object_hex = object_id.to_hex().to_string();
230+
mappings.push_str(&format!("{hash_hex}:{object_hex}\n"));
231+
232+
// Write back
233+
let _ = std::fs::write(mapping_path, mappings);
234+
}
235+
236+
/// Load hash mappings from filesystem
237+
fn load_hash_mappings(&self) {
238+
let mapping_path = self.dataset_dir.join("prolly_hash_mappings");
239+
240+
if let Ok(mappings) = std::fs::read_to_string(mapping_path) {
241+
let mut hash_map = self.hash_to_object_id.lock().unwrap();
242+
243+
for line in mappings.lines() {
244+
if let Some((hash_hex, object_hex)) = line.split_once(':') {
245+
// Parse hex string manually
246+
if hash_hex.len() == N * 2 {
247+
let mut hash_bytes = Vec::new();
248+
for i in 0..N {
249+
if let Ok(byte) = u8::from_str_radix(&hash_hex[i * 2..i * 2 + 2], 16) {
250+
hash_bytes.push(byte);
251+
} else {
252+
break;
253+
}
254+
}
255+
256+
if hash_bytes.len() == N {
257+
if let Ok(object_id) = gix::ObjectId::from_hex(object_hex.as_bytes()) {
258+
let mut hash_array = [0u8; N];
259+
hash_array.copy_from_slice(&hash_bytes);
260+
let hash = ValueDigest(hash_array);
261+
hash_map.insert(hash, object_id);
262+
}
263+
}
264+
}
265+
}
266+
}
267+
}
150268
}
151269
}
152270

@@ -186,8 +304,8 @@ mod tests {
186304

187305
#[test]
188306
fn test_git_node_storage_basic_operations() {
189-
let (_temp_dir, repo) = create_test_repo();
190-
let mut storage = GitNodeStorage::<32>::new(repo).unwrap();
307+
let (temp_dir, repo) = create_test_repo();
308+
let mut storage = GitNodeStorage::<32>::new(repo, temp_dir.path().to_path_buf()).unwrap();
191309

192310
let node = create_test_node();
193311
let hash = node.get_hash();
@@ -210,8 +328,10 @@ mod tests {
210328

211329
#[test]
212330
fn test_cache_functionality() {
213-
let (_temp_dir, repo) = create_test_repo();
214-
let mut storage = GitNodeStorage::<32>::with_cache_size(repo, 2).unwrap();
331+
let (temp_dir, repo) = create_test_repo();
332+
let dataset_dir = temp_dir.path().join("dataset");
333+
std::fs::create_dir_all(&dataset_dir).unwrap();
334+
let mut storage = GitNodeStorage::<32>::with_cache_size(repo, dataset_dir, 2).unwrap();
215335

216336
let node1 = create_test_node();
217337
let hash1 = node1.get_hash();

0 commit comments

Comments
 (0)