Skip to content

Commit ff00715

Browse files
committed
git: Fix panic in git2 due to empty repo paths
1 parent 5d08c1b commit ff00715

File tree

12 files changed

+79
-88
lines changed

12 files changed

+79
-88
lines changed

crates/editor/src/test/editor_test_context.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use buffer_diff::DiffHunkStatusKind;
66
use collections::BTreeMap;
77
use futures::Future;
88

9+
use git::repository::RepoPath;
910
use gpui::{
1011
AnyWindowHandle, App, Context, Entity, Focusable as _, Keystroke, Pixels, Point,
1112
VisualTestContext, Window, WindowHandle, prelude::*,
@@ -334,7 +335,10 @@ impl EditorTestContext {
334335
let path = self.update_buffer(|buffer, _| buffer.file().unwrap().path().clone());
335336
let mut found = None;
336337
fs.with_git_state(&Self::root_path().join(".git"), false, |git_state| {
337-
found = git_state.index_contents.get(&path.into()).cloned();
338+
found = git_state
339+
.index_contents
340+
.get(&RepoPath::from_rel_path(&path))
341+
.cloned();
338342
})
339343
.unwrap();
340344
assert_eq!(expected, found.as_deref());

crates/fs/src/fake_git_repo.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ impl GitRepository for FakeGitRepository {
272272
.ok()
273273
.map(|content| String::from_utf8(content).unwrap())?;
274274
let repo_path = RelPath::new(repo_path, PathStyle::local()).ok()?;
275-
Some((repo_path.into(), (content, is_ignored)))
275+
Some((RepoPath::from_rel_path(&repo_path), (content, is_ignored)))
276276
})
277277
.collect();
278278

@@ -432,7 +432,7 @@ impl GitRepository for FakeGitRepository {
432432
state
433433
.blames
434434
.get(&path)
435-
.with_context(|| format!("failed to get blame for {:?}", path.0))
435+
.with_context(|| format!("failed to get blame for {:?}", path))
436436
.cloned()
437437
})
438438
}

crates/fs/src/fs.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1792,7 +1792,8 @@ impl FakeFs {
17921792
for (path, content) in workdir_contents {
17931793
use util::{paths::PathStyle, rel_path::RelPath};
17941794

1795-
let repo_path: RepoPath = RelPath::new(path.strip_prefix(&workdir_path).unwrap(), PathStyle::local()).unwrap().into();
1795+
let repo_path = RelPath::new(path.strip_prefix(&workdir_path).unwrap(), PathStyle::local()).unwrap();
1796+
let repo_path = RepoPath::from_rel_path(&repo_path);
17961797
let status = statuses
17971798
.iter()
17981799
.find_map(|(p, status)| (*p == repo_path.as_unix_str()).then_some(status));

crates/git/src/repository.rs

Lines changed: 28 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ use rope::Rope;
1414
use schemars::JsonSchema;
1515
use serde::Deserialize;
1616
use smol::io::{AsyncBufReadExt, AsyncReadExt, BufReader};
17-
use std::borrow::Cow;
1817
use std::ffi::{OsStr, OsString};
1918
use std::process::{ExitStatus, Stdio};
2019
use std::{
@@ -846,7 +845,7 @@ impl GitRepository for RealGitRepository {
846845
}
847846

848847
files.push(CommitFile {
849-
path: rel_path.into(),
848+
path: RepoPath(Arc::from(rel_path)),
850849
old_text,
851850
new_text,
852851
})
@@ -930,7 +929,15 @@ impl GitRepository for RealGitRepository {
930929
index.read(false)?;
931930

932931
const STAGE_NORMAL: i32 = 0;
933-
let oid = match index.get_path(path.as_std_path(), STAGE_NORMAL) {
932+
let oid = match index.get_path(
933+
// git2 does not like empty paths and our RelPath infra turns `.` into ``
934+
if path.is_empty() {
935+
".".as_ref()
936+
} else {
937+
path.as_std_path()
938+
},
939+
STAGE_NORMAL,
940+
) {
934941
Some(entry) if entry.mode != GIT_MODE_SYMLINK => entry.id,
935942
_ => return Ok(None),
936943
};
@@ -2035,13 +2042,11 @@ fn git_status_args(path_prefixes: &[RepoPath]) -> Vec<OsString> {
20352042
OsString::from("--no-renames"),
20362043
OsString::from("-z"),
20372044
];
2038-
args.extend(path_prefixes.iter().map(|path_prefix| {
2039-
if path_prefix.is_empty() {
2040-
Path::new(".").into()
2041-
} else {
2042-
path_prefix.as_std_path().into()
2043-
}
2044-
}));
2045+
args.extend(
2046+
path_prefixes
2047+
.iter()
2048+
.map(|path_prefix| path_prefix.as_std_path().into()),
2049+
);
20452050
args
20462051
}
20472052

@@ -2291,22 +2296,26 @@ async fn run_askpass_command(
22912296
}
22922297

22932298
#[derive(Clone, Debug, Ord, Hash, PartialOrd, Eq, PartialEq)]
2294-
pub struct RepoPath(pub Arc<RelPath>);
2299+
pub struct RepoPath(Arc<RelPath>);
22952300

22962301
impl RepoPath {
22972302
pub fn new<S: AsRef<str> + ?Sized>(s: &S) -> Result<Self> {
22982303
let rel_path = RelPath::unix(s.as_ref())?;
2299-
Ok(rel_path.into())
2304+
Ok(Self::from_rel_path(rel_path))
2305+
}
2306+
2307+
pub fn from_std_path(path: &Path, path_style: PathStyle) -> Result<Self> {
2308+
let rel_path = RelPath::new(path, path_style)?;
2309+
Ok(Self::from_rel_path(&rel_path))
23002310
}
23012311

23022312
pub fn from_proto(proto: &str) -> Result<Self> {
23032313
let rel_path = RelPath::from_proto(proto)?;
2304-
Ok(rel_path.into())
2314+
Ok(Self(rel_path))
23052315
}
23062316

2307-
pub fn from_std_path(path: &Path, path_style: PathStyle) -> Result<Self> {
2308-
let rel_path = RelPath::new(path, path_style)?;
2309-
Ok(Self(rel_path.as_ref().into()))
2317+
pub fn from_rel_path(path: &RelPath) -> RepoPath {
2318+
Self(Arc::from(path))
23102319
}
23112320
}
23122321

@@ -2315,27 +2324,9 @@ pub fn repo_path<S: AsRef<str> + ?Sized>(s: &S) -> RepoPath {
23152324
RepoPath(RelPath::unix(s.as_ref()).unwrap().into())
23162325
}
23172326

2318-
impl From<&RelPath> for RepoPath {
2319-
fn from(value: &RelPath) -> Self {
2320-
RepoPath(value.into())
2321-
}
2322-
}
2323-
2324-
impl<'a> From<Cow<'a, RelPath>> for RepoPath {
2325-
fn from(value: Cow<'a, RelPath>) -> Self {
2326-
value.as_ref().into()
2327-
}
2328-
}
2329-
2330-
impl From<Arc<RelPath>> for RepoPath {
2331-
fn from(value: Arc<RelPath>) -> Self {
2332-
RepoPath(value)
2333-
}
2334-
}
2335-
2336-
impl Default for RepoPath {
2337-
fn default() -> Self {
2338-
RepoPath(RelPath::empty().into())
2327+
impl AsRef<Arc<RelPath>> for RepoPath {
2328+
fn as_ref(&self) -> &Arc<RelPath> {
2329+
&self.0
23392330
}
23402331
}
23412332

@@ -2347,12 +2338,6 @@ impl std::ops::Deref for RepoPath {
23472338
}
23482339
}
23492340

2350-
// impl AsRef<Path> for RepoPath {
2351-
// fn as_ref(&self) -> &Path {
2352-
// RelPath::as_ref(&self.0)
2353-
// }
2354-
// }
2355-
23562341
#[derive(Debug)]
23572342
pub struct RepoPathDescendants<'a>(pub &'a RepoPath);
23582343

crates/git/src/status.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ impl FromStr for GitStatus {
454454
let status = entry.as_bytes()[0..2].try_into().unwrap();
455455
let status = FileStatus::from_bytes(status).log_err()?;
456456
// git-status outputs `/`-delimited repo paths, even on Windows.
457-
let path = RepoPath(RelPath::unix(path).log_err()?.into());
457+
let path = RepoPath::from_rel_path(RelPath::unix(path).log_err()?);
458458
Some((path, status))
459459
})
460460
.collect::<Vec<_>>();
@@ -539,7 +539,7 @@ impl FromStr for TreeDiff {
539539
let mut fields = s.split('\0');
540540
let mut parsed = HashMap::default();
541541
while let Some((status, path)) = fields.next().zip(fields.next()) {
542-
let path = RepoPath(RelPath::unix(path)?.into());
542+
let path = RepoPath::from_rel_path(RelPath::unix(path)?);
543543

544544
let mut fields = status.split(" ").skip(2);
545545
let old_sha = fields

crates/git_ui/src/commit_view.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ impl language::File for GitBlob {
266266
}
267267

268268
fn path(&self) -> &Arc<RelPath> {
269-
&self.path.0
269+
self.path.as_ref()
270270
}
271271

272272
fn full_path(&self, _: &App) -> PathBuf {

crates/git_ui/src/git_panel.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -879,7 +879,7 @@ impl GitPanel {
879879
let active_repository = self.active_repository.as_ref()?.downgrade();
880880

881881
cx.spawn(async move |_, cx| {
882-
let file_path_str = repo_path.0.display(PathStyle::Posix);
882+
let file_path_str = repo_path.as_ref().display(PathStyle::Posix);
883883

884884
let repo_root = active_repository.read_with(cx, |repository, _| {
885885
repository.snapshot().work_directory_abs_path
@@ -1074,7 +1074,7 @@ impl GitPanel {
10741074
}
10751075
let mut details = entries
10761076
.iter()
1077-
.filter_map(|entry| entry.repo_path.0.file_name())
1077+
.filter_map(|entry| entry.repo_path.as_ref().file_name())
10781078
.map(|filename| filename.to_string())
10791079
.take(5)
10801080
.join("\n");
@@ -1129,7 +1129,7 @@ impl GitPanel {
11291129
.map(|entry| {
11301130
entry
11311131
.repo_path
1132-
.0
1132+
.as_ref()
11331133
.file_name()
11341134
.map(|f| f.to_string())
11351135
.unwrap_or_default()
@@ -5646,7 +5646,7 @@ mod tests {
56465646
assert_eq!(
56475647
entry.status_entry().map(|status| status
56485648
.repo_path
5649-
.0
5649+
.as_ref()
56505650
.as_std_path()
56515651
.to_string_lossy()
56525652
.to_string()),

crates/git_ui/src/project_diff.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ impl ProjectDiff {
336336
};
337337
let repo = git_repo.read(cx);
338338
let sort_prefix = sort_prefix(repo, &entry.repo_path, entry.status, cx);
339-
let path_key = PathKey::with_sort_prefix(sort_prefix, entry.repo_path.0);
339+
let path_key = PathKey::with_sort_prefix(sort_prefix, entry.repo_path.as_ref().clone());
340340

341341
self.move_to_path(path_key, window, cx)
342342
}
@@ -566,7 +566,7 @@ impl ProjectDiff {
566566
for entry in buffers_to_load.iter() {
567567
let sort_prefix = sort_prefix(&repo, &entry.repo_path, entry.file_status, cx);
568568
let path_key =
569-
PathKey::with_sort_prefix(sort_prefix, entry.repo_path.0.clone());
569+
PathKey::with_sort_prefix(sort_prefix, entry.repo_path.as_ref().clone());
570570
previous_paths.remove(&path_key);
571571
path_keys.push(path_key)
572572
}

crates/project/src/git_store.rs

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ impl sum_tree::Item for StatusEntry {
225225

226226
fn summary(&self, _: <Self::Summary as sum_tree::Summary>::Context<'_>) -> Self::Summary {
227227
PathSummary {
228-
max_path: self.repo_path.0.clone(),
228+
max_path: self.repo_path.as_ref().clone(),
229229
item_summary: self.status.summary(),
230230
}
231231
}
@@ -235,7 +235,7 @@ impl sum_tree::KeyedItem for StatusEntry {
235235
type Key = PathKey;
236236

237237
fn key(&self) -> Self::Key {
238-
PathKey(self.repo_path.0.clone())
238+
PathKey(self.repo_path.as_ref().clone())
239239
}
240240
}
241241

@@ -987,7 +987,7 @@ impl GitStore {
987987
RepositoryState::Local { backend, .. } => backend
988988
.blame(repo_path.clone(), content)
989989
.await
990-
.with_context(|| format!("Failed to blame {:?}", repo_path.0))
990+
.with_context(|| format!("Failed to blame {:?}", repo_path.as_ref()))
991991
.map(Some),
992992
RepositoryState::Remote { project_id, client } => {
993993
let response = client
@@ -2311,7 +2311,7 @@ impl GitStore {
23112311
.entries
23122312
.into_iter()
23132313
.map(|(path, status)| proto::TreeDiffStatus {
2314-
path: path.0.to_proto(),
2314+
path: path.as_ref().to_proto(),
23152315
status: match status {
23162316
TreeDiffStatus::Added {} => proto::tree_diff_status::Status::Added.into(),
23172317
TreeDiffStatus::Modified { .. } => {
@@ -3087,13 +3087,13 @@ impl RepositorySnapshot {
30873087

30883088
pub fn status_for_path(&self, path: &RepoPath) -> Option<StatusEntry> {
30893089
self.statuses_by_path
3090-
.get(&PathKey(path.0.clone()), ())
3090+
.get(&PathKey(path.as_ref().clone()), ())
30913091
.cloned()
30923092
}
30933093

30943094
pub fn pending_ops_for_path(&self, path: &RepoPath) -> Option<PendingOps> {
30953095
self.pending_ops_by_path
3096-
.get(&PathKey(path.0.clone()), ())
3096+
.get(&PathKey(path.as_ref().clone()), ())
30973097
.cloned()
30983098
}
30993099

@@ -4653,7 +4653,9 @@ impl Repository {
46534653
}
46544654
};
46554655
Some((
4656-
RepoPath(RelPath::from_proto(&entry.path).log_err()?),
4656+
RepoPath::from_rel_path(
4657+
&RelPath::from_proto(&entry.path).log_err()?,
4658+
),
46574659
status,
46584660
))
46594661
})
@@ -5209,7 +5211,8 @@ impl Repository {
52095211
let mut cursor = prev_statuses.cursor::<PathProgress>(());
52105212
for path in changed_paths.into_iter() {
52115213
if cursor.seek_forward(&PathTarget::Path(&path), Bias::Left) {
5212-
changed_path_statuses.push(Edit::Remove(PathKey(path.0)));
5214+
changed_path_statuses
5215+
.push(Edit::Remove(PathKey(path.as_ref().clone())));
52135216
}
52145217
}
52155218
changed_path_statuses
@@ -5355,10 +5358,8 @@ fn get_permalink_in_rust_registry_src(
53555358
remote,
53565359
BuildPermalinkParams::new(
53575360
&cargo_vcs_info.git.sha1,
5358-
&RepoPath(
5359-
RelPath::new(&path, PathStyle::local())
5360-
.context("invalid path")?
5361-
.into_arc(),
5361+
&RepoPath::from_rel_path(
5362+
&RelPath::new(&path, PathStyle::local()).context("invalid path")?,
53625363
),
53635364
Some(selection),
53645365
),
@@ -5560,7 +5561,11 @@ async fn compute_snapshot(
55605561
let mut events = Vec::new();
55615562
let branches = backend.branches().await?;
55625563
let branch = branches.into_iter().find(|branch| branch.is_head);
5563-
let statuses = backend.status(&[RelPath::empty().into()]).await?;
5564+
let statuses = backend
5565+
.status(&[RepoPath::from_rel_path(
5566+
&RelPath::new(".".as_ref(), PathStyle::local()).unwrap(),
5567+
)])
5568+
.await?;
55645569
let stash_entries = backend.stash_entries().await?;
55655570
let statuses_by_path = SumTree::from_iter(
55665571
statuses

crates/project/src/git_store/conflict_set.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ mod tests {
264264
use super::*;
265265
use fs::FakeFs;
266266
use git::{
267-
repository::repo_path,
267+
repository::{RepoPath, repo_path},
268268
status::{UnmergedStatus, UnmergedStatusCode},
269269
};
270270
use gpui::{BackgroundExecutor, TestAppContext};
@@ -617,7 +617,7 @@ mod tests {
617617
cx.run_until_parked();
618618
fs.with_git_state(path!("/project/.git").as_ref(), true, |state| {
619619
state.unmerged_paths.insert(
620-
rel_path("a.txt").into(),
620+
RepoPath::from_rel_path(rel_path("a.txt")),
621621
UnmergedStatus {
622622
first_head: UnmergedStatusCode::Updated,
623623
second_head: UnmergedStatusCode::Updated,

0 commit comments

Comments
 (0)