From 7169f5c7609a93ad282aa0d4b5f9969ea6f447cc Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Tue, 9 May 2023 08:36:43 -0700 Subject: [PATCH 01/34] Add git status to the file system abstraction co-authored-by: petros --- Cargo.lock | 1 + crates/fs/Cargo.toml | 1 + crates/fs/src/repository.rs | 82 +++++++++++++++++++++++++++++++++- crates/project/src/worktree.rs | 37 +++++---------- 4 files changed, 94 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bef04fce14..bd1dd4f33b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2350,6 +2350,7 @@ dependencies = [ "serde_derive", "serde_json", "smol", + "sum_tree", "tempfile", "util", ] diff --git a/crates/fs/Cargo.toml b/crates/fs/Cargo.toml index d080fe3cd1..54c6ce362a 100644 --- a/crates/fs/Cargo.toml +++ b/crates/fs/Cargo.toml @@ -13,6 +13,7 @@ gpui = { path = "../gpui" } lsp = { path = "../lsp" } rope = { path = "../rope" } util = { path = "../util" } +sum_tree = { path = "../sum_tree" } anyhow.workspace = true async-trait.workspace = true futures.workspace = true diff --git a/crates/fs/src/repository.rs b/crates/fs/src/repository.rs index 5624ce42f1..14e7e75a3d 100644 --- a/crates/fs/src/repository.rs +++ b/crates/fs/src/repository.rs @@ -1,9 +1,10 @@ use anyhow::Result; use collections::HashMap; use parking_lot::Mutex; +use sum_tree::TreeMap; use std::{ path::{Component, Path, PathBuf}, - sync::Arc, + sync::Arc, ffi::OsStr, os::unix::prelude::OsStrExt, }; use util::ResultExt; @@ -16,6 +17,8 @@ pub trait GitRepository: Send { fn load_index_text(&self, relative_file_path: &Path) -> Option; fn branch_name(&self) -> Option; + + fn statuses(&self) -> Option>; } impl std::fmt::Debug for dyn GitRepository { @@ -61,6 +64,79 @@ impl GitRepository for LibGitRepository { let branch = String::from_utf8_lossy(head.shorthand_bytes()); Some(branch.to_string()) } + + fn statuses(&self) -> Option> { + let statuses = self.statuses(None).log_err()?; + + let mut map = TreeMap::default(); + + for status in statuses.iter() { + let path = RepoPath(PathBuf::from(OsStr::from_bytes(status.path_bytes()))); + + let status_data = status.status(); + + let status = if status_data.contains(git2::Status::CONFLICTED) { + GitStatus::Conflict + } else if status_data.intersects(git2::Status::INDEX_MODIFIED + | git2::Status::WT_MODIFIED + | git2::Status::INDEX_RENAMED + | git2::Status::WT_RENAMED) { + GitStatus::Modified + } else if status_data.intersects(git2::Status::INDEX_NEW | git2::Status::WT_NEW) { + GitStatus::Added + } else { + GitStatus::Untracked + }; + + map.insert(path, status) + } + + Some(map) + } +} + +#[derive(Debug, Clone, Default)] +pub enum GitStatus { + Added, + Modified, + Conflict, + #[default] + Untracked, +} + +#[derive(Clone, Debug, Ord, Hash, PartialOrd, Eq, PartialEq)] +pub struct RepoPath(PathBuf); + +impl From<&Path> for RepoPath { + fn from(value: &Path) -> Self { + RepoPath(value.to_path_buf()) + } +} + +impl From for RepoPath { + fn from(value: PathBuf) -> Self { + RepoPath(value) + } +} + +impl Default for RepoPath { + fn default() -> Self { + RepoPath(PathBuf::new()) + } +} + +impl AsRef for RepoPath { + fn as_ref(&self) -> &Path { + self.0.as_ref() + } +} + +impl std::ops::Deref for RepoPath { + type Target = PathBuf; + + fn deref(&self) -> &Self::Target { + &self.0 + } } #[derive(Debug, Clone, Default)] @@ -93,6 +169,10 @@ impl GitRepository for FakeGitRepository { let state = self.state.lock(); state.branch_name.clone() } + + fn statuses(&self) -> Option>{ + todo!() + } } fn check_path_to_repo_path_errors(relative_file_path: &Path) -> Result<()> { diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 554304f3d3..e236d18efd 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -6,7 +6,7 @@ use anyhow::{anyhow, Context, Result}; use client::{proto, Client}; use clock::ReplicaId; use collections::{HashMap, VecDeque}; -use fs::{repository::GitRepository, Fs, LineEnding}; +use fs::{repository::{GitRepository, RepoPath, GitStatus}, Fs, LineEnding}; use futures::{ channel::{ mpsc::{self, UnboundedSender}, @@ -117,10 +117,11 @@ pub struct Snapshot { completed_scan_id: usize, } -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct RepositoryEntry { pub(crate) work_directory: WorkDirectoryEntry, pub(crate) branch: Option>, + // pub(crate) statuses: TreeMap } impl RepositoryEntry { @@ -162,6 +163,13 @@ impl Default for RepositoryWorkDirectory { } } +impl AsRef for RepositoryWorkDirectory { + fn as_ref(&self) -> &Path { + self.0.as_ref() + } +} + + #[derive(Clone, Debug, Ord, PartialOrd, Eq, PartialEq)] pub struct WorkDirectoryEntry(ProjectEntryId); @@ -178,7 +186,7 @@ impl WorkDirectoryEntry { worktree.entry_for_id(self.0).and_then(|entry| { path.strip_prefix(&entry.path) .ok() - .map(move |path| RepoPath(path.to_owned())) + .map(move |path| path.into()) }) } } @@ -197,29 +205,6 @@ impl<'a> From for WorkDirectoryEntry { } } -#[derive(Clone, Debug, Ord, PartialOrd, Eq, PartialEq)] -pub struct RepoPath(PathBuf); - -impl AsRef for RepoPath { - fn as_ref(&self) -> &Path { - self.0.as_ref() - } -} - -impl Deref for RepoPath { - type Target = PathBuf; - - fn deref(&self) -> &Self::Target { - &self.0 - } -} - -impl AsRef for RepositoryWorkDirectory { - fn as_ref(&self) -> &Path { - self.0.as_ref() - } -} - #[derive(Debug, Clone)] pub struct LocalSnapshot { ignores_by_parent_abs_path: HashMap, (Arc, usize)>, From 67491632cbce955038e907360c35027e094e4028 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Tue, 9 May 2023 10:02:58 -0700 Subject: [PATCH 02/34] WIP: Track live entry status in repository co-authored-by: petros --- crates/fs/src/repository.rs | 152 +++++++++++++++++++------------- crates/project/src/worktree.rs | 95 +++++++++++++++----- crates/sum_tree/src/tree_map.rs | 4 +- 3 files changed, 165 insertions(+), 86 deletions(-) diff --git a/crates/fs/src/repository.rs b/crates/fs/src/repository.rs index 14e7e75a3d..626fbf9e12 100644 --- a/crates/fs/src/repository.rs +++ b/crates/fs/src/repository.rs @@ -1,11 +1,14 @@ use anyhow::Result; use collections::HashMap; +use git2::Status; use parking_lot::Mutex; -use sum_tree::TreeMap; use std::{ + ffi::OsStr, + os::unix::prelude::OsStrExt, path::{Component, Path, PathBuf}, - sync::Arc, ffi::OsStr, os::unix::prelude::OsStrExt, + sync::Arc, }; +use sum_tree::TreeMap; use util::ResultExt; pub use git2::Repository as LibGitRepository; @@ -19,6 +22,8 @@ pub trait GitRepository: Send { fn branch_name(&self) -> Option; fn statuses(&self) -> Option>; + + fn file_status(&self, path: &RepoPath) -> Option; } impl std::fmt::Debug for dyn GitRepository { @@ -70,72 +75,22 @@ impl GitRepository for LibGitRepository { let mut map = TreeMap::default(); - for status in statuses.iter() { + for status in statuses + .iter() + .filter(|status| !status.status().contains(git2::Status::IGNORED)) + { let path = RepoPath(PathBuf::from(OsStr::from_bytes(status.path_bytes()))); - let status_data = status.status(); - - let status = if status_data.contains(git2::Status::CONFLICTED) { - GitStatus::Conflict - } else if status_data.intersects(git2::Status::INDEX_MODIFIED - | git2::Status::WT_MODIFIED - | git2::Status::INDEX_RENAMED - | git2::Status::WT_RENAMED) { - GitStatus::Modified - } else if status_data.intersects(git2::Status::INDEX_NEW | git2::Status::WT_NEW) { - GitStatus::Added - } else { - GitStatus::Untracked - }; - - map.insert(path, status) + map.insert(path, status.status().into()) } Some(map) } -} -#[derive(Debug, Clone, Default)] -pub enum GitStatus { - Added, - Modified, - Conflict, - #[default] - Untracked, -} + fn file_status(&self, path: &RepoPath) -> Option { + let status = self.status_file(path).log_err()?; -#[derive(Clone, Debug, Ord, Hash, PartialOrd, Eq, PartialEq)] -pub struct RepoPath(PathBuf); - -impl From<&Path> for RepoPath { - fn from(value: &Path) -> Self { - RepoPath(value.to_path_buf()) - } -} - -impl From for RepoPath { - fn from(value: PathBuf) -> Self { - RepoPath(value) - } -} - -impl Default for RepoPath { - fn default() -> Self { - RepoPath(PathBuf::new()) - } -} - -impl AsRef for RepoPath { - fn as_ref(&self) -> &Path { - self.0.as_ref() - } -} - -impl std::ops::Deref for RepoPath { - type Target = PathBuf; - - fn deref(&self) -> &Self::Target { - &self.0 + Some(status.into()) } } @@ -170,7 +125,11 @@ impl GitRepository for FakeGitRepository { state.branch_name.clone() } - fn statuses(&self) -> Option>{ + fn statuses(&self) -> Option> { + todo!() + } + + fn file_status(&self, _: &RepoPath) -> Option { todo!() } } @@ -203,3 +162,74 @@ fn check_path_to_repo_path_errors(relative_file_path: &Path) -> Result<()> { _ => Ok(()), } } + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub enum GitStatus { + Added, + Modified, + Conflict, + #[default] + Untracked, +} + +impl From for GitStatus { + fn from(value: Status) -> Self { + if value.contains(git2::Status::CONFLICTED) { + GitStatus::Conflict + } else if value.intersects( + git2::Status::INDEX_MODIFIED + | git2::Status::WT_MODIFIED + | git2::Status::INDEX_RENAMED + | git2::Status::WT_RENAMED, + ) { + GitStatus::Modified + } else if value.intersects(git2::Status::INDEX_NEW | git2::Status::WT_NEW) { + GitStatus::Added + } else { + GitStatus::Untracked + } + } +} + +#[derive(Clone, Debug, Ord, Hash, PartialOrd, Eq, PartialEq)] +pub struct RepoPath(PathBuf); + +impl RepoPath { + fn new(path: PathBuf) -> Self { + debug_assert!(path.is_relative(), "Repo paths must be relative"); + + RepoPath(path) + } +} + +impl From<&Path> for RepoPath { + fn from(value: &Path) -> Self { + RepoPath::new(value.to_path_buf()) + } +} + +impl From for RepoPath { + fn from(value: PathBuf) -> Self { + RepoPath::new(value) + } +} + +impl Default for RepoPath { + fn default() -> Self { + RepoPath(PathBuf::new()) + } +} + +impl AsRef for RepoPath { + fn as_ref(&self) -> &Path { + self.0.as_ref() + } +} + +impl std::ops::Deref for RepoPath { + type Target = PathBuf; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index e236d18efd..e43ab9257b 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -6,7 +6,10 @@ use anyhow::{anyhow, Context, Result}; use client::{proto, Client}; use clock::ReplicaId; use collections::{HashMap, VecDeque}; -use fs::{repository::{GitRepository, RepoPath, GitStatus}, Fs, LineEnding}; +use fs::{ + repository::{GitRepository, GitStatus, RepoPath}, + Fs, LineEnding, +}; use futures::{ channel::{ mpsc::{self, UnboundedSender}, @@ -121,7 +124,7 @@ pub struct Snapshot { pub struct RepositoryEntry { pub(crate) work_directory: WorkDirectoryEntry, pub(crate) branch: Option>, - // pub(crate) statuses: TreeMap + pub(crate) statuses: TreeMap, } impl RepositoryEntry { @@ -169,7 +172,6 @@ impl AsRef for RepositoryWorkDirectory { } } - #[derive(Clone, Debug, Ord, PartialOrd, Eq, PartialEq)] pub struct WorkDirectoryEntry(ProjectEntryId); @@ -219,6 +221,7 @@ pub struct LocalSnapshot { #[derive(Debug, Clone)] pub struct LocalRepositoryEntry { pub(crate) scan_id: usize, + pub(crate) full_scan_id: usize, pub(crate) repo_ptr: Arc>, /// Path to the actual .git folder. /// Note: if .git is a file, this points to the folder indicated by the .git file @@ -1412,6 +1415,8 @@ impl Snapshot { let repository = RepositoryEntry { work_directory: ProjectEntryId::from_proto(repository.work_directory_id).into(), branch: repository.branch.map(Into::into), + // TODO: status + statuses: Default::default(), }; if let Some(entry) = self.entry_for_id(repository.work_directory_id()) { self.repository_entries @@ -1572,6 +1577,10 @@ impl LocalSnapshot { current_candidate.map(|entry| entry.to_owned()) } + pub(crate) fn get_local_repo(&self, repo: &RepositoryEntry) -> Option<&LocalRepositoryEntry> { + self.git_repositories.get(&repo.work_directory.0) + } + pub(crate) fn repo_for_metadata( &self, path: &Path, @@ -1842,6 +1851,7 @@ impl LocalSnapshot { RepositoryEntry { work_directory: work_dir_id.into(), branch: repo_lock.branch_name().map(Into::into), + statuses: repo_lock.statuses().unwrap_or_default(), }, ); drop(repo_lock); @@ -1850,6 +1860,7 @@ impl LocalSnapshot { work_dir_id, LocalRepositoryEntry { scan_id, + full_scan_id: scan_id, repo_ptr: repo, git_dir_path: parent_path.clone(), }, @@ -2825,26 +2836,7 @@ impl BackgroundScanner { fs_entry.is_ignored = ignore_stack.is_all(); snapshot.insert_entry(fs_entry, self.fs.as_ref()); - let scan_id = snapshot.scan_id; - - let repo_with_path_in_dotgit = snapshot.repo_for_metadata(&path); - if let Some((entry_id, repo)) = repo_with_path_in_dotgit { - let work_dir = snapshot - .entry_for_id(entry_id) - .map(|entry| RepositoryWorkDirectory(entry.path.clone()))?; - - let repo = repo.lock(); - repo.reload_index(); - let branch = repo.branch_name(); - - snapshot.git_repositories.update(&entry_id, |entry| { - entry.scan_id = scan_id; - }); - - snapshot - .repository_entries - .update(&work_dir, |entry| entry.branch = branch.map(Into::into)); - } + self.reload_repo_for_path(&path, &mut snapshot); if let Some(scan_queue_tx) = &scan_queue_tx { let mut ancestor_inodes = snapshot.ancestor_inodes_for_path(&path); @@ -2872,6 +2864,63 @@ impl BackgroundScanner { Some(event_paths) } + fn reload_repo_for_path(&self, path: &Path, snapshot: &mut LocalSnapshot) -> Option<()> { + let scan_id = snapshot.scan_id; + + if path + .components() + .any(|component| component.as_os_str() == *DOT_GIT) + { + let (entry_id, repo) = snapshot.repo_for_metadata(&path)?; + + let work_dir = snapshot + .entry_for_id(entry_id) + .map(|entry| RepositoryWorkDirectory(entry.path.clone()))?; + + let repo = repo.lock(); + repo.reload_index(); + let branch = repo.branch_name(); + let statuses = repo.statuses().unwrap_or_default(); + + snapshot.git_repositories.update(&entry_id, |entry| { + entry.scan_id = scan_id; + entry.full_scan_id = scan_id; + }); + + snapshot.repository_entries.update(&work_dir, |entry| { + entry.branch = branch.map(Into::into); + entry.statuses = statuses; + }); + } else if let Some(repo) = snapshot.repo_for(&path) { + let status = { + let local_repo = snapshot.get_local_repo(&repo)?; + // Short circuit if we've already scanned everything + if local_repo.full_scan_id == scan_id { + return None; + } + + let repo_path = repo.work_directory.relativize(&snapshot, &path)?; + let git_ptr = local_repo.repo_ptr.lock(); + git_ptr.file_status(&repo_path)? + }; + + if status != GitStatus::Untracked { + let work_dir = repo.work_directory(snapshot)?; + let work_dir_id = repo.work_directory; + + snapshot + .git_repositories + .update(&work_dir_id, |entry| entry.scan_id = scan_id); + + snapshot + .repository_entries + .update(&work_dir, |entry| entry.statuses.insert(repo_path, status)); + } + } + + Some(()) + } + async fn update_ignore_statuses(&self) { use futures::FutureExt as _; diff --git a/crates/sum_tree/src/tree_map.rs b/crates/sum_tree/src/tree_map.rs index 1b97cbec9f..ab37d2577a 100644 --- a/crates/sum_tree/src/tree_map.rs +++ b/crates/sum_tree/src/tree_map.rs @@ -2,13 +2,13 @@ use std::{cmp::Ordering, fmt::Debug}; use crate::{Bias, Dimension, Item, KeyedItem, SeekTarget, SumTree, Summary}; -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct TreeMap(SumTree>) where K: Clone + Debug + Default + Ord, V: Clone + Debug; -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct MapEntry { key: K, value: V, From bd98f7810148076a9743fe1a5313f79c2797e29a Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Tue, 9 May 2023 10:04:44 -0700 Subject: [PATCH 03/34] Fix compile error --- crates/project/src/worktree.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index e43ab9257b..570ff94f4e 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -2892,6 +2892,8 @@ impl BackgroundScanner { entry.statuses = statuses; }); } else if let Some(repo) = snapshot.repo_for(&path) { + let repo_path = repo.work_directory.relativize(&snapshot, &path)?; + let status = { let local_repo = snapshot.get_local_repo(&repo)?; // Short circuit if we've already scanned everything @@ -2899,7 +2901,6 @@ impl BackgroundScanner { return None; } - let repo_path = repo.work_directory.relativize(&snapshot, &path)?; let git_ptr = local_repo.repo_ptr.lock(); git_ptr.file_status(&repo_path)? }; From 93f57430dad33b14c8e77d6239c5d4391ab651f9 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Tue, 9 May 2023 10:13:22 -0700 Subject: [PATCH 04/34] Track live entry status in repository --- crates/project/src/worktree.rs | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 570ff94f4e..fb8a0ce9e7 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -2853,7 +2853,9 @@ impl BackgroundScanner { } } } - Ok(None) => {} + Ok(None) => { + self.remove_repo_path(&path, &mut snapshot); + } Err(err) => { // TODO - create a special 'error' entry in the entries tree to mark this log::error!("error reading file on event {:?}", err); @@ -2864,6 +2866,31 @@ impl BackgroundScanner { Some(event_paths) } + fn remove_repo_path(&self, path: &Path, snapshot: &mut LocalSnapshot) -> Option<()> { + if !path + .components() + .any(|component| component.as_os_str() == *DOT_GIT) + { + let scan_id = snapshot.scan_id; + let repo = snapshot.repo_for(&path)?; + + let repo_path = repo.work_directory.relativize(&snapshot, &path)?; + + let work_dir = repo.work_directory(snapshot)?; + let work_dir_id = repo.work_directory; + + snapshot + .git_repositories + .update(&work_dir_id, |entry| entry.scan_id = scan_id); + + snapshot + .repository_entries + .update(&work_dir, |entry| entry.statuses.remove(&repo_path)); + } + + Some(()) + } + fn reload_repo_for_path(&self, path: &Path, snapshot: &mut LocalSnapshot) -> Option<()> { let scan_id = snapshot.scan_id; @@ -2891,11 +2918,14 @@ impl BackgroundScanner { entry.branch = branch.map(Into::into); entry.statuses = statuses; }); - } else if let Some(repo) = snapshot.repo_for(&path) { + } else { + let repo = snapshot.repo_for(&path)?; + let repo_path = repo.work_directory.relativize(&snapshot, &path)?; let status = { let local_repo = snapshot.get_local_repo(&repo)?; + // Short circuit if we've already scanned everything if local_repo.full_scan_id == scan_id { return None; From e98507d8bf088895936ed7fb85ed3302c9e6639f Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Tue, 9 May 2023 14:42:51 -0700 Subject: [PATCH 05/34] Added git status to the project panel, added worktree test --- Cargo.lock | 1 + crates/project/Cargo.toml | 1 + crates/project/src/worktree.rs | 240 ++++++++++++++++++++-- crates/project_panel/src/project_panel.rs | 23 ++- 4 files changed, 246 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bd1dd4f33b..0190b4d8f5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4717,6 +4717,7 @@ dependencies = [ "futures 0.3.25", "fuzzy", "git", + "git2", "glob", "gpui", "ignore", diff --git a/crates/project/Cargo.toml b/crates/project/Cargo.toml index 2b4892aab9..85a302bdd7 100644 --- a/crates/project/Cargo.toml +++ b/crates/project/Cargo.toml @@ -74,5 +74,6 @@ lsp = { path = "../lsp", features = ["test-support"] } settings = { path = "../settings", features = ["test-support"] } util = { path = "../util", features = ["test-support"] } rpc = { path = "../rpc", features = ["test-support"] } +git2 = { version = "0.15", default-features = false } tempdir.workspace = true unindent.workspace = true diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index fb8a0ce9e7..cf116d188f 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -120,6 +120,25 @@ pub struct Snapshot { completed_scan_id: usize, } +impl Snapshot { + pub fn repo_for(&self, path: &Path) -> Option { + let mut max_len = 0; + let mut current_candidate = None; + for (work_directory, repo) in (&self.repository_entries).iter() { + if repo.contains(self, path) { + if work_directory.0.as_os_str().len() >= max_len { + current_candidate = Some(repo); + max_len = work_directory.0.as_os_str().len(); + } else { + break; + } + } + } + + current_candidate.map(|entry| entry.to_owned()) + } +} + #[derive(Clone, Debug, PartialEq, Eq)] pub struct RepositoryEntry { pub(crate) work_directory: WorkDirectoryEntry, @@ -145,6 +164,13 @@ impl RepositoryEntry { pub(crate) fn contains(&self, snapshot: &Snapshot, path: &Path) -> bool { self.work_directory.contains(snapshot, path) } + + pub fn status_for(&self, snapshot: &Snapshot, path: &Path) -> Option { + self.work_directory + .relativize(snapshot, path) + .and_then(|repo_path| self.statuses.get(&repo_path)) + .cloned() + } } impl From<&RepositoryEntry> for proto::RepositoryEntry { @@ -1560,23 +1586,6 @@ impl Snapshot { } impl LocalSnapshot { - pub(crate) fn repo_for(&self, path: &Path) -> Option { - let mut max_len = 0; - let mut current_candidate = None; - for (work_directory, repo) in (&self.repository_entries).iter() { - if repo.contains(self, path) { - if work_directory.0.as_os_str().len() >= max_len { - current_candidate = Some(repo); - max_len = work_directory.0.as_os_str().len(); - } else { - break; - } - } - } - - current_candidate.map(|entry| entry.to_owned()) - } - pub(crate) fn get_local_repo(&self, repo: &RepositoryEntry) -> Option<&LocalRepositoryEntry> { self.git_repositories.get(&repo.work_directory.0) } @@ -3751,6 +3760,203 @@ mod tests { }); } + #[gpui::test] + async fn test_git_status(cx: &mut TestAppContext) { + #[track_caller] + fn git_init(path: &Path) -> git2::Repository { + git2::Repository::init(path).expect("Failed to initialize git repository") + } + + #[track_caller] + fn git_add(path: &Path, repo: &git2::Repository) { + let mut index = repo.index().expect("Failed to get index"); + index.add_path(path).expect("Failed to add a.txt"); + index.write().expect("Failed to write index"); + } + + #[track_caller] + fn git_remove_index(path: &Path, repo: &git2::Repository) { + let mut index = repo.index().expect("Failed to get index"); + index.remove_path(path).expect("Failed to add a.txt"); + index.write().expect("Failed to write index"); + } + + #[track_caller] + fn git_commit(msg: &'static str, repo: &git2::Repository) { + let signature = repo.signature().unwrap(); + let oid = repo.index().unwrap().write_tree().unwrap(); + let tree = repo.find_tree(oid).unwrap(); + if let Some(head) = repo.head().ok() { + let parent_obj = head + .peel(git2::ObjectType::Commit) + .unwrap(); + + let parent_commit = parent_obj + .as_commit() + .unwrap(); + + + repo.commit( + Some("HEAD"), + &signature, + &signature, + msg, + &tree, + &[parent_commit], + ) + .expect("Failed to commit with parent"); + } else { + repo.commit( + Some("HEAD"), + &signature, + &signature, + msg, + &tree, + &[], + ) + .expect("Failed to commit"); + } + } + + #[track_caller] + fn git_stash(repo: &mut git2::Repository) { + let signature = repo.signature().unwrap(); + repo.stash_save(&signature, "N/A", None) + .expect("Failed to stash"); + } + + #[track_caller] + fn git_reset(offset: usize, repo: &git2::Repository) { + let head = repo.head().expect("Couldn't get repo head"); + let object = head.peel(git2::ObjectType::Commit).unwrap(); + let commit = object.as_commit().unwrap(); + let new_head = commit + .parents() + .inspect(|parnet| { + parnet.message(); + }) + .skip(offset) + .next() + .expect("Not enough history"); + repo.reset(&new_head.as_object(), git2::ResetType::Soft, None) + .expect("Could not reset"); + } + + #[track_caller] + fn git_status(repo: &git2::Repository) -> HashMap { + repo.statuses(None) + .unwrap() + .iter() + .map(|status| { + (status.path().unwrap().to_string(), status.status()) + }) + .collect() + } + + let root = temp_tree(json!({ + "project": { + "a.txt": "a", + "b.txt": "bb", + }, + + })); + + let http_client = FakeHttpClient::with_404_response(); + let client = cx.read(|cx| Client::new(http_client, cx)); + let tree = Worktree::local( + client, + root.path(), + true, + Arc::new(RealFs), + Default::default(), + &mut cx.to_async(), + ) + .await + .unwrap(); + + const A_TXT: &'static str = "a.txt"; + const B_TXT: &'static str = "b.txt"; + let work_dir = root.path().join("project"); + + let mut repo = git_init(work_dir.as_path()); + git_add(Path::new(A_TXT), &repo); + git_commit("Initial commit", &repo); + + std::fs::write(work_dir.join(A_TXT), "aa").unwrap(); + + cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) + .await; + tree.flush_fs_events(cx).await; + + // Check that the right git state is observed on startup + tree.read_with(cx, |tree, _cx| { + let snapshot = tree.snapshot(); + assert_eq!(snapshot.repository_entries.iter().count(), 1); + let (dir, repo) = snapshot.repository_entries.iter().next().unwrap(); + assert_eq!(dir.0.as_ref(), Path::new("project")); + + assert_eq!(repo.statuses.iter().count(), 2); + assert_eq!( + repo.statuses.get(&Path::new(A_TXT).into()), + Some(&GitStatus::Modified) + ); + assert_eq!( + repo.statuses.get(&Path::new(B_TXT).into()), + Some(&GitStatus::Added) + ); + }); + + git_add(Path::new(A_TXT), &repo); + git_add(Path::new(B_TXT), &repo); + git_commit("Committing modified and added", &repo); + tree.flush_fs_events(cx).await; + + // Check that repo only changes are tracked + tree.read_with(cx, |tree, _cx| { + let snapshot = tree.snapshot(); + let (_, repo) = snapshot.repository_entries.iter().next().unwrap(); + + assert_eq!(repo.statuses.iter().count(), 0); + assert_eq!(repo.statuses.get(&Path::new(A_TXT).into()), None); + assert_eq!(repo.statuses.get(&Path::new(B_TXT).into()), None); + }); + + git_reset(0, &repo); + git_remove_index(Path::new(B_TXT), &repo); + git_stash(&mut repo); + tree.flush_fs_events(cx).await; + + // Check that more complex repo changes are tracked + tree.read_with(cx, |tree, _cx| { + let snapshot = tree.snapshot(); + let (_, repo) = snapshot.repository_entries.iter().next().unwrap(); + + + dbg!(&repo.statuses); + + + assert_eq!(repo.statuses.iter().count(), 1); + assert_eq!(repo.statuses.get(&Path::new(A_TXT).into()), None); + assert_eq!( + repo.statuses.get(&Path::new(B_TXT).into()), + Some(&GitStatus::Added) + ); + }); + + std::fs::remove_file(work_dir.join(B_TXT)).unwrap(); + tree.flush_fs_events(cx).await; + + // Check that non-repo behavior is tracked + tree.read_with(cx, |tree, _cx| { + let snapshot = tree.snapshot(); + let (_, repo) = snapshot.repository_entries.iter().next().unwrap(); + + assert_eq!(repo.statuses.iter().count(), 0); + assert_eq!(repo.statuses.get(&Path::new(A_TXT).into()), None); + assert_eq!(repo.statuses.get(&Path::new(B_TXT).into()), None); + }); + } + #[gpui::test] async fn test_write_file(cx: &mut TestAppContext) { let dir = temp_tree(json!({ diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 7602ff7db8..845ab333e1 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -13,10 +13,10 @@ use gpui::{ keymap_matcher::KeymapContext, platform::{CursorStyle, MouseButton, PromptLevel}, AnyElement, AppContext, ClipboardItem, Element, Entity, ModelHandle, Task, View, ViewContext, - ViewHandle, WeakViewHandle, + ViewHandle, WeakViewHandle, color::Color, }; use menu::{Confirm, SelectNext, SelectPrev}; -use project::{Entry, EntryKind, Project, ProjectEntryId, ProjectPath, Worktree, WorktreeId}; +use project::{Entry, EntryKind, Project, ProjectEntryId, ProjectPath, Worktree, WorktreeId, repository::GitStatus}; use settings::Settings; use std::{ cmp::Ordering, @@ -86,6 +86,7 @@ pub struct EntryDetails { is_editing: bool, is_processing: bool, is_cut: bool, + git_status: Option } actions!( @@ -1008,6 +1009,13 @@ impl ProjectPanel { let entry_range = range.start.saturating_sub(ix)..end_ix - ix; for entry in &visible_worktree_entries[entry_range] { + let path = &entry.path; + let status = snapshot.repo_for(path) + .and_then(|entry| { + entry.status_for(&snapshot, path) + }); + + let mut details = EntryDetails { filename: entry .path @@ -1028,6 +1036,7 @@ impl ProjectPanel { is_cut: self .clipboard_entry .map_or(false, |e| e.is_cut() && e.entry_id() == entry.id), + git_status: status }; if let Some(edit_state) = &self.edit_state { @@ -1069,6 +1078,15 @@ impl ProjectPanel { let kind = details.kind; let show_editor = details.is_editing && !details.is_processing; + let git_color = details.git_status.as_ref().and_then(|status| { + match status { + GitStatus::Added => Some(Color::green()), + GitStatus::Modified => Some(Color::blue()), + GitStatus::Conflict => Some(Color::red()), + GitStatus::Untracked => None, + } + }).unwrap_or(Color::transparent_black()); + Flex::row() .with_child( if kind == EntryKind::Dir { @@ -1107,6 +1125,7 @@ impl ProjectPanel { .with_height(style.height) .contained() .with_style(row_container_style) + .with_background_color(git_color) .with_padding_left(padding) .into_any_named("project panel entry visual element") } From 18cec8d64f82aa39fe8c7df059a6e030010c55b7 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Tue, 9 May 2023 14:49:35 -0700 Subject: [PATCH 06/34] Format --- crates/project/src/worktree.rs | 27 +++++--------------- crates/project_panel/src/project_panel.rs | 30 +++++++++++++---------- 2 files changed, 23 insertions(+), 34 deletions(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index cf116d188f..66cef5131b 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -3787,14 +3787,9 @@ mod tests { let oid = repo.index().unwrap().write_tree().unwrap(); let tree = repo.find_tree(oid).unwrap(); if let Some(head) = repo.head().ok() { - let parent_obj = head - .peel(git2::ObjectType::Commit) - .unwrap(); - - let parent_commit = parent_obj - .as_commit() - .unwrap(); + let parent_obj = head.peel(git2::ObjectType::Commit).unwrap(); + let parent_commit = parent_obj.as_commit().unwrap(); repo.commit( Some("HEAD"), @@ -3806,15 +3801,8 @@ mod tests { ) .expect("Failed to commit with parent"); } else { - repo.commit( - Some("HEAD"), - &signature, - &signature, - msg, - &tree, - &[], - ) - .expect("Failed to commit"); + repo.commit(Some("HEAD"), &signature, &signature, msg, &tree, &[]) + .expect("Failed to commit"); } } @@ -3842,14 +3830,13 @@ mod tests { .expect("Could not reset"); } + #[allow(dead_code)] #[track_caller] fn git_status(repo: &git2::Repository) -> HashMap { repo.statuses(None) .unwrap() .iter() - .map(|status| { - (status.path().unwrap().to_string(), status.status()) - }) + .map(|status| (status.path().unwrap().to_string(), status.status())) .collect() } @@ -3931,10 +3918,8 @@ mod tests { let snapshot = tree.snapshot(); let (_, repo) = snapshot.repository_entries.iter().next().unwrap(); - dbg!(&repo.statuses); - assert_eq!(repo.statuses.iter().count(), 1); assert_eq!(repo.statuses.get(&Path::new(A_TXT).into()), None); assert_eq!( diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 845ab333e1..971c4207ba 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -5,6 +5,7 @@ use futures::stream::StreamExt; use gpui::{ actions, anyhow::{anyhow, Result}, + color::Color, elements::{ AnchorCorner, ChildView, ContainerStyle, Empty, Flex, Label, MouseEventHandler, ParentElement, ScrollTarget, Stack, Svg, UniformList, UniformListState, @@ -13,10 +14,13 @@ use gpui::{ keymap_matcher::KeymapContext, platform::{CursorStyle, MouseButton, PromptLevel}, AnyElement, AppContext, ClipboardItem, Element, Entity, ModelHandle, Task, View, ViewContext, - ViewHandle, WeakViewHandle, color::Color, + ViewHandle, WeakViewHandle, }; use menu::{Confirm, SelectNext, SelectPrev}; -use project::{Entry, EntryKind, Project, ProjectEntryId, ProjectPath, Worktree, WorktreeId, repository::GitStatus}; +use project::{ + repository::GitStatus, Entry, EntryKind, Project, ProjectEntryId, ProjectPath, Worktree, + WorktreeId, +}; use settings::Settings; use std::{ cmp::Ordering, @@ -86,7 +90,7 @@ pub struct EntryDetails { is_editing: bool, is_processing: bool, is_cut: bool, - git_status: Option + git_status: Option, } actions!( @@ -1010,11 +1014,9 @@ impl ProjectPanel { let entry_range = range.start.saturating_sub(ix)..end_ix - ix; for entry in &visible_worktree_entries[entry_range] { let path = &entry.path; - let status = snapshot.repo_for(path) - .and_then(|entry| { - entry.status_for(&snapshot, path) - }); - + let status = snapshot + .repo_for(path) + .and_then(|entry| entry.status_for(&snapshot, path)); let mut details = EntryDetails { filename: entry @@ -1036,7 +1038,7 @@ impl ProjectPanel { is_cut: self .clipboard_entry .map_or(false, |e| e.is_cut() && e.entry_id() == entry.id), - git_status: status + git_status: status, }; if let Some(edit_state) = &self.edit_state { @@ -1078,14 +1080,16 @@ impl ProjectPanel { let kind = details.kind; let show_editor = details.is_editing && !details.is_processing; - let git_color = details.git_status.as_ref().and_then(|status| { - match status { + let git_color = details + .git_status + .as_ref() + .and_then(|status| match status { GitStatus::Added => Some(Color::green()), GitStatus::Modified => Some(Color::blue()), GitStatus::Conflict => Some(Color::red()), GitStatus::Untracked => None, - } - }).unwrap_or(Color::transparent_black()); + }) + .unwrap_or(Color::transparent_black()); Flex::row() .with_child( From a58a33fc93dd64b607112c1cae8af26903b5a510 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Tue, 9 May 2023 19:29:45 -0700 Subject: [PATCH 07/34] WIP: integrate status with collab --- crates/collab/src/tests/integration_tests.rs | 113 ++++++++++++++++++- crates/fs/src/fs.rs | 15 ++- crates/fs/src/repository.rs | 13 ++- crates/project/src/worktree.rs | 4 + crates/rpc/proto/zed.proto | 14 +++ 5 files changed, 154 insertions(+), 5 deletions(-) diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index e3b5b0be7e..764f070f0b 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -10,7 +10,7 @@ use editor::{ ConfirmRename, Editor, ExcerptRange, MultiBuffer, Redo, Rename, ToOffset, ToggleCodeActions, Undo, }; -use fs::{FakeFs, Fs as _, LineEnding, RemoveOptions}; +use fs::{repository::GitStatus, FakeFs, Fs as _, LineEnding, RemoveOptions}; use futures::StreamExt as _; use gpui::{ executor::Deterministic, geometry::vector::vec2f, test::EmptyView, AppContext, ModelHandle, @@ -2690,6 +2690,117 @@ async fn test_git_branch_name( }); } +#[gpui::test] +async fn test_git_status_sync( + deterministic: Arc, + cx_a: &mut TestAppContext, + cx_b: &mut TestAppContext, + cx_c: &mut TestAppContext, +) { + deterministic.forbid_parking(); + let mut server = TestServer::start(&deterministic).await; + let client_a = server.create_client(cx_a, "user_a").await; + let client_b = server.create_client(cx_b, "user_b").await; + let client_c = server.create_client(cx_c, "user_c").await; + server + .create_room(&mut [(&client_a, cx_a), (&client_b, cx_b), (&client_c, cx_c)]) + .await; + let active_call_a = cx_a.read(ActiveCall::global); + + client_a + .fs + .insert_tree( + "/dir", + json!({ + ".git": {}, + "a.txt": "a", + "b.txt": "b", + }), + ) + .await; + + const A_TXT: &'static str = "a.txt"; + const B_TXT: &'static str = "b.txt"; + + client_a + .fs + .as_fake() + .set_status_for_repo( + Path::new("/dir/.git"), + &[ + (&Path::new(A_TXT), GitStatus::Added), + (&Path::new(B_TXT), GitStatus::Added), + ], + ) + .await; + + let (project_local, _worktree_id) = client_a.build_local_project("/dir", cx_a).await; + let project_id = active_call_a + .update(cx_a, |call, cx| { + call.share_project(project_local.clone(), cx) + }) + .await + .unwrap(); + + let project_remote = client_b.build_remote_project(project_id, cx_b).await; + + // Wait for it to catch up to the new status + deterministic.run_until_parked(); + + #[track_caller] + fn assert_status(file: &impl AsRef, status: Option, project: &Project, cx: &AppContext) { + let file = file.as_ref(); + let worktrees = project.visible_worktrees(cx).collect::>(); + assert_eq!(worktrees.len(), 1); + let worktree = worktrees[0].clone(); + let snapshot = worktree.read(cx).snapshot(); + let root_entry = snapshot.root_git_entry().unwrap(); + assert_eq!(root_entry.status_for(&snapshot, file), status); + } + + // Smoke test status reading + project_local.read_with(cx_a, |project, cx| { + assert_status(&Path::new(A_TXT), Some(GitStatus::Added), project, cx); + assert_status(&Path::new(B_TXT), Some(GitStatus::Added), project, cx); + }); + project_remote.read_with(cx_b, |project, cx| { + assert_status(&Path::new(A_TXT), Some(GitStatus::Added), project, cx); + assert_status(&Path::new(B_TXT), Some(GitStatus::Added), project, cx); + }); + + client_a + .fs + .as_fake() + .set_status_for_repo( + Path::new("/dir/.git"), + &[ + (&Path::new(A_TXT), GitStatus::Modified), + (&Path::new(B_TXT), GitStatus::Modified), + ], + ) + .await; + + // Wait for buffer_local_a to receive it + deterministic.run_until_parked(); + + // Smoke test status reading + project_local.read_with(cx_a, |project, cx| { + assert_status(&Path::new(A_TXT), Some(GitStatus::Added), project, cx); + assert_status(&Path::new(B_TXT), Some(GitStatus::Added), project, cx); + }); + project_remote.read_with(cx_b, |project, cx| { + assert_status(&Path::new(A_TXT), Some(GitStatus::Added), project, cx); + assert_status(&Path::new(B_TXT), Some(GitStatus::Added), project, cx); + }); + + // And synchronization while joining + let project_remote_c = client_c.build_remote_project(project_id, cx_c).await; + project_remote_c.read_with(cx_c, |project, cx| { + assert_status(&Path::new(A_TXT), Some(GitStatus::Added), project, cx); + assert_status(&Path::new(B_TXT), Some(GitStatus::Added), project, cx); + }); +} + #[gpui::test(iterations = 10)] async fn test_fs_operations( deterministic: Arc, diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 945ffaea16..efc24553c4 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -7,7 +7,7 @@ use git2::Repository as LibGitRepository; use lazy_static::lazy_static; use parking_lot::Mutex; use regex::Regex; -use repository::GitRepository; +use repository::{GitRepository, GitStatus}; use rope::Rope; use smol::io::{AsyncReadExt, AsyncWriteExt}; use std::borrow::Cow; @@ -654,6 +654,19 @@ impl FakeFs { }); } + pub async fn set_status_for_repo(&self, dot_git: &Path, statuses: &[(&Path, GitStatus)]) { + self.with_git_state(dot_git, |state| { + state.git_statuses.clear(); + state.git_statuses.extend( + statuses + .iter() + .map(|(path, content)| { + ((**path).into(), content.clone()) + }), + ); + }); + } + pub fn paths(&self) -> Vec { let mut result = Vec::new(); let mut queue = collections::VecDeque::new(); diff --git a/crates/fs/src/repository.rs b/crates/fs/src/repository.rs index 626fbf9e12..7fa20bddcb 100644 --- a/crates/fs/src/repository.rs +++ b/crates/fs/src/repository.rs @@ -102,6 +102,7 @@ pub struct FakeGitRepository { #[derive(Debug, Clone, Default)] pub struct FakeGitRepositoryState { pub index_contents: HashMap, + pub git_statuses: HashMap, pub branch_name: Option, } @@ -126,11 +127,17 @@ impl GitRepository for FakeGitRepository { } fn statuses(&self) -> Option> { - todo!() + let state = self.state.lock(); + let mut map = TreeMap::default(); + for (repo_path, status) in state.git_statuses.iter() { + map.insert(repo_path.to_owned(), status.to_owned()); + } + Some(map) } - fn file_status(&self, _: &RepoPath) -> Option { - todo!() + fn file_status(&self, path: &RepoPath) -> Option { + let state = self.state.lock(); + state.git_statuses.get(path).cloned() } } diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 66cef5131b..82c719f31e 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -178,6 +178,9 @@ impl From<&RepositoryEntry> for proto::RepositoryEntry { proto::RepositoryEntry { work_directory_id: value.work_directory.to_proto(), branch: value.branch.as_ref().map(|str| str.to_string()), + // TODO: Status + removed_statuses: Default::default(), + updated_statuses: Default::default(), } } } @@ -1855,6 +1858,7 @@ impl LocalSnapshot { let scan_id = self.scan_id; let repo_lock = repo.lock(); + self.repository_entries.insert( work_directory, RepositoryEntry { diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 220ef22fb7..abe02f42bb 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -986,8 +986,22 @@ message Entry { message RepositoryEntry { uint64 work_directory_id = 1; optional string branch = 2; + repeated uint64 removed_statuses = 3; + repeated StatusEntry updated_statuses = 4; } +message StatusEntry { + uint64 entry_id = 1; + GitStatus status = 2; +} + +enum GitStatus { + Added = 0; + Modified = 1; + Conflict = 2; +} + + message BufferState { uint64 id = 1; optional File file = 2; From 94a0de4c9fe5417882075aed00f6d337f3bee86d Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Tue, 9 May 2023 19:35:15 -0700 Subject: [PATCH 08/34] Fix compile errors --- crates/collab/src/db.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index bc5b816abf..5867fa7369 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -1568,6 +1568,9 @@ impl Database { worktree.updated_repositories.push(proto::RepositoryEntry { work_directory_id: db_repository.work_directory_id as u64, branch: db_repository.branch, + removed_statuses: Default::default(), + updated_statuses: Default::default(), + }); } } @@ -2648,6 +2651,9 @@ impl Database { worktree.repository_entries.push(proto::RepositoryEntry { work_directory_id: db_repository_entry.work_directory_id as u64, branch: db_repository_entry.branch, + removed_statuses: Default::default(), + updated_statuses: Default::default(), + }); } } From f935047ff27bcc9ca2acfb4855df5b5d59b951d7 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Tue, 9 May 2023 21:06:23 -0700 Subject: [PATCH 09/34] Convert git status calculation to use Entry IDs as the key instead of repo relative paths --- Cargo.lock | 1 - crates/collab/src/tests/integration_tests.rs | 3 +- crates/fs/Cargo.toml | 1 - crates/fs/src/fs.rs | 4 +- crates/fs/src/repository.rs | 15 ++- crates/project/src/worktree.rs | 120 ++++++++++++------- crates/project_panel/src/project_panel.rs | 5 +- 7 files changed, 90 insertions(+), 59 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0190b4d8f5..8fe740267e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2350,7 +2350,6 @@ dependencies = [ "serde_derive", "serde_json", "smol", - "sum_tree", "tempfile", "util", ] diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 764f070f0b..b6046870d1 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -2755,7 +2755,8 @@ async fn test_git_status_sync( let worktree = worktrees[0].clone(); let snapshot = worktree.read(cx).snapshot(); let root_entry = snapshot.root_git_entry().unwrap(); - assert_eq!(root_entry.status_for(&snapshot, file), status); + let file_entry_id = snapshot.entry_for_path(file).unwrap().id; + assert_eq!(root_entry.status_for(file_entry_id), status); } // Smoke test status reading diff --git a/crates/fs/Cargo.toml b/crates/fs/Cargo.toml index 54c6ce362a..d080fe3cd1 100644 --- a/crates/fs/Cargo.toml +++ b/crates/fs/Cargo.toml @@ -13,7 +13,6 @@ gpui = { path = "../gpui" } lsp = { path = "../lsp" } rope = { path = "../rope" } util = { path = "../util" } -sum_tree = { path = "../sum_tree" } anyhow.workspace = true async-trait.workspace = true futures.workspace = true diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index efc24553c4..9347e7d7b1 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -7,7 +7,7 @@ use git2::Repository as LibGitRepository; use lazy_static::lazy_static; use parking_lot::Mutex; use regex::Regex; -use repository::{GitRepository, GitStatus}; +use repository::GitRepository; use rope::Rope; use smol::io::{AsyncReadExt, AsyncWriteExt}; use std::borrow::Cow; @@ -27,7 +27,7 @@ use util::ResultExt; #[cfg(any(test, feature = "test-support"))] use collections::{btree_map, BTreeMap}; #[cfg(any(test, feature = "test-support"))] -use repository::FakeGitRepositoryState; +use repository::{FakeGitRepositoryState, GitStatus}; #[cfg(any(test, feature = "test-support"))] use std::sync::Weak; diff --git a/crates/fs/src/repository.rs b/crates/fs/src/repository.rs index 7fa20bddcb..73847dac29 100644 --- a/crates/fs/src/repository.rs +++ b/crates/fs/src/repository.rs @@ -8,7 +8,6 @@ use std::{ path::{Component, Path, PathBuf}, sync::Arc, }; -use sum_tree::TreeMap; use util::ResultExt; pub use git2::Repository as LibGitRepository; @@ -21,7 +20,7 @@ pub trait GitRepository: Send { fn branch_name(&self) -> Option; - fn statuses(&self) -> Option>; + fn statuses(&self) -> Option>; fn file_status(&self, path: &RepoPath) -> Option; } @@ -70,10 +69,10 @@ impl GitRepository for LibGitRepository { Some(branch.to_string()) } - fn statuses(&self) -> Option> { + fn statuses(&self) -> Option> { let statuses = self.statuses(None).log_err()?; - let mut map = TreeMap::default(); + let mut result = HashMap::default(); for status in statuses .iter() @@ -81,10 +80,10 @@ impl GitRepository for LibGitRepository { { let path = RepoPath(PathBuf::from(OsStr::from_bytes(status.path_bytes()))); - map.insert(path, status.status().into()) + result.insert(path, status.status().into()); } - Some(map) + Some(result) } fn file_status(&self, path: &RepoPath) -> Option { @@ -126,9 +125,9 @@ impl GitRepository for FakeGitRepository { state.branch_name.clone() } - fn statuses(&self) -> Option> { + fn statuses(&self) -> Option> { let state = self.state.lock(); - let mut map = TreeMap::default(); + let mut map = HashMap::default(); for (repo_path, status) in state.git_statuses.iter() { map.insert(repo_path.to_owned(), status.to_owned()); } diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 82c719f31e..b9a4b549a1 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -143,7 +143,7 @@ impl Snapshot { pub struct RepositoryEntry { pub(crate) work_directory: WorkDirectoryEntry, pub(crate) branch: Option>, - pub(crate) statuses: TreeMap, + pub(crate) statuses: TreeMap, } impl RepositoryEntry { @@ -165,11 +165,8 @@ impl RepositoryEntry { self.work_directory.contains(snapshot, path) } - pub fn status_for(&self, snapshot: &Snapshot, path: &Path) -> Option { - self.work_directory - .relativize(snapshot, path) - .and_then(|repo_path| self.statuses.get(&repo_path)) - .cloned() + pub fn status_for(&self, entry: ProjectEntryId) -> Option { + self.statuses.get(&entry).cloned() } } @@ -1813,10 +1810,6 @@ impl LocalSnapshot { ); } - if parent_path.file_name() == Some(&DOT_GIT) { - self.build_repo(parent_path, fs); - } - let mut entries_by_path_edits = vec![Edit::Insert(parent_entry)]; let mut entries_by_id_edits = Vec::new(); @@ -1833,6 +1826,10 @@ impl LocalSnapshot { self.entries_by_path.edit(entries_by_path_edits, &()); self.entries_by_id.edit(entries_by_id_edits, &()); + + if parent_path.file_name() == Some(&DOT_GIT) { + self.build_repo(parent_path, fs); + } } fn build_repo(&mut self, parent_path: Arc, fs: &dyn Fs) -> Option<()> { @@ -1858,13 +1855,13 @@ impl LocalSnapshot { let scan_id = self.scan_id; let repo_lock = repo.lock(); - + let statuses = convert_statuses(&work_directory, repo_lock.deref(), self)?; self.repository_entries.insert( work_directory, RepositoryEntry { work_directory: work_dir_id.into(), branch: repo_lock.branch_name().map(Into::into), - statuses: repo_lock.statuses().unwrap_or_default(), + statuses, }, ); drop(repo_lock); @@ -2821,6 +2818,7 @@ impl BackgroundScanner { for (abs_path, metadata) in abs_paths.iter().zip(metadata.iter()) { if let Ok(path) = abs_path.strip_prefix(&root_canonical_path) { if matches!(metadata, Ok(None)) || doing_recursive_update { + self.remove_repo_path(&path, &mut snapshot); snapshot.remove_path(path); } event_paths.push(path.into()); @@ -2866,9 +2864,7 @@ impl BackgroundScanner { } } } - Ok(None) => { - self.remove_repo_path(&path, &mut snapshot); - } + Ok(None) => {} Err(err) => { // TODO - create a special 'error' entry in the entries tree to mark this log::error!("error reading file on event {:?}", err); @@ -2887,7 +2883,7 @@ impl BackgroundScanner { let scan_id = snapshot.scan_id; let repo = snapshot.repo_for(&path)?; - let repo_path = repo.work_directory.relativize(&snapshot, &path)?; + let repo_path_id = snapshot.entry_for_path(path)?.id; let work_dir = repo.work_directory(snapshot)?; let work_dir_id = repo.work_directory; @@ -2898,7 +2894,7 @@ impl BackgroundScanner { snapshot .repository_entries - .update(&work_dir, |entry| entry.statuses.remove(&repo_path)); + .update(&work_dir, |entry| entry.statuses.remove(&repo_path_id)); } Some(()) @@ -2911,18 +2907,19 @@ impl BackgroundScanner { .components() .any(|component| component.as_os_str() == *DOT_GIT) { - let (entry_id, repo) = snapshot.repo_for_metadata(&path)?; + let (git_dir_id, repo) = snapshot.repo_for_metadata(&path)?; let work_dir = snapshot - .entry_for_id(entry_id) + .entry_for_id(git_dir_id) .map(|entry| RepositoryWorkDirectory(entry.path.clone()))?; let repo = repo.lock(); repo.reload_index(); let branch = repo.branch_name(); - let statuses = repo.statuses().unwrap_or_default(); - snapshot.git_repositories.update(&entry_id, |entry| { + let statuses = convert_statuses(&work_dir, repo.deref(), snapshot)?; + + snapshot.git_repositories.update(&git_dir_id, |entry| { entry.scan_id = scan_id; entry.full_scan_id = scan_id; }); @@ -2936,6 +2933,8 @@ impl BackgroundScanner { let repo_path = repo.work_directory.relativize(&snapshot, &path)?; + let path_id = snapshot.entry_for_path(&path)?.id; + let status = { let local_repo = snapshot.get_local_repo(&repo)?; @@ -2958,7 +2957,7 @@ impl BackgroundScanner { snapshot .repository_entries - .update(&work_dir, |entry| entry.statuses.insert(repo_path, status)); + .update(&work_dir, |entry| entry.statuses.insert(path_id, status)); } } @@ -3167,6 +3166,19 @@ impl BackgroundScanner { } } +fn convert_statuses( + work_dir: &RepositoryWorkDirectory, + repo: &dyn GitRepository, + snapshot: &Snapshot, +) -> Option> { + let mut statuses = TreeMap::default(); + for (path, status) in repo.statuses().unwrap_or_default() { + let path_entry = snapshot.entry_for_path(&work_dir.0.join(path.as_path()))?; + statuses.insert(path_entry.id, status) + } + Some(statuses) +} + fn char_bag_for_path(root_char_bag: CharBag, path: &Path) -> CharBag { let mut result = root_char_bag; result.extend( @@ -3848,6 +3860,11 @@ mod tests { "project": { "a.txt": "a", "b.txt": "bb", + "c": { + "d": { + "e.txt": "eee" + } + } }, })); @@ -3865,18 +3882,39 @@ mod tests { .await .unwrap(); + cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) + .await; + const A_TXT: &'static str = "a.txt"; const B_TXT: &'static str = "b.txt"; + const E_TXT: &'static str = "c/d/e.txt"; let work_dir = root.path().join("project"); + let tree_clone = tree.clone(); + let (a_txt_id, b_txt_id, e_txt_id) = cx.read(|cx| { + let tree = tree_clone.read(cx); + let a_id = tree + .entry_for_path(Path::new("project").join(Path::new(A_TXT))) + .unwrap() + .id; + let b_id = tree + .entry_for_path(Path::new("project").join(Path::new(B_TXT))) + .unwrap() + .id; + let e_id = tree + .entry_for_path(Path::new("project").join(Path::new(E_TXT))) + .unwrap() + .id; + (a_id, b_id, e_id) + }); + let mut repo = git_init(work_dir.as_path()); git_add(Path::new(A_TXT), &repo); + git_add(Path::new(E_TXT), &repo); git_commit("Initial commit", &repo); std::fs::write(work_dir.join(A_TXT), "aa").unwrap(); - cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) - .await; tree.flush_fs_events(cx).await; // Check that the right git state is observed on startup @@ -3885,16 +3923,9 @@ mod tests { assert_eq!(snapshot.repository_entries.iter().count(), 1); let (dir, repo) = snapshot.repository_entries.iter().next().unwrap(); assert_eq!(dir.0.as_ref(), Path::new("project")); - assert_eq!(repo.statuses.iter().count(), 2); - assert_eq!( - repo.statuses.get(&Path::new(A_TXT).into()), - Some(&GitStatus::Modified) - ); - assert_eq!( - repo.statuses.get(&Path::new(B_TXT).into()), - Some(&GitStatus::Added) - ); + assert_eq!(repo.statuses.get(&a_txt_id), Some(&GitStatus::Modified)); + assert_eq!(repo.statuses.get(&b_txt_id), Some(&GitStatus::Added)); }); git_add(Path::new(A_TXT), &repo); @@ -3908,15 +3939,18 @@ mod tests { let (_, repo) = snapshot.repository_entries.iter().next().unwrap(); assert_eq!(repo.statuses.iter().count(), 0); - assert_eq!(repo.statuses.get(&Path::new(A_TXT).into()), None); - assert_eq!(repo.statuses.get(&Path::new(B_TXT).into()), None); + assert_eq!(repo.statuses.get(&a_txt_id), None); + assert_eq!(repo.statuses.get(&b_txt_id), None); }); git_reset(0, &repo); git_remove_index(Path::new(B_TXT), &repo); git_stash(&mut repo); + std::fs::write(work_dir.join(E_TXT), "eeee").unwrap(); tree.flush_fs_events(cx).await; + dbg!(git_status(&repo)); + // Check that more complex repo changes are tracked tree.read_with(cx, |tree, _cx| { let snapshot = tree.snapshot(); @@ -3924,15 +3958,14 @@ mod tests { dbg!(&repo.statuses); - assert_eq!(repo.statuses.iter().count(), 1); - assert_eq!(repo.statuses.get(&Path::new(A_TXT).into()), None); - assert_eq!( - repo.statuses.get(&Path::new(B_TXT).into()), - Some(&GitStatus::Added) - ); + assert_eq!(repo.statuses.iter().count(), 2); + assert_eq!(repo.statuses.get(&a_txt_id), None); + assert_eq!(repo.statuses.get(&b_txt_id), Some(&GitStatus::Added)); + assert_eq!(repo.statuses.get(&e_txt_id), Some(&GitStatus::Modified)); }); std::fs::remove_file(work_dir.join(B_TXT)).unwrap(); + std::fs::remove_dir_all(work_dir.join("c")).unwrap(); tree.flush_fs_events(cx).await; // Check that non-repo behavior is tracked @@ -3941,8 +3974,9 @@ mod tests { let (_, repo) = snapshot.repository_entries.iter().next().unwrap(); assert_eq!(repo.statuses.iter().count(), 0); - assert_eq!(repo.statuses.get(&Path::new(A_TXT).into()), None); - assert_eq!(repo.statuses.get(&Path::new(B_TXT).into()), None); + assert_eq!(repo.statuses.get(&a_txt_id), None); + assert_eq!(repo.statuses.get(&b_txt_id), None); + assert_eq!(repo.statuses.get(&e_txt_id), None); }); } diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 971c4207ba..de440b38a4 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -1013,10 +1013,9 @@ impl ProjectPanel { let entry_range = range.start.saturating_sub(ix)..end_ix - ix; for entry in &visible_worktree_entries[entry_range] { - let path = &entry.path; let status = snapshot - .repo_for(path) - .and_then(|entry| entry.status_for(&snapshot, path)); + .repo_for(&entry.path) + .and_then(|repo_entry| repo_entry.status_for(entry.id)); let mut details = EntryDetails { filename: entry From 6b4242cdedbe3c571f9704a28ed103b6d51a211f Mon Sep 17 00:00:00 2001 From: Petros Amoiridis Date: Wed, 10 May 2023 16:21:24 +0300 Subject: [PATCH 10/34] Use theme.editor.diff for the colors --- crates/project_panel/src/project_panel.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index de440b38a4..2baccc0913 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -5,7 +5,6 @@ use futures::stream::StreamExt; use gpui::{ actions, anyhow::{anyhow, Result}, - color::Color, elements::{ AnchorCorner, ChildView, ContainerStyle, Empty, Flex, Label, MouseEventHandler, ParentElement, ScrollTarget, Stack, Svg, UniformList, UniformListState, @@ -1079,16 +1078,23 @@ impl ProjectPanel { let kind = details.kind; let show_editor = details.is_editing && !details.is_processing; - let git_color = details + // Prepare colors for git statuses + let editor_theme = &cx.global::().theme.editor; + let color_for_added = Some(editor_theme.diff.inserted); + let color_for_modified = Some(editor_theme.diff.modified); + let color_for_conflict = Some(editor_theme.diff.deleted); + let color_for_untracked = None; + let mut filename_text_style = style.text.clone(); + filename_text_style.color = details .git_status .as_ref() .and_then(|status| match status { - GitStatus::Added => Some(Color::green()), - GitStatus::Modified => Some(Color::blue()), - GitStatus::Conflict => Some(Color::red()), - GitStatus::Untracked => None, + GitStatus::Added => color_for_added, + GitStatus::Modified => color_for_modified, + GitStatus::Conflict => color_for_conflict, + GitStatus::Untracked => color_for_untracked, }) - .unwrap_or(Color::transparent_black()); + .unwrap_or(style.text.color); Flex::row() .with_child( @@ -1117,7 +1123,7 @@ impl ProjectPanel { .flex(1.0, true) .into_any() } else { - Label::new(details.filename.clone(), style.text.clone()) + Label::new(details.filename.clone(), filename_text_style) .contained() .with_margin_left(style.icon_spacing) .aligned() @@ -1128,7 +1134,6 @@ impl ProjectPanel { .with_height(style.height) .contained() .with_style(row_container_style) - .with_background_color(git_color) .with_padding_left(padding) .into_any_named("project panel entry visual element") } From 21e1bdc8cd9bc880e01916337ee60a5a034bd30c Mon Sep 17 00:00:00 2001 From: Petros Amoiridis Date: Wed, 10 May 2023 17:05:53 +0300 Subject: [PATCH 11/34] Fix yellow to be yellow --- crates/gpui/src/color.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/gpui/src/color.rs b/crates/gpui/src/color.rs index cc725776b9..b6c1e3aff9 100644 --- a/crates/gpui/src/color.rs +++ b/crates/gpui/src/color.rs @@ -42,7 +42,7 @@ impl Color { } pub fn yellow() -> Self { - Self(ColorU::from_u32(0x00ffffff)) + Self(ColorU::from_u32(0xffff00ff)) } pub fn new(r: u8, g: u8, b: u8, a: u8) -> Self { From 0082d68d4af653a9dfd9c1dbeb1bb70373a2de51 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 10 May 2023 08:49:30 -0700 Subject: [PATCH 12/34] Revert "Convert git status calculation to use Entry IDs as the key instead of repo relative paths" This reverts commit 728c6892c924ebeabb086e308ec4b5f56c4fd72a. --- Cargo.lock | 1 + crates/collab/src/tests/integration_tests.rs | 3 +- crates/fs/Cargo.toml | 1 + crates/fs/src/fs.rs | 4 +- crates/fs/src/repository.rs | 15 +-- crates/project/src/worktree.rs | 120 +++++++------------ crates/project_panel/src/project_panel.rs | 5 +- 7 files changed, 59 insertions(+), 90 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8fe740267e..0190b4d8f5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2350,6 +2350,7 @@ dependencies = [ "serde_derive", "serde_json", "smol", + "sum_tree", "tempfile", "util", ] diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index b6046870d1..764f070f0b 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -2755,8 +2755,7 @@ async fn test_git_status_sync( let worktree = worktrees[0].clone(); let snapshot = worktree.read(cx).snapshot(); let root_entry = snapshot.root_git_entry().unwrap(); - let file_entry_id = snapshot.entry_for_path(file).unwrap().id; - assert_eq!(root_entry.status_for(file_entry_id), status); + assert_eq!(root_entry.status_for(&snapshot, file), status); } // Smoke test status reading diff --git a/crates/fs/Cargo.toml b/crates/fs/Cargo.toml index d080fe3cd1..54c6ce362a 100644 --- a/crates/fs/Cargo.toml +++ b/crates/fs/Cargo.toml @@ -13,6 +13,7 @@ gpui = { path = "../gpui" } lsp = { path = "../lsp" } rope = { path = "../rope" } util = { path = "../util" } +sum_tree = { path = "../sum_tree" } anyhow.workspace = true async-trait.workspace = true futures.workspace = true diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 9347e7d7b1..efc24553c4 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -7,7 +7,7 @@ use git2::Repository as LibGitRepository; use lazy_static::lazy_static; use parking_lot::Mutex; use regex::Regex; -use repository::GitRepository; +use repository::{GitRepository, GitStatus}; use rope::Rope; use smol::io::{AsyncReadExt, AsyncWriteExt}; use std::borrow::Cow; @@ -27,7 +27,7 @@ use util::ResultExt; #[cfg(any(test, feature = "test-support"))] use collections::{btree_map, BTreeMap}; #[cfg(any(test, feature = "test-support"))] -use repository::{FakeGitRepositoryState, GitStatus}; +use repository::FakeGitRepositoryState; #[cfg(any(test, feature = "test-support"))] use std::sync::Weak; diff --git a/crates/fs/src/repository.rs b/crates/fs/src/repository.rs index 73847dac29..7fa20bddcb 100644 --- a/crates/fs/src/repository.rs +++ b/crates/fs/src/repository.rs @@ -8,6 +8,7 @@ use std::{ path::{Component, Path, PathBuf}, sync::Arc, }; +use sum_tree::TreeMap; use util::ResultExt; pub use git2::Repository as LibGitRepository; @@ -20,7 +21,7 @@ pub trait GitRepository: Send { fn branch_name(&self) -> Option; - fn statuses(&self) -> Option>; + fn statuses(&self) -> Option>; fn file_status(&self, path: &RepoPath) -> Option; } @@ -69,10 +70,10 @@ impl GitRepository for LibGitRepository { Some(branch.to_string()) } - fn statuses(&self) -> Option> { + fn statuses(&self) -> Option> { let statuses = self.statuses(None).log_err()?; - let mut result = HashMap::default(); + let mut map = TreeMap::default(); for status in statuses .iter() @@ -80,10 +81,10 @@ impl GitRepository for LibGitRepository { { let path = RepoPath(PathBuf::from(OsStr::from_bytes(status.path_bytes()))); - result.insert(path, status.status().into()); + map.insert(path, status.status().into()) } - Some(result) + Some(map) } fn file_status(&self, path: &RepoPath) -> Option { @@ -125,9 +126,9 @@ impl GitRepository for FakeGitRepository { state.branch_name.clone() } - fn statuses(&self) -> Option> { + fn statuses(&self) -> Option> { let state = self.state.lock(); - let mut map = HashMap::default(); + let mut map = TreeMap::default(); for (repo_path, status) in state.git_statuses.iter() { map.insert(repo_path.to_owned(), status.to_owned()); } diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index b9a4b549a1..82c719f31e 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -143,7 +143,7 @@ impl Snapshot { pub struct RepositoryEntry { pub(crate) work_directory: WorkDirectoryEntry, pub(crate) branch: Option>, - pub(crate) statuses: TreeMap, + pub(crate) statuses: TreeMap, } impl RepositoryEntry { @@ -165,8 +165,11 @@ impl RepositoryEntry { self.work_directory.contains(snapshot, path) } - pub fn status_for(&self, entry: ProjectEntryId) -> Option { - self.statuses.get(&entry).cloned() + pub fn status_for(&self, snapshot: &Snapshot, path: &Path) -> Option { + self.work_directory + .relativize(snapshot, path) + .and_then(|repo_path| self.statuses.get(&repo_path)) + .cloned() } } @@ -1810,6 +1813,10 @@ impl LocalSnapshot { ); } + if parent_path.file_name() == Some(&DOT_GIT) { + self.build_repo(parent_path, fs); + } + let mut entries_by_path_edits = vec![Edit::Insert(parent_entry)]; let mut entries_by_id_edits = Vec::new(); @@ -1826,10 +1833,6 @@ impl LocalSnapshot { self.entries_by_path.edit(entries_by_path_edits, &()); self.entries_by_id.edit(entries_by_id_edits, &()); - - if parent_path.file_name() == Some(&DOT_GIT) { - self.build_repo(parent_path, fs); - } } fn build_repo(&mut self, parent_path: Arc, fs: &dyn Fs) -> Option<()> { @@ -1855,13 +1858,13 @@ impl LocalSnapshot { let scan_id = self.scan_id; let repo_lock = repo.lock(); - let statuses = convert_statuses(&work_directory, repo_lock.deref(), self)?; + self.repository_entries.insert( work_directory, RepositoryEntry { work_directory: work_dir_id.into(), branch: repo_lock.branch_name().map(Into::into), - statuses, + statuses: repo_lock.statuses().unwrap_or_default(), }, ); drop(repo_lock); @@ -2818,7 +2821,6 @@ impl BackgroundScanner { for (abs_path, metadata) in abs_paths.iter().zip(metadata.iter()) { if let Ok(path) = abs_path.strip_prefix(&root_canonical_path) { if matches!(metadata, Ok(None)) || doing_recursive_update { - self.remove_repo_path(&path, &mut snapshot); snapshot.remove_path(path); } event_paths.push(path.into()); @@ -2864,7 +2866,9 @@ impl BackgroundScanner { } } } - Ok(None) => {} + Ok(None) => { + self.remove_repo_path(&path, &mut snapshot); + } Err(err) => { // TODO - create a special 'error' entry in the entries tree to mark this log::error!("error reading file on event {:?}", err); @@ -2883,7 +2887,7 @@ impl BackgroundScanner { let scan_id = snapshot.scan_id; let repo = snapshot.repo_for(&path)?; - let repo_path_id = snapshot.entry_for_path(path)?.id; + let repo_path = repo.work_directory.relativize(&snapshot, &path)?; let work_dir = repo.work_directory(snapshot)?; let work_dir_id = repo.work_directory; @@ -2894,7 +2898,7 @@ impl BackgroundScanner { snapshot .repository_entries - .update(&work_dir, |entry| entry.statuses.remove(&repo_path_id)); + .update(&work_dir, |entry| entry.statuses.remove(&repo_path)); } Some(()) @@ -2907,19 +2911,18 @@ impl BackgroundScanner { .components() .any(|component| component.as_os_str() == *DOT_GIT) { - let (git_dir_id, repo) = snapshot.repo_for_metadata(&path)?; + let (entry_id, repo) = snapshot.repo_for_metadata(&path)?; let work_dir = snapshot - .entry_for_id(git_dir_id) + .entry_for_id(entry_id) .map(|entry| RepositoryWorkDirectory(entry.path.clone()))?; let repo = repo.lock(); repo.reload_index(); let branch = repo.branch_name(); + let statuses = repo.statuses().unwrap_or_default(); - let statuses = convert_statuses(&work_dir, repo.deref(), snapshot)?; - - snapshot.git_repositories.update(&git_dir_id, |entry| { + snapshot.git_repositories.update(&entry_id, |entry| { entry.scan_id = scan_id; entry.full_scan_id = scan_id; }); @@ -2933,8 +2936,6 @@ impl BackgroundScanner { let repo_path = repo.work_directory.relativize(&snapshot, &path)?; - let path_id = snapshot.entry_for_path(&path)?.id; - let status = { let local_repo = snapshot.get_local_repo(&repo)?; @@ -2957,7 +2958,7 @@ impl BackgroundScanner { snapshot .repository_entries - .update(&work_dir, |entry| entry.statuses.insert(path_id, status)); + .update(&work_dir, |entry| entry.statuses.insert(repo_path, status)); } } @@ -3166,19 +3167,6 @@ impl BackgroundScanner { } } -fn convert_statuses( - work_dir: &RepositoryWorkDirectory, - repo: &dyn GitRepository, - snapshot: &Snapshot, -) -> Option> { - let mut statuses = TreeMap::default(); - for (path, status) in repo.statuses().unwrap_or_default() { - let path_entry = snapshot.entry_for_path(&work_dir.0.join(path.as_path()))?; - statuses.insert(path_entry.id, status) - } - Some(statuses) -} - fn char_bag_for_path(root_char_bag: CharBag, path: &Path) -> CharBag { let mut result = root_char_bag; result.extend( @@ -3860,11 +3848,6 @@ mod tests { "project": { "a.txt": "a", "b.txt": "bb", - "c": { - "d": { - "e.txt": "eee" - } - } }, })); @@ -3882,39 +3865,18 @@ mod tests { .await .unwrap(); - cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) - .await; - const A_TXT: &'static str = "a.txt"; const B_TXT: &'static str = "b.txt"; - const E_TXT: &'static str = "c/d/e.txt"; let work_dir = root.path().join("project"); - let tree_clone = tree.clone(); - let (a_txt_id, b_txt_id, e_txt_id) = cx.read(|cx| { - let tree = tree_clone.read(cx); - let a_id = tree - .entry_for_path(Path::new("project").join(Path::new(A_TXT))) - .unwrap() - .id; - let b_id = tree - .entry_for_path(Path::new("project").join(Path::new(B_TXT))) - .unwrap() - .id; - let e_id = tree - .entry_for_path(Path::new("project").join(Path::new(E_TXT))) - .unwrap() - .id; - (a_id, b_id, e_id) - }); - let mut repo = git_init(work_dir.as_path()); git_add(Path::new(A_TXT), &repo); - git_add(Path::new(E_TXT), &repo); git_commit("Initial commit", &repo); std::fs::write(work_dir.join(A_TXT), "aa").unwrap(); + cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) + .await; tree.flush_fs_events(cx).await; // Check that the right git state is observed on startup @@ -3923,9 +3885,16 @@ mod tests { assert_eq!(snapshot.repository_entries.iter().count(), 1); let (dir, repo) = snapshot.repository_entries.iter().next().unwrap(); assert_eq!(dir.0.as_ref(), Path::new("project")); + assert_eq!(repo.statuses.iter().count(), 2); - assert_eq!(repo.statuses.get(&a_txt_id), Some(&GitStatus::Modified)); - assert_eq!(repo.statuses.get(&b_txt_id), Some(&GitStatus::Added)); + assert_eq!( + repo.statuses.get(&Path::new(A_TXT).into()), + Some(&GitStatus::Modified) + ); + assert_eq!( + repo.statuses.get(&Path::new(B_TXT).into()), + Some(&GitStatus::Added) + ); }); git_add(Path::new(A_TXT), &repo); @@ -3939,18 +3908,15 @@ mod tests { let (_, repo) = snapshot.repository_entries.iter().next().unwrap(); assert_eq!(repo.statuses.iter().count(), 0); - assert_eq!(repo.statuses.get(&a_txt_id), None); - assert_eq!(repo.statuses.get(&b_txt_id), None); + assert_eq!(repo.statuses.get(&Path::new(A_TXT).into()), None); + assert_eq!(repo.statuses.get(&Path::new(B_TXT).into()), None); }); git_reset(0, &repo); git_remove_index(Path::new(B_TXT), &repo); git_stash(&mut repo); - std::fs::write(work_dir.join(E_TXT), "eeee").unwrap(); tree.flush_fs_events(cx).await; - dbg!(git_status(&repo)); - // Check that more complex repo changes are tracked tree.read_with(cx, |tree, _cx| { let snapshot = tree.snapshot(); @@ -3958,14 +3924,15 @@ mod tests { dbg!(&repo.statuses); - assert_eq!(repo.statuses.iter().count(), 2); - assert_eq!(repo.statuses.get(&a_txt_id), None); - assert_eq!(repo.statuses.get(&b_txt_id), Some(&GitStatus::Added)); - assert_eq!(repo.statuses.get(&e_txt_id), Some(&GitStatus::Modified)); + assert_eq!(repo.statuses.iter().count(), 1); + assert_eq!(repo.statuses.get(&Path::new(A_TXT).into()), None); + assert_eq!( + repo.statuses.get(&Path::new(B_TXT).into()), + Some(&GitStatus::Added) + ); }); std::fs::remove_file(work_dir.join(B_TXT)).unwrap(); - std::fs::remove_dir_all(work_dir.join("c")).unwrap(); tree.flush_fs_events(cx).await; // Check that non-repo behavior is tracked @@ -3974,9 +3941,8 @@ mod tests { let (_, repo) = snapshot.repository_entries.iter().next().unwrap(); assert_eq!(repo.statuses.iter().count(), 0); - assert_eq!(repo.statuses.get(&a_txt_id), None); - assert_eq!(repo.statuses.get(&b_txt_id), None); - assert_eq!(repo.statuses.get(&e_txt_id), None); + assert_eq!(repo.statuses.get(&Path::new(A_TXT).into()), None); + assert_eq!(repo.statuses.get(&Path::new(B_TXT).into()), None); }); } diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 2baccc0913..16b6232e8b 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -1012,9 +1012,10 @@ impl ProjectPanel { let entry_range = range.start.saturating_sub(ix)..end_ix - ix; for entry in &visible_worktree_entries[entry_range] { + let path = &entry.path; let status = snapshot - .repo_for(&entry.path) - .and_then(|repo_entry| repo_entry.status_for(entry.id)); + .repo_for(path) + .and_then(|entry| entry.status_for(&snapshot, path)); let mut details = EntryDetails { filename: entry From 23a19d85b817a70474f7ac2f9588191b46b7449a Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 10 May 2023 09:55:10 -0700 Subject: [PATCH 13/34] Fix bug in status detection when removing a directory --- crates/collab/src/tests/integration_tests.rs | 32 +++---- crates/fs/src/fs.rs | 8 +- crates/fs/src/repository.rs | 62 ++++++------- crates/project/src/worktree.rs | 94 ++++++++++++-------- crates/project_panel/src/project_panel.rs | 17 ++-- 5 files changed, 108 insertions(+), 105 deletions(-) diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 764f070f0b..185e6c6354 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -10,7 +10,7 @@ use editor::{ ConfirmRename, Editor, ExcerptRange, MultiBuffer, Redo, Rename, ToOffset, ToggleCodeActions, Undo, }; -use fs::{repository::GitStatus, FakeFs, Fs as _, LineEnding, RemoveOptions}; +use fs::{repository::GitFileStatus, FakeFs, Fs as _, LineEnding, RemoveOptions}; use futures::StreamExt as _; use gpui::{ executor::Deterministic, geometry::vector::vec2f, test::EmptyView, AppContext, ModelHandle, @@ -2728,8 +2728,8 @@ async fn test_git_status_sync( .set_status_for_repo( Path::new("/dir/.git"), &[ - (&Path::new(A_TXT), GitStatus::Added), - (&Path::new(B_TXT), GitStatus::Added), + (&Path::new(A_TXT), GitFileStatus::Added), + (&Path::new(B_TXT), GitFileStatus::Added), ], ) .await; @@ -2748,7 +2748,7 @@ async fn test_git_status_sync( deterministic.run_until_parked(); #[track_caller] - fn assert_status(file: &impl AsRef, status: Option, project: &Project, cx: &AppContext) { + fn assert_status(file: &impl AsRef, status: Option, project: &Project, cx: &AppContext) { let file = file.as_ref(); let worktrees = project.visible_worktrees(cx).collect::>(); assert_eq!(worktrees.len(), 1); @@ -2760,12 +2760,12 @@ async fn test_git_status_sync( // Smoke test status reading project_local.read_with(cx_a, |project, cx| { - assert_status(&Path::new(A_TXT), Some(GitStatus::Added), project, cx); - assert_status(&Path::new(B_TXT), Some(GitStatus::Added), project, cx); + assert_status(&Path::new(A_TXT), Some(GitFileStatus::Added), project, cx); + assert_status(&Path::new(B_TXT), Some(GitFileStatus::Added), project, cx); }); project_remote.read_with(cx_b, |project, cx| { - assert_status(&Path::new(A_TXT), Some(GitStatus::Added), project, cx); - assert_status(&Path::new(B_TXT), Some(GitStatus::Added), project, cx); + assert_status(&Path::new(A_TXT), Some(GitFileStatus::Added), project, cx); + assert_status(&Path::new(B_TXT), Some(GitFileStatus::Added), project, cx); }); client_a @@ -2774,8 +2774,8 @@ async fn test_git_status_sync( .set_status_for_repo( Path::new("/dir/.git"), &[ - (&Path::new(A_TXT), GitStatus::Modified), - (&Path::new(B_TXT), GitStatus::Modified), + (&Path::new(A_TXT), GitFileStatus::Modified), + (&Path::new(B_TXT), GitFileStatus::Modified), ], ) .await; @@ -2785,19 +2785,19 @@ async fn test_git_status_sync( // Smoke test status reading project_local.read_with(cx_a, |project, cx| { - assert_status(&Path::new(A_TXT), Some(GitStatus::Added), project, cx); - assert_status(&Path::new(B_TXT), Some(GitStatus::Added), project, cx); + assert_status(&Path::new(A_TXT), Some(GitFileStatus::Added), project, cx); + assert_status(&Path::new(B_TXT), Some(GitFileStatus::Added), project, cx); }); project_remote.read_with(cx_b, |project, cx| { - assert_status(&Path::new(A_TXT), Some(GitStatus::Added), project, cx); - assert_status(&Path::new(B_TXT), Some(GitStatus::Added), project, cx); + assert_status(&Path::new(A_TXT), Some(GitFileStatus::Added), project, cx); + assert_status(&Path::new(B_TXT), Some(GitFileStatus::Added), project, cx); }); // And synchronization while joining let project_remote_c = client_c.build_remote_project(project_id, cx_c).await; project_remote_c.read_with(cx_c, |project, cx| { - assert_status(&Path::new(A_TXT), Some(GitStatus::Added), project, cx); - assert_status(&Path::new(B_TXT), Some(GitStatus::Added), project, cx); + assert_status(&Path::new(A_TXT), Some(GitFileStatus::Added), project, cx); + assert_status(&Path::new(B_TXT), Some(GitFileStatus::Added), project, cx); }); } diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index efc24553c4..fd094160f5 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -7,7 +7,7 @@ use git2::Repository as LibGitRepository; use lazy_static::lazy_static; use parking_lot::Mutex; use regex::Regex; -use repository::{GitRepository, GitStatus}; +use repository::{GitRepository, GitFileStatus}; use rope::Rope; use smol::io::{AsyncReadExt, AsyncWriteExt}; use std::borrow::Cow; @@ -654,10 +654,10 @@ impl FakeFs { }); } - pub async fn set_status_for_repo(&self, dot_git: &Path, statuses: &[(&Path, GitStatus)]) { + pub async fn set_status_for_repo(&self, dot_git: &Path, statuses: &[(&Path, GitFileStatus)]) { self.with_git_state(dot_git, |state| { - state.git_statuses.clear(); - state.git_statuses.extend( + state.worktree_statuses.clear(); + state.worktree_statuses.extend( statuses .iter() .map(|(path, content)| { diff --git a/crates/fs/src/repository.rs b/crates/fs/src/repository.rs index 7fa20bddcb..3fb562570e 100644 --- a/crates/fs/src/repository.rs +++ b/crates/fs/src/repository.rs @@ -1,6 +1,5 @@ use anyhow::Result; use collections::HashMap; -use git2::Status; use parking_lot::Mutex; use std::{ ffi::OsStr, @@ -21,9 +20,9 @@ pub trait GitRepository: Send { fn branch_name(&self) -> Option; - fn statuses(&self) -> Option>; + fn worktree_statuses(&self) -> Option>; - fn file_status(&self, path: &RepoPath) -> Option; + fn worktree_status(&self, path: &RepoPath) -> Option; } impl std::fmt::Debug for dyn GitRepository { @@ -70,7 +69,7 @@ impl GitRepository for LibGitRepository { Some(branch.to_string()) } - fn statuses(&self) -> Option> { + fn worktree_statuses(&self) -> Option> { let statuses = self.statuses(None).log_err()?; let mut map = TreeMap::default(); @@ -80,17 +79,31 @@ impl GitRepository for LibGitRepository { .filter(|status| !status.status().contains(git2::Status::IGNORED)) { let path = RepoPath(PathBuf::from(OsStr::from_bytes(status.path_bytes()))); + let Some(status) = read_status(status.status()) else { + continue + }; - map.insert(path, status.status().into()) + map.insert(path, status) } Some(map) } - fn file_status(&self, path: &RepoPath) -> Option { + fn worktree_status(&self, path: &RepoPath) -> Option { let status = self.status_file(path).log_err()?; + read_status(status) + } +} - Some(status.into()) +fn read_status(status: git2::Status) -> Option { + if status.contains(git2::Status::CONFLICTED) { + Some(GitFileStatus::Conflict) + } else if status.intersects(git2::Status::WT_MODIFIED | git2::Status::WT_RENAMED) { + Some(GitFileStatus::Modified) + } else if status.intersects(git2::Status::WT_NEW) { + Some(GitFileStatus::Added) + } else { + None } } @@ -102,7 +115,7 @@ pub struct FakeGitRepository { #[derive(Debug, Clone, Default)] pub struct FakeGitRepositoryState { pub index_contents: HashMap, - pub git_statuses: HashMap, + pub worktree_statuses: HashMap, pub branch_name: Option, } @@ -126,18 +139,18 @@ impl GitRepository for FakeGitRepository { state.branch_name.clone() } - fn statuses(&self) -> Option> { + fn worktree_statuses(&self) -> Option> { let state = self.state.lock(); let mut map = TreeMap::default(); - for (repo_path, status) in state.git_statuses.iter() { + for (repo_path, status) in state.worktree_statuses.iter() { map.insert(repo_path.to_owned(), status.to_owned()); } Some(map) } - fn file_status(&self, path: &RepoPath) -> Option { + fn worktree_status(&self, path: &RepoPath) -> Option { let state = self.state.lock(); - state.git_statuses.get(path).cloned() + state.worktree_statuses.get(path).cloned() } } @@ -170,32 +183,11 @@ fn check_path_to_repo_path_errors(relative_file_path: &Path) -> Result<()> { } } -#[derive(Debug, Clone, Default, PartialEq, Eq)] -pub enum GitStatus { +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum GitFileStatus { Added, Modified, Conflict, - #[default] - Untracked, -} - -impl From for GitStatus { - fn from(value: Status) -> Self { - if value.contains(git2::Status::CONFLICTED) { - GitStatus::Conflict - } else if value.intersects( - git2::Status::INDEX_MODIFIED - | git2::Status::WT_MODIFIED - | git2::Status::INDEX_RENAMED - | git2::Status::WT_RENAMED, - ) { - GitStatus::Modified - } else if value.intersects(git2::Status::INDEX_NEW | git2::Status::WT_NEW) { - GitStatus::Added - } else { - GitStatus::Untracked - } - } } #[derive(Clone, Debug, Ord, Hash, PartialOrd, Eq, PartialEq)] diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 82c719f31e..1fc4fcf5a8 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -7,7 +7,7 @@ use client::{proto, Client}; use clock::ReplicaId; use collections::{HashMap, VecDeque}; use fs::{ - repository::{GitRepository, GitStatus, RepoPath}, + repository::{GitRepository, GitFileStatus, RepoPath}, Fs, LineEnding, }; use futures::{ @@ -143,7 +143,7 @@ impl Snapshot { pub struct RepositoryEntry { pub(crate) work_directory: WorkDirectoryEntry, pub(crate) branch: Option>, - pub(crate) statuses: TreeMap, + pub(crate) worktree_statuses: TreeMap, } impl RepositoryEntry { @@ -165,10 +165,10 @@ impl RepositoryEntry { self.work_directory.contains(snapshot, path) } - pub fn status_for(&self, snapshot: &Snapshot, path: &Path) -> Option { + pub fn status_for(&self, snapshot: &Snapshot, path: &Path) -> Option { self.work_directory .relativize(snapshot, path) - .and_then(|repo_path| self.statuses.get(&repo_path)) + .and_then(|repo_path| self.worktree_statuses.get(&repo_path)) .cloned() } } @@ -1445,7 +1445,7 @@ impl Snapshot { work_directory: ProjectEntryId::from_proto(repository.work_directory_id).into(), branch: repository.branch.map(Into::into), // TODO: status - statuses: Default::default(), + worktree_statuses: Default::default(), }; if let Some(entry) = self.entry_for_id(repository.work_directory_id()) { self.repository_entries @@ -1864,7 +1864,7 @@ impl LocalSnapshot { RepositoryEntry { work_directory: work_dir_id.into(), branch: repo_lock.branch_name().map(Into::into), - statuses: repo_lock.statuses().unwrap_or_default(), + worktree_statuses: repo_lock.worktree_statuses().unwrap_or_default(), }, ); drop(repo_lock); @@ -2896,9 +2896,12 @@ impl BackgroundScanner { .git_repositories .update(&work_dir_id, |entry| entry.scan_id = scan_id); + // TODO: Status Replace linear scan with smarter sum tree traversal snapshot .repository_entries - .update(&work_dir, |entry| entry.statuses.remove(&repo_path)); + .update(&work_dir, |entry| entry.worktree_statuses.retain(|stored_path, _| { + !stored_path.starts_with(&repo_path) + })); } Some(()) @@ -2920,7 +2923,7 @@ impl BackgroundScanner { let repo = repo.lock(); repo.reload_index(); let branch = repo.branch_name(); - let statuses = repo.statuses().unwrap_or_default(); + let statuses = repo.worktree_statuses().unwrap_or_default(); snapshot.git_repositories.update(&entry_id, |entry| { entry.scan_id = scan_id; @@ -2929,7 +2932,7 @@ impl BackgroundScanner { snapshot.repository_entries.update(&work_dir, |entry| { entry.branch = branch.map(Into::into); - entry.statuses = statuses; + entry.worktree_statuses = statuses; }); } else { let repo = snapshot.repo_for(&path)?; @@ -2945,21 +2948,19 @@ impl BackgroundScanner { } let git_ptr = local_repo.repo_ptr.lock(); - git_ptr.file_status(&repo_path)? + git_ptr.worktree_status(&repo_path)? }; - if status != GitStatus::Untracked { - let work_dir = repo.work_directory(snapshot)?; - let work_dir_id = repo.work_directory; + let work_dir = repo.work_directory(snapshot)?; + let work_dir_id = repo.work_directory; - snapshot - .git_repositories - .update(&work_dir_id, |entry| entry.scan_id = scan_id); + snapshot + .git_repositories + .update(&work_dir_id, |entry| entry.scan_id = scan_id); - snapshot - .repository_entries - .update(&work_dir, |entry| entry.statuses.insert(repo_path, status)); - } + snapshot + .repository_entries + .update(&work_dir, |entry| entry.worktree_statuses.insert(repo_path, status)); } Some(()) @@ -3848,6 +3849,11 @@ mod tests { "project": { "a.txt": "a", "b.txt": "bb", + "c": { + "d": { + "e.txt": "eee" + } + } }, })); @@ -3865,18 +3871,22 @@ mod tests { .await .unwrap(); + cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) + .await; + const A_TXT: &'static str = "a.txt"; const B_TXT: &'static str = "b.txt"; + const E_TXT: &'static str = "c/d/e.txt"; + let work_dir = root.path().join("project"); let mut repo = git_init(work_dir.as_path()); git_add(Path::new(A_TXT), &repo); + git_add(Path::new(E_TXT), &repo); git_commit("Initial commit", &repo); std::fs::write(work_dir.join(A_TXT), "aa").unwrap(); - cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) - .await; tree.flush_fs_events(cx).await; // Check that the right git state is observed on startup @@ -3886,14 +3896,14 @@ mod tests { let (dir, repo) = snapshot.repository_entries.iter().next().unwrap(); assert_eq!(dir.0.as_ref(), Path::new("project")); - assert_eq!(repo.statuses.iter().count(), 2); + assert_eq!(repo.worktree_statuses.iter().count(), 2); assert_eq!( - repo.statuses.get(&Path::new(A_TXT).into()), - Some(&GitStatus::Modified) + repo.worktree_statuses.get(&Path::new(A_TXT).into()), + Some(&GitFileStatus::Modified) ); assert_eq!( - repo.statuses.get(&Path::new(B_TXT).into()), - Some(&GitStatus::Added) + repo.worktree_statuses.get(&Path::new(B_TXT).into()), + Some(&GitFileStatus::Added) ); }); @@ -3907,14 +3917,15 @@ mod tests { let snapshot = tree.snapshot(); let (_, repo) = snapshot.repository_entries.iter().next().unwrap(); - assert_eq!(repo.statuses.iter().count(), 0); - assert_eq!(repo.statuses.get(&Path::new(A_TXT).into()), None); - assert_eq!(repo.statuses.get(&Path::new(B_TXT).into()), None); + assert_eq!(repo.worktree_statuses.iter().count(), 0); + assert_eq!(repo.worktree_statuses.get(&Path::new(A_TXT).into()), None); + assert_eq!(repo.worktree_statuses.get(&Path::new(B_TXT).into()), None); }); git_reset(0, &repo); git_remove_index(Path::new(B_TXT), &repo); git_stash(&mut repo); + std::fs::write(work_dir.join(E_TXT), "eeee").unwrap(); tree.flush_fs_events(cx).await; // Check that more complex repo changes are tracked @@ -3922,17 +3933,21 @@ mod tests { let snapshot = tree.snapshot(); let (_, repo) = snapshot.repository_entries.iter().next().unwrap(); - dbg!(&repo.statuses); - - assert_eq!(repo.statuses.iter().count(), 1); - assert_eq!(repo.statuses.get(&Path::new(A_TXT).into()), None); + assert_eq!(repo.worktree_statuses.iter().count(), 2); + assert_eq!(repo.worktree_statuses.get(&Path::new(A_TXT).into()), None); assert_eq!( - repo.statuses.get(&Path::new(B_TXT).into()), - Some(&GitStatus::Added) + repo.worktree_statuses.get(&Path::new(B_TXT).into()), + Some(&GitFileStatus::Added) + ); + assert_eq!( + repo.worktree_statuses.get(&Path::new(E_TXT).into()), + Some(&GitFileStatus::Modified) ); }); std::fs::remove_file(work_dir.join(B_TXT)).unwrap(); + std::fs::remove_dir_all(work_dir.join("c")).unwrap(); + tree.flush_fs_events(cx).await; // Check that non-repo behavior is tracked @@ -3940,9 +3955,10 @@ mod tests { let snapshot = tree.snapshot(); let (_, repo) = snapshot.repository_entries.iter().next().unwrap(); - assert_eq!(repo.statuses.iter().count(), 0); - assert_eq!(repo.statuses.get(&Path::new(A_TXT).into()), None); - assert_eq!(repo.statuses.get(&Path::new(B_TXT).into()), None); + assert_eq!(repo.worktree_statuses.iter().count(), 0); + assert_eq!(repo.worktree_statuses.get(&Path::new(A_TXT).into()), None); + assert_eq!(repo.worktree_statuses.get(&Path::new(B_TXT).into()), None); + assert_eq!(repo.worktree_statuses.get(&Path::new(E_TXT).into()), None); }); } diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 16b6232e8b..49741ea49f 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -17,7 +17,7 @@ use gpui::{ }; use menu::{Confirm, SelectNext, SelectPrev}; use project::{ - repository::GitStatus, Entry, EntryKind, Project, ProjectEntryId, ProjectPath, Worktree, + repository::GitFileStatus, Entry, EntryKind, Project, ProjectEntryId, ProjectPath, Worktree, WorktreeId, }; use settings::Settings; @@ -89,7 +89,7 @@ pub struct EntryDetails { is_editing: bool, is_processing: bool, is_cut: bool, - git_status: Option, + git_status: Option, } actions!( @@ -1081,19 +1081,14 @@ impl ProjectPanel { // Prepare colors for git statuses let editor_theme = &cx.global::().theme.editor; - let color_for_added = Some(editor_theme.diff.inserted); - let color_for_modified = Some(editor_theme.diff.modified); - let color_for_conflict = Some(editor_theme.diff.deleted); - let color_for_untracked = None; let mut filename_text_style = style.text.clone(); filename_text_style.color = details .git_status .as_ref() - .and_then(|status| match status { - GitStatus::Added => color_for_added, - GitStatus::Modified => color_for_modified, - GitStatus::Conflict => color_for_conflict, - GitStatus::Untracked => color_for_untracked, + .map(|status| match status { + GitFileStatus::Added => editor_theme.diff.inserted, + GitFileStatus::Modified => editor_theme.diff.modified, + GitFileStatus::Conflict => editor_theme.diff.deleted, }) .unwrap_or(style.text.color); From 00b345fdfe426b00c7da0219585eda03c20a9171 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 10 May 2023 11:28:39 -0700 Subject: [PATCH 14/34] Use sum tree traversal to remove paths --- crates/project/src/worktree.rs | 26 ++++++++----- crates/sum_tree/src/tree_map.rs | 68 +++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 10 deletions(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 1fc4fcf5a8..ff9f7fde9a 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -7,7 +7,7 @@ use client::{proto, Client}; use clock::ReplicaId; use collections::{HashMap, VecDeque}; use fs::{ - repository::{GitRepository, GitFileStatus, RepoPath}, + repository::{GitFileStatus, GitRepository, RepoPath}, Fs, LineEnding, }; use futures::{ @@ -46,6 +46,7 @@ use std::{ future::Future, mem, ops::{Deref, DerefMut}, + path::{Path, PathBuf}, pin::Pin, sync::{ @@ -2896,12 +2897,13 @@ impl BackgroundScanner { .git_repositories .update(&work_dir_id, |entry| entry.scan_id = scan_id); - // TODO: Status Replace linear scan with smarter sum tree traversal - snapshot - .repository_entries - .update(&work_dir, |entry| entry.worktree_statuses.retain(|stored_path, _| { - !stored_path.starts_with(&repo_path) - })); + snapshot.repository_entries.update(&work_dir, |entry| { + entry + .worktree_statuses + .remove_from_while(&repo_path, |stored_path, _| { + stored_path.starts_with(&repo_path) + }) + }); } Some(()) @@ -2958,9 +2960,9 @@ impl BackgroundScanner { .git_repositories .update(&work_dir_id, |entry| entry.scan_id = scan_id); - snapshot - .repository_entries - .update(&work_dir, |entry| entry.worktree_statuses.insert(repo_path, status)); + snapshot.repository_entries.update(&work_dir, |entry| { + entry.worktree_statuses.insert(repo_path, status) + }); } Some(()) @@ -3948,6 +3950,8 @@ mod tests { std::fs::remove_file(work_dir.join(B_TXT)).unwrap(); std::fs::remove_dir_all(work_dir.join("c")).unwrap(); + dbg!(git_status(&repo)); + tree.flush_fs_events(cx).await; // Check that non-repo behavior is tracked @@ -3955,6 +3959,8 @@ mod tests { let snapshot = tree.snapshot(); let (_, repo) = snapshot.repository_entries.iter().next().unwrap(); + dbg!(&repo.worktree_statuses); + assert_eq!(repo.worktree_statuses.iter().count(), 0); assert_eq!(repo.worktree_statuses.get(&Path::new(A_TXT).into()), None); assert_eq!(repo.worktree_statuses.get(&Path::new(B_TXT).into()), None); diff --git a/crates/sum_tree/src/tree_map.rs b/crates/sum_tree/src/tree_map.rs index ab37d2577a..359137d439 100644 --- a/crates/sum_tree/src/tree_map.rs +++ b/crates/sum_tree/src/tree_map.rs @@ -82,6 +82,36 @@ impl TreeMap { cursor.item().map(|item| (&item.key, &item.value)) } + pub fn remove_between(&mut self, from: &K, until: &K) + { + let mut cursor = self.0.cursor::>(); + let from_key = MapKeyRef(Some(from)); + let mut new_tree = cursor.slice(&from_key, Bias::Left, &()); + let until_key = MapKeyRef(Some(until)); + cursor.seek_forward(&until_key, Bias::Left, &()); + new_tree.push_tree(cursor.suffix(&()), &()); + drop(cursor); + self.0 = new_tree; + } + + pub fn remove_from_while(&mut self, from: &K, mut f: F) + where F: FnMut(&K, &V) -> bool + { + let mut cursor = self.0.cursor::>(); + let from_key = MapKeyRef(Some(from)); + let mut new_tree = cursor.slice(&from_key, Bias::Left, &()); + while let Some(item) = cursor.item() { + if !f(&item.key, &item.value) { + break; + } + cursor.next(&()); + } + new_tree.push_tree(cursor.suffix(&()), &()); + drop(cursor); + self.0 = new_tree; + } + + pub fn update(&mut self, key: &K, f: F) -> Option where F: FnOnce(&mut V) -> T, @@ -272,4 +302,42 @@ mod tests { map.retain(|key, _| *key % 2 == 0); assert_eq!(map.iter().collect::>(), vec![(&4, &"d"), (&6, &"f")]); } + + #[test] + fn test_remove_between() { + let mut map = TreeMap::default(); + + map.insert("a", 1); + map.insert("b", 2); + map.insert("baa", 3); + map.insert("baaab", 4); + map.insert("c", 5); + + map.remove_between(&"ba", &"bb"); + + assert_eq!(map.get(&"a"), Some(&1)); + assert_eq!(map.get(&"b"), Some(&2)); + assert_eq!(map.get(&"baaa"), None); + assert_eq!(map.get(&"baaaab"), None); + assert_eq!(map.get(&"c"), Some(&5)); + } + + #[test] + fn test_remove_from_while() { + let mut map = TreeMap::default(); + + map.insert("a", 1); + map.insert("b", 2); + map.insert("baa", 3); + map.insert("baaab", 4); + map.insert("c", 5); + + map.remove_from_while(&"ba", |key, _| key.starts_with(&"ba")); + + assert_eq!(map.get(&"a"), Some(&1)); + assert_eq!(map.get(&"b"), Some(&2)); + assert_eq!(map.get(&"baaa"), None); + assert_eq!(map.get(&"baaaab"), None); + assert_eq!(map.get(&"c"), Some(&5)); + } } From 2b80dfa81d2e4ed6f36443b28a18906412ef1419 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 10 May 2023 11:59:04 -0700 Subject: [PATCH 15/34] Update protos --- crates/collab/src/db.rs | 8 ++++---- crates/project/src/worktree.rs | 25 +++++++++++++------------ crates/rpc/proto/zed.proto | 6 +++--- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 5867fa7369..b9bd1374eb 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -1568,8 +1568,8 @@ impl Database { worktree.updated_repositories.push(proto::RepositoryEntry { work_directory_id: db_repository.work_directory_id as u64, branch: db_repository.branch, - removed_statuses: Default::default(), - updated_statuses: Default::default(), + removed_worktree_repo_paths: Default::default(), + updated_worktree_statuses: Default::default(), }); } @@ -2651,8 +2651,8 @@ impl Database { worktree.repository_entries.push(proto::RepositoryEntry { work_directory_id: db_repository_entry.work_directory_id as u64, branch: db_repository_entry.branch, - removed_statuses: Default::default(), - updated_statuses: Default::default(), + removed_worktree_repo_paths: Default::default(), + updated_worktree_statuses: Default::default(), }); } diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index ff9f7fde9a..7b760ae354 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -180,8 +180,8 @@ impl From<&RepositoryEntry> for proto::RepositoryEntry { work_directory_id: value.work_directory.to_proto(), branch: value.branch.as_ref().map(|str| str.to_string()), // TODO: Status - removed_statuses: Default::default(), - updated_statuses: Default::default(), + removed_worktree_repo_paths: Default::default(), + updated_worktree_statuses: Default::default(), } } } @@ -1597,12 +1597,11 @@ impl LocalSnapshot { pub(crate) fn repo_for_metadata( &self, path: &Path, - ) -> Option<(ProjectEntryId, Arc>)> { - let (entry_id, local_repo) = self + ) -> Option<(&ProjectEntryId, &LocalRepositoryEntry)> { + self .git_repositories .iter() - .find(|(_, repo)| repo.in_dot_git(path))?; - Some((*entry_id, local_repo.repo_ptr.to_owned())) + .find(|(_, repo)| repo.in_dot_git(path)) } #[cfg(test)] @@ -2916,13 +2915,19 @@ impl BackgroundScanner { .components() .any(|component| component.as_os_str() == *DOT_GIT) { - let (entry_id, repo) = snapshot.repo_for_metadata(&path)?; + let (entry_id, repo_ptr) = { + let (entry_id, repo) = snapshot.repo_for_metadata(&path)?; + if repo.full_scan_id == scan_id { + return None; + } + (*entry_id, repo.repo_ptr.to_owned()) + }; let work_dir = snapshot .entry_for_id(entry_id) .map(|entry| RepositoryWorkDirectory(entry.path.clone()))?; - let repo = repo.lock(); + let repo = repo_ptr.lock(); repo.reload_index(); let branch = repo.branch_name(); let statuses = repo.worktree_statuses().unwrap_or_default(); @@ -3950,8 +3955,6 @@ mod tests { std::fs::remove_file(work_dir.join(B_TXT)).unwrap(); std::fs::remove_dir_all(work_dir.join("c")).unwrap(); - dbg!(git_status(&repo)); - tree.flush_fs_events(cx).await; // Check that non-repo behavior is tracked @@ -3959,8 +3962,6 @@ mod tests { let snapshot = tree.snapshot(); let (_, repo) = snapshot.repository_entries.iter().next().unwrap(); - dbg!(&repo.worktree_statuses); - assert_eq!(repo.worktree_statuses.iter().count(), 0); assert_eq!(repo.worktree_statuses.get(&Path::new(A_TXT).into()), None); assert_eq!(repo.worktree_statuses.get(&Path::new(B_TXT).into()), None); diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index abe02f42bb..8e45435b89 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -986,12 +986,12 @@ message Entry { message RepositoryEntry { uint64 work_directory_id = 1; optional string branch = 2; - repeated uint64 removed_statuses = 3; - repeated StatusEntry updated_statuses = 4; + repeated string removed_worktree_repo_paths = 3; + repeated StatusEntry updated_worktree_statuses = 4; } message StatusEntry { - uint64 entry_id = 1; + string repo_path = 1; GitStatus status = 2; } From e20eaca59513622b903f201410b89fb2ac21525a Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 10 May 2023 16:07:41 -0700 Subject: [PATCH 16/34] Got basic replication working :) --- crates/collab/src/db.rs | 2 - crates/collab/src/tests/integration_tests.rs | 49 ++++++- crates/fs/src/fs.rs | 6 +- crates/fs/src/repository.rs | 2 +- crates/project/src/worktree.rs | 129 ++++++++++++++++--- crates/rpc/src/proto.rs | 65 +++++++++- crates/sum_tree/src/tree_map.rs | 44 ++++++- 7 files changed, 256 insertions(+), 41 deletions(-) diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index b9bd1374eb..217987984a 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -1570,7 +1570,6 @@ impl Database { branch: db_repository.branch, removed_worktree_repo_paths: Default::default(), updated_worktree_statuses: Default::default(), - }); } } @@ -2653,7 +2652,6 @@ impl Database { branch: db_repository_entry.branch, removed_worktree_repo_paths: Default::default(), updated_worktree_statuses: Default::default(), - }); } } diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 185e6c6354..7dd8f86b8e 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -2748,7 +2748,12 @@ async fn test_git_status_sync( deterministic.run_until_parked(); #[track_caller] - fn assert_status(file: &impl AsRef, status: Option, project: &Project, cx: &AppContext) { + fn assert_status( + file: &impl AsRef, + status: Option, + project: &Project, + cx: &AppContext, + ) { let file = file.as_ref(); let worktrees = project.visible_worktrees(cx).collect::>(); assert_eq!(worktrees.len(), 1); @@ -2785,19 +2790,49 @@ async fn test_git_status_sync( // Smoke test status reading project_local.read_with(cx_a, |project, cx| { - assert_status(&Path::new(A_TXT), Some(GitFileStatus::Added), project, cx); - assert_status(&Path::new(B_TXT), Some(GitFileStatus::Added), project, cx); + assert_status( + &Path::new(A_TXT), + Some(GitFileStatus::Modified), + project, + cx, + ); + assert_status( + &Path::new(B_TXT), + Some(GitFileStatus::Modified), + project, + cx, + ); }); project_remote.read_with(cx_b, |project, cx| { - assert_status(&Path::new(A_TXT), Some(GitFileStatus::Added), project, cx); - assert_status(&Path::new(B_TXT), Some(GitFileStatus::Added), project, cx); + assert_status( + &Path::new(A_TXT), + Some(GitFileStatus::Modified), + project, + cx, + ); + assert_status( + &Path::new(B_TXT), + Some(GitFileStatus::Modified), + project, + cx, + ); }); // And synchronization while joining let project_remote_c = client_c.build_remote_project(project_id, cx_c).await; project_remote_c.read_with(cx_c, |project, cx| { - assert_status(&Path::new(A_TXT), Some(GitFileStatus::Added), project, cx); - assert_status(&Path::new(B_TXT), Some(GitFileStatus::Added), project, cx); + assert_status( + &Path::new(A_TXT), + Some(GitFileStatus::Modified), + project, + cx, + ); + assert_status( + &Path::new(B_TXT), + Some(GitFileStatus::Modified), + project, + cx, + ); }); } diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index fd094160f5..09ddce2ffa 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -7,7 +7,7 @@ use git2::Repository as LibGitRepository; use lazy_static::lazy_static; use parking_lot::Mutex; use regex::Regex; -use repository::{GitRepository, GitFileStatus}; +use repository::{GitFileStatus, GitRepository}; use rope::Rope; use smol::io::{AsyncReadExt, AsyncWriteExt}; use std::borrow::Cow; @@ -660,9 +660,7 @@ impl FakeFs { state.worktree_statuses.extend( statuses .iter() - .map(|(path, content)| { - ((**path).into(), content.clone()) - }), + .map(|(path, content)| ((**path).into(), content.clone())), ); }); } diff --git a/crates/fs/src/repository.rs b/crates/fs/src/repository.rs index 3fb562570e..90b3761677 100644 --- a/crates/fs/src/repository.rs +++ b/crates/fs/src/repository.rs @@ -194,7 +194,7 @@ pub enum GitFileStatus { pub struct RepoPath(PathBuf); impl RepoPath { - fn new(path: PathBuf) -> Self { + pub fn new(path: PathBuf) -> Self { debug_assert!(path.is_relative(), "Repo paths must be relative"); RepoPath(path) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 7b760ae354..0d4a02775d 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -46,7 +46,6 @@ use std::{ future::Future, mem, ops::{Deref, DerefMut}, - path::{Path, PathBuf}, pin::Pin, sync::{ @@ -147,6 +146,14 @@ pub struct RepositoryEntry { pub(crate) worktree_statuses: TreeMap, } +fn read_git_status(git_status: i32) -> Option { + proto::GitStatus::from_i32(git_status).map(|status| match status { + proto::GitStatus::Added => GitFileStatus::Added, + proto::GitStatus::Modified => GitFileStatus::Modified, + proto::GitStatus::Conflict => GitFileStatus::Conflict, + }) +} + impl RepositoryEntry { pub fn branch(&self) -> Option> { self.branch.clone() @@ -172,6 +179,70 @@ impl RepositoryEntry { .and_then(|repo_path| self.worktree_statuses.get(&repo_path)) .cloned() } + + pub fn build_update(&self, other: &Self) -> proto::RepositoryEntry { + let mut updated_statuses: Vec = Vec::new(); + let mut removed_statuses: Vec = Vec::new(); + + let mut self_statuses = self.worktree_statuses.iter().peekable(); + let mut other_statuses = other.worktree_statuses.iter().peekable(); + loop { + match (self_statuses.peek(), other_statuses.peek()) { + (Some((self_repo_path, self_status)), Some((other_repo_path, other_status))) => { + match Ord::cmp(self_repo_path, other_repo_path) { + Ordering::Less => { + updated_statuses.push(make_status_entry(self_repo_path, self_status)); + self_statuses.next(); + } + Ordering::Equal => { + if self_status != other_status { + updated_statuses + .push(make_status_entry(self_repo_path, self_status)); + } + + self_statuses.next(); + other_statuses.next(); + } + Ordering::Greater => { + removed_statuses.push(make_repo_path(other_repo_path)); + other_statuses.next(); + } + } + } + (Some((self_repo_path, self_status)), None) => { + updated_statuses.push(make_status_entry(self_repo_path, self_status)); + self_statuses.next(); + } + (None, Some((other_repo_path, _))) => { + removed_statuses.push(make_repo_path(other_repo_path)); + other_statuses.next(); + } + (None, None) => break, + } + } + + proto::RepositoryEntry { + work_directory_id: self.work_directory_id().to_proto(), + branch: self.branch.as_ref().map(|str| str.to_string()), + removed_worktree_repo_paths: removed_statuses, + updated_worktree_statuses: updated_statuses, + } + } +} + +fn make_repo_path(path: &RepoPath) -> String { + path.as_os_str().to_string_lossy().to_string() +} + +fn make_status_entry(path: &RepoPath, status: &GitFileStatus) -> proto::StatusEntry { + proto::StatusEntry { + repo_path: make_repo_path(path), + status: match status { + GitFileStatus::Added => proto::GitStatus::Added.into(), + GitFileStatus::Modified => proto::GitStatus::Modified.into(), + GitFileStatus::Conflict => proto::GitStatus::Conflict.into(), + }, + } } impl From<&RepositoryEntry> for proto::RepositoryEntry { @@ -179,9 +250,12 @@ impl From<&RepositoryEntry> for proto::RepositoryEntry { proto::RepositoryEntry { work_directory_id: value.work_directory.to_proto(), branch: value.branch.as_ref().map(|str| str.to_string()), - // TODO: Status + updated_worktree_statuses: value + .worktree_statuses + .iter() + .map(|(repo_path, status)| make_status_entry(repo_path, status)) + .collect(), removed_worktree_repo_paths: Default::default(), - updated_worktree_statuses: Default::default(), } } } @@ -1442,15 +1516,41 @@ impl Snapshot { }); for repository in update.updated_repositories { - let repository = RepositoryEntry { - work_directory: ProjectEntryId::from_proto(repository.work_directory_id).into(), - branch: repository.branch.map(Into::into), - // TODO: status - worktree_statuses: Default::default(), - }; - if let Some(entry) = self.entry_for_id(repository.work_directory_id()) { - self.repository_entries - .insert(RepositoryWorkDirectory(entry.path.clone()), repository) + let work_directory_entry: WorkDirectoryEntry = + ProjectEntryId::from_proto(repository.work_directory_id).into(); + + if let Some(entry) = self.entry_for_id(*work_directory_entry) { + let mut statuses = TreeMap::default(); + for status_entry in repository.updated_worktree_statuses { + let Some(git_file_status) = read_git_status(status_entry.status) else { + continue; + }; + + let repo_path = RepoPath::new(status_entry.repo_path.into()); + statuses.insert(repo_path, git_file_status); + } + + let work_directory = RepositoryWorkDirectory(entry.path.clone()); + if self.repository_entries.get(&work_directory).is_some() { + self.repository_entries.update(&work_directory, |repo| { + repo.branch = repository.branch.map(Into::into); + repo.worktree_statuses.insert_tree(statuses); + + for repo_path in repository.removed_worktree_repo_paths { + let repo_path = RepoPath::new(repo_path.into()); + repo.worktree_statuses.remove(&repo_path); + } + }); + } else { + self.repository_entries.insert( + work_directory, + RepositoryEntry { + work_directory: work_directory_entry, + branch: repository.branch.map(Into::into), + worktree_statuses: statuses, + }, + ) + } } else { log::error!("no work directory entry for repository {:?}", repository) } @@ -1598,8 +1698,7 @@ impl LocalSnapshot { &self, path: &Path, ) -> Option<(&ProjectEntryId, &LocalRepositoryEntry)> { - self - .git_repositories + self.git_repositories .iter() .find(|(_, repo)| repo.in_dot_git(path)) } @@ -1691,7 +1790,7 @@ impl LocalSnapshot { } Ordering::Equal => { if self_repo != other_repo { - updated_repositories.push((*self_repo).into()); + updated_repositories.push(self_repo.build_update(other_repo)); } self_repos.next(); diff --git a/crates/rpc/src/proto.rs b/crates/rpc/src/proto.rs index 20a457cc4b..32f40ad7db 100644 --- a/crates/rpc/src/proto.rs +++ b/crates/rpc/src/proto.rs @@ -484,9 +484,11 @@ pub fn split_worktree_update( mut message: UpdateWorktree, max_chunk_size: usize, ) -> impl Iterator { - let mut done = false; + let mut done_files = false; + let mut done_statuses = false; + let mut repository_index = 0; iter::from_fn(move || { - if done { + if done_files && done_statuses { return None; } @@ -502,22 +504,71 @@ pub fn split_worktree_update( .drain(..removed_entries_chunk_size) .collect(); - done = message.updated_entries.is_empty() && message.removed_entries.is_empty(); + done_files = message.updated_entries.is_empty() && message.removed_entries.is_empty(); // Wait to send repositories until after we've guaranteed that their associated entries // will be read - let updated_repositories = if done { - mem::take(&mut message.updated_repositories) + let updated_repositories = if done_files { + let mut total_statuses = 0; + let mut updated_repositories = Vec::new(); + while total_statuses < max_chunk_size + && repository_index < message.updated_repositories.len() + { + let updated_statuses_chunk_size = cmp::min( + message.updated_repositories[repository_index] + .updated_worktree_statuses + .len(), + max_chunk_size - total_statuses, + ); + + let updated_statuses: Vec<_> = message.updated_repositories[repository_index] + .updated_worktree_statuses + .drain(..updated_statuses_chunk_size) + .collect(); + + total_statuses += updated_statuses.len(); + + let done_this_repo = message.updated_repositories[repository_index] + .updated_worktree_statuses + .is_empty(); + + let removed_repo_paths = if done_this_repo { + mem::take( + &mut message.updated_repositories[repository_index] + .removed_worktree_repo_paths, + ) + } else { + Default::default() + }; + + updated_repositories.push(RepositoryEntry { + work_directory_id: message.updated_repositories[repository_index] + .work_directory_id, + branch: message.updated_repositories[repository_index] + .branch + .clone(), + updated_worktree_statuses: updated_statuses, + removed_worktree_repo_paths: removed_repo_paths, + }); + + if done_this_repo { + repository_index += 1; + } + } + + updated_repositories } else { Default::default() }; - let removed_repositories = if done { + let removed_repositories = if done_files && done_statuses { mem::take(&mut message.removed_repositories) } else { Default::default() }; + done_statuses = repository_index >= message.updated_repositories.len(); + Some(UpdateWorktree { project_id: message.project_id, worktree_id: message.worktree_id, @@ -526,7 +577,7 @@ pub fn split_worktree_update( updated_entries, removed_entries, scan_id: message.scan_id, - is_last_update: done && message.is_last_update, + is_last_update: done_files && message.is_last_update, updated_repositories, removed_repositories, }) diff --git a/crates/sum_tree/src/tree_map.rs b/crates/sum_tree/src/tree_map.rs index 359137d439..3942d00b29 100644 --- a/crates/sum_tree/src/tree_map.rs +++ b/crates/sum_tree/src/tree_map.rs @@ -1,6 +1,6 @@ use std::{cmp::Ordering, fmt::Debug}; -use crate::{Bias, Dimension, Item, KeyedItem, SeekTarget, SumTree, Summary}; +use crate::{Bias, Dimension, Edit, Item, KeyedItem, SeekTarget, SumTree, Summary}; #[derive(Clone, Debug, PartialEq, Eq)] pub struct TreeMap(SumTree>) @@ -82,8 +82,7 @@ impl TreeMap { cursor.item().map(|item| (&item.key, &item.value)) } - pub fn remove_between(&mut self, from: &K, until: &K) - { + pub fn remove_between(&mut self, from: &K, until: &K) { let mut cursor = self.0.cursor::>(); let from_key = MapKeyRef(Some(from)); let mut new_tree = cursor.slice(&from_key, Bias::Left, &()); @@ -95,7 +94,8 @@ impl TreeMap { } pub fn remove_from_while(&mut self, from: &K, mut f: F) - where F: FnMut(&K, &V) -> bool + where + F: FnMut(&K, &V) -> bool, { let mut cursor = self.0.cursor::>(); let from_key = MapKeyRef(Some(from)); @@ -111,7 +111,6 @@ impl TreeMap { self.0 = new_tree; } - pub fn update(&mut self, key: &K, f: F) -> Option where F: FnOnce(&mut V) -> T, @@ -155,6 +154,20 @@ impl TreeMap { pub fn values(&self) -> impl Iterator + '_ { self.0.iter().map(|entry| &entry.value) } + + pub fn insert_tree(&mut self, other: TreeMap) { + let edits = other + .iter() + .map(|(key, value)| { + Edit::Insert(MapEntry { + key: key.to_owned(), + value: value.to_owned(), + }) + }) + .collect(); + + self.0.edit(edits, &()); + } } impl Default for TreeMap @@ -340,4 +353,25 @@ mod tests { assert_eq!(map.get(&"baaaab"), None); assert_eq!(map.get(&"c"), Some(&5)); } + + #[test] + fn test_insert_tree() { + let mut map = TreeMap::default(); + map.insert("a", 1); + map.insert("b", 2); + map.insert("c", 3); + + let mut other = TreeMap::default(); + other.insert("a", 2); + other.insert("b", 2); + other.insert("d", 4); + + map.insert_tree(other); + + assert_eq!(map.iter().count(), 4); + assert_eq!(map.get(&"a"), Some(&2)); + assert_eq!(map.get(&"b"), Some(&2)); + assert_eq!(map.get(&"c"), Some(&3)); + assert_eq!(map.get(&"d"), Some(&4)); + } } From 65d4c4f6ed62bd25997c528b3fbd1c0fe8d044ce Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 10 May 2023 17:36:16 -0700 Subject: [PATCH 17/34] Add integration test for git status --- .../20221109000000_test_schema.sql | 19 ++- crates/collab/src/db.rs | 134 +++++++++++++++++- crates/collab/src/rpc.rs | 2 +- crates/collab/src/tests/integration_tests.rs | 2 + 4 files changed, 148 insertions(+), 9 deletions(-) diff --git a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql index 684b6bffe0..7c6a49f179 100644 --- a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql +++ b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql @@ -86,8 +86,8 @@ CREATE TABLE "worktree_repositories" ( "project_id" INTEGER NOT NULL, "worktree_id" INTEGER NOT NULL, "work_directory_id" INTEGER NOT NULL, - "scan_id" INTEGER NOT NULL, "branch" VARCHAR, + "scan_id" INTEGER NOT NULL, "is_deleted" BOOL NOT NULL, PRIMARY KEY(project_id, worktree_id, work_directory_id), FOREIGN KEY(project_id, worktree_id) REFERENCES worktrees (project_id, id) ON DELETE CASCADE, @@ -96,6 +96,23 @@ CREATE TABLE "worktree_repositories" ( CREATE INDEX "index_worktree_repositories_on_project_id" ON "worktree_repositories" ("project_id"); CREATE INDEX "index_worktree_repositories_on_project_id_and_worktree_id" ON "worktree_repositories" ("project_id", "worktree_id"); +CREATE TABLE "worktree_repository_statuses" ( + "project_id" INTEGER NOT NULL, + "worktree_id" INTEGER NOT NULL, + "work_directory_id" INTEGER NOT NULL, + "repo_path" VARCHAR NOT NULL, + "status" INTEGER NOT NULL, + "scan_id" INTEGER NOT NULL, + "is_deleted" BOOL NOT NULL, + PRIMARY KEY(project_id, worktree_id, work_directory_id, repo_path), + FOREIGN KEY(project_id, worktree_id) REFERENCES worktrees (project_id, id) ON DELETE CASCADE, + FOREIGN KEY(project_id, worktree_id, work_directory_id) REFERENCES worktree_entries (project_id, worktree_id, id) ON DELETE CASCADE +); +CREATE INDEX "index_worktree_repository_statuses_on_project_id" ON "worktree_repository_statuses" ("project_id"); +CREATE INDEX "index_worktree_repository_statuses_on_project_id_and_worktree_id" ON "worktree_repository_statuses" ("project_id", "worktree_id"); +CREATE INDEX "index_worktree_repository_statuses_on_project_id_and_worktree_id_and_work_directory_id" ON "worktree_repository_statuses" ("project_id", "worktree_id", "work_directory_id"); + + CREATE TABLE "worktree_diagnostic_summaries" ( "project_id" INTEGER NOT NULL, "worktree_id" INTEGER NOT NULL, diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 217987984a..cc85d4f369 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -15,6 +15,7 @@ mod worktree; mod worktree_diagnostic_summary; mod worktree_entry; mod worktree_repository; +mod worktree_repository_statuses; use crate::executor::Executor; use crate::{Error, Result}; @@ -2397,6 +2398,74 @@ impl Database { ) .exec(&*tx) .await?; + + for repository in update.updated_repositories.iter() { + if !repository.updated_worktree_statuses.is_empty() { + worktree_repository_statuses::Entity::insert_many( + repository + .updated_worktree_statuses + .iter() + .map(|status_entry| worktree_repository_statuses::ActiveModel { + project_id: ActiveValue::set(project_id), + worktree_id: ActiveValue::set(worktree_id), + work_directory_id: ActiveValue::set( + repository.work_directory_id as i64, + ), + repo_path: ActiveValue::set(status_entry.repo_path.clone()), + status: ActiveValue::set(status_entry.status as i64), + scan_id: ActiveValue::set(update.scan_id as i64), + is_deleted: ActiveValue::set(false), + }), + ) + .on_conflict( + OnConflict::columns([ + worktree_repository_statuses::Column::ProjectId, + worktree_repository_statuses::Column::WorktreeId, + worktree_repository_statuses::Column::WorkDirectoryId, + worktree_repository_statuses::Column::RepoPath, + ]) + .update_columns([ + worktree_repository_statuses::Column::ScanId, + worktree_repository_statuses::Column::Status, + ]) + .to_owned(), + ) + .exec(&*tx) + .await?; + } + + if !repository.removed_worktree_repo_paths.is_empty() { + worktree_repository_statuses::Entity::update_many() + .filter( + worktree_repository_statuses::Column::ProjectId + .eq(project_id) + .and( + worktree_repository_statuses::Column::WorktreeId + .eq(worktree_id), + ) + .and( + worktree_repository_statuses::Column::WorkDirectoryId + .eq(repository.work_directory_id), + ) + .and( + worktree_repository_statuses::Column::RepoPath.is_in( + repository + .removed_worktree_repo_paths + .iter() + .cloned() + .collect::>(), + ), + ), + ) + .set(worktree_repository_statuses::ActiveModel { + is_deleted: ActiveValue::Set(true), + scan_id: ActiveValue::Set(update.scan_id as i64), + ..Default::default() + }) + .exec(&*tx) + .await?; + } + } } if !update.removed_repositories.is_empty() { @@ -2417,6 +2486,25 @@ impl Database { }) .exec(&*tx) .await?; + + // Flip all status entries associated with a given repository_entry + worktree_repository_statuses::Entity::update_many() + .filter( + worktree_repository_statuses::Column::ProjectId + .eq(project_id) + .and(worktree_repository_statuses::Column::WorktreeId.eq(worktree_id)) + .and( + worktree_repository_statuses::Column::WorkDirectoryId + .is_in(update.removed_repositories.iter().map(|id| *id as i64)), + ), + ) + .set(worktree_repository_statuses::ActiveModel { + is_deleted: ActiveValue::Set(true), + scan_id: ActiveValue::Set(update.scan_id as i64), + ..Default::default() + }) + .exec(&*tx) + .await?; } let connection_ids = self.project_guest_connection_ids(project_id, &tx).await?; @@ -2647,12 +2735,44 @@ impl Database { if let Some(worktree) = worktrees.get_mut(&(db_repository_entry.worktree_id as u64)) { - worktree.repository_entries.push(proto::RepositoryEntry { - work_directory_id: db_repository_entry.work_directory_id as u64, - branch: db_repository_entry.branch, - removed_worktree_repo_paths: Default::default(), - updated_worktree_statuses: Default::default(), - }); + worktree.repository_entries.insert( + db_repository_entry.work_directory_id as u64, + proto::RepositoryEntry { + work_directory_id: db_repository_entry.work_directory_id as u64, + branch: db_repository_entry.branch, + removed_worktree_repo_paths: Default::default(), + updated_worktree_statuses: Default::default(), + }, + ); + } + } + } + + { + let mut db_status_entries = worktree_repository_statuses::Entity::find() + .filter( + Condition::all() + .add(worktree_repository_statuses::Column::ProjectId.eq(project_id)) + .add(worktree_repository_statuses::Column::IsDeleted.eq(false)), + ) + .stream(&*tx) + .await?; + + while let Some(db_status_entry) = db_status_entries.next().await { + let db_status_entry = db_status_entry?; + if let Some(worktree) = worktrees.get_mut(&(db_status_entry.worktree_id as u64)) + { + if let Some(repository_entry) = worktree + .repository_entries + .get_mut(&(db_status_entry.work_directory_id as u64)) + { + repository_entry + .updated_worktree_statuses + .push(proto::StatusEntry { + repo_path: db_status_entry.repo_path, + status: db_status_entry.status as i32, + }); + } } } } @@ -3394,7 +3514,7 @@ pub struct Worktree { pub root_name: String, pub visible: bool, pub entries: Vec, - pub repository_entries: Vec, + pub repository_entries: BTreeMap, pub diagnostic_summaries: Vec, pub scan_id: u64, pub completed_scan_id: u64, diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 23935904d3..001f3462d0 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -1385,7 +1385,7 @@ async fn join_project( removed_entries: Default::default(), scan_id: worktree.scan_id, is_last_update: worktree.scan_id == worktree.completed_scan_id, - updated_repositories: worktree.repository_entries, + updated_repositories: worktree.repository_entries.into_values().collect(), removed_repositories: Default::default(), }; for update in proto::split_worktree_update(message, MAX_CHUNK_SIZE) { diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 7dd8f86b8e..aefc172268 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -2820,6 +2820,8 @@ async fn test_git_status_sync( // And synchronization while joining let project_remote_c = client_c.build_remote_project(project_id, cx_c).await; + deterministic.run_until_parked(); + project_remote_c.read_with(cx_c, |project, cx| { assert_status( &Path::new(A_TXT), From c7166fde3bcb88162862411d6d9e9390fe8c17a4 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 10 May 2023 17:38:29 -0700 Subject: [PATCH 18/34] Bump protocol version --- crates/rpc/src/rpc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/rpc/src/rpc.rs b/crates/rpc/src/rpc.rs index e51ded5969..64fbf19462 100644 --- a/crates/rpc/src/rpc.rs +++ b/crates/rpc/src/rpc.rs @@ -6,4 +6,4 @@ pub use conn::Connection; pub use peer::*; mod macros; -pub const PROTOCOL_VERSION: u32 = 54; +pub const PROTOCOL_VERSION: u32 = 55; From 18becabfa593ec2da7735c6c7edec75ff18c6445 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 10 May 2023 17:50:30 -0700 Subject: [PATCH 19/34] Add postgres migration --- ...20230511004019_add_repository_statuses.sql | 15 ++++++++++++ .../src/db/worktree_repository_statuses.rs | 23 +++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 crates/collab/migrations/20230511004019_add_repository_statuses.sql create mode 100644 crates/collab/src/db/worktree_repository_statuses.rs diff --git a/crates/collab/migrations/20230511004019_add_repository_statuses.sql b/crates/collab/migrations/20230511004019_add_repository_statuses.sql new file mode 100644 index 0000000000..862561c686 --- /dev/null +++ b/crates/collab/migrations/20230511004019_add_repository_statuses.sql @@ -0,0 +1,15 @@ +CREATE TABLE "worktree_repository_statuses" ( + "project_id" INTEGER NOT NULL, + "worktree_id" INT8 NOT NULL, + "work_directory_id" INT8 NOT NULL, + "repo_path" VARCHAR NOT NULL, + "status" INT8 NOT NULL, + "scan_id" INT8 NOT NULL, + "is_deleted" BOOL NOT NULL, + PRIMARY KEY(project_id, worktree_id, work_directory_id, repo_path), + FOREIGN KEY(project_id, worktree_id) REFERENCES worktrees (project_id, id) ON DELETE CASCADE, + FOREIGN KEY(project_id, worktree_id, work_directory_id) REFERENCES worktree_entries (project_id, worktree_id, id) ON DELETE CASCADE +); +CREATE INDEX "index_wt_repos_statuses_on_project_id" ON "worktree_repository_statuses" ("project_id"); +CREATE INDEX "index_wt_repos_statuses_on_project_id_and_wt_id" ON "worktree_repository_statuses" ("project_id", "worktree_id"); +CREATE INDEX "index_wt_repos_statuses_on_project_id_and_wt_id_and_wd_id" ON "worktree_repository_statuses" ("project_id", "worktree_id", "work_directory_id"); diff --git a/crates/collab/src/db/worktree_repository_statuses.rs b/crates/collab/src/db/worktree_repository_statuses.rs new file mode 100644 index 0000000000..fc15efc816 --- /dev/null +++ b/crates/collab/src/db/worktree_repository_statuses.rs @@ -0,0 +1,23 @@ +use super::ProjectId; +use sea_orm::entity::prelude::*; + +#[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] +#[sea_orm(table_name = "worktree_repository_statuses")] +pub struct Model { + #[sea_orm(primary_key)] + pub project_id: ProjectId, + #[sea_orm(primary_key)] + pub worktree_id: i64, + #[sea_orm(primary_key)] + pub work_directory_id: i64, + #[sea_orm(primary_key)] + pub repo_path: String, + pub status: i64, + pub scan_id: i64, + pub is_deleted: bool, +} + +#[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] +pub enum Relation {} + +impl ActiveModelBehavior for ActiveModel {} From f55ca7ae3c65d18581c36ba4515ad6f725e88477 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 10 May 2023 17:52:23 -0700 Subject: [PATCH 20/34] Fix incorrect import --- crates/fs/src/fs.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 09ddce2ffa..3285eb328a 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -7,7 +7,7 @@ use git2::Repository as LibGitRepository; use lazy_static::lazy_static; use parking_lot::Mutex; use regex::Regex; -use repository::{GitFileStatus, GitRepository}; +use repository::GitRepository; use rope::Rope; use smol::io::{AsyncReadExt, AsyncWriteExt}; use std::borrow::Cow; @@ -27,7 +27,7 @@ use util::ResultExt; #[cfg(any(test, feature = "test-support"))] use collections::{btree_map, BTreeMap}; #[cfg(any(test, feature = "test-support"))] -use repository::FakeGitRepositoryState; +use repository::{FakeGitRepositoryState, GitFileStatus}; #[cfg(any(test, feature = "test-support"))] use std::sync::Weak; From 9800a149a61480cd3464acc373c3e48a728e465b Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 10 May 2023 17:59:33 -0700 Subject: [PATCH 21/34] Remove some external context from git status test --- crates/project/src/worktree.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 0d4a02775d..a970067230 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -3556,6 +3556,7 @@ impl<'a> TryFrom<(&'a CharBag, proto::Entry)> for Entry { mod tests { use super::*; use fs::{FakeFs, RealFs}; + use git2::Signature; use gpui::{executor::Deterministic, TestAppContext}; use pretty_assertions::assert_eq; use rand::prelude::*; @@ -3894,7 +3895,7 @@ mod tests { #[track_caller] fn git_commit(msg: &'static str, repo: &git2::Repository) { - let signature = repo.signature().unwrap(); + let signature = Signature::now("test", "test@zed.dev").unwrap(); let oid = repo.index().unwrap().write_tree().unwrap(); let tree = repo.find_tree(oid).unwrap(); if let Some(head) = repo.head().ok() { From fca3bb3b93a38119771e04272d67c60f3e062783 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 10 May 2023 19:21:27 -0700 Subject: [PATCH 22/34] Add randomized test for git statuses --- crates/collab/src/db.rs | 39 +++ .../src/tests/randomized_integration_tests.rs | 259 +++++++++++------- crates/fs/src/repository.rs | 3 +- 3 files changed, 207 insertions(+), 94 deletions(-) diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index cc85d4f369..e881121758 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -1576,6 +1576,45 @@ impl Database { } } + // Repository Status Entries + for repository in worktree.updated_repositories.iter_mut() { + let repository_status_entry_filter = + if let Some(rejoined_worktree) = rejoined_worktree { + worktree_repository_statuses::Column::ScanId + .gt(rejoined_worktree.scan_id) + } else { + worktree_repository_statuses::Column::IsDeleted.eq(false) + }; + + let mut db_repository_statuses = + worktree_repository_statuses::Entity::find() + .filter( + Condition::all() + .add( + worktree_repository_statuses::Column::WorktreeId + .eq(worktree.id), + ) + .add( + worktree_repository_statuses::Column::WorkDirectoryId + .eq(repository.work_directory_id), + ) + .add(repository_status_entry_filter), + ) + .stream(&*tx) + .await?; + + while let Some(db_status_entry) = db_repository_statuses.next().await { + let db_status_entry = db_status_entry?; + if db_status_entry.is_deleted { + repository.removed_worktree_repo_paths.push(db_status_entry.repo_path); + } else { + repository.updated_worktree_statuses.push(proto::StatusEntry { + repo_path: db_status_entry.repo_path, status: db_status_entry.status as i32 + }); + } + } + } + worktrees.push(worktree); } diff --git a/crates/collab/src/tests/randomized_integration_tests.rs b/crates/collab/src/tests/randomized_integration_tests.rs index c4326be101..d5ed47675a 100644 --- a/crates/collab/src/tests/randomized_integration_tests.rs +++ b/crates/collab/src/tests/randomized_integration_tests.rs @@ -8,7 +8,7 @@ use call::ActiveCall; use client::RECEIVE_TIMEOUT; use collections::BTreeMap; use editor::Bias; -use fs::{FakeFs, Fs as _}; +use fs::{repository::GitFileStatus, FakeFs, Fs as _}; use futures::StreamExt as _; use gpui::{executor::Deterministic, ModelHandle, Task, TestAppContext}; use language::{range_to_lsp, FakeLspAdapter, Language, LanguageConfig, PointUtf16}; @@ -32,6 +32,7 @@ use std::{ }, }; use util::ResultExt; +use pretty_assertions::assert_eq; lazy_static::lazy_static! { static ref PLAN_LOAD_PATH: Option = path_env_var("LOAD_PLAN"); @@ -763,53 +764,81 @@ async fn apply_client_operation( } } - ClientOperation::WriteGitIndex { - repo_path, - contents, - } => { - if !client.fs.directories().contains(&repo_path) { - return Err(TestError::Inapplicable); - } - - log::info!( - "{}: writing git index for repo {:?}: {:?}", - client.username, + ClientOperation::GitOperation { operation } => match operation { + GitOperation::WriteGitIndex { repo_path, - contents - ); + contents, + } => { + if !client.fs.directories().contains(&repo_path) { + return Err(TestError::Inapplicable); + } - let dot_git_dir = repo_path.join(".git"); - let contents = contents - .iter() - .map(|(path, contents)| (path.as_path(), contents.clone())) - .collect::>(); - if client.fs.metadata(&dot_git_dir).await?.is_none() { - client.fs.create_dir(&dot_git_dir).await?; + log::info!( + "{}: writing git index for repo {:?}: {:?}", + client.username, + repo_path, + contents + ); + + let dot_git_dir = repo_path.join(".git"); + let contents = contents + .iter() + .map(|(path, contents)| (path.as_path(), contents.clone())) + .collect::>(); + if client.fs.metadata(&dot_git_dir).await?.is_none() { + client.fs.create_dir(&dot_git_dir).await?; + } + client.fs.set_index_for_repo(&dot_git_dir, &contents).await; } - client.fs.set_index_for_repo(&dot_git_dir, &contents).await; - } - - ClientOperation::WriteGitBranch { - repo_path, - new_branch, - } => { - if !client.fs.directories().contains(&repo_path) { - return Err(TestError::Inapplicable); - } - - log::info!( - "{}: writing git branch for repo {:?}: {:?}", - client.username, + GitOperation::WriteGitBranch { repo_path, - new_branch - ); + new_branch, + } => { + if !client.fs.directories().contains(&repo_path) { + return Err(TestError::Inapplicable); + } - let dot_git_dir = repo_path.join(".git"); - if client.fs.metadata(&dot_git_dir).await?.is_none() { - client.fs.create_dir(&dot_git_dir).await?; + log::info!( + "{}: writing git branch for repo {:?}: {:?}", + client.username, + repo_path, + new_branch + ); + + let dot_git_dir = repo_path.join(".git"); + if client.fs.metadata(&dot_git_dir).await?.is_none() { + client.fs.create_dir(&dot_git_dir).await?; + } + client.fs.set_branch_name(&dot_git_dir, new_branch).await; } - client.fs.set_branch_name(&dot_git_dir, new_branch).await; - } + GitOperation::WriteGitStatuses { + repo_path, + statuses, + } => { + if !client.fs.directories().contains(&repo_path) { + return Err(TestError::Inapplicable); + } + + log::info!( + "{}: writing git statuses for repo {:?}: {:?}", + client.username, + repo_path, + statuses + ); + + let dot_git_dir = repo_path.join(".git"); + + let statuses = statuses.iter() + .map(|(path, val)| (path.as_path(), val.clone())) + .collect::>(); + + if client.fs.metadata(&dot_git_dir).await?.is_none() { + client.fs.create_dir(&dot_git_dir).await?; + } + + client.fs.set_status_for_repo(&dot_git_dir, statuses.as_slice()).await; + }, + }, } Ok(()) } @@ -1178,6 +1207,13 @@ enum ClientOperation { is_dir: bool, content: String, }, + GitOperation { + operation: GitOperation, + }, +} + +#[derive(Clone, Debug, Serialize, Deserialize)] +enum GitOperation { WriteGitIndex { repo_path: PathBuf, contents: Vec<(PathBuf, String)>, @@ -1186,6 +1222,10 @@ enum ClientOperation { repo_path: PathBuf, new_branch: Option, }, + WriteGitStatuses { + repo_path: PathBuf, + statuses: Vec<(PathBuf, GitFileStatus)>, + }, } #[derive(Clone, Debug, Serialize, Deserialize)] @@ -1698,57 +1738,10 @@ impl TestPlan { } } - // Update a git index - 91..=93 => { - let repo_path = client - .fs - .directories() - .into_iter() - .choose(&mut self.rng) - .unwrap() - .clone(); - - let mut file_paths = client - .fs - .files() - .into_iter() - .filter(|path| path.starts_with(&repo_path)) - .collect::>(); - let count = self.rng.gen_range(0..=file_paths.len()); - file_paths.shuffle(&mut self.rng); - file_paths.truncate(count); - - let mut contents = Vec::new(); - for abs_child_file_path in &file_paths { - let child_file_path = abs_child_file_path - .strip_prefix(&repo_path) - .unwrap() - .to_path_buf(); - let new_base = Alphanumeric.sample_string(&mut self.rng, 16); - contents.push((child_file_path, new_base)); - } - - break ClientOperation::WriteGitIndex { - repo_path, - contents, - }; - } - - // Update a git branch - 94..=95 => { - let repo_path = client - .fs - .directories() - .choose(&mut self.rng) - .unwrap() - .clone(); - - let new_branch = (self.rng.gen_range(0..10) > 3) - .then(|| Alphanumeric.sample_string(&mut self.rng, 8)); - - break ClientOperation::WriteGitBranch { - repo_path, - new_branch, + // Update a git related action + 91..=95 => { + break ClientOperation::GitOperation { + operation: self.generate_git_operation(client), }; } @@ -1786,6 +1779,86 @@ impl TestPlan { }) } + fn generate_git_operation(&mut self, client: &TestClient) -> GitOperation { + fn generate_file_paths( + repo_path: &Path, + rng: &mut StdRng, + client: &TestClient, + ) -> Vec { + let mut paths = client + .fs + .files() + .into_iter() + .filter(|path| path.starts_with(repo_path)) + .collect::>(); + + let count = rng.gen_range(0..=paths.len()); + paths.shuffle(rng); + paths.truncate(count); + + paths + .iter() + .map(|path| path.strip_prefix(repo_path).unwrap().to_path_buf()) + .collect::>() + } + + let repo_path = client + .fs + .directories() + .choose(&mut self.rng) + .unwrap() + .clone(); + + match self.rng.gen_range(0..100_u32) { + 0..=25 => { + let file_paths = generate_file_paths(&repo_path, &mut self.rng, client); + + let contents = file_paths + .into_iter() + .map(|path| (path, Alphanumeric.sample_string(&mut self.rng, 16))) + .collect(); + + GitOperation::WriteGitIndex { + repo_path, + contents, + } + } + 26..=63 => { + let new_branch = (self.rng.gen_range(0..10) > 3) + .then(|| Alphanumeric.sample_string(&mut self.rng, 8)); + + GitOperation::WriteGitBranch { + repo_path, + new_branch, + } + } + 64..=100 => { + let file_paths = generate_file_paths(&repo_path, &mut self.rng, client); + + let statuses = file_paths + .into_iter() + .map(|paths| { + ( + paths, + match self.rng.gen_range(0..3_u32) { + 0 => GitFileStatus::Added, + 1 => GitFileStatus::Modified, + 2 => GitFileStatus::Conflict, + _ => unreachable!(), + }, + ) + }) + .collect::>(); + + GitOperation::WriteGitStatuses { + repo_path, + statuses, + } + } + _ => unreachable!(), + } + } + fn next_root_dir_name(&mut self, user_id: UserId) -> String { let user_ix = self .users diff --git a/crates/fs/src/repository.rs b/crates/fs/src/repository.rs index 90b3761677..6bf0a43230 100644 --- a/crates/fs/src/repository.rs +++ b/crates/fs/src/repository.rs @@ -1,6 +1,7 @@ use anyhow::Result; use collections::HashMap; use parking_lot::Mutex; +use serde_derive::{Serialize, Deserialize}; use std::{ ffi::OsStr, os::unix::prelude::OsStrExt, @@ -183,7 +184,7 @@ fn check_path_to_repo_path_errors(relative_file_path: &Path) -> Result<()> { } } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub enum GitFileStatus { Added, Modified, From f5c633e80cc22583b7e72369f29388982170f19d Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 10 May 2023 19:54:02 -0700 Subject: [PATCH 23/34] Fixed bug in status deletion marking --- crates/collab/src/db.rs | 32 -------------------------------- 1 file changed, 32 deletions(-) diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index e881121758..cac84a0ccc 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -2472,38 +2472,6 @@ impl Database { .exec(&*tx) .await?; } - - if !repository.removed_worktree_repo_paths.is_empty() { - worktree_repository_statuses::Entity::update_many() - .filter( - worktree_repository_statuses::Column::ProjectId - .eq(project_id) - .and( - worktree_repository_statuses::Column::WorktreeId - .eq(worktree_id), - ) - .and( - worktree_repository_statuses::Column::WorkDirectoryId - .eq(repository.work_directory_id), - ) - .and( - worktree_repository_statuses::Column::RepoPath.is_in( - repository - .removed_worktree_repo_paths - .iter() - .cloned() - .collect::>(), - ), - ), - ) - .set(worktree_repository_statuses::ActiveModel { - is_deleted: ActiveValue::Set(true), - scan_id: ActiveValue::Set(update.scan_id as i64), - ..Default::default() - }) - .exec(&*tx) - .await?; - } } } From adfbbf21b2267eef6e7f6deeb39731e8e7133e27 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 10 May 2023 20:09:37 -0700 Subject: [PATCH 24/34] fmt --- crates/collab/src/db.rs | 13 +++++++++---- .../src/tests/randomized_integration_tests.rs | 12 ++++++++---- crates/fs/src/repository.rs | 2 +- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index cac84a0ccc..b95bb49b4e 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -1606,11 +1606,16 @@ impl Database { while let Some(db_status_entry) = db_repository_statuses.next().await { let db_status_entry = db_status_entry?; if db_status_entry.is_deleted { - repository.removed_worktree_repo_paths.push(db_status_entry.repo_path); + repository + .removed_worktree_repo_paths + .push(db_status_entry.repo_path); } else { - repository.updated_worktree_statuses.push(proto::StatusEntry { - repo_path: db_status_entry.repo_path, status: db_status_entry.status as i32 - }); + repository + .updated_worktree_statuses + .push(proto::StatusEntry { + repo_path: db_status_entry.repo_path, + status: db_status_entry.status as i32, + }); } } } diff --git a/crates/collab/src/tests/randomized_integration_tests.rs b/crates/collab/src/tests/randomized_integration_tests.rs index d5ed47675a..fe4b6190ed 100644 --- a/crates/collab/src/tests/randomized_integration_tests.rs +++ b/crates/collab/src/tests/randomized_integration_tests.rs @@ -14,6 +14,7 @@ use gpui::{executor::Deterministic, ModelHandle, Task, TestAppContext}; use language::{range_to_lsp, FakeLspAdapter, Language, LanguageConfig, PointUtf16}; use lsp::FakeLanguageServer; use parking_lot::Mutex; +use pretty_assertions::assert_eq; use project::{search::SearchQuery, Project, ProjectPath}; use rand::{ distributions::{Alphanumeric, DistString}, @@ -32,7 +33,6 @@ use std::{ }, }; use util::ResultExt; -use pretty_assertions::assert_eq; lazy_static::lazy_static! { static ref PLAN_LOAD_PATH: Option = path_env_var("LOAD_PLAN"); @@ -828,7 +828,8 @@ async fn apply_client_operation( let dot_git_dir = repo_path.join(".git"); - let statuses = statuses.iter() + let statuses = statuses + .iter() .map(|(path, val)| (path.as_path(), val.clone())) .collect::>(); @@ -836,8 +837,11 @@ async fn apply_client_operation( client.fs.create_dir(&dot_git_dir).await?; } - client.fs.set_status_for_repo(&dot_git_dir, statuses.as_slice()).await; - }, + client + .fs + .set_status_for_repo(&dot_git_dir, statuses.as_slice()) + .await; + } }, } Ok(()) diff --git a/crates/fs/src/repository.rs b/crates/fs/src/repository.rs index 6bf0a43230..2fe31f5569 100644 --- a/crates/fs/src/repository.rs +++ b/crates/fs/src/repository.rs @@ -1,7 +1,7 @@ use anyhow::Result; use collections::HashMap; use parking_lot::Mutex; -use serde_derive::{Serialize, Deserialize}; +use serde_derive::{Deserialize, Serialize}; use std::{ ffi::OsStr, os::unix::prelude::OsStrExt, From 191ac86f0926d317ff77498558ecb95e98e4da9f Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Thu, 11 May 2023 09:24:36 -0700 Subject: [PATCH 25/34] Remove the CORRECT, overly agressive deletion codepath --- crates/collab/src/db.rs | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index b95bb49b4e..69b561c054 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -2477,6 +2477,26 @@ impl Database { .exec(&*tx) .await?; } + if !repository.removed_worktree_repo_paths.is_empty() { + worktree_repository_statuses::Entity::update_many() + .filter( + worktree_repository_statuses::Column::ProjectId + .eq(project_id) + .and(worktree_repository_statuses::Column::WorktreeId.eq(worktree_id)) + .and(worktree_repository_statuses::Column::WorkDirectoryId.eq(repository.work_directory_id as i64)) + .and( + worktree_repository_statuses::Column::RepoPath + .is_in(repository.removed_worktree_repo_paths.iter().map(String::as_str)), + ), + ) + .set(worktree_repository_statuses::ActiveModel { + is_deleted: ActiveValue::Set(true), + scan_id: ActiveValue::Set(update.scan_id as i64), + ..Default::default() + }) + .exec(&*tx) + .await?; + } } } @@ -2498,25 +2518,6 @@ impl Database { }) .exec(&*tx) .await?; - - // Flip all status entries associated with a given repository_entry - worktree_repository_statuses::Entity::update_many() - .filter( - worktree_repository_statuses::Column::ProjectId - .eq(project_id) - .and(worktree_repository_statuses::Column::WorktreeId.eq(worktree_id)) - .and( - worktree_repository_statuses::Column::WorkDirectoryId - .is_in(update.removed_repositories.iter().map(|id| *id as i64)), - ), - ) - .set(worktree_repository_statuses::ActiveModel { - is_deleted: ActiveValue::Set(true), - scan_id: ActiveValue::Set(update.scan_id as i64), - ..Default::default() - }) - .exec(&*tx) - .await?; } let connection_ids = self.project_guest_connection_ids(project_id, &tx).await?; From 5accf7cf4e5c745ca4cff0bf40cc94a37aecd472 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Thu, 11 May 2023 10:21:25 -0700 Subject: [PATCH 26/34] Update is_deleted when sending new repositories --- crates/collab/src/db.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 69b561c054..f5175a16a9 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -2471,12 +2471,14 @@ impl Database { .update_columns([ worktree_repository_statuses::Column::ScanId, worktree_repository_statuses::Column::Status, + worktree_repository_statuses::Column::IsDeleted, ]) .to_owned(), ) .exec(&*tx) .await?; } + if !repository.removed_worktree_repo_paths.is_empty() { worktree_repository_statuses::Entity::update_many() .filter( From 5b2ee63f80d8b83583713b65e8bfe2ee5f0589b2 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Thu, 11 May 2023 12:01:42 -0700 Subject: [PATCH 27/34] Added status trickle up --- Cargo.lock | 7 ++++ crates/collab/src/tests/integration_tests.rs | 2 +- crates/fs/src/repository.rs | 2 +- crates/project/src/worktree.rs | 28 ++++++++++++- crates/project_panel/src/project_panel.rs | 10 +++-- crates/sum_tree/src/tree_map.rs | 44 +++++++++++++++++++- crates/util/Cargo.toml | 1 + crates/util/src/util.rs | 2 + 8 files changed, 88 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0190b4d8f5..81e4a4e025 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6537,6 +6537,12 @@ dependencies = [ "winx", ] +[[package]] +name = "take-until" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8bdb6fa0dfa67b38c1e66b7041ba9dcf23b99d8121907cd31c807a332f7a0bbb" + [[package]] name = "target-lexicon" version = "0.12.5" @@ -7596,6 +7602,7 @@ dependencies = [ "serde", "serde_json", "smol", + "take-until", "tempdir", "url", ] diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index aefc172268..47455c0a70 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -2760,7 +2760,7 @@ async fn test_git_status_sync( let worktree = worktrees[0].clone(); let snapshot = worktree.read(cx).snapshot(); let root_entry = snapshot.root_git_entry().unwrap(); - assert_eq!(root_entry.status_for(&snapshot, file), status); + assert_eq!(root_entry.status_for_file(&snapshot, file), status); } // Smoke test status reading diff --git a/crates/fs/src/repository.rs b/crates/fs/src/repository.rs index 2fe31f5569..13f55b9c94 100644 --- a/crates/fs/src/repository.rs +++ b/crates/fs/src/repository.rs @@ -184,7 +184,7 @@ fn check_path_to_repo_path_errors(relative_file_path: &Path) -> Result<()> { } } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] pub enum GitFileStatus { Added, Modified, diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index a970067230..07302b4e2e 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -55,7 +55,7 @@ use std::{ time::{Duration, SystemTime}, }; use sum_tree::{Bias, Edit, SeekTarget, SumTree, TreeMap, TreeSet}; -use util::{paths::HOME, ResultExt, TryFutureExt}; +use util::{paths::HOME, ResultExt, TakeUntilExt, TryFutureExt}; #[derive(Copy, Clone, PartialEq, Eq, Debug, Hash, PartialOrd, Ord)] pub struct WorktreeId(usize); @@ -173,13 +173,37 @@ impl RepositoryEntry { self.work_directory.contains(snapshot, path) } - pub fn status_for(&self, snapshot: &Snapshot, path: &Path) -> Option { + pub fn status_for_file(&self, snapshot: &Snapshot, path: &Path) -> Option { self.work_directory .relativize(snapshot, path) .and_then(|repo_path| self.worktree_statuses.get(&repo_path)) .cloned() } + pub fn status_for_path(&self, snapshot: &Snapshot, path: &Path) -> Option { + self.work_directory + .relativize(snapshot, path) + .and_then(|repo_path| { + self.worktree_statuses + .get_from_while(&repo_path, |repo_path, key, _| key.starts_with(repo_path)) + .map(|(_, status)| status) + // Short circut once we've found the highest level + .take_until(|status| status == &&GitFileStatus::Conflict) + .reduce( + |status_first, status_second| match (status_first, status_second) { + (GitFileStatus::Conflict, _) | (_, GitFileStatus::Conflict) => { + &GitFileStatus::Conflict + } + (GitFileStatus::Added, _) | (_, GitFileStatus::Added) => { + &GitFileStatus::Added + } + _ => &GitFileStatus::Modified, + }, + ) + .copied() + }) + } + pub fn build_update(&self, other: &Self) -> proto::RepositoryEntry { let mut updated_statuses: Vec = Vec::new(); let mut removed_statuses: Vec = Vec::new(); diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 49741ea49f..1066875022 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -1013,9 +1013,13 @@ impl ProjectPanel { let entry_range = range.start.saturating_sub(ix)..end_ix - ix; for entry in &visible_worktree_entries[entry_range] { let path = &entry.path; - let status = snapshot - .repo_for(path) - .and_then(|entry| entry.status_for(&snapshot, path)); + let status = (entry.path.parent().is_some() && !entry.is_ignored) + .then(|| { + snapshot + .repo_for(path) + .and_then(|entry| entry.status_for_path(&snapshot, path)) + }) + .flatten(); let mut details = EntryDetails { filename: entry diff --git a/crates/sum_tree/src/tree_map.rs b/crates/sum_tree/src/tree_map.rs index 3942d00b29..e59b05f00f 100644 --- a/crates/sum_tree/src/tree_map.rs +++ b/crates/sum_tree/src/tree_map.rs @@ -1,4 +1,4 @@ -use std::{cmp::Ordering, fmt::Debug}; +use std::{cmp::Ordering, fmt::Debug, iter}; use crate::{Bias, Dimension, Edit, Item, KeyedItem, SeekTarget, SumTree, Summary}; @@ -111,6 +111,26 @@ impl TreeMap { self.0 = new_tree; } + + pub fn get_from_while<'tree, F>(&'tree self, from: &'tree K, mut f: F) -> impl Iterator + '_ + where + F: FnMut(&K, &K, &V) -> bool + 'tree, + { + let mut cursor = self.0.cursor::>(); + let from_key = MapKeyRef(Some(from)); + cursor.seek(&from_key, Bias::Left, &()); + + iter::from_fn(move || { + let result = cursor.item().and_then(|item| { + (f(from, &item.key, &item.value)) + .then(|| (&item.key, &item.value)) + }); + cursor.next(&()); + result + }) + } + + pub fn update(&mut self, key: &K, f: F) -> Option where F: FnOnce(&mut V) -> T, @@ -354,6 +374,28 @@ mod tests { assert_eq!(map.get(&"c"), Some(&5)); } + #[test] + fn test_get_from_while() { + let mut map = TreeMap::default(); + + map.insert("a", 1); + map.insert("b", 2); + map.insert("baa", 3); + map.insert("baaab", 4); + map.insert("c", 5); + + let result = map.get_from_while(&"ba", |key, _| key.starts_with(&"ba")).collect::>(); + + assert_eq!(result.len(), 2); + assert!(result.iter().find(|(k, _)| k == &&"baa").is_some()); + assert!(result.iter().find(|(k, _)| k == &&"baaab").is_some()); + + let result = map.get_from_while(&"c", |key, _| key.starts_with(&"c")).collect::>(); + + assert_eq!(result.len(), 1); + assert!(result.iter().find(|(k, _)| k == &&"c").is_some()); + } + #[test] fn test_insert_tree() { let mut map = TreeMap::default(); diff --git a/crates/util/Cargo.toml b/crates/util/Cargo.toml index 319d815d17..4ec8f7553c 100644 --- a/crates/util/Cargo.toml +++ b/crates/util/Cargo.toml @@ -26,6 +26,7 @@ serde.workspace = true serde_json.workspace = true git2 = { version = "0.15", default-features = false, optional = true } dirs = "3.0" +take-until = "0.2.0" [dev-dependencies] tempdir.workspace = true diff --git a/crates/util/src/util.rs b/crates/util/src/util.rs index 903b0eec59..63b2d5f279 100644 --- a/crates/util/src/util.rs +++ b/crates/util/src/util.rs @@ -17,6 +17,8 @@ pub use backtrace::Backtrace; use futures::Future; use rand::{seq::SliceRandom, Rng}; +pub use take_until::*; + #[macro_export] macro_rules! debug_panic { ( $($fmt_arg:tt)* ) => { From dfb6a2f7fca54d22b42ded92dc9fc1fca4cbfcdc Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Thu, 11 May 2023 12:02:25 -0700 Subject: [PATCH 28/34] fmt --- crates/collab/src/db.rs | 18 ++++++++++--- crates/sum_tree/src/tree_map.rs | 45 ++++++++++++++++++--------------- 2 files changed, 39 insertions(+), 24 deletions(-) diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index f5175a16a9..1047b207b9 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -2484,11 +2484,21 @@ impl Database { .filter( worktree_repository_statuses::Column::ProjectId .eq(project_id) - .and(worktree_repository_statuses::Column::WorktreeId.eq(worktree_id)) - .and(worktree_repository_statuses::Column::WorkDirectoryId.eq(repository.work_directory_id as i64)) .and( - worktree_repository_statuses::Column::RepoPath - .is_in(repository.removed_worktree_repo_paths.iter().map(String::as_str)), + worktree_repository_statuses::Column::WorktreeId + .eq(worktree_id), + ) + .and( + worktree_repository_statuses::Column::WorkDirectoryId + .eq(repository.work_directory_id as i64), + ) + .and( + worktree_repository_statuses::Column::RepoPath.is_in( + repository + .removed_worktree_repo_paths + .iter() + .map(String::as_str), + ), ), ) .set(worktree_repository_statuses::ActiveModel { diff --git a/crates/sum_tree/src/tree_map.rs b/crates/sum_tree/src/tree_map.rs index e59b05f00f..b18af3633a 100644 --- a/crates/sum_tree/src/tree_map.rs +++ b/crates/sum_tree/src/tree_map.rs @@ -111,25 +111,26 @@ impl TreeMap { self.0 = new_tree; } + pub fn get_from_while<'tree, F>( + &'tree self, + from: &'tree K, + mut f: F, + ) -> impl Iterator + '_ + where + F: FnMut(&K, &K, &V) -> bool + 'tree, + { + let mut cursor = self.0.cursor::>(); + let from_key = MapKeyRef(Some(from)); + cursor.seek(&from_key, Bias::Left, &()); - pub fn get_from_while<'tree, F>(&'tree self, from: &'tree K, mut f: F) -> impl Iterator + '_ - where - F: FnMut(&K, &K, &V) -> bool + 'tree, - { - let mut cursor = self.0.cursor::>(); - let from_key = MapKeyRef(Some(from)); - cursor.seek(&from_key, Bias::Left, &()); - - iter::from_fn(move || { - let result = cursor.item().and_then(|item| { - (f(from, &item.key, &item.value)) - .then(|| (&item.key, &item.value)) - }); - cursor.next(&()); - result - }) - } - + iter::from_fn(move || { + let result = cursor.item().and_then(|item| { + (f(from, &item.key, &item.value)).then(|| (&item.key, &item.value)) + }); + cursor.next(&()); + result + }) + } pub fn update(&mut self, key: &K, f: F) -> Option where @@ -384,13 +385,17 @@ mod tests { map.insert("baaab", 4); map.insert("c", 5); - let result = map.get_from_while(&"ba", |key, _| key.starts_with(&"ba")).collect::>(); + let result = map + .get_from_while(&"ba", |key, _| key.starts_with(&"ba")) + .collect::>(); assert_eq!(result.len(), 2); assert!(result.iter().find(|(k, _)| k == &&"baa").is_some()); assert!(result.iter().find(|(k, _)| k == &&"baaab").is_some()); - let result = map.get_from_while(&"c", |key, _| key.starts_with(&"c")).collect::>(); + let result = map + .get_from_while(&"c", |key, _| key.starts_with(&"c")) + .collect::>(); assert_eq!(result.len(), 1); assert!(result.iter().find(|(k, _)| k == &&"c").is_some()); From 1bb34e08bb14a48d7a06efa6490568b74ca30af6 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Thu, 11 May 2023 12:03:39 -0700 Subject: [PATCH 29/34] Fix test --- crates/sum_tree/src/tree_map.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/sum_tree/src/tree_map.rs b/crates/sum_tree/src/tree_map.rs index b18af3633a..bfc8db5bf6 100644 --- a/crates/sum_tree/src/tree_map.rs +++ b/crates/sum_tree/src/tree_map.rs @@ -386,7 +386,7 @@ mod tests { map.insert("c", 5); let result = map - .get_from_while(&"ba", |key, _| key.starts_with(&"ba")) + .get_from_while(&"ba", |_, key, _| key.starts_with(&"ba")) .collect::>(); assert_eq!(result.len(), 2); @@ -394,7 +394,7 @@ mod tests { assert!(result.iter().find(|(k, _)| k == &&"baaab").is_some()); let result = map - .get_from_while(&"c", |key, _| key.starts_with(&"c")) + .get_from_while(&"c", |_, key, _| key.starts_with(&"c")) .collect::>(); assert_eq!(result.len(), 1); From 6f87f9c51f0d8f3602d6e97113da1192b5712eb2 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Thu, 11 May 2023 13:25:07 -0700 Subject: [PATCH 30/34] Don't scan for statuses in files that are ignored --- crates/project/src/worktree.rs | 59 ++++++++++++++++++++++++++++++---- 1 file changed, 52 insertions(+), 7 deletions(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 07302b4e2e..8ce087e9d9 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -3065,6 +3065,15 @@ impl BackgroundScanner { entry.worktree_statuses = statuses; }); } else { + if snapshot + .entry_for_path(&path) + .map(|entry| entry.is_ignored) + .unwrap_or(false) + { + self.remove_repo_path(&path, snapshot); + return None; + } + let repo = snapshot.repo_for(&path)?; let repo_path = repo.work_directory.relativize(&snapshot, &path)?; @@ -3580,7 +3589,6 @@ impl<'a> TryFrom<(&'a CharBag, proto::Entry)> for Entry { mod tests { use super::*; use fs::{FakeFs, RealFs}; - use git2::Signature; use gpui::{executor::Deterministic, TestAppContext}; use pretty_assertions::assert_eq; use rand::prelude::*; @@ -3919,6 +3927,8 @@ mod tests { #[track_caller] fn git_commit(msg: &'static str, repo: &git2::Repository) { + use git2::Signature; + let signature = Signature::now("test", "test@zed.dev").unwrap(); let oid = repo.index().unwrap().write_tree().unwrap(); let tree = repo.find_tree(oid).unwrap(); @@ -3944,7 +3954,9 @@ mod tests { #[track_caller] fn git_stash(repo: &mut git2::Repository) { - let signature = repo.signature().unwrap(); + use git2::Signature; + + let signature = Signature::now("test", "test@zed.dev").unwrap(); repo.stash_save(&signature, "N/A", None) .expect("Failed to stash"); } @@ -3976,6 +3988,8 @@ mod tests { .collect() } + const IGNORE_RULE: &'static str = "**/target"; + let root = temp_tree(json!({ "project": { "a.txt": "a", @@ -3984,7 +3998,12 @@ mod tests { "d": { "e.txt": "eee" } - } + }, + "f.txt": "ffff", + "target": { + "build_file": "???" + }, + ".gitignore": IGNORE_RULE }, })); @@ -4008,12 +4027,16 @@ mod tests { const A_TXT: &'static str = "a.txt"; const B_TXT: &'static str = "b.txt"; const E_TXT: &'static str = "c/d/e.txt"; + const F_TXT: &'static str = "f.txt"; + const DOTGITIGNORE: &'static str = ".gitignore"; + const BUILD_FILE: &'static str = "target/build_file"; let work_dir = root.path().join("project"); - let mut repo = git_init(work_dir.as_path()); + repo.add_ignore_rule(IGNORE_RULE).unwrap(); git_add(Path::new(A_TXT), &repo); git_add(Path::new(E_TXT), &repo); + git_add(Path::new(DOTGITIGNORE), &repo); git_commit("Initial commit", &repo); std::fs::write(work_dir.join(A_TXT), "aa").unwrap(); @@ -4027,7 +4050,7 @@ mod tests { let (dir, repo) = snapshot.repository_entries.iter().next().unwrap(); assert_eq!(dir.0.as_ref(), Path::new("project")); - assert_eq!(repo.worktree_statuses.iter().count(), 2); + assert_eq!(repo.worktree_statuses.iter().count(), 3); assert_eq!( repo.worktree_statuses.get(&Path::new(A_TXT).into()), Some(&GitFileStatus::Modified) @@ -4036,6 +4059,10 @@ mod tests { repo.worktree_statuses.get(&Path::new(B_TXT).into()), Some(&GitFileStatus::Added) ); + assert_eq!( + repo.worktree_statuses.get(&Path::new(F_TXT).into()), + Some(&GitFileStatus::Added) + ); }); git_add(Path::new(A_TXT), &repo); @@ -4048,15 +4075,20 @@ mod tests { let snapshot = tree.snapshot(); let (_, repo) = snapshot.repository_entries.iter().next().unwrap(); - assert_eq!(repo.worktree_statuses.iter().count(), 0); + assert_eq!(repo.worktree_statuses.iter().count(), 1); assert_eq!(repo.worktree_statuses.get(&Path::new(A_TXT).into()), None); assert_eq!(repo.worktree_statuses.get(&Path::new(B_TXT).into()), None); + assert_eq!( + repo.worktree_statuses.get(&Path::new(F_TXT).into()), + Some(&GitFileStatus::Added) + ); }); git_reset(0, &repo); git_remove_index(Path::new(B_TXT), &repo); git_stash(&mut repo); std::fs::write(work_dir.join(E_TXT), "eeee").unwrap(); + std::fs::write(work_dir.join(BUILD_FILE), "this should be ignored").unwrap(); tree.flush_fs_events(cx).await; // Check that more complex repo changes are tracked @@ -4064,7 +4096,7 @@ mod tests { let snapshot = tree.snapshot(); let (_, repo) = snapshot.repository_entries.iter().next().unwrap(); - assert_eq!(repo.worktree_statuses.iter().count(), 2); + assert_eq!(repo.worktree_statuses.iter().count(), 3); assert_eq!(repo.worktree_statuses.get(&Path::new(A_TXT).into()), None); assert_eq!( repo.worktree_statuses.get(&Path::new(B_TXT).into()), @@ -4074,22 +4106,35 @@ mod tests { repo.worktree_statuses.get(&Path::new(E_TXT).into()), Some(&GitFileStatus::Modified) ); + assert_eq!( + repo.worktree_statuses.get(&Path::new(F_TXT).into()), + Some(&GitFileStatus::Added) + ); }); std::fs::remove_file(work_dir.join(B_TXT)).unwrap(); std::fs::remove_dir_all(work_dir.join("c")).unwrap(); + std::fs::write(work_dir.join(DOTGITIGNORE), [IGNORE_RULE, "f.txt"].join("\n")).unwrap(); + + git_add(Path::new(DOTGITIGNORE), &repo); + git_commit("Committing modified git ignore", &repo); tree.flush_fs_events(cx).await; + dbg!(git_status(&repo)); + // Check that non-repo behavior is tracked tree.read_with(cx, |tree, _cx| { let snapshot = tree.snapshot(); let (_, repo) = snapshot.repository_entries.iter().next().unwrap(); + dbg!(&repo.worktree_statuses); + assert_eq!(repo.worktree_statuses.iter().count(), 0); assert_eq!(repo.worktree_statuses.get(&Path::new(A_TXT).into()), None); assert_eq!(repo.worktree_statuses.get(&Path::new(B_TXT).into()), None); assert_eq!(repo.worktree_statuses.get(&Path::new(E_TXT).into()), None); + assert_eq!(repo.worktree_statuses.get(&Path::new(F_TXT).into()), None); }); } From 72655fc41d154684b674f2d4cba9ff24c32641d6 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Thu, 11 May 2023 13:25:57 -0700 Subject: [PATCH 31/34] fmt --- crates/project/src/worktree.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 8ce087e9d9..2112bd92b1 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -4114,7 +4114,11 @@ mod tests { std::fs::remove_file(work_dir.join(B_TXT)).unwrap(); std::fs::remove_dir_all(work_dir.join("c")).unwrap(); - std::fs::write(work_dir.join(DOTGITIGNORE), [IGNORE_RULE, "f.txt"].join("\n")).unwrap(); + std::fs::write( + work_dir.join(DOTGITIGNORE), + [IGNORE_RULE, "f.txt"].join("\n"), + ) + .unwrap(); git_add(Path::new(DOTGITIGNORE), &repo); git_commit("Committing modified git ignore", &repo); From d538994c7f03618cfc1df60c54c01a7b166e8a1b Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Thu, 11 May 2023 16:06:25 -0700 Subject: [PATCH 32/34] Use more efficient sum tree traversals for removal and improve ergonomics with iter_from co-authored-by: Nathan --- crates/project/src/worktree.rs | 5 +- crates/sum_tree/src/tree_map.rs | 120 +++++++++++++++++++++----------- 2 files changed, 84 insertions(+), 41 deletions(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 2112bd92b1..305bcbbf16 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -185,7 +185,8 @@ impl RepositoryEntry { .relativize(snapshot, path) .and_then(|repo_path| { self.worktree_statuses - .get_from_while(&repo_path, |repo_path, key, _| key.starts_with(repo_path)) + .iter_from(&repo_path) + .take_while(|(key, _)| key.starts_with(&repo_path)) .map(|(_, status)| status) // Short circut once we've found the highest level .take_until(|status| status == &&GitFileStatus::Conflict) @@ -3022,7 +3023,7 @@ impl BackgroundScanner { snapshot.repository_entries.update(&work_dir, |entry| { entry .worktree_statuses - .remove_from_while(&repo_path, |stored_path, _| { + .remove_by(&repo_path, |stored_path, _| { stored_path.starts_with(&repo_path) }) }); diff --git a/crates/sum_tree/src/tree_map.rs b/crates/sum_tree/src/tree_map.rs index bfc8db5bf6..fdafdaeb3a 100644 --- a/crates/sum_tree/src/tree_map.rs +++ b/crates/sum_tree/src/tree_map.rs @@ -1,4 +1,4 @@ -use std::{cmp::Ordering, fmt::Debug, iter}; +use std::{cmp::Ordering, fmt::Debug}; use crate::{Bias, Dimension, Edit, Item, KeyedItem, SeekTarget, SumTree, Summary}; @@ -93,43 +93,14 @@ impl TreeMap { self.0 = new_tree; } - pub fn remove_from_while(&mut self, from: &K, mut f: F) - where - F: FnMut(&K, &V) -> bool, - { - let mut cursor = self.0.cursor::>(); - let from_key = MapKeyRef(Some(from)); - let mut new_tree = cursor.slice(&from_key, Bias::Left, &()); - while let Some(item) = cursor.item() { - if !f(&item.key, &item.value) { - break; - } - cursor.next(&()); - } - new_tree.push_tree(cursor.suffix(&()), &()); - drop(cursor); - self.0 = new_tree; - } - - pub fn get_from_while<'tree, F>( - &'tree self, - from: &'tree K, - mut f: F, - ) -> impl Iterator + '_ - where - F: FnMut(&K, &K, &V) -> bool + 'tree, - { + pub fn iter_from<'a>(&'a self, from: &'a K) -> impl Iterator + '_ { let mut cursor = self.0.cursor::>(); let from_key = MapKeyRef(Some(from)); cursor.seek(&from_key, Bias::Left, &()); - iter::from_fn(move || { - let result = cursor.item().and_then(|item| { - (f(from, &item.key, &item.value)).then(|| (&item.key, &item.value)) - }); - cursor.next(&()); - result - }) + cursor + .into_iter() + .map(|map_entry| (&map_entry.key, &map_entry.value)) } pub fn update(&mut self, key: &K, f: F) -> Option @@ -189,6 +160,51 @@ impl TreeMap { self.0.edit(edits, &()); } + + pub fn remove_by(&mut self, key: &K, f: F) + where + F: Fn(&K) -> bool, + { + let mut cursor = self.0.cursor::>(); + let key = MapKeyRef(Some(key)); + let mut new_tree = cursor.slice(&key, Bias::Left, &()); + let until = RemoveByTarget(key, &f); + cursor.seek_forward(&until, Bias::Right, &()); + new_tree.push_tree(cursor.suffix(&()), &()); + drop(cursor); + self.0 = new_tree; + } +} + +struct RemoveByTarget<'a, K>(MapKeyRef<'a, K>, &'a dyn Fn(&K) -> bool); + +impl<'a, K: Debug> Debug for RemoveByTarget<'a, K> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("RemoveByTarget") + .field("key", &self.0) + .field("F", &"<...>") + .finish() + } +} + +impl<'a, K: Debug + Clone + Default + Ord> SeekTarget<'a, MapKey, MapKeyRef<'a, K>> + for RemoveByTarget<'_, K> +{ + fn cmp( + &self, + cursor_location: &MapKeyRef<'a, K>, + _cx: & as Summary>::Context, + ) -> Ordering { + if let Some(cursor_location) = cursor_location.0 { + if (self.1)(cursor_location) { + Ordering::Equal + } else { + self.0 .0.unwrap().cmp(cursor_location) + } + } else { + Ordering::Greater + } + } } impl Default for TreeMap @@ -357,26 +373,50 @@ mod tests { } #[test] - fn test_remove_from_while() { + fn test_remove_by() { let mut map = TreeMap::default(); map.insert("a", 1); + map.insert("aa", 1); map.insert("b", 2); map.insert("baa", 3); map.insert("baaab", 4); map.insert("c", 5); + map.insert("ca", 6); - map.remove_from_while(&"ba", |key, _| key.starts_with(&"ba")); + map.remove_by(&"ba", |key| key.starts_with("ba")); assert_eq!(map.get(&"a"), Some(&1)); + assert_eq!(map.get(&"aa"), Some(&1)); assert_eq!(map.get(&"b"), Some(&2)); assert_eq!(map.get(&"baaa"), None); assert_eq!(map.get(&"baaaab"), None); assert_eq!(map.get(&"c"), Some(&5)); + assert_eq!(map.get(&"ca"), Some(&6)); + + + map.remove_by(&"c", |key| key.starts_with("c")); + + assert_eq!(map.get(&"a"), Some(&1)); + assert_eq!(map.get(&"aa"), Some(&1)); + assert_eq!(map.get(&"b"), Some(&2)); + assert_eq!(map.get(&"c"), None); + assert_eq!(map.get(&"ca"), None); + + map.remove_by(&"a", |key| key.starts_with("a")); + + assert_eq!(map.get(&"a"), None); + assert_eq!(map.get(&"aa"), None); + assert_eq!(map.get(&"b"), Some(&2)); + + map.remove_by(&"b", |key| key.starts_with("b")); + + assert_eq!(map.get(&"b"), None); + } #[test] - fn test_get_from_while() { + fn test_iter_from() { let mut map = TreeMap::default(); map.insert("a", 1); @@ -386,7 +426,8 @@ mod tests { map.insert("c", 5); let result = map - .get_from_while(&"ba", |_, key, _| key.starts_with(&"ba")) + .iter_from(&"ba") + .take_while(|(key, _)| key.starts_with(&"ba")) .collect::>(); assert_eq!(result.len(), 2); @@ -394,7 +435,8 @@ mod tests { assert!(result.iter().find(|(k, _)| k == &&"baaab").is_some()); let result = map - .get_from_while(&"c", |_, key, _| key.starts_with(&"c")) + .iter_from(&"c") + .take_while(|(key, _)| key.starts_with(&"c")) .collect::>(); assert_eq!(result.len(), 1); From d526fa6f1fb8d84958ee345178880fe649150113 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Thu, 11 May 2023 16:06:56 -0700 Subject: [PATCH 33/34] fmt --- crates/sum_tree/src/tree_map.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/sum_tree/src/tree_map.rs b/crates/sum_tree/src/tree_map.rs index fdafdaeb3a..509a79ec47 100644 --- a/crates/sum_tree/src/tree_map.rs +++ b/crates/sum_tree/src/tree_map.rs @@ -394,7 +394,6 @@ mod tests { assert_eq!(map.get(&"c"), Some(&5)); assert_eq!(map.get(&"ca"), Some(&6)); - map.remove_by(&"c", |key| key.starts_with("c")); assert_eq!(map.get(&"a"), Some(&1)); @@ -412,7 +411,6 @@ mod tests { map.remove_by(&"b", |key| key.starts_with("b")); assert_eq!(map.get(&"b"), None); - } #[test] From 5fe8b73f0491ec4f62a2b3204484918a10f5aa53 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Thu, 11 May 2023 16:07:41 -0700 Subject: [PATCH 34/34] =?UTF-8?q?compile=20error=20=F0=9F=98=85?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/project/src/worktree.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 305bcbbf16..fcda45fd6f 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -3023,7 +3023,7 @@ impl BackgroundScanner { snapshot.repository_entries.update(&work_dir, |entry| { entry .worktree_statuses - .remove_by(&repo_path, |stored_path, _| { + .remove_by(&repo_path, |stored_path| { stored_path.starts_with(&repo_path) }) });