From 03ae1b747cb231dfff1bf784c8641f37d5f04201 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Sun, 16 May 2021 21:55:51 -0700 Subject: [PATCH] repo_path: remove FileRepoPath in favor of just RepoPath I had initially hoped that the type-safety provided by the separate `FileRepoPath` and `DirRepoPath` types would help prevent bugs. I'm not sure if it has prevented any bugs so far. It has turned out that there are more cases than I had hoped where it's unknown whether a path is for a directory or a file. One such example is for the path of a conflict. Since it can be conflict between a directory and a file, it doesn't make sense to use either. Instead we end up with quite a bit of conversion between the types. I feel like they are not worth the extra complexity. This patch therefore starts simplifying it by replacing uses of `FileRepoPath` by `RepoPath`. `DirRepoPath` is a little more complicated because its string form ends with a '/'. I'll address that in separate patches. --- lib/src/conflicts.rs | 11 +- lib/src/git_store.rs | 12 +- lib/src/local_store.rs | 10 +- lib/src/matchers.rs | 50 +++---- lib/src/repo_path.rs | 156 ++++------------------ lib/src/store.rs | 10 +- lib/src/store_wrapper.rs | 10 +- lib/src/testutils.rs | 22 +-- lib/src/tree.rs | 8 +- lib/src/trees.rs | 10 +- lib/src/working_copy.rs | 33 ++--- lib/tests/test_commit_builder.rs | 10 +- lib/tests/test_diff_summary.rs | 32 ++--- lib/tests/test_evolution.rs | 10 +- lib/tests/test_merge_trees.rs | 24 ++-- lib/tests/test_mut_repo.rs | 16 +-- lib/tests/test_working_copy.rs | 56 ++++---- lib/tests/test_working_copy_concurrent.rs | 10 +- src/commands.rs | 24 +--- src/diff_edit.rs | 5 +- 20 files changed, 196 insertions(+), 323 deletions(-) diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index db46b89a7..246fe04eb 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -48,19 +48,18 @@ pub fn materialize_conflict( let mut left_contents: Vec = vec![]; let mut base_contents: Vec = vec![]; let mut right_contents: Vec = vec![]; - let file_path = path.to_file_repo_path(); store - .read_file(&file_path, &left_id) + .read_file(&path, &left_id) .unwrap() .read_to_end(&mut left_contents) .unwrap(); store - .read_file(&file_path, &base_id) + .read_file(&path, &base_id) .unwrap() .read_to_end(&mut base_contents) .unwrap(); store - .read_file(&file_path, &right_id) + .read_file(&path, &right_id) .unwrap() .read_to_end(&mut right_contents) .unwrap(); @@ -108,9 +107,7 @@ pub fn conflict_to_materialized_value( ) -> TreeValue { let mut buf = vec![]; materialize_conflict(store, &path, &conflict, &mut buf); - let file_id = store - .write_file(&path.to_file_repo_path(), &mut Cursor::new(&buf)) - .unwrap(); + let file_id = store.write_file(path, &mut Cursor::new(&buf)).unwrap(); TreeValue::Normal { id: file_id, executable: false, diff --git a/lib/src/git_store.rs b/lib/src/git_store.rs index b4d179fbc..79ce301c1 100644 --- a/lib/src/git_store.rs +++ b/lib/src/git_store.rs @@ -24,7 +24,7 @@ use git2::Oid; use protobuf::Message; use uuid::Uuid; -use crate::repo_path::{DirRepoPath, FileRepoPath}; +use crate::repo_path::{DirRepoPath, RepoPath}; use crate::store::{ ChangeId, Commit, CommitId, Conflict, ConflictId, ConflictPart, FileId, MillisSinceEpoch, Signature, Store, StoreError, StoreResult, SymlinkId, Timestamp, Tree, TreeId, TreeValue, @@ -176,7 +176,7 @@ impl Store for GitStore { Some(git2::Repository::open(&path).unwrap()) } - fn read_file(&self, _path: &FileRepoPath, id: &FileId) -> StoreResult> { + fn read_file(&self, _path: &RepoPath, id: &FileId) -> StoreResult> { if id.0.len() != self.hash_length() { return Err(StoreError::NotFound); } @@ -188,7 +188,7 @@ impl Store for GitStore { Ok(Box::new(Cursor::new(content))) } - fn write_file(&self, _path: &FileRepoPath, contents: &mut dyn Read) -> StoreResult { + fn write_file(&self, _path: &RepoPath, contents: &mut dyn Read) -> StoreResult { let mut bytes = Vec::new(); contents.read_to_end(&mut bytes).unwrap(); let locked_repo = self.repo.lock().unwrap(); @@ -196,7 +196,7 @@ impl Store for GitStore { Ok(FileId(oid.as_bytes().to_vec())) } - fn read_symlink(&self, _path: &FileRepoPath, id: &SymlinkId) -> Result { + fn read_symlink(&self, _path: &RepoPath, id: &SymlinkId) -> Result { if id.0.len() != self.hash_length() { return Err(StoreError::NotFound); } @@ -208,7 +208,7 @@ impl Store for GitStore { Ok(target) } - fn write_symlink(&self, _path: &FileRepoPath, target: &str) -> Result { + fn write_symlink(&self, _path: &RepoPath, target: &str) -> Result { let locked_repo = self.repo.lock().unwrap(); let oid = locked_repo.blob(target.as_bytes()).unwrap(); Ok(SymlinkId(oid.as_bytes().to_vec())) @@ -391,7 +391,7 @@ impl Store for GitStore { } fn read_conflict(&self, id: &ConflictId) -> StoreResult { - let mut file = self.read_file(&FileRepoPath::from("unused"), &FileId(id.0.clone()))?; + let mut file = self.read_file(&RepoPath::from("unused"), &FileId(id.0.clone()))?; let mut data = String::new(); file.read_to_string(&mut data)?; let json: serde_json::Value = serde_json::from_str(&data).unwrap(); diff --git a/lib/src/local_store.rs b/lib/src/local_store.rs index a0cd1a813..094d6abf8 100644 --- a/lib/src/local_store.rs +++ b/lib/src/local_store.rs @@ -22,7 +22,7 @@ use blake2::{Blake2b, Digest}; use protobuf::{Message, ProtobufError}; use tempfile::{NamedTempFile, PersistError}; -use crate::repo_path::{DirRepoPath, FileRepoPath}; +use crate::repo_path::{DirRepoPath, RepoPath}; use crate::store::{ ChangeId, Commit, CommitId, Conflict, ConflictId, ConflictPart, FileId, MillisSinceEpoch, Signature, Store, StoreError, StoreResult, SymlinkId, Timestamp, Tree, TreeId, TreeValue, @@ -113,13 +113,13 @@ impl Store for LocalStore { None } - fn read_file(&self, _path: &FileRepoPath, id: &FileId) -> StoreResult> { + fn read_file(&self, _path: &RepoPath, id: &FileId) -> StoreResult> { let path = self.file_path(&id); let file = File::open(path).map_err(not_found_to_store_error)?; Ok(Box::new(zstd::Decoder::new(file)?)) } - fn write_file(&self, _path: &FileRepoPath, contents: &mut dyn Read) -> StoreResult { + fn write_file(&self, _path: &RepoPath, contents: &mut dyn Read) -> StoreResult { let temp_file = NamedTempFile::new_in(&self.path)?; let mut encoder = zstd::Encoder::new(temp_file.as_file(), 0)?; let mut hasher = Blake2b::new(); @@ -144,7 +144,7 @@ impl Store for LocalStore { Ok(id) } - fn read_symlink(&self, _path: &FileRepoPath, id: &SymlinkId) -> Result { + fn read_symlink(&self, _path: &RepoPath, id: &SymlinkId) -> Result { let path = self.symlink_path(&id); let mut file = File::open(path).map_err(not_found_to_store_error)?; let mut target = String::new(); @@ -152,7 +152,7 @@ impl Store for LocalStore { Ok(target) } - fn write_symlink(&self, _path: &FileRepoPath, target: &str) -> Result { + fn write_symlink(&self, _path: &RepoPath, target: &str) -> Result { let mut temp_file = NamedTempFile::new_in(&self.path)?; temp_file.write_all(target.as_bytes()).unwrap(); let mut hasher = Blake2b::new(); diff --git a/lib/src/matchers.rs b/lib/src/matchers.rs index 9758702f4..f2e92a8fa 100644 --- a/lib/src/matchers.rs +++ b/lib/src/matchers.rs @@ -16,7 +16,7 @@ use std::collections::{HashMap, HashSet}; -use crate::repo_path::{DirRepoPath, DirRepoPathComponent, FileRepoPath, FileRepoPathComponent}; +use crate::repo_path::{DirRepoPath, DirRepoPathComponent, RepoPath, RepoPathComponent}; #[derive(PartialEq, Eq, Debug)] pub struct Visit<'a> { @@ -33,11 +33,11 @@ pub enum VisitDirs<'a> { #[derive(PartialEq, Eq, Debug)] pub enum VisitFiles<'a> { All, - Set(&'a HashSet), + Set(&'a HashSet), } pub trait Matcher { - fn matches(&self, file: &FileRepoPath) -> bool; + fn matches(&self, file: &RepoPath) -> bool; fn visit(&self, dir: &DirRepoPath) -> Visit; } @@ -45,7 +45,7 @@ pub trait Matcher { pub struct EverythingMatcher; impl Matcher for EverythingMatcher { - fn matches(&self, _file: &FileRepoPath) -> bool { + fn matches(&self, _file: &RepoPath) -> bool { true } @@ -59,12 +59,12 @@ impl Matcher for EverythingMatcher { #[derive(PartialEq, Eq, Debug)] pub struct FilesMatcher { - files: HashSet, + files: HashSet, dirs: Dirs, } impl FilesMatcher { - fn new(files: HashSet) -> Self { + fn new(files: HashSet) -> Self { let mut dirs = Dirs::new(); for f in &files { dirs.add_file(f); @@ -74,7 +74,7 @@ impl FilesMatcher { } impl Matcher for FilesMatcher { - fn matches(&self, file: &FileRepoPath) -> bool { + fn matches(&self, file: &RepoPath) -> bool { self.files.contains(file) } @@ -93,9 +93,9 @@ impl Matcher for FilesMatcher { #[derive(PartialEq, Eq, Debug)] struct Dirs { dirs: HashMap>, - files: HashMap>, + files: HashMap>, empty_dirs: HashSet, - empty_files: HashSet, + empty_files: HashSet, } impl Dirs { @@ -129,8 +129,10 @@ impl Dirs { } } - fn add_file(&mut self, file: &FileRepoPath) { - let (dir, basename) = file.split(); + fn add_file(&mut self, file: &RepoPath) { + let (dir, basename) = file + .split() + .unwrap_or_else(|| panic!("got empty filename: {:?}", file)); self.add_dir(dir.clone()); self.files .entry(dir.clone()) @@ -142,7 +144,7 @@ impl Dirs { self.dirs.get(&dir).unwrap_or(&self.empty_dirs) } - fn get_files(&self, dir: &DirRepoPath) -> &HashSet { + fn get_files(&self, dir: &DirRepoPath) -> &HashSet { self.files.get(&dir).unwrap_or(&self.empty_files) } } @@ -152,9 +154,7 @@ mod tests { use std::collections::HashSet; use super::*; - use crate::repo_path::{ - DirRepoPath, DirRepoPathComponent, FileRepoPath, FileRepoPathComponent, - }; + use crate::repo_path::{DirRepoPath, DirRepoPathComponent, RepoPath, RepoPathComponent}; #[test] fn dirs_empty() { @@ -182,7 +182,7 @@ mod tests { #[test] fn dirs_file() { let mut dirs = Dirs::new(); - dirs.add_file(&FileRepoPath::from("dir/file")); + dirs.add_file(&RepoPath::from("dir/file")); assert_eq!( dirs.get_dirs(&DirRepoPath::root()), &hashset! {DirRepoPathComponent::from("dir")} @@ -193,8 +193,8 @@ mod tests { #[test] fn filesmatcher_empty() { let m = FilesMatcher::new(HashSet::new()); - assert!(!m.matches(&FileRepoPath::from("file"))); - assert!(!m.matches(&FileRepoPath::from("dir/file"))); + assert!(!m.matches(&RepoPath::from("file"))); + assert!(!m.matches(&RepoPath::from("dir/file"))); assert_eq!( m.visit(&DirRepoPath::root()), Visit { @@ -207,17 +207,17 @@ mod tests { #[test] fn filesmatcher_nonempty() { let m = FilesMatcher::new(hashset! { - FileRepoPath::from("dir1/subdir1/file1"), - FileRepoPath::from("dir1/subdir1/file2"), - FileRepoPath::from("dir1/subdir2/file3"), - FileRepoPath::from("file4"), + RepoPath::from("dir1/subdir1/file1"), + RepoPath::from("dir1/subdir1/file2"), + RepoPath::from("dir1/subdir2/file3"), + RepoPath::from("file4"), }); assert_eq!( m.visit(&DirRepoPath::root()), Visit { dirs: VisitDirs::Set(&hashset! {DirRepoPathComponent::from("dir1")}), - files: VisitFiles::Set(&hashset! {FileRepoPathComponent::from("file4")}), + files: VisitFiles::Set(&hashset! {RepoPathComponent::from("file4")}), } ); assert_eq!( @@ -234,7 +234,7 @@ mod tests { Visit { dirs: VisitDirs::Set(&hashset! {}), files: VisitFiles::Set( - &hashset! {FileRepoPathComponent::from("file1"), FileRepoPathComponent::from("file2")} + &hashset! {RepoPathComponent::from("file1"), RepoPathComponent::from("file2")} ), } ); @@ -242,7 +242,7 @@ mod tests { m.visit(&DirRepoPath::from("dir1/subdir2/")), Visit { dirs: VisitDirs::Set(&hashset! {}), - files: VisitFiles::Set(&hashset! {FileRepoPathComponent::from("file3")}), + files: VisitFiles::Set(&hashset! {RepoPathComponent::from("file3")}), } ); } diff --git a/lib/src/repo_path.rs b/lib/src/repo_path.rs index cfd897fc9..0950e39e4 100644 --- a/lib/src/repo_path.rs +++ b/lib/src/repo_path.rs @@ -56,27 +56,6 @@ impl From<&str> for DirRepoPathComponent { } } -#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)] -pub struct FileRepoPathComponent { - value: String, -} - -impl FileRepoPathComponent { - pub fn value(&self) -> &str { - &self.value - } -} - -impl From<&str> for FileRepoPathComponent { - fn from(value: &str) -> Self { - assert!(!value.contains('/')); - assert!(!value.is_empty()); - FileRepoPathComponent { - value: value.to_owned(), - } - } -} - #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct RepoPath { dir: DirRepoPath, @@ -109,14 +88,6 @@ impl RepoPath { self.dir.to_internal_string() + self.basename.value() } - pub fn to_file_repo_path(&self) -> FileRepoPath { - FileRepoPath { - dir: self.dir.clone(), - basename: FileRepoPathComponent { - value: self.basename.value.clone(), - }, - } - } pub fn to_dir_repo_path(&self) -> DirRepoPath { if self.is_root() { DirRepoPath::root() @@ -208,7 +179,7 @@ impl DirRepoPath { other.value.starts_with(&self.value) } - pub fn contains_file(&self, other: &FileRepoPath) -> bool { + pub fn contains_file(&self, other: &RepoPath) -> bool { other.dir.value.starts_with(&self.value) } @@ -258,63 +229,6 @@ impl From<&str> for DirRepoPath { } } -#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct FileRepoPath { - dir: DirRepoPath, - basename: FileRepoPathComponent, -} - -impl Debug for FileRepoPath { - fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> { - f.write_fmt(format_args!("{:?}", &self.to_internal_string())) - } -} - -impl FileRepoPath { - /// The full string form used internally, not for presenting to users (where - /// we may want to use the platform's separator). - pub fn to_internal_string(&self) -> String { - self.dir.to_internal_string() + self.basename.value() - } - - pub fn dir(&self) -> &DirRepoPath { - &self.dir - } - - pub fn split(&self) -> (&DirRepoPath, &FileRepoPathComponent) { - (&self.dir, &self.basename) - } - - pub fn to_repo_path(&self) -> RepoPath { - RepoPath { - dir: self.dir.clone(), - basename: RepoPathComponent { - value: self.basename.value.clone(), - }, - } - } - - pub fn to_fs_path(&self, base: &Path) -> PathBuf { - self.to_repo_path().to_fs_path(base) - } -} - -impl From<&str> for FileRepoPath { - fn from(value: &str) -> Self { - assert!(!value.ends_with('/')); - match value.rfind('/') { - None => FileRepoPath { - dir: DirRepoPath::root(), - basename: FileRepoPathComponent::from(value), - }, - Some(i) => FileRepoPath { - dir: DirRepoPath::from(&value[..=i]), - basename: FileRepoPathComponent::from(&value[i + 1..]), - }, - } - } -} - pub trait RepoPathJoin { type Result; @@ -331,17 +245,6 @@ impl RepoPathJoin for DirRepoPath { } } -impl RepoPathJoin for DirRepoPath { - type Result = FileRepoPath; - - fn join(&self, entry: &FileRepoPathComponent) -> FileRepoPath { - FileRepoPath { - dir: self.clone(), - basename: entry.clone(), - } - } -} - impl RepoPathJoin for DirRepoPath { type Result = RepoPath; @@ -379,11 +282,6 @@ mod tests { DirRepoPath::from("dir/subdir/").to_internal_string(), "dir/subdir/" ); - assert_eq!(FileRepoPath::from("file").to_internal_string(), "file"); - assert_eq!( - FileRepoPath::from("dir/file").to_internal_string(), - "dir/file" - ); } #[test] @@ -394,11 +292,11 @@ mod tests { assert!(DirRepoPath::from("dir/") < DirRepoPath::from("dir#/")); assert!(DirRepoPath::from("dir/") < DirRepoPath::from("dir/sub/")); - assert!(FileRepoPath::from("abc") < FileRepoPath::from("dir/file")); - assert!(FileRepoPath::from("dir") < FileRepoPath::from("dir/file")); - assert!(FileRepoPath::from("dis") < FileRepoPath::from("dir/file")); - assert!(FileRepoPath::from("xyz") < FileRepoPath::from("dir/file")); - assert!(FileRepoPath::from("dir1/xyz") < FileRepoPath::from("dir2/abc")); + assert!(RepoPath::from("abc") < RepoPath::from("dir/file")); + assert!(RepoPath::from("dir") < RepoPath::from("dir/file")); + assert!(RepoPath::from("dis") < RepoPath::from("dir/file")); + assert!(RepoPath::from("xyz") < RepoPath::from("dir/file")); + assert!(RepoPath::from("dir1/xyz") < RepoPath::from("dir2/abc")); } #[test] @@ -406,16 +304,16 @@ mod tests { let root = DirRepoPath::root(); let dir_component = DirRepoPathComponent::from("dir"); let subdir_component = DirRepoPathComponent::from("subdir"); - let file_component = FileRepoPathComponent::from("file"); - assert_eq!(root.join(&file_component), FileRepoPath::from("file")); + let file_component = RepoPathComponent::from("file"); + assert_eq!(root.join(&file_component), RepoPath::from("file")); let dir = root.join(&dir_component); assert_eq!(dir, DirRepoPath::from("dir/")); - assert_eq!(dir.join(&file_component), FileRepoPath::from("dir/file")); + assert_eq!(dir.join(&file_component), RepoPath::from("dir/file")); let subdir = dir.join(&subdir_component); assert_eq!(subdir, DirRepoPath::from("dir/subdir/")); assert_eq!( subdir.join(&file_component), - FileRepoPath::from("dir/subdir/file") + RepoPath::from("dir/subdir/file") ); } @@ -451,27 +349,30 @@ mod tests { fn test_split_file() { let root = DirRepoPath::root(); let dir_component = DirRepoPathComponent::from("dir"); - let file_component = FileRepoPathComponent::from("file"); + let file_component = RepoPathComponent::from("file"); let dir = root.join(&dir_component); assert_eq!( root.join(&file_component).split(), - (&root, &file_component.clone()) + Some((&root, &file_component.clone())) + ); + assert_eq!( + dir.join(&file_component).split(), + Some((&dir, &file_component)) ); - assert_eq!(dir.join(&file_component).split(), (&dir, &file_component)); } #[test] fn test_dir() { let root = DirRepoPath::root(); let dir_component = DirRepoPathComponent::from("dir"); - let file_component = FileRepoPathComponent::from("file"); + let file_component = RepoPathComponent::from("file"); let dir = root.join(&dir_component); - assert_eq!(root.join(&file_component).dir(), &root); - assert_eq!(dir.join(&file_component).dir(), &dir); + assert_eq!(root.join(&file_component).dir(), Some(&root)); + assert_eq!(dir.join(&file_component).dir(), Some(&dir)); } #[test] @@ -493,15 +394,20 @@ mod tests { #[test] fn test_to_fs_path() { assert_eq!( - FileRepoPath::from("file").to_fs_path(&Path::new("base/dir")), + RepoPath::from("").to_fs_path(&Path::new("base/dir")), + Path::new("base/dir") + ); + assert_eq!(RepoPath::from("").to_fs_path(&Path::new("")), Path::new("")); + assert_eq!( + RepoPath::from("file").to_fs_path(&Path::new("base/dir")), Path::new("base/dir/file") ); assert_eq!( - FileRepoPath::from("some/deep/dir/file").to_fs_path(&Path::new("base/dir")), + RepoPath::from("some/deep/dir/file").to_fs_path(&Path::new("base/dir")), Path::new("base/dir/some/deep/dir/file") ); assert_eq!( - FileRepoPath::from("dir/file").to_fs_path(&Path::new("")), + RepoPath::from("dir/file").to_fs_path(&Path::new("")), Path::new("dir/file") ); } @@ -517,13 +423,5 @@ mod tests { RepoPath::from("dir/subdir").to_dir_repo_path(), DirRepoPath::from("dir/subdir/") ); - assert_eq!( - RepoPath::from("file").to_file_repo_path(), - FileRepoPath::from("file") - ); - assert_eq!( - RepoPath::from("dir/file").to_file_repo_path(), - FileRepoPath::from("dir/file") - ); } } diff --git a/lib/src/store.rs b/lib/src/store.rs index ac0931d6d..0630a5913 100644 --- a/lib/src/store.rs +++ b/lib/src/store.rs @@ -21,7 +21,7 @@ use std::vec::Vec; use thiserror::Error; -use crate::repo_path::{DirRepoPath, FileRepoPath}; +use crate::repo_path::{DirRepoPath, RepoPath}; #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Hash)] pub struct CommitId(pub Vec); @@ -334,13 +334,13 @@ pub trait Store: Send + Sync + Debug { fn git_repo(&self) -> Option; - fn read_file(&self, path: &FileRepoPath, id: &FileId) -> StoreResult>; + fn read_file(&self, path: &RepoPath, id: &FileId) -> StoreResult>; - fn write_file(&self, path: &FileRepoPath, contents: &mut dyn Read) -> StoreResult; + fn write_file(&self, path: &RepoPath, contents: &mut dyn Read) -> StoreResult; - fn read_symlink(&self, path: &FileRepoPath, id: &SymlinkId) -> StoreResult; + fn read_symlink(&self, path: &RepoPath, id: &SymlinkId) -> StoreResult; - fn write_symlink(&self, path: &FileRepoPath, target: &str) -> StoreResult; + fn write_symlink(&self, path: &RepoPath, target: &str) -> StoreResult; fn empty_tree_id(&self) -> &TreeId; diff --git a/lib/src/store_wrapper.rs b/lib/src/store_wrapper.rs index 0f64ff2a9..aa77dc827 100644 --- a/lib/src/store_wrapper.rs +++ b/lib/src/store_wrapper.rs @@ -21,7 +21,7 @@ use std::sync::{Arc, RwLock, Weak}; use crate::commit::Commit; use crate::git_store::GitStore; use crate::local_store::LocalStore; -use crate::repo_path::{DirRepoPath, FileRepoPath}; +use crate::repo_path::{DirRepoPath, RepoPath}; use crate::store; use crate::store::{ ChangeId, CommitId, Conflict, ConflictId, FileId, MillisSinceEpoch, Signature, Store, @@ -197,19 +197,19 @@ impl StoreWrapper { self.store.write_tree(path, contents) } - pub fn read_file(&self, path: &FileRepoPath, id: &FileId) -> StoreResult> { + pub fn read_file(&self, path: &RepoPath, id: &FileId) -> StoreResult> { self.store.read_file(path, id) } - pub fn write_file(&self, path: &FileRepoPath, contents: &mut dyn Read) -> StoreResult { + pub fn write_file(&self, path: &RepoPath, contents: &mut dyn Read) -> StoreResult { self.store.write_file(path, contents) } - pub fn read_symlink(&self, path: &FileRepoPath, id: &SymlinkId) -> StoreResult { + pub fn read_symlink(&self, path: &RepoPath, id: &SymlinkId) -> StoreResult { self.store.read_symlink(path, id) } - pub fn write_symlink(&self, path: &FileRepoPath, contents: &str) -> StoreResult { + pub fn write_symlink(&self, path: &RepoPath, contents: &str) -> StoreResult { self.store.write_symlink(path, contents) } diff --git a/lib/src/testutils.rs b/lib/src/testutils.rs index dbc3d1d43..839826c90 100644 --- a/lib/src/testutils.rs +++ b/lib/src/testutils.rs @@ -22,7 +22,7 @@ use tempfile::TempDir; use crate::commit::Commit; use crate::commit_builder::CommitBuilder; use crate::repo::{MutableRepo, ReadonlyRepo}; -use crate::repo_path::{DirRepoPath, FileRepoPath}; +use crate::repo_path::{DirRepoPath, RepoPath}; use crate::settings::UserSettings; use crate::store::{FileId, TreeId, TreeValue}; use crate::store_wrapper::StoreWrapper; @@ -61,14 +61,14 @@ pub fn init_repo(settings: &UserSettings, use_git: bool) -> (TempDir, Arc FileId { +pub fn write_file(store: &StoreWrapper, path: &RepoPath, contents: &str) -> FileId { store.write_file(path, &mut contents.as_bytes()).unwrap() } -pub fn write_normal_file(tree_builder: &mut TreeBuilder, path: &FileRepoPath, contents: &str) { +pub fn write_normal_file(tree_builder: &mut TreeBuilder, path: &RepoPath, contents: &str) { let id = write_file(tree_builder.repo(), path, contents); tree_builder.set( - path.to_repo_path(), + path.clone(), TreeValue::Normal { id, executable: false, @@ -76,10 +76,10 @@ pub fn write_normal_file(tree_builder: &mut TreeBuilder, path: &FileRepoPath, co ); } -pub fn write_executable_file(tree_builder: &mut TreeBuilder, path: &FileRepoPath, contents: &str) { +pub fn write_executable_file(tree_builder: &mut TreeBuilder, path: &RepoPath, contents: &str) { let id = write_file(tree_builder.repo(), path, contents); tree_builder.set( - path.to_repo_path(), + path.clone(), TreeValue::Normal { id, executable: true, @@ -87,12 +87,12 @@ pub fn write_executable_file(tree_builder: &mut TreeBuilder, path: &FileRepoPath ); } -pub fn write_symlink(tree_builder: &mut TreeBuilder, path: &FileRepoPath, target: &str) { +pub fn write_symlink(tree_builder: &mut TreeBuilder, path: &RepoPath, target: &str) { let id = tree_builder.repo().write_symlink(path, target).unwrap(); - tree_builder.set(path.to_repo_path(), TreeValue::Symlink(id)); + tree_builder.set(path.clone(), TreeValue::Symlink(id)); } -pub fn create_tree(repo: &ReadonlyRepo, path_contents: &[(&FileRepoPath, &str)]) -> Tree { +pub fn create_tree(repo: &ReadonlyRepo, path_contents: &[(&RepoPath, &str)]) -> Tree { let store = repo.store(); let mut tree_builder = store.tree_builder(store.empty_tree_id().clone()); for (path, contents) in path_contents { @@ -108,7 +108,7 @@ pub fn create_random_tree(repo: &ReadonlyRepo) -> TreeId { .store() .tree_builder(repo.store().empty_tree_id().clone()); let number = rand::random::(); - let path = FileRepoPath::from(format!("file{}", number).as_str()); + let path = RepoPath::from(format!("file{}", number).as_str()); write_normal_file(&mut tree_builder, &path, "contents"); tree_builder.write_tree() } @@ -121,7 +121,7 @@ pub fn create_random_commit(settings: &UserSettings, repo: &ReadonlyRepo) -> Com .set_description(format!("random commit {}", number)) } -pub fn write_working_copy_file(repo: &ReadonlyRepo, path: &FileRepoPath, contents: &str) { +pub fn write_working_copy_file(repo: &ReadonlyRepo, path: &RepoPath, contents: &str) { let mut file = OpenOptions::new() .write(true) .create(true) diff --git a/lib/src/tree.rs b/lib/src/tree.rs index ac89b9451..6a457cf2b 100644 --- a/lib/src/tree.rs +++ b/lib/src/tree.rs @@ -18,7 +18,7 @@ use std::pin::Pin; use std::sync::Arc; use crate::repo_path::{ - DirRepoPath, DirRepoPathComponent, FileRepoPath, RepoPath, RepoPathComponent, RepoPathJoin, + DirRepoPath, DirRepoPathComponent, RepoPath, RepoPathComponent, RepoPathJoin, }; use crate::store; use crate::store::{ConflictId, TreeEntriesNonRecursiveIter, TreeEntry, TreeId, TreeValue}; @@ -44,9 +44,9 @@ impl Debug for Tree { #[derive(Debug, PartialEq, Eq, Clone)] pub struct DiffSummary { - pub modified: Vec, - pub added: Vec, - pub removed: Vec, + pub modified: Vec, + pub added: Vec, + pub removed: Vec, } impl Tree { diff --git a/lib/src/trees.rs b/lib/src/trees.rs index 69a70c744..5eab4ccbd 100644 --- a/lib/src/trees.rs +++ b/lib/src/trees.rs @@ -19,7 +19,7 @@ use std::pin::Pin; use crate::files; use crate::files::MergeResult; use crate::repo_path::{ - DirRepoPath, DirRepoPathComponent, FileRepoPath, FileRepoPathComponent, RepoPathJoin, + DirRepoPath, DirRepoPathComponent, RepoPath, RepoPathComponent, RepoPathJoin, }; use crate::store::{ Conflict, ConflictPart, StoreError, TreeEntriesNonRecursiveIter, TreeId, TreeValue, @@ -137,7 +137,7 @@ pub struct TreeDiffIterator { // This is used for making sure that when a directory gets replaced by a file, we // yield the value for the addition of the file after we yield the values // for removing files in the directory. - added_file: Option<(FileRepoPath, TreeValue)>, + added_file: Option<(RepoPath, TreeValue)>, // Iterator over the diffs of a subdirectory, if we're currently visiting one. subdir_iterator: Option>, } @@ -161,7 +161,7 @@ impl TreeDiffIterator { } impl Iterator for TreeDiffIterator { - type Item = (FileRepoPath, Diff); + type Item = (RepoPath, Diff); fn next(&mut self) -> Option { loop { @@ -178,7 +178,7 @@ impl Iterator for TreeDiffIterator { // Note: whenever we say "file" below, it may also be a symlink or a conflict. if let Some((name, diff)) = self.entry_iterator.next() { - let file_path = self.dir.join(&FileRepoPathComponent::from(name.as_str())); + let file_path = self.dir.join(&RepoPathComponent::from(name.as_str())); let subdir = DirRepoPathComponent::from(name.as_str()); let subdir_path = self.dir.join(&subdir); // TODO: simplify this mess @@ -351,7 +351,7 @@ fn merge_tree_value( *side1_executable }; - let filename = dir.join(&FileRepoPathComponent::from(basename)); + let filename = dir.join(&RepoPathComponent::from(basename)); let mut base_content = vec![]; store .read_file(&filename, &base_id)? diff --git a/lib/src/working_copy.rs b/lib/src/working_copy.rs index fdb14e0c5..8efda0d00 100644 --- a/lib/src/working_copy.rs +++ b/lib/src/working_copy.rs @@ -39,7 +39,7 @@ use crate::gitignore::GitIgnoreFile; use crate::lock::FileLock; use crate::repo::ReadonlyRepo; use crate::repo_path::{ - DirRepoPath, DirRepoPathComponent, FileRepoPath, FileRepoPathComponent, RepoPathJoin, + DirRepoPath, DirRepoPathComponent, RepoPath, RepoPathComponent, RepoPathJoin, }; use crate::settings::UserSettings; use crate::store::{CommitId, FileId, MillisSinceEpoch, StoreError, SymlinkId, TreeId, TreeValue}; @@ -86,7 +86,7 @@ pub struct TreeState { working_copy_path: PathBuf, state_path: PathBuf, tree_id: TreeId, - file_states: BTreeMap, + file_states: BTreeMap, read_time: MillisSinceEpoch, } @@ -118,10 +118,10 @@ fn file_state_to_proto(file_state: &FileState) -> crate::protos::working_copy::F fn file_states_from_proto( proto: &crate::protos::working_copy::TreeState, -) -> BTreeMap { +) -> BTreeMap { let mut file_states = BTreeMap::new(); for (path_str, proto_file_state) in &proto.file_states { - let path = FileRepoPath::from(path_str.as_str()); + let path = RepoPath::from(path_str.as_str()); file_states.insert(path, file_state_from_proto(&proto_file_state)); } file_states @@ -160,7 +160,7 @@ impl TreeState { &self.tree_id } - pub fn file_states(&self) -> &BTreeMap { + pub fn file_states(&self) -> &BTreeMap { &self.file_states } @@ -273,12 +273,12 @@ impl TreeState { }) } - fn write_file_to_store(&self, path: &FileRepoPath, disk_path: &Path) -> FileId { + fn write_file_to_store(&self, path: &RepoPath, disk_path: &Path) -> FileId { let file = File::open(disk_path).unwrap(); self.store.write_file(path, &mut Box::new(file)).unwrap() } - fn write_symlink_to_store(&self, path: &FileRepoPath, disk_path: &Path) -> SymlinkId { + fn write_symlink_to_store(&self, path: &RepoPath, disk_path: &Path) -> SymlinkId { let target = disk_path.read_link().unwrap(); let str_target = target.to_str().unwrap(); self.store.write_symlink(path, str_target).unwrap() @@ -326,7 +326,7 @@ impl TreeState { git_ignore, )]; let mut tree_builder = self.store.tree_builder(self.tree_id.clone()); - let mut deleted_files: HashSet<&FileRepoPath> = self.file_states.keys().collect(); + let mut deleted_files: HashSet<&RepoPath> = self.file_states.keys().collect(); let mut modified_files = BTreeMap::new(); while !work.is_empty() { let (dir, disk_dir, git_ignore) = work.pop().unwrap(); @@ -350,7 +350,7 @@ impl TreeState { // some file in it. TODO: This is pretty ugly... Also, we should // optimize it to check exactly the already-tracked files (we know that // we won't have to consider new files in the directory). - let first_file_in_dir = dir.join(&FileRepoPathComponent::from("\0")); + let first_file_in_dir = dir.join(&RepoPathComponent::from("\0")); if let Some((maybe_subdir_file, _)) = self .file_states .range((Bound::Included(&first_file_in_dir), Bound::Unbounded)) @@ -358,6 +358,7 @@ impl TreeState { { if !maybe_subdir_file .dir() + .unwrap() .components() .starts_with(dir.components()) { @@ -368,7 +369,7 @@ impl TreeState { let disk_subdir = disk_dir.join(file_name); work.push((subdir, disk_subdir, git_ignore.clone())); } else { - let file = dir.join(&FileRepoPathComponent::from(name)); + let file = dir.join(&RepoPathComponent::from(name)); let disk_file = disk_dir.join(file_name); deleted_files.remove(&file); let new_file_state = self.file_state(&entry.path()).unwrap(); @@ -410,18 +411,18 @@ impl TreeState { TreeValue::Symlink(id) } }; - tree_builder.set(file.to_repo_path(), file_value); + tree_builder.set(file.clone(), file_value); modified_files.insert(file, new_file_state); } } } } - let deleted_files: Vec = deleted_files.iter().cloned().cloned().collect(); + let deleted_files: Vec = deleted_files.iter().cloned().cloned().collect(); for file in &deleted_files { self.file_states.remove(file); - tree_builder.remove(file.to_repo_path()); + tree_builder.remove(file.clone()); } for (file, file_state) in modified_files { self.file_states.insert(file, file_state); @@ -434,7 +435,7 @@ impl TreeState { fn write_file( &self, disk_path: &Path, - path: &FileRepoPath, + path: &RepoPath, id: &FileId, executable: bool, ) -> FileState { @@ -462,7 +463,7 @@ impl TreeState { file_state } - fn write_symlink(&self, disk_path: &Path, path: &FileRepoPath, id: &SymlinkId) -> FileState { + fn write_symlink(&self, disk_path: &Path, path: &RepoPath, id: &SymlinkId) -> FileState { create_parent_dirs(disk_path); #[cfg(windows)] { @@ -710,7 +711,7 @@ impl WorkingCopy { .clone() } - pub fn file_states(&self) -> BTreeMap { + pub fn file_states(&self) -> BTreeMap { self.tree_state().as_ref().unwrap().file_states().clone() } diff --git a/lib/tests/test_commit_builder.rs b/lib/tests/test_commit_builder.rs index c43cf5b44..5e43c655c 100644 --- a/lib/tests/test_commit_builder.rs +++ b/lib/tests/test_commit_builder.rs @@ -13,7 +13,7 @@ // limitations under the License. use jujutsu_lib::commit_builder::CommitBuilder; -use jujutsu_lib::repo_path::FileRepoPath; +use jujutsu_lib::repo_path::RepoPath; use jujutsu_lib::settings::UserSettings; use jujutsu_lib::testutils; use jujutsu_lib::tree::DiffSummary; @@ -26,8 +26,8 @@ fn test_initial(use_git: bool) { let (_temp_dir, repo) = testutils::init_repo(&settings, use_git); let store = repo.store(); - let root_file_path = FileRepoPath::from("file"); - let dir_file_path = FileRepoPath::from("dir/file"); + let root_file_path = RepoPath::from("file"); + let dir_file_path = RepoPath::from("dir/file"); let tree = testutils::create_tree( &repo, &[ @@ -65,8 +65,8 @@ fn test_rewrite(use_git: bool) { let (_temp_dir, repo) = testutils::init_repo(&settings, use_git); let store = repo.store().clone(); - let root_file_path = FileRepoPath::from("file"); - let dir_file_path = FileRepoPath::from("dir/file"); + let root_file_path = RepoPath::from("file"); + let dir_file_path = RepoPath::from("dir/file"); let initial_tree = testutils::create_tree( &repo, &[ diff --git a/lib/tests/test_diff_summary.rs b/lib/tests/test_diff_summary.rs index cb54b4d04..a8a82f6e7 100644 --- a/lib/tests/test_diff_summary.rs +++ b/lib/tests/test_diff_summary.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use jujutsu_lib::repo_path::FileRepoPath; +use jujutsu_lib::repo_path::RepoPath; use jujutsu_lib::testutils; use jujutsu_lib::tree::DiffSummary; use test_case::test_case; @@ -23,10 +23,10 @@ fn test_types(use_git: bool) { let settings = testutils::user_settings(); let (_temp_dir, repo) = testutils::init_repo(&settings, use_git); - let clean_path = FileRepoPath::from("clean"); - let modified_path = FileRepoPath::from("modified"); - let added_path = FileRepoPath::from("added"); - let removed_path = FileRepoPath::from("removed"); + let clean_path = RepoPath::from("clean"); + let modified_path = RepoPath::from("modified"); + let added_path = RepoPath::from("added"); + let removed_path = RepoPath::from("removed"); let tree1 = testutils::create_tree( &repo, @@ -62,8 +62,8 @@ fn test_tree_file_transition(use_git: bool) { let settings = testutils::user_settings(); let (_temp_dir, repo) = testutils::init_repo(&settings, use_git); - let dir_file_path = FileRepoPath::from("dir/file"); - let dir_path = FileRepoPath::from("dir"); + let dir_file_path = RepoPath::from("dir/file"); + let dir_path = RepoPath::from("dir"); let tree1 = testutils::create_tree(&repo, &[(&dir_file_path, "contents")]); let tree2 = testutils::create_tree(&repo, &[(&dir_path, "contents")]); @@ -92,15 +92,15 @@ fn test_sorting(use_git: bool) { let settings = testutils::user_settings(); let (_temp_dir, repo) = testutils::init_repo(&settings, use_git); - let a_path = FileRepoPath::from("a"); - let b_path = FileRepoPath::from("b"); - let f_a_path = FileRepoPath::from("f/a"); - let f_b_path = FileRepoPath::from("f/b"); - let f_f_a_path = FileRepoPath::from("f/f/a"); - let f_f_b_path = FileRepoPath::from("f/f/b"); - let n_path = FileRepoPath::from("n"); - let s_b_path = FileRepoPath::from("s/b"); - let z_path = FileRepoPath::from("z"); + let a_path = RepoPath::from("a"); + let b_path = RepoPath::from("b"); + let f_a_path = RepoPath::from("f/a"); + let f_b_path = RepoPath::from("f/b"); + let f_f_a_path = RepoPath::from("f/f/a"); + let f_f_b_path = RepoPath::from("f/f/b"); + let n_path = RepoPath::from("n"); + let s_b_path = RepoPath::from("s/b"); + let z_path = RepoPath::from("z"); let tree1 = testutils::create_tree( &repo, diff --git a/lib/tests/test_evolution.rs b/lib/tests/test_evolution.rs index 9c8ad83c4..277c981c7 100644 --- a/lib/tests/test_evolution.rs +++ b/lib/tests/test_evolution.rs @@ -20,7 +20,7 @@ use jujutsu_lib::evolution::{ DivergenceResolution, DivergenceResolver, OrphanResolution, OrphanResolver, }; use jujutsu_lib::repo::ReadonlyRepo; -use jujutsu_lib::repo_path::FileRepoPath; +use jujutsu_lib::repo_path::RepoPath; use jujutsu_lib::settings::UserSettings; use jujutsu_lib::testutils; use test_case::test_case; @@ -668,10 +668,10 @@ fn test_evolve_divergent(use_git: bool) { // commit 6 has a later commit time than commit 4). It should have files C, // X, Y, Z. - let path_a = FileRepoPath::from("A"); - let path_x = FileRepoPath::from("X"); - let path_y = FileRepoPath::from("Y"); - let path_z = FileRepoPath::from("Z"); + let path_a = RepoPath::from("A"); + let path_x = RepoPath::from("X"); + let path_y = RepoPath::from("Y"); + let path_z = RepoPath::from("Z"); let tree1 = testutils::create_tree(&repo, &[(&path_a, "A")]); let tree2 = testutils::create_tree(&repo, &[(&path_a, "A"), (&path_x, "X")]); let tree3 = testutils::create_tree(&repo, &[(&path_a, "B")]); diff --git a/lib/tests/test_merge_trees.rs b/lib/tests/test_merge_trees.rs index 766977d21..bee440db7 100644 --- a/lib/tests/test_merge_trees.rs +++ b/lib/tests/test_merge_trees.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use jujutsu_lib::repo_path::{DirRepoPath, FileRepoPath, RepoPath}; +use jujutsu_lib::repo_path::{DirRepoPath, RepoPath}; use jujutsu_lib::store::{ConflictPart, TreeValue}; use jujutsu_lib::tree::Tree; use jujutsu_lib::{testutils, trees}; @@ -52,11 +52,7 @@ fn test_same_type(use_git: bool) { for path in &files { let contents = &path[index..index + 1]; if contents != "_" { - testutils::write_normal_file( - &mut tree_builder, - &FileRepoPath::from(*path), - contents, - ); + testutils::write_normal_file(&mut tree_builder, &RepoPath::from(*path), contents); } } let tree_id = tree_builder.write_tree(); @@ -186,7 +182,7 @@ fn test_subtrees(use_git: bool) { for path in paths { testutils::write_normal_file( &mut tree_builder, - &FileRepoPath::from(path), + &RepoPath::from(path), &format!("contents of {:?}", path), ); } @@ -244,7 +240,7 @@ fn test_subtree_becomes_empty(use_git: bool) { for path in paths { testutils::write_normal_file( &mut tree_builder, - &FileRepoPath::from(path), + &RepoPath::from(path), &format!("contents of {:?}", path), ); } @@ -278,17 +274,17 @@ fn test_types(use_git: bool) { let mut side2_tree_builder = store.tree_builder(store.empty_tree_id().clone()); testutils::write_normal_file( &mut base_tree_builder, - &FileRepoPath::from("normal_executable_symlink"), + &RepoPath::from("normal_executable_symlink"), "contents", ); testutils::write_executable_file( &mut side1_tree_builder, - &FileRepoPath::from("normal_executable_symlink"), + &RepoPath::from("normal_executable_symlink"), "contents", ); testutils::write_symlink( &mut side2_tree_builder, - &FileRepoPath::from("normal_executable_symlink"), + &RepoPath::from("normal_executable_symlink"), "contents", ); let tree_id = store.empty_tree_id().clone(); @@ -298,12 +294,12 @@ fn test_types(use_git: bool) { ); testutils::write_normal_file( &mut side1_tree_builder, - &FileRepoPath::from("tree_normal_symlink"), + &RepoPath::from("tree_normal_symlink"), "contents", ); testutils::write_symlink( &mut side2_tree_builder, - &FileRepoPath::from("tree_normal_symlink"), + &RepoPath::from("tree_normal_symlink"), "contents", ); let base_tree_id = base_tree_builder.write_tree(); @@ -389,7 +385,7 @@ fn test_simplify_conflict(use_git: bool) { let store = repo.store(); let write_tree = |contents: &str| -> Tree { - testutils::create_tree(&repo, &[(&FileRepoPath::from("file"), contents)]) + testutils::create_tree(&repo, &[(&RepoPath::from("file"), contents)]) }; let base_tree = write_tree("base contents"); diff --git a/lib/tests/test_mut_repo.rs b/lib/tests/test_mut_repo.rs index 6dcfa0633..199f9cf42 100644 --- a/lib/tests/test_mut_repo.rs +++ b/lib/tests/test_mut_repo.rs @@ -15,7 +15,7 @@ use std::sync::Arc; use jujutsu_lib::commit_builder::CommitBuilder; -use jujutsu_lib::repo_path::FileRepoPath; +use jujutsu_lib::repo_path::RepoPath; use jujutsu_lib::store::{Conflict, ConflictId, ConflictPart, TreeValue}; use jujutsu_lib::store_wrapper::StoreWrapper; use jujutsu_lib::testutils; @@ -77,12 +77,12 @@ fn test_checkout_open_with_conflict(use_git: bool) { let (_temp_dir, repo) = testutils::init_repo(&settings, use_git); let store = repo.store(); - let file_path = FileRepoPath::from("file"); + let file_path = RepoPath::from("file"); let conflict_id = write_conflict(store, &file_path); let mut tree_builder = repo .store() .tree_builder(repo.store().empty_tree_id().clone()); - tree_builder.set(file_path.to_repo_path(), TreeValue::Conflict(conflict_id)); + tree_builder.set(file_path.clone(), TreeValue::Conflict(conflict_id)); let tree_id = tree_builder.write_tree(); let mut tx = repo.start_transaction("test"); @@ -93,7 +93,7 @@ fn test_checkout_open_with_conflict(use_git: bool) { let mut tx = repo.start_transaction("test"); let actual_checkout = tx.mut_repo().check_out(&settings, &requested_checkout); - let file_value = actual_checkout.tree().path_value(&file_path.to_repo_path()); + let file_value = actual_checkout.tree().path_value(&file_path); match file_value { Some(TreeValue::Normal { id: _, @@ -116,12 +116,12 @@ fn test_checkout_closed_with_conflict(use_git: bool) { let (_temp_dir, repo) = testutils::init_repo(&settings, use_git); let store = repo.store(); - let file_path = FileRepoPath::from("file"); + let file_path = RepoPath::from("file"); let conflict_id = write_conflict(store, &file_path); let mut tree_builder = repo .store() .tree_builder(repo.store().empty_tree_id().clone()); - tree_builder.set(file_path.to_repo_path(), TreeValue::Conflict(conflict_id)); + tree_builder.set(file_path.clone(), TreeValue::Conflict(conflict_id)); let tree_id = tree_builder.write_tree(); let mut tx = repo.start_transaction("test"); @@ -132,7 +132,7 @@ fn test_checkout_closed_with_conflict(use_git: bool) { let mut tx = repo.start_transaction("test"); let actual_checkout = tx.mut_repo().check_out(&settings, &requested_checkout); - let file_value = actual_checkout.tree().path_value(&file_path.to_repo_path()); + let file_value = actual_checkout.tree().path_value(&file_path); match file_value { Some(TreeValue::Normal { id: _, @@ -146,7 +146,7 @@ fn test_checkout_closed_with_conflict(use_git: bool) { assert_eq!(repo.view().checkout(), actual_checkout.id()); } -fn write_conflict(store: &Arc, file_path: &FileRepoPath) -> ConflictId { +fn write_conflict(store: &Arc, file_path: &RepoPath) -> ConflictId { let file_id1 = testutils::write_file(store, &file_path, "a\n"); let file_id2 = testutils::write_file(store, &file_path, "b\n"); let file_id3 = testutils::write_file(store, &file_path, "c\n"); diff --git a/lib/tests/test_working_copy.rs b/lib/tests/test_working_copy.rs index 19d11b7a9..538e20b30 100644 --- a/lib/tests/test_working_copy.rs +++ b/lib/tests/test_working_copy.rs @@ -20,7 +20,7 @@ use std::sync::Arc; use jujutsu_lib::commit_builder::CommitBuilder; use jujutsu_lib::repo::ReadonlyRepo; -use jujutsu_lib::repo_path::{FileRepoPath, RepoPath}; +use jujutsu_lib::repo_path::RepoPath; use jujutsu_lib::settings::UserSettings; use jujutsu_lib::store::TreeValue; use jujutsu_lib::testutils; @@ -86,18 +86,15 @@ fn test_checkout_file_transitions(use_git: bool) { } Kind::Normal => { let id = - testutils::write_file(store, &FileRepoPath::from(path), "normal file contents"); + testutils::write_file(store, &RepoPath::from(path), "normal file contents"); TreeValue::Normal { id, executable: false, } } Kind::Executable => { - let id = testutils::write_file( - store, - &FileRepoPath::from(path), - "executable file contents", - ); + let id = + testutils::write_file(store, &RepoPath::from(path), "executable file contents"); TreeValue::Normal { id, executable: true, @@ -105,7 +102,7 @@ fn test_checkout_file_transitions(use_git: bool) { } Kind::Symlink => { let id = store - .write_symlink(&FileRepoPath::from(path), "target") + .write_symlink(&RepoPath::from(path), "target") .unwrap(); TreeValue::Symlink(id) } @@ -279,13 +276,13 @@ fn test_gitignores(use_git: bool) { let settings = testutils::user_settings(); let (_temp_dir, repo) = testutils::init_repo(&settings, use_git); - let gitignore_path = FileRepoPath::from(".gitignore"); - let added_path = FileRepoPath::from("added"); - let modified_path = FileRepoPath::from("modified"); - let removed_path = FileRepoPath::from("removed"); - let ignored_path = FileRepoPath::from("ignored"); - let subdir_modified_path = FileRepoPath::from("dir/modified"); - let subdir_ignored_path = FileRepoPath::from("dir/ignored"); + let gitignore_path = RepoPath::from(".gitignore"); + let added_path = RepoPath::from("added"); + let modified_path = RepoPath::from("modified"); + let removed_path = RepoPath::from("removed"); + let ignored_path = RepoPath::from("ignored"); + let subdir_modified_path = RepoPath::from("dir/modified"); + let subdir_ignored_path = RepoPath::from("dir/ignored"); testutils::write_working_copy_file(&repo, &gitignore_path, "ignored\n"); testutils::write_working_copy_file(&repo, &modified_path, "1"); @@ -303,10 +300,10 @@ fn test_gitignores(use_git: bool) { assert_eq!( files1, vec![ - gitignore_path.to_repo_path(), - subdir_modified_path.to_repo_path(), - modified_path.to_repo_path(), - removed_path.to_repo_path() + gitignore_path.clone(), + subdir_modified_path.clone(), + modified_path.clone(), + removed_path.clone(), ] ); @@ -328,10 +325,10 @@ fn test_gitignores(use_git: bool) { assert_eq!( files2, vec![ - gitignore_path.to_repo_path(), - added_path.to_repo_path(), - subdir_modified_path.to_repo_path(), - modified_path.to_repo_path() + gitignore_path.clone(), + added_path.clone(), + subdir_modified_path.clone(), + modified_path.clone(), ] ); } @@ -347,9 +344,9 @@ fn test_gitignores_checkout_overwrites_ignored(use_git: bool) { let (_temp_dir, repo) = testutils::init_repo(&settings, use_git); // Write an ignored file called "modified" to disk - let gitignore_path = FileRepoPath::from(".gitignore"); + let gitignore_path = RepoPath::from(".gitignore"); testutils::write_working_copy_file(&repo, &gitignore_path, "modified\n"); - let modified_path = FileRepoPath::from("modified"); + let modified_path = RepoPath::from("modified"); testutils::write_working_copy_file(&repo, &modified_path, "garbage"); // Create a commit that adds the same file but with different contents @@ -394,9 +391,9 @@ fn test_gitignores_ignored_directory_already_tracked(use_git: bool) { let (_temp_dir, repo) = testutils::init_repo(&settings, use_git); // Add a .gitignore file saying to ignore the directory "ignored/" - let gitignore_path = FileRepoPath::from(".gitignore"); + let gitignore_path = RepoPath::from(".gitignore"); testutils::write_working_copy_file(&repo, &gitignore_path, "/ignored/\n"); - let file_path = FileRepoPath::from("ignored/file"); + let file_path = RepoPath::from("ignored/file"); // Create a commit that adds a file in the ignored directory let mut tx = repo.start_transaction("test"); @@ -417,8 +414,5 @@ fn test_gitignores_ignored_directory_already_tracked(use_git: bool) { // Check that the file is still in the commit created by committing the working // copy (that it didn't get removed because the directory is ignored) let (_repo, new_commit) = repo.working_copy_locked().commit(&settings, repo.clone()); - assert!(new_commit - .tree() - .path_value(&file_path.to_repo_path()) - .is_some()); + assert!(new_commit.tree().path_value(&file_path).is_some()); } diff --git a/lib/tests/test_working_copy_concurrent.rs b/lib/tests/test_working_copy_concurrent.rs index 211c94515..556b0dea8 100644 --- a/lib/tests/test_working_copy_concurrent.rs +++ b/lib/tests/test_working_copy_concurrent.rs @@ -18,7 +18,7 @@ use std::thread; use jujutsu_lib::commit_builder::CommitBuilder; use jujutsu_lib::repo::ReadonlyRepo; -use jujutsu_lib::repo_path::FileRepoPath; +use jujutsu_lib::repo_path::RepoPath; use jujutsu_lib::store::CommitId; use jujutsu_lib::testutils; use jujutsu_lib::working_copy::CheckoutError; @@ -82,7 +82,7 @@ fn test_concurrent_commit(use_git: bool) { // Commit from another process (simulated by another repo instance) let repo2 = ReadonlyRepo::load(&settings, repo1.working_copy_path().clone()).unwrap(); - testutils::write_working_copy_file(&repo2, &FileRepoPath::from("file2"), "contents2"); + testutils::write_working_copy_file(&repo2, &RepoPath::from("file2"), "contents2"); let owned_wc2 = repo2.working_copy().clone(); let wc2 = owned_wc2.lock().unwrap(); let commit2 = wc2.commit(&settings, repo2).1; @@ -91,7 +91,7 @@ fn test_concurrent_commit(use_git: bool) { // Creating another commit (via the first repo instance) should result in a // successor of the commit created from the other process. - testutils::write_working_copy_file(&repo1, &FileRepoPath::from("file3"), "contents3"); + testutils::write_working_copy_file(&repo1, &RepoPath::from("file3"), "contents3"); let commit3 = wc1.commit(&settings, repo1).1; assert_eq!(commit3.predecessors(), vec![commit2]); } @@ -108,7 +108,7 @@ fn test_checkout_parallel(use_git: bool) { let num_threads = max(num_cpus::get(), 4); let mut commit_ids = vec![]; for i in 0..num_threads { - let path = FileRepoPath::from(format!("file{}", i).as_str()); + let path = RepoPath::from(format!("file{}", i).as_str()); let tree = testutils::create_tree(&repo, &[(&path, "contents")]); let commit = CommitBuilder::for_new_commit(&settings, store, tree.id().clone()) .set_open(true) @@ -118,7 +118,7 @@ fn test_checkout_parallel(use_git: bool) { // Create another commit just so we can test the update stats reliably from the // first update - let tree = testutils::create_tree(&repo, &[(&FileRepoPath::from("other file"), "contents")]); + let tree = testutils::create_tree(&repo, &[(&RepoPath::from("other file"), "contents")]); let mut tx = repo.start_transaction("test"); let commit = CommitBuilder::for_new_commit(&settings, store, tree.id().clone()) .set_open(true) diff --git a/src/commands.rs b/src/commands.rs index 241c3f877..1dac215f1 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -970,7 +970,7 @@ fn cmd_diff( let mut styler = ui.styler(); styler.add_label(String::from("diff"))?; for (path, diff) in from_tree.diff(&to_tree) { - let ui_path = ui.format_file_path(repo.working_copy_path(), &path.to_repo_path()); + let ui_path = ui.format_file_path(repo.working_copy_path(), &path); match diff { Diff::Added(TreeValue::Normal { id, @@ -1028,7 +1028,7 @@ fn cmd_diff( let mut buffer_left = vec![]; conflicts::materialize_conflict( repo.store(), - &path.to_repo_path(), + &path, &conflict_left, &mut buffer_left, ); @@ -1059,7 +1059,7 @@ fn cmd_diff( let mut buffer_right = vec![]; conflicts::materialize_conflict( repo.store(), - &path.to_repo_path(), + &path, &conflict_right, &mut buffer_right, ); @@ -1098,25 +1098,13 @@ fn cmd_diff( fn show_diff_summary(ui: &mut Ui, wc_path: &Path, from: &Tree, to: &Tree) -> io::Result<()> { let summary = from.diff_summary(&to); for file in summary.modified { - writeln!( - ui, - "M {}", - ui.format_file_path(wc_path, &file.to_repo_path()) - )?; + writeln!(ui, "M {}", ui.format_file_path(wc_path, &file))?; } for file in summary.added { - writeln!( - ui, - "A {}", - ui.format_file_path(wc_path, &file.to_repo_path()) - )?; + writeln!(ui, "A {}", ui.format_file_path(wc_path, &file))?; } for file in summary.removed { - writeln!( - ui, - "R {}", - ui.format_file_path(wc_path, &file.to_repo_path()) - )?; + writeln!(ui, "R {}", ui.format_file_path(wc_path, &file))?; } Ok(()) } diff --git a/src/diff_edit.rs b/src/diff_edit.rs index 7afe6f579..8f2de83e1 100644 --- a/src/diff_edit.rs +++ b/src/diff_edit.rs @@ -106,12 +106,11 @@ pub fn edit_diff( let mut right_tree_builder = store.tree_builder(store.empty_tree_id().clone()); for (file_path, diff) in left_tree.diff(&right_tree) { let (left_value, right_value) = diff.as_options(); - let repo_path = file_path.to_repo_path(); if let Some(value) = left_value { - add_to_tree(store, &mut left_tree_builder, &repo_path, value).unwrap(); + add_to_tree(store, &mut left_tree_builder, &file_path, value).unwrap(); } if let Some(value) = right_value { - add_to_tree(store, &mut right_tree_builder, &repo_path, value).unwrap(); + add_to_tree(store, &mut right_tree_builder, &file_path, value).unwrap(); } } let left_partial_tree_id = left_tree_builder.write_tree();