From 26581750fea5d65a69129abe4c13c1913760cdf3 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Wed, 30 Aug 2023 07:57:13 -0700 Subject: [PATCH] store: add a `empty_merged_tree_id()` helper Many (most?) callers of `Store::empty_tree_id()` really want a `MergedTreeId`, so let's create a helper for that. It returns the `Legacy` variant, which is what all current callers used. That should be all we need since the two variants compare equal these days, and since trees built based on the legacy variant can get promoted to the new variant on write if the config is enabled. --- lib/src/rewrite.rs | 4 ++-- lib/src/store.rs | 4 ++++ lib/src/working_copy.rs | 4 ++-- lib/tests/test_commit_builder.rs | 6 +++--- lib/tests/test_git.rs | 5 ++--- lib/tests/test_id_prefix.rs | 4 ++-- lib/tests/test_init.rs | 6 +----- lib/tests/test_merged_tree.rs | 3 +-- lib/tests/test_mut_repo.rs | 9 ++++----- lib/tests/test_revset.rs | 6 ++---- lib/tests/test_working_copy.rs | 20 ++++++-------------- 11 files changed, 29 insertions(+), 42 deletions(-) diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs index 8973dbc7b..042448aea 100644 --- a/lib/src/rewrite.rs +++ b/lib/src/rewrite.rs @@ -20,7 +20,7 @@ use std::sync::Arc; use itertools::{process_results, Itertools}; use tracing::instrument; -use crate::backend::{BackendError, CommitId, MergedTreeId, ObjectId}; +use crate::backend::{BackendError, CommitId, ObjectId}; use crate::commit::Commit; use crate::dag_walk; use crate::index::Index; @@ -48,7 +48,7 @@ pub fn merge_commit_trees_without_repo( commits: &[Commit], ) -> Result { if commits.is_empty() { - Ok(store.get_root_tree(&MergedTreeId::Legacy(store.empty_tree_id().clone()))?) + Ok(store.get_root_tree(&store.empty_merged_tree_id())?) } else { let mut new_tree = commits[0].tree()?; let commit_ids = commits diff --git a/lib/src/store.rs b/lib/src/store.rs index 34f34512d..98eb58384 100644 --- a/lib/src/store.rs +++ b/lib/src/store.rs @@ -72,6 +72,10 @@ impl Store { self.backend.empty_tree_id() } + pub fn empty_merged_tree_id(&self) -> MergedTreeId { + MergedTreeId::Legacy(self.backend.empty_tree_id().clone()) + } + pub fn root_commit_id(&self) -> &CommitId { self.backend.root_commit_id() } diff --git a/lib/src/working_copy.rs b/lib/src/working_copy.rs index 828ef5b3a..1eab8df5f 100644 --- a/lib/src/working_copy.rs +++ b/lib/src/working_copy.rs @@ -455,14 +455,14 @@ impl TreeState { } fn empty(store: Arc, working_copy_path: PathBuf, state_path: PathBuf) -> TreeState { - let tree_id = store.empty_tree_id().clone(); + let tree_id = store.empty_merged_tree_id(); // Canonicalize the working copy path because "repo/." makes libgit2 think that // everything should be ignored TreeState { store, working_copy_path: working_copy_path.canonicalize().unwrap(), state_path, - tree_id: MergedTreeId::Legacy(tree_id.clone()), + tree_id, file_states: BTreeMap::new(), sparse_patterns: vec![RepoPath::root()], own_mtime: MillisSinceEpoch(0), diff --git a/lib/tests/test_commit_builder.rs b/lib/tests/test_commit_builder.rs index 98f6e470c..c7f38a5aa 100644 --- a/lib/tests/test_commit_builder.rs +++ b/lib/tests/test_commit_builder.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use jj_lib::backend::{ChangeId, MergedTreeId, MillisSinceEpoch, ObjectId, Signature, Timestamp}; +use jj_lib::backend::{ChangeId, MillisSinceEpoch, ObjectId, Signature, Timestamp}; use jj_lib::matchers::EverythingMatcher; use jj_lib::merged_tree::DiffSummary; use jj_lib::repo::Repo; @@ -202,7 +202,7 @@ fn test_rewrite_update_missing_user(use_git: bool) { .new_commit( &missing_user_settings, vec![repo.store().root_commit_id().clone()], - MergedTreeId::Legacy(repo.store().empty_tree_id().clone()), + repo.store().empty_merged_tree_id(), ) .write() .unwrap(); @@ -258,7 +258,7 @@ fn test_commit_builder_descendants(use_git: bool) { .new_commit( &settings, vec![store.root_commit_id().clone()], - MergedTreeId::Legacy(store.empty_tree_id().clone()), + store.empty_merged_tree_id(), ) .write() .unwrap(); diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index 1dba11d0a..18f0f7070 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -21,8 +21,7 @@ use assert_matches::assert_matches; use git2::Oid; use itertools::Itertools; use jj_lib::backend::{ - BackendError, ChangeId, CommitId, MergedTreeId, MillisSinceEpoch, ObjectId, Signature, - Timestamp, + BackendError, ChangeId, CommitId, MillisSinceEpoch, ObjectId, Signature, Timestamp, }; use jj_lib::commit::Commit; use jj_lib::commit_builder::CommitBuilder; @@ -2304,7 +2303,7 @@ fn create_rooted_commit<'repo>( .new_commit( settings, vec![mut_repo.store().root_commit_id().clone()], - MergedTreeId::Legacy(mut_repo.store().empty_tree_id().clone()), + mut_repo.store().empty_merged_tree_id(), ) .set_author(signature.clone()) .set_committer(signature) diff --git a/lib/tests/test_id_prefix.rs b/lib/tests/test_id_prefix.rs index e735ccedf..27dd7635b 100644 --- a/lib/tests/test_id_prefix.rs +++ b/lib/tests/test_id_prefix.rs @@ -13,7 +13,7 @@ // limitations under the License. use itertools::Itertools; -use jj_lib::backend::{CommitId, MergedTreeId, MillisSinceEpoch, ObjectId, Signature, Timestamp}; +use jj_lib::backend::{CommitId, MillisSinceEpoch, ObjectId, Signature, Timestamp}; use jj_lib::id_prefix::IdPrefixContext; use jj_lib::index::HexPrefix; use jj_lib::index::PrefixResolution::{AmbiguousMatch, NoMatch, SingleMatch}; @@ -43,7 +43,7 @@ fn test_id_prefix() { .new_commit( &settings, vec![parent_id.clone()], - MergedTreeId::Legacy(repo.store().empty_tree_id().clone()), + repo.store().empty_merged_tree_id(), ) .set_author(signature.clone()) .set_committer(signature) diff --git a/lib/tests/test_init.rs b/lib/tests/test_init.rs index ff6b7057d..4bbdaaef4 100644 --- a/lib/tests/test_init.rs +++ b/lib/tests/test_init.rs @@ -14,7 +14,6 @@ use std::path::{Path, PathBuf}; -use jj_lib::backend::MergedTreeId; use jj_lib::git_backend::GitBackend; use jj_lib::op_store::WorkspaceId; use jj_lib::repo::Repo; @@ -124,10 +123,7 @@ fn test_init_checkout(use_git: bool) { .get_wc_commit_id(&WorkspaceId::default()) .unwrap(); let wc_commit = repo.store().get_commit(wc_commit_id).unwrap(); - assert_eq!( - *wc_commit.tree_id(), - MergedTreeId::Legacy(repo.store().empty_tree_id().clone()) - ); + assert_eq!(*wc_commit.tree_id(), repo.store().empty_merged_tree_id()); assert_eq!( wc_commit.store_commit().parents, vec![repo.store().root_commit_id().clone()] diff --git a/lib/tests/test_merged_tree.rs b/lib/tests/test_merged_tree.rs index 0d465d587..eca606446 100644 --- a/lib/tests/test_merged_tree.rs +++ b/lib/tests/test_merged_tree.rs @@ -184,8 +184,7 @@ fn test_from_legacy_tree() { // Also test that MergedTreeBuilder can create the same tree by starting from an // empty legacy tree. - let mut tree_builder = - MergedTreeBuilder::new(MergedTreeId::Legacy(store.empty_tree_id().clone())); + let mut tree_builder = MergedTreeBuilder::new(store.empty_merged_tree_id()); for (path, value) in tree.entries() { tree_builder.set_or_remove(path, Merge::normal(value)); } diff --git a/lib/tests/test_mut_repo.rs b/lib/tests/test_mut_repo.rs index fd3eaed5c..d5706a120 100644 --- a/lib/tests/test_mut_repo.rs +++ b/lib/tests/test_mut_repo.rs @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -use jj_lib::backend::MergedTreeId; use jj_lib::op_store::{RefTarget, WorkspaceId}; use jj_lib::repo::Repo; use maplit::hashset; @@ -104,7 +103,7 @@ fn test_checkout_previous_empty(use_git: bool) { .new_commit( &settings, vec![repo.store().root_commit_id().clone()], - MergedTreeId::Legacy(repo.store().empty_tree_id().clone()), + repo.store().empty_merged_tree_id(), ) .write() .unwrap(); @@ -135,7 +134,7 @@ fn test_checkout_previous_empty_with_description(use_git: bool) { .new_commit( &settings, vec![repo.store().root_commit_id().clone()], - MergedTreeId::Legacy(repo.store().empty_tree_id().clone()), + repo.store().empty_merged_tree_id(), ) .set_description("not empty") .write() @@ -167,7 +166,7 @@ fn test_checkout_previous_empty_with_local_branch(use_git: bool) { .new_commit( &settings, vec![repo.store().root_commit_id().clone()], - MergedTreeId::Legacy(repo.store().empty_tree_id().clone()), + repo.store().empty_merged_tree_id(), ) .write() .unwrap(); @@ -199,7 +198,7 @@ fn test_checkout_previous_empty_non_head(use_git: bool) { .new_commit( &settings, vec![repo.store().root_commit_id().clone()], - MergedTreeId::Legacy(repo.store().empty_tree_id().clone()), + repo.store().empty_merged_tree_id(), ) .write() .unwrap(); diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index 1b3af1917..7a38a1854 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -21,9 +21,7 @@ use std::path::Path; use assert_matches::assert_matches; use itertools::Itertools; -use jj_lib::backend::{ - ChangeId, CommitId, MergedTreeId, MillisSinceEpoch, ObjectId, Signature, Timestamp, -}; +use jj_lib::backend::{ChangeId, CommitId, MillisSinceEpoch, ObjectId, Signature, Timestamp}; use jj_lib::commit::Commit; use jj_lib::git; use jj_lib::git_backend::GitBackend; @@ -119,7 +117,7 @@ fn test_resolve_symbol_commit_id() { .new_commit( &settings, vec![repo.store().root_commit_id().clone()], - MergedTreeId::Legacy(repo.store().empty_tree_id().clone()), + repo.store().empty_merged_tree_id(), ) .set_description(format!("test {i}")) .set_author(signature.clone()) diff --git a/lib/tests/test_working_copy.rs b/lib/tests/test_working_copy.rs index ede1a22da..260ac443a 100644 --- a/lib/tests/test_working_copy.rs +++ b/lib/tests/test_working_copy.rs @@ -54,10 +54,7 @@ fn test_root(use_git: bool) { .unwrap(); let wc_commit = repo.store().get_commit(wc_commit_id).unwrap(); assert_eq!(new_tree.id(), *wc_commit.tree_id()); - assert_eq!( - new_tree.id(), - MergedTreeId::Legacy(repo.store().empty_tree_id().clone()) - ); + assert_eq!(new_tree.id(), repo.store().empty_merged_tree_id()); } #[test_case(false ; "local backend")] @@ -176,10 +173,8 @@ fn test_checkout_file_transitions(use_git: bool) { if use_git { kinds.push(Kind::GitSubmodule); } - let mut left_tree_builder = - MergedTreeBuilder::new(MergedTreeId::Legacy(store.empty_tree_id().clone())); - let mut right_tree_builder = - MergedTreeBuilder::new(MergedTreeId::Legacy(store.empty_tree_id().clone())); + let mut left_tree_builder = MergedTreeBuilder::new(store.empty_merged_tree_id()); + let mut right_tree_builder = MergedTreeBuilder::new(store.empty_merged_tree_id()); let mut files = vec![]; for left_kind in &kinds { for right_kind in &kinds { @@ -435,7 +430,7 @@ fn test_snapshot_racy_timestamps(use_git: bool) { let workspace_root = test_workspace.workspace.workspace_root().clone(); let file_path = workspace_root.join("file"); - let mut previous_tree_id = MergedTreeId::Legacy(repo.store().empty_tree_id().clone()); + let mut previous_tree_id = repo.store().empty_merged_tree_id(); let wc = test_workspace.workspace.working_copy_mut(); for i in 0..100 { { @@ -724,7 +719,7 @@ fn test_dotgit_ignored(use_git: bool) { "contents", ); let new_tree = test_workspace.snapshot().unwrap(); - let empty_tree_id = MergedTreeId::Legacy(store.empty_tree_id().clone()); + let empty_tree_id = store.empty_merged_tree_id(); assert_eq!(new_tree.id(), empty_tree_id); std::fs::remove_dir_all(&dotgit_path).unwrap(); @@ -870,10 +865,7 @@ fn test_fsmonitor() { { let mut locked_wc = wc.start_mutation().unwrap(); let tree_id = snapshot(&mut locked_wc, &[]); - assert_eq!( - tree_id, - MergedTreeId::Legacy(repo.store().empty_tree_id().clone()) - ); + assert_eq!(tree_id, repo.store().empty_merged_tree_id()); locked_wc.discard(); }