From b72665728bd86bd3be1397dc27a7af296c6c79e2 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Thu, 7 Sep 2023 19:43:07 +0900 Subject: [PATCH 1/4] git: do not import unknown commits by read_commit(), use explicit method call The main goal of this change is to enable tree-level conflict format, but it also allows us to bulk-import commits on clone/init. I think a separate method will help if we want to provide progress information, enable check for .jjconflict entries under certain condition, etc. Since git::import_refs() now depends on GitBackend type, it might be better to remove git_repo from the function arguments. --- lib/src/git.rs | 31 ++++++----- lib/src/git_backend.rs | 113 +++++++++++++++++++++++++++-------------- lib/tests/test_git.rs | 16 ++++-- 3 files changed, 104 insertions(+), 56 deletions(-) diff --git a/lib/src/git.rs b/lib/src/git.rs index 30cd6aa1bc..93ad6c6039 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -26,7 +26,7 @@ use tempfile::NamedTempFile; use thiserror::Error; use crate::backend::{BackendError, CommitId, ObjectId}; -use crate::git_backend::NO_GC_REF_NAMESPACE; +use crate::git_backend::{GitBackend, NO_GC_REF_NAMESPACE}; use crate::op_store::{BranchTarget, RefTarget, RefTargetOptionExt}; use crate::repo::{MutableRepo, Repo}; use crate::revset; @@ -228,27 +228,30 @@ pub fn import_some_refs( // Import new heads let store = mut_repo.store(); + // TODO: It might be better to obtain both git_repo and git_backend from + // mut_repo, and return error if the repo isn't backed by Git. + let git_backend = store.backend_impl().downcast_ref::().unwrap(); let mut head_commits = Vec::new(); + // TODO: Import commits in bulk (but we'll need to adjust error handling.) + let get_commit = |id| { + git_backend.import_head_commits([id])?; + store.get_commit(id) + }; if let Some(new_head_target) = &changed_git_head { for id in new_head_target.added_ids() { - let commit = store - .get_commit(id) - .map_err(|err| GitImportError::MissingHeadTarget { - id: id.clone(), - err, - })?; + let commit = get_commit(id).map_err(|err| GitImportError::MissingHeadTarget { + id: id.clone(), + err, + })?; head_commits.push(commit); } } for (ref_name, (_, new_git_target)) in &changed_git_refs { for id in new_git_target.added_ids() { - let commit = - store - .get_commit(id) - .map_err(|err| GitImportError::MissingRefAncestor { - ref_name: ref_name.to_string(), - err, - })?; + let commit = get_commit(id).map_err(|err| GitImportError::MissingRefAncestor { + ref_name: ref_name.to_string(), + err, + })?; head_commits.push(commit); } } diff --git a/lib/src/git_backend.rs b/lib/src/git_backend.rs index 8679ad516f..ed04364501 100644 --- a/lib/src/git_backend.rs +++ b/lib/src/git_backend.rs @@ -16,11 +16,11 @@ use std::any::Any; use std::fmt::{Debug, Error, Formatter}; +use std::fs; use std::io::{Cursor, Read}; use std::ops::Deref; use std::path::Path; use std::sync::{Arc, Mutex, MutexGuard}; -use std::{fs, slice}; use git2::Oid; use itertools::Itertools; @@ -200,6 +200,41 @@ impl GitBackend { *self.cached_extra_metadata.lock().unwrap() = Some(table); Ok(()) } + + /// Imports the given commits and ancestors from the backing Git repo. + #[tracing::instrument(skip(self, head_ids))] + pub fn import_head_commits<'a>( + &self, + head_ids: impl IntoIterator, + ) -> BackendResult<()> { + let table = self.cached_extra_metadata_table()?; + let mut missing_head_ids = head_ids + .into_iter() + .filter(|&id| *id != self.root_commit_id && table.get_value(id.as_bytes()).is_none()) + .collect_vec(); + if missing_head_ids.is_empty() { + return Ok(()); + } + + // These commits are imported from Git. Make our change ids persist (otherwise + // future write_commit() could reassign new change id.) + tracing::debug!( + heads_count = missing_head_ids.len(), + "import extra metadata entries" + ); + let locked_repo = self.repo.lock().unwrap(); + let (table, table_lock) = self.read_extra_metadata_table_locked()?; + let mut mut_table = table.start_mutation(); + // Concurrent write_commit() might have updated the table before taking a lock. + missing_head_ids.retain(|&id| mut_table.get_value(id.as_bytes()).is_none()); + import_extra_metadata_entries_from_heads( + &locked_repo, + &mut mut_table, + &table_lock, + &missing_head_ids, + )?; + self.save_extra_metadata_table(mut_table, &table_lock) + } } fn commit_from_git_without_root_parent(commit: &git2::Commit) -> Commit { @@ -373,17 +408,16 @@ fn import_extra_metadata_entries_from_heads( git_repo: &git2::Repository, mut_table: &mut MutableTable, _table_lock: &FileLock, - head_ids: &[CommitId], + missing_head_ids: &[&CommitId], ) -> BackendResult<()> { - let mut work_ids = head_ids - .iter() - .filter(|id| mut_table.get_value(id.as_bytes()).is_none()) - .cloned() - .collect_vec(); + let mut work_ids = missing_head_ids.iter().map(|&id| id.clone()).collect_vec(); while let Some(id) = work_ids.pop() { let git_commit = git_repo .find_commit(validate_git_object_id(&id)?) .map_err(|err| map_not_found_err(err, &id))?; + // TODO(#1624): Should we read the root tree here and check if it has a + // `.jjconflict-...` entries? That could happen if the user used `git` to e.g. + // change the description of a commit with tree-level conflicts. let commit = commit_from_git_without_root_parent(&git_commit); mut_table.add_entry(id.to_bytes(), serialize_extras(&commit)); work_ids.extend( @@ -614,7 +648,6 @@ impl Backend for GitBackend { Ok(ConflictId::from_bytes(oid.as_bytes())) } - #[tracing::instrument(skip(self))] fn read_commit(&self, id: &CommitId) -> BackendResult { if *id == self.root_commit_id { return Ok(make_root_commit( @@ -634,36 +667,15 @@ impl Backend for GitBackend { }; let table = self.cached_extra_metadata_table()?; - if let Some(extras) = table.get_value(id.as_bytes()) { - deserialize_extras(&mut commit, extras); - } else { - let (table, table_lock) = self.read_extra_metadata_table_locked()?; - if let Some(extras) = table.get_value(id.as_bytes()) { - // Concurrent write_commit() would update extras before taking a lock. - deserialize_extras(&mut commit, extras); - *self.cached_extra_metadata.lock().unwrap() = Some(table); - } else { - // This commit is imported from Git. Make our change id persist (otherwise - // future write_commit() could reassign new change id.) It's likely that - // the commit is a branch head, so bulk-import metadata as much as possible. - tracing::debug!("import extra metadata entries"); - let mut mut_table = table.start_mutation(); - // TODO(#1624): Should we read the root tree here and check if it has a - // `.jjconflict-...` entries? That could happen if the user used `git` to e.g. - // change the description of a commit with tree-level conflicts. - mut_table.add_entry(id.to_bytes(), serialize_extras(&commit)); - if commit.parents != slice::from_ref(&self.root_commit_id) { - import_extra_metadata_entries_from_heads( - &locked_repo, - &mut mut_table, - &table_lock, - &commit.parents, - )?; - } - self.save_extra_metadata_table(mut_table, &table_lock)?; - } - } - + let extras = + table + .get_value(id.as_bytes()) + .ok_or_else(|| BackendError::ObjectNotFound { + object_type: id.object_type(), + hash: id.hex(), + source: "Not imported from Git".into(), + })?; + deserialize_extras(&mut commit, extras); Ok(commit) } @@ -914,7 +926,24 @@ mod tests { // Check that the git commit above got the hash we expect assert_eq!(git_commit_id.as_bytes(), commit_id.as_bytes()); + // Add an empty commit on top + let git_commit_id2 = git_repo + .commit( + None, + &git_author, + &git_committer, + "git commit message 2", + &git_tree, + &[&git_repo.find_commit(git_commit_id).unwrap()], + ) + .unwrap(); + let commit_id2 = CommitId::from_bytes(git_commit_id2.as_bytes()); + let store = GitBackend::init_external(store_path, &git_repo_path).unwrap(); + + // Import the head commit and its ancestors + store.import_head_commits([&commit_id2]).unwrap(); + let commit = store.read_commit(&commit_id).unwrap(); assert_eq!(&commit.change_id, &change_id); assert_eq!(commit.parents, vec![CommitId::from_bytes(&[0; 20])]); @@ -977,6 +1006,14 @@ mod tests { symlink.value(), &TreeValue::Symlink(SymlinkId::from_bytes(blob2.as_bytes())) ); + + let commit2 = store.read_commit(&commit_id2).unwrap(); + assert_eq!(commit2.parents, vec![commit_id.clone()]); + assert_eq!(commit.predecessors, vec![]); + assert_eq!( + commit.root_tree, + MergedTreeId::Legacy(TreeId::from_bytes(root_tree_id.as_bytes())) + ); } #[test] diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index a0b63f2174..2eed1c61cf 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -70,12 +70,15 @@ fn git_id(commit: &Commit) -> Oid { Oid::from_bytes(commit.id().as_bytes()).unwrap() } -fn get_git_repo(repo: &Arc) -> git2::Repository { +fn get_git_backend(repo: &Arc) -> &GitBackend { repo.store() .backend_impl() .downcast_ref::() .unwrap() - .git_repo_clone() +} + +fn get_git_repo(repo: &Arc) -> git2::Repository { + get_git_backend(repo).git_repo_clone() } #[test] @@ -1851,6 +1854,9 @@ fn set_up_push_repos(settings: &UserSettings, temp_dir: &TempDir) -> PushTestSet ReadonlyRepo::default_submodule_store_factory(), ) .unwrap(); + get_git_backend(&jj_repo) + .import_head_commits(&[jj_id(&initial_git_commit)]) + .unwrap(); let mut tx = jj_repo.start_transaction(settings, "test"); let new_commit = create_random_commit(tx.mut_repo(), settings) .set_parents(vec![jj_id(&initial_git_commit)]) @@ -2278,13 +2284,15 @@ fn test_concurrent_read_write_commit() { barrier.wait(); while !pending_commit_ids.is_empty() { repo = repo.reload_at_head(settings).unwrap(); + let git_backend = get_git_backend(&repo); let mut tx = repo.start_transaction(settings, &format!("reader {i}")); pending_commit_ids = pending_commit_ids .into_iter() .filter_map(|commit_id| { - match repo.store().get_commit(&commit_id) { - Ok(commit) => { + match git_backend.import_head_commits([&commit_id]) { + Ok(()) => { // update index as git::import_refs() would do + let commit = repo.store().get_commit(&commit_id).unwrap(); tx.mut_repo().add_head(&commit); None } From fb1ab169a58c392fb312d1a11a9e9f119a92361b Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Fri, 8 Sep 2023 16:29:09 +0900 Subject: [PATCH 2/4] git: create no-gc ref by GitBackend Since we now have an explicit method to import heads, it makes more sense to manage no-gc refs by the backend. --- lib/src/git.rs | 17 +---------------- lib/src/git_backend.rs | 25 ++++++++++++++++++++++++- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/lib/src/git.rs b/lib/src/git.rs index 93ad6c6039..d6066f8648 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -26,7 +26,7 @@ use tempfile::NamedTempFile; use thiserror::Error; use crate::backend::{BackendError, CommitId, ObjectId}; -use crate::git_backend::{GitBackend, NO_GC_REF_NAMESPACE}; +use crate::git_backend::GitBackend; use crate::op_store::{BranchTarget, RefTarget, RefTargetOptionExt}; use crate::repo::{MutableRepo, Repo}; use crate::revset; @@ -172,18 +172,6 @@ pub fn get_local_git_tracking_branch<'a>(view: &'a View, branch: &str) -> &'a Re view.get_git_ref(&format!("refs/heads/{branch}")) } -fn prevent_gc(git_repo: &git2::Repository, id: &CommitId) -> Result<(), git2::Error> { - // If multiple processes do git::import_refs() in parallel, this can fail to - // acquire a lock file even with force=true. - git_repo.reference( - &format!("{}{}", NO_GC_REF_NAMESPACE, id.hex()), - Oid::from_bytes(id.as_bytes()).unwrap(), - true, - "used by jj", - )?; - Ok(()) -} - /// Reflect changes made in the underlying Git repo in the Jujutsu repo. /// /// This function detects conflicts (if both Git and JJ modified a branch) and @@ -255,9 +243,6 @@ pub fn import_some_refs( head_commits.push(commit); } } - for commit in &head_commits { - prevent_gc(git_repo, commit.id())?; - } mut_repo.add_heads(&head_commits); // Apply the change that happened in git since last time we imported refs. diff --git a/lib/src/git_backend.rs b/lib/src/git_backend.rs index ed04364501..09a032de06 100644 --- a/lib/src/git_backend.rs +++ b/lib/src/git_backend.rs @@ -43,7 +43,7 @@ use crate::stacked_table::{ const HASH_LENGTH: usize = 20; const CHANGE_ID_LENGTH: usize = 16; /// Ref namespace used only for preventing GC. -pub const NO_GC_REF_NAMESPACE: &str = "refs/jj/keep/"; +const NO_GC_REF_NAMESPACE: &str = "refs/jj/keep/"; const CONFLICT_SUFFIX: &str = ".jjconflict"; #[derive(Debug, Error)] @@ -233,6 +233,9 @@ impl GitBackend { &table_lock, &missing_head_ids, )?; + for &id in &missing_head_ids { + prevent_gc(&locked_repo, id)?; + } self.save_extra_metadata_table(mut_table, &table_lock) } } @@ -371,6 +374,18 @@ fn create_no_gc_ref() -> String { format!("{NO_GC_REF_NAMESPACE}{}", hex::encode(random_bytes)) } +fn prevent_gc(git_repo: &git2::Repository, id: &CommitId) -> Result<(), BackendError> { + git_repo + .reference( + &format!("{NO_GC_REF_NAMESPACE}{}", id.hex()), + Oid::from_bytes(id.as_bytes()).unwrap(), + true, + "used by jj", + ) + .map_err(|err| BackendError::Other(Box::new(err)))?; + Ok(()) +} + fn validate_git_object_id(id: &impl ObjectId) -> Result { if id.as_bytes().len() != HASH_LENGTH { return Err(BackendError::InvalidHashLength { @@ -943,6 +958,14 @@ mod tests { // Import the head commit and its ancestors store.import_head_commits([&commit_id2]).unwrap(); + // Ref should be created only for the head commit + let git_refs = store + .git_repo() + .references_glob("refs/jj/keep/*") + .unwrap() + .map(|git_ref| git_ref.unwrap().target().unwrap()) + .collect_vec(); + assert_eq!(git_refs, vec![git_commit_id2]); let commit = store.read_commit(&commit_id).unwrap(); assert_eq!(&commit.change_id, &change_id); From 3a1720459d0858d8226756d297c976d9c496b0ae Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Fri, 8 Sep 2023 11:05:54 +0900 Subject: [PATCH 3/4] git: import new head commits all at once --- lib/src/git.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/src/git.rs b/lib/src/git.rs index d6066f8648..45505efd5b 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -219,10 +219,19 @@ pub fn import_some_refs( // TODO: It might be better to obtain both git_repo and git_backend from // mut_repo, and return error if the repo isn't backed by Git. let git_backend = store.backend_impl().downcast_ref::().unwrap(); + // Bulk-import all reachable commits to reduce overhead of table merging. + let head_ids = itertools::chain( + &changed_git_head, + changed_git_refs.values().map(|(_, new_target)| new_target), + ) + .flat_map(|target| target.added_ids()); + let heads_imported = git_backend.import_head_commits(head_ids).is_ok(); let mut head_commits = Vec::new(); - // TODO: Import commits in bulk (but we'll need to adjust error handling.) let get_commit = |id| { - git_backend.import_head_commits([id])?; + // If bulk-import failed, try again to find bad head or ref. + if !heads_imported { + git_backend.import_head_commits([id])?; + } store.get_commit(id) }; if let Some(new_head_target) = &changed_git_head { From 445a8f7dd8c03042ac3e6c08c7850d99480e185f Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Thu, 7 Sep 2023 22:11:51 +0900 Subject: [PATCH 4/4] git: promote imported commits to tree-conflict format if configured --- lib/src/git.rs | 6 ++++-- lib/src/git_backend.rs | 46 ++++++++++++++++++++++++++++++++---------- lib/tests/test_git.rs | 10 +++++++-- 3 files changed, 47 insertions(+), 15 deletions(-) diff --git a/lib/src/git.rs b/lib/src/git.rs index 45505efd5b..55caeae1d5 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -225,12 +225,14 @@ pub fn import_some_refs( changed_git_refs.values().map(|(_, new_target)| new_target), ) .flat_map(|target| target.added_ids()); - let heads_imported = git_backend.import_head_commits(head_ids).is_ok(); + let heads_imported = git_backend + .import_head_commits(head_ids, store.use_tree_conflict_format()) + .is_ok(); let mut head_commits = Vec::new(); let get_commit = |id| { // If bulk-import failed, try again to find bad head or ref. if !heads_imported { - git_backend.import_head_commits([id])?; + git_backend.import_head_commits([id], store.use_tree_conflict_format())?; } store.get_commit(id) }; diff --git a/lib/src/git_backend.rs b/lib/src/git_backend.rs index 09a032de06..bac37ffe40 100644 --- a/lib/src/git_backend.rs +++ b/lib/src/git_backend.rs @@ -206,6 +206,7 @@ impl GitBackend { pub fn import_head_commits<'a>( &self, head_ids: impl IntoIterator, + uses_tree_conflict_format: bool, ) -> BackendResult<()> { let table = self.cached_extra_metadata_table()?; let mut missing_head_ids = head_ids @@ -232,6 +233,7 @@ impl GitBackend { &mut mut_table, &table_lock, &missing_head_ids, + uses_tree_conflict_format, )?; for &id in &missing_head_ids { prevent_gc(&locked_repo, id)?; @@ -240,7 +242,10 @@ impl GitBackend { } } -fn commit_from_git_without_root_parent(commit: &git2::Commit) -> Commit { +fn commit_from_git_without_root_parent( + commit: &git2::Commit, + uses_tree_conflict_format: bool, +) -> Commit { // We reverse the bits of the commit id to create the change id. We don't want // to use the first bytes unmodified because then it would be ambiguous // if a given hash prefix refers to the commit id or the change id. It @@ -261,7 +266,11 @@ fn commit_from_git_without_root_parent(commit: &git2::Commit) -> Commit { let tree_id = TreeId::from_bytes(commit.tree_id().as_bytes()); // If this commit is a conflict, we'll update the root tree later, when we read // the extra metadata. - let root_tree = MergedTreeId::Legacy(tree_id); + let root_tree = if uses_tree_conflict_format { + MergedTreeId::resolved(tree_id) + } else { + MergedTreeId::Legacy(tree_id) + }; let description = commit.message().unwrap_or("").to_owned(); let author = signature_from_git(commit.author()); let committer = signature_from_git(commit.committer()); @@ -424,6 +433,7 @@ fn import_extra_metadata_entries_from_heads( mut_table: &mut MutableTable, _table_lock: &FileLock, missing_head_ids: &[&CommitId], + uses_tree_conflict_format: bool, ) -> BackendResult<()> { let mut work_ids = missing_head_ids.iter().map(|&id| id.clone()).collect_vec(); while let Some(id) = work_ids.pop() { @@ -433,7 +443,7 @@ fn import_extra_metadata_entries_from_heads( // TODO(#1624): Should we read the root tree here and check if it has a // `.jjconflict-...` entries? That could happen if the user used `git` to e.g. // change the description of a commit with tree-level conflicts. - let commit = commit_from_git_without_root_parent(&git_commit); + let commit = commit_from_git_without_root_parent(&git_commit, uses_tree_conflict_format); mut_table.add_entry(id.to_bytes(), serialize_extras(&commit)); work_ids.extend( commit @@ -676,7 +686,7 @@ impl Backend for GitBackend { let commit = locked_repo .find_commit(git_commit_id) .map_err(|err| map_not_found_err(err, id))?; - let mut commit = commit_from_git_without_root_parent(&commit); + let mut commit = commit_from_git_without_root_parent(&commit, false); if commit.parents.is_empty() { commit.parents.push(self.root_commit_id.clone()); }; @@ -889,12 +899,14 @@ fn bytes_vec_from_json(value: &serde_json::Value) -> Vec { #[cfg(test)] mod tests { use assert_matches::assert_matches; + use test_case::test_case; use super::*; use crate::backend::{FileId, MillisSinceEpoch}; - #[test] - fn read_plain_git_commit() { + #[test_case(false; "legacy tree format")] + #[test_case(true; "tree-level conflict format")] + fn read_plain_git_commit(uses_tree_conflict_format: bool) { let temp_dir = testutils::new_temp_dir(); let store_path = temp_dir.path(); let git_repo_path = temp_dir.path().join("git"); @@ -957,7 +969,9 @@ mod tests { let store = GitBackend::init_external(store_path, &git_repo_path).unwrap(); // Import the head commit and its ancestors - store.import_head_commits([&commit_id2]).unwrap(); + store + .import_head_commits([&commit_id2], uses_tree_conflict_format) + .unwrap(); // Ref should be created only for the head commit let git_refs = store .git_repo() @@ -972,9 +986,14 @@ mod tests { assert_eq!(commit.parents, vec![CommitId::from_bytes(&[0; 20])]); assert_eq!(commit.predecessors, vec![]); assert_eq!( - commit.root_tree, - MergedTreeId::Legacy(TreeId::from_bytes(root_tree_id.as_bytes())) + commit.root_tree.to_merge(), + Merge::resolved(TreeId::from_bytes(root_tree_id.as_bytes())) ); + if uses_tree_conflict_format { + assert_matches!(commit.root_tree, MergedTreeId::Merge(_)); + } else { + assert_matches!(commit.root_tree, MergedTreeId::Legacy(_)); + } assert_eq!(commit.description, "git commit message"); assert_eq!(commit.author.name, "git author"); assert_eq!(commit.author.email, "git.author@example.com"); @@ -1034,9 +1053,14 @@ mod tests { assert_eq!(commit2.parents, vec![commit_id.clone()]); assert_eq!(commit.predecessors, vec![]); assert_eq!( - commit.root_tree, - MergedTreeId::Legacy(TreeId::from_bytes(root_tree_id.as_bytes())) + commit.root_tree.to_merge(), + Merge::resolved(TreeId::from_bytes(root_tree_id.as_bytes())) ); + if uses_tree_conflict_format { + assert_matches!(commit.root_tree, MergedTreeId::Merge(_)); + } else { + assert_matches!(commit.root_tree, MergedTreeId::Legacy(_)); + } } #[test] diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index 2eed1c61cf..e22bac5bd1 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -1855,7 +1855,10 @@ fn set_up_push_repos(settings: &UserSettings, temp_dir: &TempDir) -> PushTestSet ) .unwrap(); get_git_backend(&jj_repo) - .import_head_commits(&[jj_id(&initial_git_commit)]) + .import_head_commits( + &[jj_id(&initial_git_commit)], + jj_repo.store().use_tree_conflict_format(), + ) .unwrap(); let mut tx = jj_repo.start_transaction(settings, "test"); let new_commit = create_random_commit(tx.mut_repo(), settings) @@ -2289,7 +2292,10 @@ fn test_concurrent_read_write_commit() { pending_commit_ids = pending_commit_ids .into_iter() .filter_map(|commit_id| { - match git_backend.import_head_commits([&commit_id]) { + match git_backend.import_head_commits( + [&commit_id], + repo.store().use_tree_conflict_format(), + ) { Ok(()) => { // update index as git::import_refs() would do let commit = repo.store().get_commit(&commit_id).unwrap();