From 3a6c6d8bf4cdc1a477a69f768ecdb9674ba3be1e Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Thu, 22 Sep 2022 17:35:23 +0900 Subject: [PATCH] commit: make parent_ids() return slice instead of cloned vec I feel it doesn't make sense for a simple getter function to create an owned vec after 010867308714 "backend: let each backend handle root commit on write." --- lib/src/commit.rs | 4 +- lib/src/index.rs | 146 ++++++++++++++++++--------------------------- lib/src/repo.rs | 2 +- lib/src/rewrite.rs | 2 +- src/commands.rs | 2 +- 5 files changed, 64 insertions(+), 92 deletions(-) diff --git a/lib/src/commit.rs b/lib/src/commit.rs index a165671e5..744c422da 100644 --- a/lib/src/commit.rs +++ b/lib/src/commit.rs @@ -75,8 +75,8 @@ impl Commit { &self.id } - pub fn parent_ids(&self) -> Vec { - self.data.parents.clone() + pub fn parent_ids(&self) -> &[CommitId] { + &self.data.parents } pub fn parents(&self) -> Vec { diff --git a/lib/src/index.rs b/lib/src/index.rs index b19541b6f..2f326186a 100644 --- a/lib/src/index.rs +++ b/lib/src/index.rs @@ -367,7 +367,7 @@ impl MutableIndex { &mut self, commit_id: CommitId, change_id: ChangeId, - parent_ids: Vec, + parent_ids: &[CommitId], ) { if self.has_id(&commit_id) { return; @@ -380,7 +380,7 @@ impl MutableIndex { }; for parent_id in parent_ids { let parent_entry = self - .entry_by_id(&parent_id) + .entry_by_id(parent_id) .expect("parent commit is not indexed"); entry.generation_number = max( entry.generation_number, @@ -404,7 +404,7 @@ impl MutableIndex { .iter() .map(|entry| entry.commit_id()) .collect_vec(); - self.add_commit_data(entry.commit_id(), entry.change_id(), parent_ids); + self.add_commit_data(entry.commit_id(), entry.change_id(), &parent_ids); } } @@ -1502,7 +1502,7 @@ mod tests { let mut index = MutableIndex::full(3); let id_0 = CommitId::from_hex("000000"); let change_id0 = new_change_id(); - index.add_commit_data(id_0.clone(), change_id0.clone(), vec![]); + index.add_commit_data(id_0.clone(), change_id0.clone(), &[]); let mut _saved_index = None; let index = if on_disk { _saved_index = Some(index.save_in(temp_dir.path().to_owned()).unwrap()); @@ -1540,7 +1540,7 @@ mod tests { let mut index = MutableIndex::full(3); let id_0 = CommitId::from_hex("000000"); let id_1 = CommitId::from_hex("111111"); - index.add_commit_data(id_1, new_change_id(), vec![id_0]); + index.add_commit_data(id_1, new_change_id(), &[id_0]); } #[test_case(false, false; "full in memory")] @@ -1563,9 +1563,9 @@ mod tests { let change_id1 = new_change_id(); let id_2 = CommitId::from_hex("222222"); let change_id2 = change_id1.clone(); - index.add_commit_data(id_0.clone(), change_id0, vec![]); - index.add_commit_data(id_1.clone(), change_id1.clone(), vec![id_0.clone()]); - index.add_commit_data(id_2.clone(), change_id2.clone(), vec![id_0.clone()]); + index.add_commit_data(id_0.clone(), change_id0, &[]); + index.add_commit_data(id_1.clone(), change_id1.clone(), &[id_0.clone()]); + index.add_commit_data(id_2.clone(), change_id2.clone(), &[id_0.clone()]); // If testing incremental indexing, write the first three commits to one file // now and build the remainder as another segment on top. @@ -1580,9 +1580,9 @@ mod tests { let change_id4 = new_change_id(); let id_5 = CommitId::from_hex("555555"); let change_id5 = change_id3.clone(); - index.add_commit_data(id_3.clone(), change_id3.clone(), vec![id_2.clone()]); - index.add_commit_data(id_4.clone(), change_id4, vec![id_1.clone()]); - index.add_commit_data(id_5.clone(), change_id5, vec![id_4.clone(), id_2.clone()]); + index.add_commit_data(id_3.clone(), change_id3.clone(), &[id_2.clone()]); + index.add_commit_data(id_4.clone(), change_id4, &[id_1.clone()]); + index.add_commit_data(id_5.clone(), change_id5, &[id_4.clone(), id_2.clone()]); let mut _saved_index = None; let index = if on_disk { _saved_index = Some(index.save_in(temp_dir.path().to_owned()).unwrap()); @@ -1662,16 +1662,16 @@ mod tests { let id_4 = CommitId::from_hex("444444"); let id_5 = CommitId::from_hex("555555"); let id_6 = CommitId::from_hex("666666"); - index.add_commit_data(id_0.clone(), new_change_id(), vec![]); - index.add_commit_data(id_1.clone(), new_change_id(), vec![id_0.clone()]); - index.add_commit_data(id_2.clone(), new_change_id(), vec![id_0.clone()]); - index.add_commit_data(id_3.clone(), new_change_id(), vec![id_0.clone()]); - index.add_commit_data(id_4.clone(), new_change_id(), vec![id_0.clone()]); - index.add_commit_data(id_5.clone(), new_change_id(), vec![id_0]); + index.add_commit_data(id_0.clone(), new_change_id(), &[]); + index.add_commit_data(id_1.clone(), new_change_id(), &[id_0.clone()]); + index.add_commit_data(id_2.clone(), new_change_id(), &[id_0.clone()]); + index.add_commit_data(id_3.clone(), new_change_id(), &[id_0.clone()]); + index.add_commit_data(id_4.clone(), new_change_id(), &[id_0.clone()]); + index.add_commit_data(id_5.clone(), new_change_id(), &[id_0]); index.add_commit_data( id_6.clone(), new_change_id(), - vec![id_1, id_2, id_3, id_4, id_5], + &[id_1, id_2, id_3, id_4, id_5], ); let mut _saved_index = None; let index = if on_disk { @@ -1714,9 +1714,9 @@ mod tests { let id_0 = CommitId::from_hex("000000"); let id_1 = CommitId::from_hex("009999"); let id_2 = CommitId::from_hex("055488"); - index.add_commit_data(id_0.clone(), new_change_id(), vec![]); - index.add_commit_data(id_1.clone(), new_change_id(), vec![]); - index.add_commit_data(id_2.clone(), new_change_id(), vec![]); + index.add_commit_data(id_0.clone(), new_change_id(), &[]); + index.add_commit_data(id_1.clone(), new_change_id(), &[]); + index.add_commit_data(id_2.clone(), new_change_id(), &[]); // Write the first three commits to one file and build the remainder on top. let initial_file = index.save_in(temp_dir.path().to_owned()).unwrap(); @@ -1725,9 +1725,9 @@ mod tests { let id_3 = CommitId::from_hex("055444"); let id_4 = CommitId::from_hex("055555"); let id_5 = CommitId::from_hex("033333"); - index.add_commit_data(id_3, new_change_id(), vec![]); - index.add_commit_data(id_4, new_change_id(), vec![]); - index.add_commit_data(id_5, new_change_id(), vec![]); + index.add_commit_data(id_3, new_change_id(), &[]); + index.add_commit_data(id_4, new_change_id(), &[]); + index.add_commit_data(id_5, new_change_id(), &[]); // Can find commits given the full hex number assert_eq!( @@ -1788,16 +1788,12 @@ mod tests { let id_3 = CommitId::from_hex("333333"); let id_4 = CommitId::from_hex("444444"); let id_5 = CommitId::from_hex("555555"); - index.add_commit_data(id_0.clone(), new_change_id(), vec![]); - index.add_commit_data(id_1.clone(), new_change_id(), vec![id_0.clone()]); - index.add_commit_data(id_2.clone(), new_change_id(), vec![id_0.clone()]); - index.add_commit_data(id_3.clone(), new_change_id(), vec![id_2.clone()]); - index.add_commit_data(id_4.clone(), new_change_id(), vec![id_1.clone()]); - index.add_commit_data( - id_5.clone(), - new_change_id(), - vec![id_4.clone(), id_2.clone()], - ); + index.add_commit_data(id_0.clone(), new_change_id(), &[]); + index.add_commit_data(id_1.clone(), new_change_id(), &[id_0.clone()]); + index.add_commit_data(id_2.clone(), new_change_id(), &[id_0.clone()]); + index.add_commit_data(id_3.clone(), new_change_id(), &[id_2.clone()]); + index.add_commit_data(id_4.clone(), new_change_id(), &[id_1.clone()]); + index.add_commit_data(id_5.clone(), new_change_id(), &[id_4.clone(), id_2.clone()]); assert!(index.is_ancestor(&id_0, &id_0)); assert!(index.is_ancestor(&id_0, &id_1)); @@ -1829,16 +1825,12 @@ mod tests { let id_3 = CommitId::from_hex("333333"); let id_4 = CommitId::from_hex("444444"); let id_5 = CommitId::from_hex("555555"); - index.add_commit_data(id_0.clone(), new_change_id(), vec![]); - index.add_commit_data(id_1.clone(), new_change_id(), vec![id_0.clone()]); - index.add_commit_data(id_2.clone(), new_change_id(), vec![id_0.clone()]); - index.add_commit_data(id_3.clone(), new_change_id(), vec![id_0.clone()]); - index.add_commit_data(id_4.clone(), new_change_id(), vec![id_1.clone()]); - index.add_commit_data( - id_5.clone(), - new_change_id(), - vec![id_4.clone(), id_2.clone()], - ); + index.add_commit_data(id_0.clone(), new_change_id(), &[]); + index.add_commit_data(id_1.clone(), new_change_id(), &[id_0.clone()]); + index.add_commit_data(id_2.clone(), new_change_id(), &[id_0.clone()]); + index.add_commit_data(id_3.clone(), new_change_id(), &[id_0.clone()]); + index.add_commit_data(id_4.clone(), new_change_id(), &[id_1.clone()]); + index.add_commit_data(id_5.clone(), new_change_id(), &[id_4.clone(), id_2.clone()]); assert_eq!( index.common_ancestors(&[id_0.clone()], &[id_0.clone()]), @@ -1910,19 +1902,11 @@ mod tests { let id_2 = CommitId::from_hex("222222"); let id_3 = CommitId::from_hex("333333"); let id_4 = CommitId::from_hex("444444"); - index.add_commit_data(id_0.clone(), new_change_id(), vec![]); - index.add_commit_data(id_1.clone(), new_change_id(), vec![id_0.clone()]); - index.add_commit_data(id_2.clone(), new_change_id(), vec![id_0]); - index.add_commit_data( - id_3.clone(), - new_change_id(), - vec![id_1.clone(), id_2.clone()], - ); - index.add_commit_data( - id_4.clone(), - new_change_id(), - vec![id_1.clone(), id_2.clone()], - ); + index.add_commit_data(id_0.clone(), new_change_id(), &[]); + index.add_commit_data(id_1.clone(), new_change_id(), &[id_0.clone()]); + index.add_commit_data(id_2.clone(), new_change_id(), &[id_0]); + index.add_commit_data(id_3.clone(), new_change_id(), &[id_1.clone(), id_2.clone()]); + index.add_commit_data(id_4.clone(), new_change_id(), &[id_1.clone(), id_2.clone()]); let mut common_ancestors = index.common_ancestors(&[id_3], &[id_4]); common_ancestors.sort(); @@ -1943,16 +1927,12 @@ mod tests { let id_3 = CommitId::from_hex("333333"); let id_4 = CommitId::from_hex("444444"); let id_5 = CommitId::from_hex("555555"); - index.add_commit_data(id_0.clone(), new_change_id(), vec![]); - index.add_commit_data(id_1, new_change_id(), vec![id_0.clone()]); - index.add_commit_data(id_2.clone(), new_change_id(), vec![id_0.clone()]); - index.add_commit_data(id_3, new_change_id(), vec![id_0.clone()]); - index.add_commit_data( - id_4.clone(), - new_change_id(), - vec![id_0.clone(), id_2.clone()], - ); - index.add_commit_data(id_5.clone(), new_change_id(), vec![id_0, id_2.clone()]); + index.add_commit_data(id_0.clone(), new_change_id(), &[]); + index.add_commit_data(id_1, new_change_id(), &[id_0.clone()]); + index.add_commit_data(id_2.clone(), new_change_id(), &[id_0.clone()]); + index.add_commit_data(id_3, new_change_id(), &[id_0.clone()]); + index.add_commit_data(id_4.clone(), new_change_id(), &[id_0.clone(), id_2.clone()]); + index.add_commit_data(id_5.clone(), new_change_id(), &[id_0, id_2.clone()]); let mut common_ancestors = index.common_ancestors(&[id_4], &[id_5]); common_ancestors.sort(); @@ -1975,16 +1955,12 @@ mod tests { let id_3 = CommitId::from_hex("333333"); let id_4 = CommitId::from_hex("444444"); let id_5 = CommitId::from_hex("555555"); - index.add_commit_data(id_0.clone(), new_change_id(), vec![]); - index.add_commit_data(id_1.clone(), new_change_id(), vec![id_0.clone()]); - index.add_commit_data(id_2.clone(), new_change_id(), vec![id_0.clone()]); - index.add_commit_data(id_3.clone(), new_change_id(), vec![id_2.clone()]); - index.add_commit_data(id_4.clone(), new_change_id(), vec![id_1.clone()]); - index.add_commit_data( - id_5.clone(), - new_change_id(), - vec![id_4.clone(), id_2.clone()], - ); + index.add_commit_data(id_0.clone(), new_change_id(), &[]); + index.add_commit_data(id_1.clone(), new_change_id(), &[id_0.clone()]); + index.add_commit_data(id_2.clone(), new_change_id(), &[id_0.clone()]); + index.add_commit_data(id_3.clone(), new_change_id(), &[id_2.clone()]); + index.add_commit_data(id_4.clone(), new_change_id(), &[id_1.clone()]); + index.add_commit_data(id_5.clone(), new_change_id(), &[id_4.clone(), id_2.clone()]); let walk_commit_ids = |wanted: &[CommitId], unwanted: &[CommitId]| { index @@ -2057,16 +2033,12 @@ mod tests { let id_3 = CommitId::from_hex("333333"); let id_4 = CommitId::from_hex("444444"); let id_5 = CommitId::from_hex("555555"); - index.add_commit_data(id_0.clone(), new_change_id(), vec![]); - index.add_commit_data(id_1.clone(), new_change_id(), vec![id_0.clone()]); - index.add_commit_data(id_2.clone(), new_change_id(), vec![id_0.clone()]); - index.add_commit_data(id_3.clone(), new_change_id(), vec![id_2.clone()]); - index.add_commit_data(id_4.clone(), new_change_id(), vec![id_1.clone()]); - index.add_commit_data( - id_5.clone(), - new_change_id(), - vec![id_4.clone(), id_2.clone()], - ); + index.add_commit_data(id_0.clone(), new_change_id(), &[]); + index.add_commit_data(id_1.clone(), new_change_id(), &[id_0.clone()]); + index.add_commit_data(id_2.clone(), new_change_id(), &[id_0.clone()]); + index.add_commit_data(id_3.clone(), new_change_id(), &[id_2.clone()]); + index.add_commit_data(id_4.clone(), new_change_id(), &[id_1.clone()]); + index.add_commit_data(id_5.clone(), new_change_id(), &[id_4.clone(), id_2.clone()]); // Empty input assert!(index.heads(&[]).is_empty()); diff --git a/lib/src/repo.rs b/lib/src/repo.rs index 2317a54d4..3863fac6c 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -649,7 +649,7 @@ impl MutableRepo { self.index.add_commit(head); self.view.get_mut().add_head(head.id()); for parent_id in head.parent_ids() { - self.view.get_mut().remove_head(&parent_id); + self.view.get_mut().remove_head(parent_id); } } else { let missing_commits = topo_order_reverse( diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs index 559843068..9c73d62d2 100644 --- a/lib/src/rewrite.rs +++ b/lib/src/rewrite.rs @@ -381,7 +381,7 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> { } let old_commit = self.mut_repo.store().get_commit(&old_commit_id)?; let old_parent_ids = old_commit.parent_ids(); - let new_parent_ids = self.new_parents(&old_parent_ids); + let new_parent_ids = self.new_parents(old_parent_ids); if self.abandoned.contains(&old_commit_id) { // Update the `new_parents` map so descendants are rebased correctly. self.new_parents diff --git a/src/commands.rs b/src/commands.rs index 81589ff28..9ad50a6a9 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -2846,7 +2846,7 @@ aborted. let description = combine_messages(ui, workspace_command.repo(), parent, &commit, true)?; // Commit the new child on top of the parent's parents. CommitBuilder::for_rewrite_from(ui.settings(), &commit) - .set_parents(parent.parent_ids()) + .set_parents(parent.parent_ids().to_vec()) .set_description(description) .write_to_repo(tx.mut_repo()); } else {