From d5a98df046d6f5feecab9be07e997145ac582624 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Thu, 7 Sep 2023 17:39:31 +0900 Subject: [PATCH] git_backend: teach "format.tree-level-conflicts" config by constructor Since GitBackend constructors now depend on &UserSettings, it makes sense to initialize the formatting options there. --- lib/src/git.rs | 6 ++---- lib/src/git_backend.rs | 47 +++++++++++++++++++++++++++++------------- lib/tests/test_git.rs | 10 ++------- 3 files changed, 37 insertions(+), 26 deletions(-) diff --git a/lib/src/git.rs b/lib/src/git.rs index 2ce1099c1..8088897c5 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -267,16 +267,14 @@ pub fn import_some_refs( // but such targets should have already been imported to the backend. ) .flat_map(|target| target.added_ids()); - let heads_imported = git_backend - .import_head_commits(head_ids, store.use_tree_conflict_format()) - .is_ok(); + let heads_imported = git_backend.import_head_commits(head_ids).is_ok(); // Import new remote heads 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], store.use_tree_conflict_format())?; + git_backend.import_head_commits([id])?; } store.get_commit(id) }; diff --git a/lib/src/git_backend.rs b/lib/src/git_backend.rs index b7806cb45..110dc02b2 100644 --- a/lib/src/git_backend.rs +++ b/lib/src/git_backend.rs @@ -116,6 +116,8 @@ pub struct GitBackend { empty_tree_id: TreeId, extra_metadata_store: TableStore, cached_extra_metadata: Mutex>>, + /// Whether tree of imported commit should be promoted to non-legacy format. + imported_commit_uses_tree_conflict_format: bool, } impl GitBackend { @@ -123,7 +125,11 @@ impl GitBackend { "git" } - fn new(base_repo: gix::ThreadSafeRepository, extra_metadata_store: TableStore) -> Self { + fn new( + base_repo: gix::ThreadSafeRepository, + extra_metadata_store: TableStore, + imported_commit_uses_tree_conflict_format: bool, + ) -> Self { let repo = Mutex::new(base_repo.to_thread_local()); let root_commit_id = CommitId::from_bytes(&[0; HASH_LENGTH]); let root_change_id = ChangeId::from_bytes(&[0; CHANGE_ID_LENGTH]); @@ -136,6 +142,7 @@ impl GitBackend { empty_tree_id, extra_metadata_store, cached_extra_metadata: Mutex::new(None), + imported_commit_uses_tree_conflict_format, } } @@ -151,7 +158,7 @@ impl GitBackend { gix_open_opts_from_settings(settings), ) .map_err(GitBackendInitError::InitRepository)?; - Self::init_with_repo(store_path, git_repo_path, git_repo) + Self::init_with_repo(settings, store_path, git_repo_path, git_repo) } /// Initializes backend by creating a new Git repo at the specified @@ -175,7 +182,7 @@ impl GitBackend { ) .map_err(GitBackendInitError::InitRepository)?; let git_repo_path = workspace_root.join(".git"); - Self::init_with_repo(store_path, &git_repo_path, git_repo) + Self::init_with_repo(settings, store_path, &git_repo_path, git_repo) } /// Initializes backend with an existing Git repo at the specified path. @@ -195,10 +202,11 @@ impl GitBackend { gix_open_opts_from_settings(settings), ) .map_err(GitBackendInitError::OpenRepository)?; - Self::init_with_repo(store_path, git_repo_path, git_repo) + Self::init_with_repo(settings, store_path, git_repo_path, git_repo) } fn init_with_repo( + settings: &UserSettings, store_path: &Path, git_repo_path: &Path, git_repo: gix::ThreadSafeRepository, @@ -228,7 +236,11 @@ impl GitBackend { .map_err(GitBackendInitError::Path)?; }; let extra_metadata_store = TableStore::init(extra_path, HASH_LENGTH); - Ok(GitBackend::new(git_repo, extra_metadata_store)) + Ok(GitBackend::new( + git_repo, + extra_metadata_store, + settings.use_tree_conflict_format(), + )) } pub fn load( @@ -251,7 +263,11 @@ impl GitBackend { ) .map_err(GitBackendLoadError::OpenRepository)?; let extra_metadata_store = TableStore::load(store_path.join("extra"), HASH_LENGTH); - Ok(GitBackend::new(repo, extra_metadata_store)) + Ok(GitBackend::new( + repo, + extra_metadata_store, + settings.use_tree_conflict_format(), + )) } fn lock_git_repo(&self) -> MutexGuard<'_, gix::Repository> { @@ -321,7 +337,6 @@ 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 @@ -348,7 +363,7 @@ impl GitBackend { &mut mut_table, &table_lock, &missing_head_ids, - uses_tree_conflict_format, + self.imported_commit_uses_tree_conflict_format, )?; for &id in &missing_head_ids { prevent_gc(&locked_repo, id)?; @@ -919,8 +934,7 @@ impl Backend for GitBackend { // imported by jj < 0.8.0 might not have extras (#924). // https://github.com/martinvonz/jj/issues/2343 tracing::info!("unimported Git commit found"); - let uses_tree_conflict_format = false; - self.import_head_commits([id], uses_tree_conflict_format)?; + self.import_head_commits([id])?; let table = self.cached_extra_metadata_table()?; let extras = table.get_value(id.as_bytes()).unwrap(); deserialize_extras(&mut commit, extras); @@ -1167,7 +1181,14 @@ mod tests { #[test_case(false; "legacy tree format")] #[test_case(true; "tree-level conflict format")] fn read_plain_git_commit(uses_tree_conflict_format: bool) { - let settings = user_settings(); + let settings = { + let config = config::Config::builder() + .set_override("format.tree-level-conflicts", uses_tree_conflict_format) + .unwrap() + .build() + .unwrap(); + UserSettings::from_config(config) + }; let temp_dir = testutils::new_temp_dir(); let store_path = temp_dir.path(); let git_repo_path = temp_dir.path().join("git"); @@ -1230,9 +1251,7 @@ mod tests { let backend = GitBackend::init_external(&settings, store_path, git_repo.path()).unwrap(); // Import the head commit and its ancestors - backend - .import_head_commits([&commit_id2], uses_tree_conflict_format) - .unwrap(); + backend.import_head_commits([&commit_id2]).unwrap(); // Ref should be created only for the head commit let git_refs = backend .open_git_repo() diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index 75b29deb6..ada243d0d 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -2310,10 +2310,7 @@ 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)], - jj_repo.store().use_tree_conflict_format(), - ) + .import_head_commits(&[jj_id(&initial_git_commit)]) .unwrap(); let initial_commit = jj_repo .store() @@ -2909,10 +2906,7 @@ 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], - repo.store().use_tree_conflict_format(), - ) { + 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();