Skip to content

Commit

Permalink
store: make get_tree() functions take owned repo path
Browse files Browse the repository at this point in the history
The function needs an owned value, so we might as well pass it one and
avoid a few clone calls.
  • Loading branch information
martinvonz committed Nov 28, 2024
1 parent b9e9392 commit 409be2e
Show file tree
Hide file tree
Showing 10 changed files with 35 additions and 34 deletions.
2 changes: 1 addition & 1 deletion cli/src/commands/debug/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ pub fn cmd_debug_tree(
RepoPathBuf::root()
};
let store = workspace_command.repo().store();
let tree = store.get_tree(&dir, &tree_id)?;
let tree = store.get_tree(dir, &tree_id)?;
MergedTree::resolved(tree)
} else {
let commit = workspace_command
Expand Down
2 changes: 1 addition & 1 deletion lib/src/merge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,7 @@ where
if let Some(tree_id_merge) = tree_id_merge {
let get_tree = |id: &Option<&TreeId>| -> BackendResult<Tree> {
if let Some(id) = id {
store.get_tree(dir, id)
store.get_tree(dir.to_owned(), id)
} else {
Ok(Tree::empty(store.clone(), dir.to_owned()))
}
Expand Down
14 changes: 7 additions & 7 deletions lib/src/merged_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ impl MergedTree {
.into_iter()
.map(|builder| {
let tree_id = builder.write_tree()?;
store.get_tree(RepoPath::root(), &tree_id)
store.get_tree(RepoPathBuf::root(), &tree_id)
})
.try_collect()?;
Ok(MergedTree {
Expand Down Expand Up @@ -199,15 +199,15 @@ impl MergedTree {
Ok(Some(TreeValue::Tree(sub_tree_id))) => {
let subdir = self.dir().join(name);
Ok(Some(MergedTree::resolved(
self.store().get_tree(&subdir, sub_tree_id)?,
self.store().get_tree(subdir, sub_tree_id)?,
)))
}
Ok(_) => Ok(None),
Err(merge) => {
let trees = merge.try_map(|value| match value {
Some(TreeValue::Tree(sub_tree_id)) => {
let subdir = self.dir().join(name);
self.store().get_tree(&subdir, sub_tree_id)
self.store().get_tree(subdir, sub_tree_id)
}
_ => {
let subdir = self.dir().join(name);
Expand Down Expand Up @@ -939,12 +939,12 @@ impl<'matcher> TreeDiffStreamImpl<'matcher> {

async fn single_tree(
store: &Arc<Store>,
dir: &RepoPath,
dir: RepoPathBuf,
value: Option<&TreeValue>,
) -> BackendResult<Tree> {
match value {
Some(TreeValue::Tree(tree_id)) => store.get_tree_async(dir, tree_id).await,
_ => Ok(Tree::empty(store.clone(), dir.to_owned())),
_ => Ok(Tree::empty(store.clone(), dir.clone())),
}
}

Expand All @@ -956,7 +956,7 @@ impl<'matcher> TreeDiffStreamImpl<'matcher> {
) -> BackendResult<MergedTree> {
let trees = if values.is_tree() {
let builder: MergeBuilder<Tree> = futures::stream::iter(values.iter())
.then(|value| Self::single_tree(&store, &dir, value.as_ref()))
.then(|value| Self::single_tree(&store, dir.clone(), value.as_ref()))
.try_collect()
.await?;
builder.build()
Expand Down Expand Up @@ -1147,7 +1147,7 @@ impl MergedTreeBuilder {
pub fn write_tree(self, store: &Arc<Store>) -> BackendResult<MergedTreeId> {
let base_tree_ids = match self.base_tree_id.clone() {
MergedTreeId::Legacy(base_tree_id) => {
let legacy_base_tree = store.get_tree(RepoPath::root(), &base_tree_id)?;
let legacy_base_tree = store.get_tree(RepoPathBuf::root(), &base_tree_id)?;
let base_tree = MergedTree::from_legacy_tree(legacy_base_tree)?;
base_tree.id().to_merge()
}
Expand Down
12 changes: 6 additions & 6 deletions lib/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,17 +170,17 @@ impl Store {
Ok(Commit::new(self.clone(), commit_id, data))
}

pub fn get_tree(self: &Arc<Self>, dir: &RepoPath, id: &TreeId) -> BackendResult<Tree> {
pub fn get_tree(self: &Arc<Self>, dir: RepoPathBuf, id: &TreeId) -> BackendResult<Tree> {
self.get_tree_async(dir, id).block_on()
}

pub async fn get_tree_async(
self: &Arc<Self>,
dir: &RepoPath,
dir: RepoPathBuf,
id: &TreeId,
) -> BackendResult<Tree> {
let data = self.get_backend_tree(dir, id).await?;
Ok(Tree::new(self.clone(), dir.to_owned(), id.clone(), data))
let data = self.get_backend_tree(&dir, id).await?;
Ok(Tree::new(self.clone(), dir, id.clone(), data))
}

async fn get_backend_tree(
Expand All @@ -205,11 +205,11 @@ impl Store {
pub fn get_root_tree(self: &Arc<Self>, id: &MergedTreeId) -> BackendResult<MergedTree> {
match &id {
MergedTreeId::Legacy(id) => {
let tree = self.get_tree(RepoPath::root(), id)?;
let tree = self.get_tree(RepoPathBuf::root(), id)?;
MergedTree::from_legacy_tree(tree)
}
MergedTreeId::Merge(ids) => {
let trees = ids.try_map(|id| self.get_tree(RepoPath::root(), id))?;
let trees = ids.try_map(|id| self.get_tree(RepoPathBuf::root(), id))?;
Ok(MergedTree::new(trees))
}
}
Expand Down
12 changes: 6 additions & 6 deletions lib/src/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ impl Tree {
match sub_tree {
TreeValue::Tree(sub_tree_id) => {
let subdir = self.dir.join(name);
let sub_tree = self.store.get_tree(&subdir, sub_tree_id)?;
let sub_tree = self.store.get_tree(subdir, sub_tree_id)?;
Ok(Some(sub_tree))
}
_ => Ok(None),
Expand All @@ -165,7 +165,7 @@ impl Tree {
}
}

fn known_sub_tree(&self, subdir: &RepoPath, id: &TreeId) -> Tree {
fn known_sub_tree(&self, subdir: RepoPathBuf, id: &TreeId) -> Tree {
self.store.get_tree(subdir, id).unwrap()
}

Expand Down Expand Up @@ -251,7 +251,7 @@ impl Iterator for TreeEntriesIterator<'_> {
if self.matcher.visit(&path).is_nothing() {
continue;
}
let subtree = top.tree.known_sub_tree(&path, &id);
let subtree = top.tree.known_sub_tree(path, &id);
self.stack.push(TreeEntriesDirItem::from(subtree));
}
value => {
Expand Down Expand Up @@ -369,9 +369,9 @@ fn merge_tree_value(
Ok(match (base_tree_id, side1_tree_id, side2_tree_id) {
(Some(base_id), Some(side1_id), Some(side2_id)) => {
let subdir = dir.join(basename);
let base_tree = store.get_tree(&subdir, base_id)?;
let side1_tree = store.get_tree(&subdir, side1_id)?;
let side2_tree = store.get_tree(&subdir, side2_id)?;
let base_tree = store.get_tree(subdir.clone(), base_id)?;
let side1_tree = store.get_tree(subdir.clone(), side1_id)?;
let side2_tree = store.get_tree(subdir, side2_id)?;
let merged_tree = merge_trees(&side1_tree, &base_tree, &side2_tree)?;
if merged_tree.id() == empty_tree_id {
None
Expand Down
2 changes: 1 addition & 1 deletion lib/src/tree_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ impl TreeBuilder {
let store = &self.store;
let mut tree_cache = {
let dir = RepoPathBuf::root();
let tree = store.get_tree(&dir, &self.base_tree_id)?;
let tree = store.get_tree(dir.clone(), &self.base_tree_id)?;
BTreeMap::from([(dir, tree)])
};

Expand Down
4 changes: 2 additions & 2 deletions lib/tests/test_local_working_copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ fn test_tree_builder_file_directory_transition() {
let mut ws = test_workspace.workspace;
let workspace_root = ws.workspace_root().to_owned();
let mut check_out_tree = |tree_id: &TreeId| {
let tree = repo.store().get_tree(RepoPath::root(), tree_id).unwrap();
let tree = repo.store().get_tree(RepoPathBuf::root(), tree_id).unwrap();
let commit = commit_with_tree(repo.store(), MergedTreeId::Legacy(tree.id().clone()));
ws.check_out(
repo.op_id().clone(),
Expand Down Expand Up @@ -1140,7 +1140,7 @@ fn test_gitignores_ignored_directory_already_tracked() {
}
}
let id = tree_builder.write_tree().unwrap();
MergedTree::resolved(store.get_tree(RepoPath::root(), &id).unwrap())
MergedTree::resolved(store.get_tree(RepoPathBuf::root(), &id).unwrap())
};

let gitignore_path = RepoPath::from_internal_string(".gitignore");
Expand Down
17 changes: 9 additions & 8 deletions lib/tests/test_merge_trees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use itertools::Itertools;
use jj_lib::backend::TreeValue;
use jj_lib::repo::Repo;
use jj_lib::repo_path::RepoPath;
use jj_lib::repo_path::RepoPathBuf;
use jj_lib::repo_path::RepoPathComponent;
use jj_lib::rewrite::rebase_commit;
use jj_lib::tree::merge_trees;
Expand Down Expand Up @@ -65,7 +66,7 @@ fn test_same_type() {
}
}
let tree_id = tree_builder.write_tree().unwrap();
store.get_tree(RepoPath::root(), &tree_id).unwrap()
store.get_tree(RepoPathBuf::root(), &tree_id).unwrap()
};

let base_tree = write_tree(0);
Expand Down Expand Up @@ -207,7 +208,7 @@ fn test_executable() {
}
}
let tree_id = tree_builder.write_tree().unwrap();
store.get_tree(RepoPath::root(), &tree_id).unwrap()
store.get_tree(RepoPathBuf::root(), &tree_id).unwrap()
};

fn contents_in_tree<'a>(files: &[&'a str], index: usize) -> Vec<(&'a str, bool)> {
Expand Down Expand Up @@ -255,7 +256,7 @@ fn test_subtrees() {
);
}
let tree_id = tree_builder.write_tree().unwrap();
store.get_tree(RepoPath::root(), &tree_id).unwrap()
store.get_tree(RepoPathBuf::root(), &tree_id).unwrap()
};

let base_tree = write_tree(vec!["f1", "d1/f1", "d1/d1/f1", "d1/d1/d1/f1"]);
Expand Down Expand Up @@ -309,7 +310,7 @@ fn test_subtree_becomes_empty() {
);
}
let tree_id = tree_builder.write_tree().unwrap();
store.get_tree(RepoPath::root(), &tree_id).unwrap()
store.get_tree(RepoPathBuf::root(), &tree_id).unwrap()
};

let base_tree = write_tree(vec!["f1", "d1/f1", "d1/d1/d1/f1", "d1/d1/d1/f2"]);
Expand Down Expand Up @@ -338,7 +339,7 @@ fn test_subtree_one_missing() {
);
}
let tree_id = tree_builder.write_tree().unwrap();
store.get_tree(RepoPath::root(), &tree_id).unwrap()
store.get_tree(RepoPathBuf::root(), &tree_id).unwrap()
};

let tree1 = write_tree(vec![]);
Expand Down Expand Up @@ -405,11 +406,11 @@ fn test_types() {
"contents",
);
let base_tree_id = base_tree_builder.write_tree().unwrap();
let base_tree = store.get_tree(RepoPath::root(), &base_tree_id).unwrap();
let base_tree = store.get_tree(RepoPathBuf::root(), &base_tree_id).unwrap();
let side1_tree_id = side1_tree_builder.write_tree().unwrap();
let side1_tree = store.get_tree(RepoPath::root(), &side1_tree_id).unwrap();
let side1_tree = store.get_tree(RepoPathBuf::root(), &side1_tree_id).unwrap();
let side2_tree_id = side2_tree_builder.write_tree().unwrap();
let side2_tree = store.get_tree(RepoPath::root(), &side2_tree_id).unwrap();
let side2_tree = store.get_tree(RepoPathBuf::root(), &side2_tree_id).unwrap();

// Created the merged tree
let merged_tree = merge_trees(&side1_tree, &base_tree, &side2_tree).unwrap();
Expand Down
2 changes: 1 addition & 1 deletion lib/tests/test_merged_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ fn test_from_legacy_tree() {
tree_builder.set(dir1_filename.to_owned(), file_value(&dir1_filename_id));

let tree_id = tree_builder.write_tree().unwrap();
let tree = store.get_tree(RepoPath::root(), &tree_id).unwrap();
let tree = store.get_tree(RepoPathBuf::root(), &tree_id).unwrap();

let merged_tree = MergedTree::from_legacy_tree(tree.clone()).unwrap();
assert_eq!(
Expand Down
2 changes: 1 addition & 1 deletion lib/testutils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ pub fn create_single_tree(repo: &Arc<ReadonlyRepo>, path_contents: &[(&RepoPath,
write_normal_file(&mut tree_builder, path, contents);
}
let id = tree_builder.write_tree().unwrap();
store.get_tree(RepoPath::root(), &id).unwrap()
store.get_tree(RepoPathBuf::root(), &id).unwrap()
}

pub fn create_tree(repo: &Arc<ReadonlyRepo>, path_contents: &[(&RepoPath, &str)]) -> MergedTree {
Expand Down

0 comments on commit 409be2e

Please sign in to comment.