git: Fix panic in git2 due to empty repo paths (#42304)

Fixes ZED-1VR

Release Notes:

- Fixed sporadic panic in git features

Author: Veykril

crates/agent_servers/src/acp.rs

@@ -136,7 +136,7 @@ impl AcpConnection {
             while let Ok(n) = stderr.read_line(&mut line).await
                 && n > 0
             {
-                log::warn!("agent stderr: {}", &line);
+                log::warn!("agent stderr: {}", line.trim());
                 line.clear();
             }
             Ok(())

crates/editor/src/test/editor_test_context.rs

@@ -6,6 +6,7 @@ use buffer_diff::DiffHunkStatusKind;
 use collections::BTreeMap;
 use futures::Future;
 
+use git::repository::RepoPath;
 use gpui::{
     AnyWindowHandle, App, Context, Entity, Focusable as _, Keystroke, Pixels, Point,
     VisualTestContext, Window, WindowHandle, prelude::*,
@@ -334,7 +335,10 @@ impl EditorTestContext {
         let path = self.update_buffer(|buffer, _| buffer.file().unwrap().path().clone());
         let mut found = None;
         fs.with_git_state(&Self::root_path().join(".git"), false, |git_state| {
-            found = git_state.index_contents.get(&path.into()).cloned();
+            found = git_state
+                .index_contents
+                .get(&RepoPath::from_rel_path(&path))
+                .cloned();
         })
         .unwrap();
         assert_eq!(expected, found.as_deref());

crates/fs/src/fake_git_repo.rs

@@ -272,7 +272,7 @@ impl GitRepository for FakeGitRepository {
                     .ok()
                     .map(|content| String::from_utf8(content).unwrap())?;
                 let repo_path = RelPath::new(repo_path, PathStyle::local()).ok()?;
-                Some((repo_path.into(), (content, is_ignored)))
+                Some((RepoPath::from_rel_path(&repo_path), (content, is_ignored)))
             })
             .collect();
 
@@ -436,7 +436,7 @@ impl GitRepository for FakeGitRepository {
             state
                 .blames
                 .get(&path)
-                .with_context(|| format!("failed to get blame for {:?}", path.0))
+                .with_context(|| format!("failed to get blame for {:?}", path))
                 .cloned()
         })
     }

crates/fs/src/fs.rs

@@ -1792,7 +1792,8 @@ impl FakeFs {
             for (path, content) in workdir_contents {
                 use util::{paths::PathStyle, rel_path::RelPath};
 
-                let repo_path: RepoPath = RelPath::new(path.strip_prefix(&workdir_path).unwrap(), PathStyle::local()).unwrap().into();
+                let repo_path = RelPath::new(path.strip_prefix(&workdir_path).unwrap(), PathStyle::local()).unwrap();
+                let repo_path = RepoPath::from_rel_path(&repo_path);
                 let status = statuses
                     .iter()
                     .find_map(|(p, status)| (*p == repo_path.as_unix_str()).then_some(status));

crates/git/src/repository.rs

@@ -14,7 +14,6 @@ use rope::Rope;
 use schemars::JsonSchema;
 use serde::Deserialize;
 use smol::io::{AsyncBufReadExt, AsyncReadExt, BufReader};
-use std::borrow::Cow;
 use std::ffi::{OsStr, OsString};
 use std::process::{ExitStatus, Stdio};
 use std::{
@@ -848,7 +847,7 @@ impl GitRepository for RealGitRepository {
                 }
 
                 files.push(CommitFile {
-                    path: rel_path.into(),
+                    path: RepoPath(Arc::from(rel_path)),
                     old_text,
                     new_text,
                 })
@@ -2049,6 +2048,11 @@ fn git_status_args(path_prefixes: &[RepoPath]) -> Vec<OsString> {
         OsString::from("--no-renames"),
         OsString::from("-z"),
     ];
+    args.extend(
+        path_prefixes
+            .iter()
+            .map(|path_prefix| path_prefix.as_std_path().into()),
+    );
     args.extend(path_prefixes.iter().map(|path_prefix| {
         if path_prefix.is_empty() {
             Path::new(".").into()
@@ -2304,52 +2308,54 @@ async fn run_askpass_command(
     }
 }
 
-#[derive(Clone, Debug, Ord, Hash, PartialOrd, Eq, PartialEq)]
-pub struct RepoPath(pub Arc<RelPath>);
+#[derive(Clone, Ord, Hash, PartialOrd, Eq, PartialEq)]
+pub struct RepoPath(Arc<RelPath>);
+
+impl std::fmt::Debug for RepoPath {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.0.fmt(f)
+    }
+}
 
 impl RepoPath {
     pub fn new<S: AsRef<str> + ?Sized>(s: &S) -> Result<Self> {
         let rel_path = RelPath::unix(s.as_ref())?;
-        Ok(rel_path.into())
-    }
-
-    pub fn from_proto(proto: &str) -> Result<Self> {
-        let rel_path = RelPath::from_proto(proto)?;
-        Ok(rel_path.into())
+        Ok(Self::from_rel_path(rel_path))
     }
 
     pub fn from_std_path(path: &Path, path_style: PathStyle) -> Result<Self> {
         let rel_path = RelPath::new(path, path_style)?;
-        Ok(Self(rel_path.as_ref().into()))
+        Ok(Self::from_rel_path(&rel_path))
     }
-}
 
-#[cfg(any(test, feature = "test-support"))]
-pub fn repo_path<S: AsRef<str> + ?Sized>(s: &S) -> RepoPath {
-    RepoPath(RelPath::unix(s.as_ref()).unwrap().into())
-}
+    pub fn from_proto(proto: &str) -> Result<Self> {
+        let rel_path = RelPath::from_proto(proto)?;
+        Ok(Self(rel_path))
+    }
 
-impl From<&RelPath> for RepoPath {
-    fn from(value: &RelPath) -> Self {
-        RepoPath(value.into())
+    pub fn from_rel_path(path: &RelPath) -> RepoPath {
+        Self(Arc::from(path))
     }
-}
 
-impl<'a> From<Cow<'a, RelPath>> for RepoPath {
-    fn from(value: Cow<'a, RelPath>) -> Self {
-        value.as_ref().into()
+    pub fn as_std_path(&self) -> &Path {
+        // git2 does not like empty paths and our RelPath infra turns `.` into ``
+        // so undo that here
+        if self.is_empty() {
+            Path::new(".")
+        } else {
+            self.0.as_std_path()
+        }
     }
 }
 
-impl From<Arc<RelPath>> for RepoPath {
-    fn from(value: Arc<RelPath>) -> Self {
-        RepoPath(value)
-    }
+#[cfg(any(test, feature = "test-support"))]
+pub fn repo_path<S: AsRef<str> + ?Sized>(s: &S) -> RepoPath {
+    RepoPath(RelPath::unix(s.as_ref()).unwrap().into())
 }
 
-impl Default for RepoPath {
-    fn default() -> Self {
-        RepoPath(RelPath::empty().into())
+impl AsRef<Arc<RelPath>> for RepoPath {
+    fn as_ref(&self) -> &Arc<RelPath> {
+        &self.0
     }
 }
 
@@ -2361,12 +2367,6 @@ impl std::ops::Deref for RepoPath {
     }
 }
 
-// impl AsRef<Path> for RepoPath {
-//     fn as_ref(&self) -> &Path {
-//         RelPath::as_ref(&self.0)
-//     }
-// }
-
 #[derive(Debug)]
 pub struct RepoPathDescendants<'a>(pub &'a RepoPath);
 

crates/git/src/status.rs

@@ -454,7 +454,7 @@ impl FromStr for GitStatus {
                 let status = entry.as_bytes()[0..2].try_into().unwrap();
                 let status = FileStatus::from_bytes(status).log_err()?;
                 // git-status outputs `/`-delimited repo paths, even on Windows.
-                let path = RepoPath(RelPath::unix(path).log_err()?.into());
+                let path = RepoPath::from_rel_path(RelPath::unix(path).log_err()?);
                 Some((path, status))
             })
             .collect::<Vec<_>>();
@@ -539,7 +539,7 @@ impl FromStr for TreeDiff {
         let mut fields = s.split('\0');
         let mut parsed = HashMap::default();
         while let Some((status, path)) = fields.next().zip(fields.next()) {
-            let path = RepoPath(RelPath::unix(path)?.into());
+            let path = RepoPath::from_rel_path(RelPath::unix(path)?);
 
             let mut fields = status.split(" ").skip(2);
             let old_sha = fields

crates/git_ui/src/commit_view.rs

@@ -266,7 +266,7 @@ impl language::File for GitBlob {
     }
 
     fn path(&self) -> &Arc<RelPath> {
-        &self.path.0
+        self.path.as_ref()
     }
 
     fn full_path(&self, _: &App) -> PathBuf {

crates/git_ui/src/git_panel.rs

@@ -879,7 +879,7 @@ impl GitPanel {
             let active_repository = self.active_repository.as_ref()?.downgrade();
 
             cx.spawn(async move |_, cx| {
-                let file_path_str = repo_path.0.display(PathStyle::Posix);
+                let file_path_str = repo_path.as_ref().display(PathStyle::Posix);
 
                 let repo_root = active_repository.read_with(cx, |repository, _| {
                     repository.snapshot().work_directory_abs_path
@@ -1074,7 +1074,7 @@ impl GitPanel {
         }
         let mut details = entries
             .iter()
-            .filter_map(|entry| entry.repo_path.0.file_name())
+            .filter_map(|entry| entry.repo_path.as_ref().file_name())
             .map(|filename| filename.to_string())
             .take(5)
             .join("\n");
@@ -1129,7 +1129,7 @@ impl GitPanel {
             .map(|entry| {
                 entry
                     .repo_path
-                    .0
+                    .as_ref()
                     .file_name()
                     .map(|f| f.to_string())
                     .unwrap_or_default()
@@ -5647,7 +5647,7 @@ mod tests {
             assert_eq!(
                 entry.status_entry().map(|status| status
                     .repo_path
-                    .0
+                    .as_ref()
                     .as_std_path()
                     .to_string_lossy()
                     .to_string()),

crates/git_ui/src/project_diff.rs

@@ -336,7 +336,7 @@ impl ProjectDiff {
         };
         let repo = git_repo.read(cx);
         let sort_prefix = sort_prefix(repo, &entry.repo_path, entry.status, cx);
-        let path_key = PathKey::with_sort_prefix(sort_prefix, entry.repo_path.0);
+        let path_key = PathKey::with_sort_prefix(sort_prefix, entry.repo_path.as_ref().clone());
 
         self.move_to_path(path_key, window, cx)
     }
@@ -566,7 +566,7 @@ impl ProjectDiff {
                 for entry in buffers_to_load.iter() {
                     let sort_prefix = sort_prefix(&repo, &entry.repo_path, entry.file_status, cx);
                     let path_key =
-                        PathKey::with_sort_prefix(sort_prefix, entry.repo_path.0.clone());
+                        PathKey::with_sort_prefix(sort_prefix, entry.repo_path.as_ref().clone());
                     previous_paths.remove(&path_key);
                     path_keys.push(path_key)
                 }

crates/project/src/git_store.rs

@@ -227,7 +227,7 @@ impl sum_tree::Item for StatusEntry {
 
     fn summary(&self, _: <Self::Summary as sum_tree::Summary>::Context<'_>) -> Self::Summary {
         PathSummary {
-            max_path: self.repo_path.0.clone(),
+            max_path: self.repo_path.as_ref().clone(),
             item_summary: self.status.summary(),
         }
     }
@@ -237,7 +237,7 @@ impl sum_tree::KeyedItem for StatusEntry {
     type Key = PathKey;
 
     fn key(&self) -> Self::Key {
-        PathKey(self.repo_path.0.clone())
+        PathKey(self.repo_path.as_ref().clone())
     }
 }
 
@@ -990,7 +990,7 @@ impl GitStore {
                     RepositoryState::Local { backend, .. } => backend
                         .blame(repo_path.clone(), content)
                         .await
-                        .with_context(|| format!("Failed to blame {:?}", repo_path.0))
+                        .with_context(|| format!("Failed to blame {:?}", repo_path.as_ref()))
                         .map(Some),
                     RepositoryState::Remote { project_id, client } => {
                         let response = client
@@ -2376,7 +2376,7 @@ impl GitStore {
                 .entries
                 .into_iter()
                 .map(|(path, status)| proto::TreeDiffStatus {
-                    path: path.0.to_proto(),
+                    path: path.as_ref().to_proto(),
                     status: match status {
                         TreeDiffStatus::Added {} => proto::tree_diff_status::Status::Added.into(),
                         TreeDiffStatus::Modified { .. } => {
@@ -3152,13 +3152,13 @@ impl RepositorySnapshot {
 
     pub fn status_for_path(&self, path: &RepoPath) -> Option<StatusEntry> {
         self.statuses_by_path
-            .get(&PathKey(path.0.clone()), ())
+            .get(&PathKey(path.as_ref().clone()), ())
             .cloned()
     }
 
     pub fn pending_ops_for_path(&self, path: &RepoPath) -> Option<PendingOps> {
         self.pending_ops_by_path
-            .get(&PathKey(path.0.clone()), ())
+            .get(&PathKey(path.as_ref().clone()), ())
             .cloned()
     }
 
@@ -4727,7 +4727,9 @@ impl Repository {
                                 }
                             };
                             Some((
-                                RepoPath(RelPath::from_proto(&entry.path).log_err()?),
+                                RepoPath::from_rel_path(
+                                    &RelPath::from_proto(&entry.path).log_err()?,
+                                ),
                                 status,
                             ))
                         })
@@ -5289,7 +5291,8 @@ impl Repository {
                         let mut cursor = prev_statuses.cursor::<PathProgress>(());
                         for path in changed_paths.into_iter() {
                             if cursor.seek_forward(&PathTarget::Path(&path), Bias::Left) {
-                                changed_path_statuses.push(Edit::Remove(PathKey(path.0)));
+                                changed_path_statuses
+                                    .push(Edit::Remove(PathKey(path.as_ref().clone())));
                             }
                         }
                         changed_path_statuses
@@ -5435,10 +5438,8 @@ fn get_permalink_in_rust_registry_src(
         remote,
         BuildPermalinkParams::new(
             &cargo_vcs_info.git.sha1,
-            &RepoPath(
-                RelPath::new(&path, PathStyle::local())
-                    .context("invalid path")?
-                    .into_arc(),
+            &RepoPath::from_rel_path(
+                &RelPath::new(&path, PathStyle::local()).context("invalid path")?,
             ),
             Some(selection),
         ),
@@ -5640,7 +5641,11 @@ async fn compute_snapshot(
     let mut events = Vec::new();
     let branches = backend.branches().await?;
     let branch = branches.into_iter().find(|branch| branch.is_head);
-    let statuses = backend.status(&[RelPath::empty().into()]).await?;
+    let statuses = backend
+        .status(&[RepoPath::from_rel_path(
+            &RelPath::new(".".as_ref(), PathStyle::local()).unwrap(),
+        )])
+        .await?;
     let stash_entries = backend.stash_entries().await?;
     let statuses_by_path = SumTree::from_iter(
         statuses