diff --git a/lib/src/index.rs b/lib/src/index.rs index 2036fb8a5..e0ce216e2 100644 --- a/lib/src/index.rs +++ b/lib/src/index.rs @@ -36,6 +36,92 @@ use crate::store_wrapper::StoreWrapper; use std::fmt::{Debug, Formatter}; use std::ops::Bound; +#[derive(Clone)] +pub enum IndexRef<'a> { + Readonly(Arc), + Mutable(&'a MutableIndex), +} + +impl From> for IndexRef<'_> { + fn from(index: Arc) -> Self { + IndexRef::Readonly(index) + } +} + +impl<'a> From<&'a MutableIndex> for IndexRef<'a> { + fn from(index: &'a MutableIndex) -> Self { + IndexRef::Mutable(index) + } +} + +impl<'a> IndexRef<'a> { + pub fn num_commits(&self) -> u32 { + match self { + IndexRef::Readonly(index) => index.num_commits(), + IndexRef::Mutable(index) => index.num_commits(), + } + } + + pub fn stats(&self) -> IndexStats { + match self { + IndexRef::Readonly(index) => index.stats(), + IndexRef::Mutable(index) => index.stats(), + } + } + + pub fn commit_id_to_pos(&self, commit_id: &CommitId) -> Option { + match self { + IndexRef::Readonly(index) => index.commit_id_to_pos(commit_id), + IndexRef::Mutable(index) => index.commit_id_to_pos(commit_id), + } + } + + pub fn resolve_prefix(&self, prefix: &HexPrefix) -> PrefixResolution { + match self { + IndexRef::Readonly(index) => index.resolve_prefix(prefix), + IndexRef::Mutable(index) => index.resolve_prefix(prefix), + } + } + + pub fn entry_by_id(&self, commit_id: &CommitId) -> Option { + match self { + IndexRef::Readonly(index) => index.entry_by_id(commit_id), + IndexRef::Mutable(index) => index.entry_by_id(commit_id), + } + } + + pub fn has_id(&self, commit_id: &CommitId) -> bool { + match self { + IndexRef::Readonly(index) => index.has_id(commit_id), + IndexRef::Mutable(index) => index.has_id(commit_id), + } + } + + pub fn is_ancestor(&self, ancestor_id: &CommitId, descendant_id: &CommitId) -> bool { + match self { + IndexRef::Readonly(index) => index.is_ancestor(ancestor_id, descendant_id), + IndexRef::Mutable(index) => index.is_ancestor(ancestor_id, descendant_id), + } + } + + pub fn walk_revs(&self, wanted: &[CommitId], unwanted: &[CommitId]) -> RevWalk { + match self { + IndexRef::Readonly(index) => index.walk_revs(wanted, unwanted), + IndexRef::Mutable(index) => index.walk_revs(wanted, unwanted), + } + } + + pub fn heads<'candidates>( + &self, + candidates: impl IntoIterator, + ) -> Vec { + match self { + IndexRef::Readonly(index) => index.heads(candidates), + IndexRef::Mutable(index) => index.heads(candidates), + } + } +} + struct CommitGraphEntry<'a> { data: &'a [u8], hash_length: usize, @@ -273,7 +359,7 @@ impl MutableIndex { } } - fn incremental(parent_file: Arc) -> Self { + pub fn incremental(parent_file: Arc) -> Self { let num_parent_commits = parent_file.num_parent_commits + parent_file.num_local_commits; let hash_length = parent_file.hash_length; Self { @@ -285,8 +371,8 @@ impl MutableIndex { } } - pub fn as_composite(&self) -> CompositeIndex { - CompositeIndex(self) + pub fn as_index_ref(&self) -> IndexRef { + IndexRef::Mutable(self) } fn add_commit(&mut self, commit: &Commit) { @@ -437,10 +523,8 @@ trait IndexSegment { fn segment_entry_by_pos(&self, pos: u32, local_pos: u32) -> IndexEntry; } -// TODO: This is a weird name for a public type in this module. The callers -// shouldn't need to know that it's composite. Rename. #[derive(Clone)] -pub struct CompositeIndex<'a>(&'a dyn IndexSegment); +struct CompositeIndex<'a>(&'a dyn IndexSegment); impl<'a> CompositeIndex<'a> { pub fn num_commits(&self) -> u32 { @@ -497,7 +581,7 @@ impl<'a> CompositeIndex<'a> { // The parent ReadonlyIndex outlives the child let parent_file: &'a ReadonlyIndex = unsafe { std::mem::transmute(parent_file) }; - parent_file.as_composite().entry_by_pos(pos) + CompositeIndex(parent_file).entry_by_pos(pos) } } @@ -507,7 +591,7 @@ impl<'a> CompositeIndex<'a> { self.0 .segment_parent_file() .as_ref() - .and_then(|file| file.as_composite().commit_id_to_pos(commit_id)) + .and_then(|file| IndexRef::Readonly(file.clone()).commit_id_to_pos(commit_id)) }) } @@ -1106,10 +1190,6 @@ impl ReadonlyIndex { Ok(index_file) } - pub fn as_composite(&self) -> CompositeIndex { - CompositeIndex(self) - } - pub fn num_commits(&self) -> u32 { CompositeIndex(self).num_commits() } @@ -1227,13 +1307,14 @@ mod tests { fn index_empty(use_file: bool) { let index = MutableIndex::full(3); let temp_dir; - let source: Box = if use_file { + let index = if use_file { temp_dir = tempfile::tempdir().unwrap(); - Box::new(ReadonlyIndex::from_mutable(temp_dir.path(), index).unwrap()) + IndexRef::Readonly(Arc::new( + ReadonlyIndex::from_mutable(temp_dir.path(), index).unwrap(), + )) } else { - Box::new(index) + IndexRef::Mutable(&index) }; - let index = CompositeIndex(source.as_ref()); // Stats are as expected let stats = index.stats(); @@ -1255,13 +1336,14 @@ mod tests { let id_0 = CommitId::from_hex("000000"); index.add_commit_data(id_0.clone(), vec![]); let temp_dir; - let source: Box = if use_file { + let index = if use_file { temp_dir = tempfile::tempdir().unwrap(); - Box::new(ReadonlyIndex::from_mutable(temp_dir.path(), index).unwrap()) + IndexRef::Readonly(Arc::new( + ReadonlyIndex::from_mutable(temp_dir.path(), index).unwrap(), + )) } else { - Box::new(index) + IndexRef::Mutable(&index) }; - let index = CompositeIndex(source.as_ref()); // Stats are as expected let stats = index.stats(); @@ -1281,10 +1363,6 @@ mod tests { assert_eq!(entry.generation_number(), 0); assert_eq!(entry.num_parents(), 0); assert_eq!(entry.parents_positions(), Vec::::new()); - // Can get same entry by position - let entry = index.entry_by_pos(0); - assert_eq!(entry.pos, 0); - assert_eq!(entry.commit_id(), id_0); } #[test] @@ -1331,12 +1409,13 @@ mod tests { index.add_commit_data(id_3.clone(), vec![id_2.clone()]); index.add_commit_data(id_4.clone(), vec![id_1.clone()]); index.add_commit_data(id_5.clone(), vec![id_4.clone(), id_2.clone()]); - let source: Box = if use_file { - Box::new(ReadonlyIndex::from_mutable(temp_dir.path(), index).unwrap()) + let index = if use_file { + IndexRef::Readonly(Arc::new( + ReadonlyIndex::from_mutable(temp_dir.path(), index).unwrap(), + )) } else { - Box::new(index) + IndexRef::Mutable(&index) }; - let index = CompositeIndex(source.as_ref()); // Stats are as expected let stats = index.stats(); @@ -1435,7 +1514,6 @@ mod tests { index.add_commit_data(id_3.clone(), vec![id_2.clone()]); index.add_commit_data(id_4.clone(), vec![id_1.clone()]); index.add_commit_data(id_5.clone(), vec![id_4.clone(), id_2.clone()]); - let index = CompositeIndex(&index); assert!(index.is_ancestor(&id_0, &id_0)); assert!(index.is_ancestor(&id_0, &id_1)); @@ -1472,7 +1550,6 @@ mod tests { index.add_commit_data(id_3.clone(), vec![id_2.clone()]); index.add_commit_data(id_4.clone(), vec![id_1.clone()]); index.add_commit_data(id_5.clone(), vec![id_4.clone(), id_2.clone()]); - let index = CompositeIndex(&index); // No wanted commits let revs: Vec = index.walk_revs(&[], &[]).collect(); @@ -1540,7 +1617,6 @@ mod tests { index.add_commit_data(id_3.clone(), vec![id_2.clone()]); index.add_commit_data(id_4.clone(), vec![id_1.clone()]); index.add_commit_data(id_5.clone(), vec![id_4.clone(), id_2.clone()]); - let index = CompositeIndex(&index); // Empty input assert!(index.heads(&[]).is_empty()); diff --git a/lib/tests/test_index.rs b/lib/tests/test_index.rs index 66537cb8b..ee2495d75 100644 --- a/lib/tests/test_index.rs +++ b/lib/tests/test_index.rs @@ -14,7 +14,7 @@ use jujube_lib::commit::Commit; use jujube_lib::commit_builder::CommitBuilder; -use jujube_lib::index::CompositeIndex; +use jujube_lib::index::IndexRef; use jujube_lib::repo::ReadonlyRepo; use jujube_lib::settings::UserSettings; use jujube_lib::store::CommitId; @@ -28,8 +28,12 @@ fn child_commit(settings: &UserSettings, repo: &ReadonlyRepo, commit: &Commit) - } // Helper just to reduce line wrapping -fn generation_number(index: &CompositeIndex, commit_id: &CommitId) -> u32 { - index.entry_by_id(commit_id).unwrap().generation_number() +fn generation_number<'a>(index: impl Into>, commit_id: &CommitId) -> u32 { + index + .into() + .entry_by_id(commit_id) + .unwrap() + .generation_number() } #[test_case(false ; "local store")] @@ -39,14 +43,19 @@ fn test_index_commits_empty_repo(use_git: bool) { let (_temp_dir, repo) = testutils::init_repo(&settings, use_git); let index = repo.index(); - let index = index.as_composite(); // There should be the root commit and the working copy commit assert_eq!(index.num_commits(), 2); // Check the generation numbers of the root and the working copy - assert_eq!(generation_number(&index, repo.store().root_commit_id()), 0); assert_eq!( - generation_number(&index, &repo.working_copy_locked().current_commit_id()), + generation_number(index.clone(), repo.store().root_commit_id()), + 0 + ); + assert_eq!( + generation_number( + index.clone(), + &repo.working_copy_locked().current_commit_id() + ), 1 ); } @@ -88,7 +97,6 @@ fn test_index_commits_standard_cases(use_git: bool) { Arc::get_mut(&mut repo).unwrap().reload(); let index = repo.index(); - let index = index.as_composite(); // There should be the root commit and the working copy commit, plus // 8 more assert_eq!(index.num_commits(), 2 + 8); @@ -98,16 +106,16 @@ fn test_index_commits_standard_cases(use_git: bool) { assert_eq!(stats.num_merges, 1); assert_eq!(stats.max_generation_number, 6); - assert_eq!(generation_number(&index, root_commit.id()), 0); - assert_eq!(generation_number(&index, wc_commit.id()), 1); - assert_eq!(generation_number(&index, commit_a.id()), 1); - assert_eq!(generation_number(&index, commit_b.id()), 2); - assert_eq!(generation_number(&index, commit_c.id()), 2); - assert_eq!(generation_number(&index, commit_d.id()), 3); - assert_eq!(generation_number(&index, commit_e.id()), 4); - assert_eq!(generation_number(&index, commit_f.id()), 5); - assert_eq!(generation_number(&index, commit_g.id()), 6); - assert_eq!(generation_number(&index, commit_h.id()), 5); + assert_eq!(generation_number(index.clone(), root_commit.id()), 0); + assert_eq!(generation_number(index.clone(), wc_commit.id()), 1); + assert_eq!(generation_number(index.clone(), commit_a.id()), 1); + assert_eq!(generation_number(index.clone(), commit_b.id()), 2); + assert_eq!(generation_number(index.clone(), commit_c.id()), 2); + assert_eq!(generation_number(index.clone(), commit_d.id()), 3); + assert_eq!(generation_number(index.clone(), commit_e.id()), 4); + assert_eq!(generation_number(index.clone(), commit_f.id()), 5); + assert_eq!(generation_number(index.clone(), commit_g.id()), 6); + assert_eq!(generation_number(index.clone(), commit_h.id()), 5); assert!(index.is_ancestor(root_commit.id(), commit_a.id())); assert!(!index.is_ancestor(commit_a.id(), root_commit.id())); @@ -161,7 +169,6 @@ fn test_index_commits_criss_cross(use_git: bool) { Arc::get_mut(&mut repo).unwrap().reload(); let index = repo.index(); - let index = index.as_composite(); // There should the root commit and the working copy commit, plus 2 for each // generation assert_eq!(index.num_commits(), 2 + 2 * (num_generations as u32)); @@ -175,11 +182,11 @@ fn test_index_commits_criss_cross(use_git: bool) { // Check generation numbers for gen in 0..num_generations { assert_eq!( - generation_number(&index, left_commits[gen].id()), + generation_number(index.clone(), left_commits[gen].id()), (gen as u32) + 1 ); assert_eq!( - generation_number(&index, right_commits[gen].id()), + generation_number(index.clone(), right_commits[gen].id()), (gen as u32) + 1 ); } @@ -276,7 +283,6 @@ fn test_index_commits_previous_operations(use_git: bool) { let repo = ReadonlyRepo::load(&settings, repo.working_copy_path().clone()).unwrap(); let index = repo.index(); - let index = index.as_composite(); // There should be the root commit and the working copy commit, plus // 3 more assert_eq!(index.num_commits(), 2 + 3); @@ -286,9 +292,9 @@ fn test_index_commits_previous_operations(use_git: bool) { assert_eq!(stats.num_merges, 0); assert_eq!(stats.max_generation_number, 3); - assert_eq!(generation_number(&index, commit_a.id()), 1); - assert_eq!(generation_number(&index, commit_b.id()), 2); - assert_eq!(generation_number(&index, commit_c.id()), 3); + assert_eq!(generation_number(index.clone(), commit_a.id()), 1); + assert_eq!(generation_number(index.clone(), commit_b.id()), 2); + assert_eq!(generation_number(index.clone(), commit_c.id()), 3); } #[test_case(false ; "local store")] @@ -312,7 +318,6 @@ fn test_index_commits_incremental(use_git: bool) { Arc::get_mut(&mut repo).unwrap().reload(); let index = repo.index(); - let index = index.as_composite(); // There should be the root commit and the working copy commit, plus // 1 more assert_eq!(index.num_commits(), 2 + 1); @@ -324,7 +329,6 @@ fn test_index_commits_incremental(use_git: bool) { let repo = ReadonlyRepo::load(&settings, repo.working_copy_path().clone()).unwrap(); let index = repo.index(); - let index = index.as_composite(); // There should be the root commit and the working copy commit, plus // 3 more assert_eq!(index.num_commits(), 2 + 3); @@ -338,10 +342,10 @@ fn test_index_commits_incremental(use_git: bool) { assert_eq!(stats.levels[1].num_commits, 3); assert_ne!(stats.levels[1].name, stats.levels[0].name); - assert_eq!(generation_number(&index, root_commit.id()), 0); - assert_eq!(generation_number(&index, commit_a.id()), 1); - assert_eq!(generation_number(&index, commit_b.id()), 2); - assert_eq!(generation_number(&index, commit_c.id()), 3); + assert_eq!(generation_number(index.clone(), root_commit.id()), 0); + assert_eq!(generation_number(index.clone(), commit_a.id()), 1); + assert_eq!(generation_number(index.clone(), commit_b.id()), 2); + assert_eq!(generation_number(index.clone(), commit_c.id()), 3); } #[test_case(false ; "local store")] @@ -363,7 +367,6 @@ fn test_index_commits_incremental_empty_transaction(use_git: bool) { Arc::get_mut(&mut repo).unwrap().reload(); let index = repo.index(); - let index = index.as_composite(); // There should be the root commit and the working copy commit, plus // 1 more assert_eq!(index.num_commits(), 2 + 1); @@ -372,7 +375,6 @@ fn test_index_commits_incremental_empty_transaction(use_git: bool) { let repo = ReadonlyRepo::load(&settings, repo.working_copy_path().clone()).unwrap(); let index = repo.index(); - let index = index.as_composite(); // There should be the root commit and the working copy commit, plus // 1 more assert_eq!(index.num_commits(), 2 + 1); @@ -385,6 +387,6 @@ fn test_index_commits_incremental_empty_transaction(use_git: bool) { assert_eq!(stats.levels[0].num_commits, 0); assert_eq!(stats.levels[1].num_commits, 3); - assert_eq!(generation_number(&index, root_commit.id()), 0); - assert_eq!(generation_number(&index, commit_a.id()), 1); + assert_eq!(generation_number(index.clone(), root_commit.id()), 0); + assert_eq!(generation_number(index.clone(), commit_a.id()), 1); }