diff --git a/lib/src/commit.rs b/lib/src/commit.rs index 6d8341499..a165671e5 100644 --- a/lib/src/commit.rs +++ b/lib/src/commit.rs @@ -76,11 +76,7 @@ impl Commit { } pub fn parent_ids(&self) -> Vec { - if self.data.parents.is_empty() && &self.id != self.store.root_commit_id() { - vec![self.store.root_commit_id().clone()] - } else { - self.data.parents.clone() - } + self.data.parents.clone() } pub fn parents(&self) -> Vec { @@ -88,9 +84,6 @@ impl Commit { for parent in &self.data.parents { parents.push(self.store.get_commit(parent).unwrap()); } - if parents.is_empty() && &self.id != self.store.root_commit_id() { - parents.push(self.store.root_commit()) - } parents } diff --git a/lib/src/commit_builder.rs b/lib/src/commit_builder.rs index 46fc80da5..147c0684b 100644 --- a/lib/src/commit_builder.rs +++ b/lib/src/commit_builder.rs @@ -37,6 +37,7 @@ impl CommitBuilder { tree_id: TreeId, ) -> CommitBuilder { let signature = settings.signature(); + assert!(!parents.is_empty()); let commit = backend::Commit { parents, predecessors: vec![], @@ -94,6 +95,7 @@ impl CommitBuilder { } pub fn set_parents(mut self, parents: Vec) -> Self { + assert!(!parents.is_empty()); self.commit.parents = parents; self } @@ -138,12 +140,7 @@ impl CommitBuilder { self } - pub fn write_to_repo(mut self, repo: &mut MutableRepo) -> Commit { - let parents = &mut self.commit.parents; - if parents.contains(repo.store().root_commit_id()) { - assert_eq!(parents.len(), 1); - parents.clear(); - } + pub fn write_to_repo(self, repo: &mut MutableRepo) -> Commit { let mut rewrite_source_id = None; if let Some(rewrite_source) = self.rewrite_source { if *rewrite_source.change_id() == self.commit.change_id { diff --git a/lib/src/git_backend.rs b/lib/src/git_backend.rs index 2a49edaef..cb4a0744c 100644 --- a/lib/src/git_backend.rs +++ b/lib/src/git_backend.rs @@ -366,10 +366,13 @@ impl Backend for GitBackend { .map(|b| b.reverse_bits()) .collect(), ); - let parents = commit + let mut parents = commit .parent_ids() .map(|oid| CommitId::from_bytes(oid.as_bytes())) .collect_vec(); + if parents.is_empty() { + parents.push(self.root_commit_id.clone()); + }; let tree_id = TreeId::from_bytes(commit.tree_id().as_bytes()); let description = commit.message().unwrap_or("").to_owned(); let author = signature_from_git(commit.author()); @@ -406,9 +409,17 @@ impl Backend for GitBackend { let mut parents = vec![]; for parent_id in &contents.parents { - let parent_git_commit = - locked_repo.find_commit(Oid::from_bytes(parent_id.as_bytes())?)?; - parents.push(parent_git_commit); + if *parent_id == self.root_commit_id { + // Git doesn't have a root commit, so if the parent is the root commit, we don't + // add it to the list of parents to write in the Git commit. We also check that + // there are no other parents since Git cannot represent a merge between a root + // commit and another commit. + assert_eq!(contents.parents.len(), 1); + } else { + let parent_git_commit = + locked_repo.find_commit(Oid::from_bytes(parent_id.as_bytes())?)?; + parents.push(parent_git_commit); + } } let parent_refs = parents.iter().collect_vec(); let git_id = locked_repo.commit( @@ -573,7 +584,7 @@ mod tests { let store = GitBackend::init_external(store_path, git_repo_path); let commit = store.read_commit(&commit_id).unwrap(); assert_eq!(&commit.change_id, &change_id); - assert_eq!(commit.parents, vec![]); + assert_eq!(commit.parents, vec![CommitId::from_bytes(&[0; 20])]); assert_eq!(commit.predecessors, vec![]); assert_eq!(commit.root_tree.as_bytes(), root_tree_id.as_bytes()); assert!(!commit.is_open); diff --git a/lib/src/store.rs b/lib/src/store.rs index 944bf1a0c..ba8197f54 100644 --- a/lib/src/store.rs +++ b/lib/src/store.rs @@ -83,6 +83,7 @@ impl Store { } pub fn write_commit(self: &Arc, commit: backend::Commit) -> Commit { + assert!(!commit.parents.is_empty()); let commit_id = self.backend.write_commit(&commit).unwrap(); let data = Arc::new(commit); { diff --git a/lib/src/testutils.rs b/lib/src/testutils.rs index 1eeae0e1d..1b55fcff6 100644 --- a/lib/src/testutils.rs +++ b/lib/src/testutils.rs @@ -189,8 +189,12 @@ pub fn create_random_tree(repo: &ReadonlyRepo) -> TreeId { pub fn create_random_commit(settings: &UserSettings, repo: &ReadonlyRepo) -> CommitBuilder { let tree_id = create_random_tree(repo); let number = rand::random::(); - CommitBuilder::for_new_commit(settings, vec![], tree_id) - .set_description(format!("random commit {}", number)) + CommitBuilder::for_new_commit( + settings, + vec![repo.store().root_commit_id().clone()], + tree_id, + ) + .set_description(format!("random commit {}", number)) } pub fn write_working_copy_file(workspace_root: &Path, path: &RepoPath, contents: &str) { diff --git a/lib/tests/test_commit_builder.rs b/lib/tests/test_commit_builder.rs index 3288d8602..7d8b38634 100644 --- a/lib/tests/test_commit_builder.rs +++ b/lib/tests/test_commit_builder.rs @@ -170,7 +170,7 @@ fn test_rewrite_update_missing_user(use_git: bool) { let mut tx = repo.start_transaction("test"); let initial_commit = CommitBuilder::for_new_commit( &missing_user_settings, - vec![], + vec![repo.store().root_commit_id().clone()], repo.store().empty_tree_id().clone(), ) .write_to_repo(tx.mut_repo()); @@ -219,8 +219,12 @@ fn test_commit_builder_descendants(use_git: bool) { // Test with for_new_commit() let mut tx = repo.start_transaction("test"); - CommitBuilder::for_new_commit(&settings, vec![], store.empty_tree_id().clone()) - .write_to_repo(tx.mut_repo()); + CommitBuilder::for_new_commit( + &settings, + vec![store.root_commit_id().clone()], + store.empty_tree_id().clone(), + ) + .write_to_repo(tx.mut_repo()); let mut rebaser = tx.mut_repo().create_descendant_rebaser(&settings); assert!(rebaser.rebase_next().unwrap().is_none()); diff --git a/lib/tests/test_init.rs b/lib/tests/test_init.rs index 97fd17c6a..3e3d0eb3d 100644 --- a/lib/tests/test_init.rs +++ b/lib/tests/test_init.rs @@ -119,7 +119,10 @@ fn test_init_checkout(use_git: bool) { .unwrap(); let wc_commit = repo.store().get_commit(wc_commit_id).unwrap(); assert_eq!(wc_commit.tree_id(), repo.store().empty_tree_id()); - assert_eq!(wc_commit.store_commit().parents, vec![]); + assert_eq!( + wc_commit.store_commit().parents, + vec![repo.store().root_commit_id().clone()] + ); assert_eq!(wc_commit.predecessors(), vec![]); assert_eq!(wc_commit.description(), ""); assert!(wc_commit.is_open()); diff --git a/lib/tests/test_merge_trees.rs b/lib/tests/test_merge_trees.rs index 0d53746cd..2d3340c6a 100644 --- a/lib/tests/test_merge_trees.rs +++ b/lib/tests/test_merge_trees.rs @@ -573,8 +573,12 @@ fn test_simplify_conflict_after_resolving_parent(use_git: bool) { let path = RepoPath::from_internal_string("dir/file"); let mut tx = repo.start_transaction("test"); let tree_a = testutils::create_tree(repo, &[(&path, "abc\ndef\nghi\n")]); - let commit_a = CommitBuilder::for_new_commit(&settings, vec![], tree_a.id().clone()) - .write_to_repo(tx.mut_repo()); + let commit_a = CommitBuilder::for_new_commit( + &settings, + vec![repo.store().root_commit_id().clone()], + tree_a.id().clone(), + ) + .write_to_repo(tx.mut_repo()); let tree_b = testutils::create_tree(repo, &[(&path, "Abc\ndef\nghi\n")]); let commit_b = CommitBuilder::for_new_commit(&settings, vec![commit_a.id().clone()], tree_b.id().clone()) diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index c1864d833..1cc76a63f 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -55,12 +55,15 @@ fn test_resolve_symbol_commit_id() { let mut commits = vec![]; for i in &[1, 167, 895] { - let commit = - CommitBuilder::for_new_commit(&settings, vec![], repo.store().empty_tree_id().clone()) - .set_description(format!("test {}", i)) - .set_author(signature.clone()) - .set_committer(signature.clone()) - .write_to_repo(mut_repo); + let commit = CommitBuilder::for_new_commit( + &settings, + vec![repo.store().root_commit_id().clone()], + repo.store().empty_tree_id().clone(), + ) + .set_description(format!("test {}", i)) + .set_author(signature.clone()) + .set_committer(signature.clone()) + .write_to_repo(mut_repo); commits.push(commit); } let repo = tx.commit(); @@ -1755,8 +1758,12 @@ fn test_filter_by_diff(use_git: bool) { // added_modified_removed, ], ); - let commit1 = CommitBuilder::for_new_commit(&settings, vec![], tree1.id().clone()) - .write_to_repo(mut_repo); + let commit1 = CommitBuilder::for_new_commit( + &settings, + vec![repo.store().root_commit_id().clone()], + tree1.id().clone(), + ) + .write_to_repo(mut_repo); let commit2 = CommitBuilder::for_new_commit(&settings, vec![commit1.id().clone()], tree2.id().clone()) .write_to_repo(mut_repo); diff --git a/lib/tests/test_rewrite.rs b/lib/tests/test_rewrite.rs index e08f7be84..ae7426850 100644 --- a/lib/tests/test_rewrite.rs +++ b/lib/tests/test_rewrite.rs @@ -849,8 +849,12 @@ fn test_rebase_descendants_contents(use_git: bool) { let mut tx = repo.start_transaction("test"); let path1 = RepoPath::from_internal_string("file1"); let tree1 = testutils::create_tree(repo, &[(&path1, "content")]); - let commit_a = CommitBuilder::for_new_commit(&settings, vec![], tree1.id().clone()) - .write_to_repo(tx.mut_repo()); + let commit_a = CommitBuilder::for_new_commit( + &settings, + vec![repo.store().root_commit_id().clone()], + tree1.id().clone(), + ) + .write_to_repo(tx.mut_repo()); let path2 = RepoPath::from_internal_string("file2"); let tree2 = testutils::create_tree(repo, &[(&path2, "content")]); let commit_b =