Unnamed repository; edit this file 'description' to name the repository.
fix git repo detection on symlinks (#11732)
| -rw-r--r-- | helix-vcs/src/git.rs | 14 | ||||
| -rw-r--r-- | helix-vcs/src/git/test.rs | 45 |
2 files changed, 49 insertions, 10 deletions
diff --git a/helix-vcs/src/git.rs b/helix-vcs/src/git.rs index 78e58243..48220f4d 100644 --- a/helix-vcs/src/git.rs +++ b/helix-vcs/src/git.rs @@ -22,18 +22,24 @@ use crate::FileChange; #[cfg(test)] mod test; +#[inline] +fn get_repo_dir(file: &Path) -> Result<&Path> { + file.parent().context("file has no parent directory") +} + pub fn get_diff_base(file: &Path) -> Result<Vec<u8>> { debug_assert!(!file.exists() || file.is_file()); debug_assert!(file.is_absolute()); + let file = gix::path::realpath(file).context("resolve symlinks")?; // TODO cache repository lookup - let repo_dir = file.parent().context("file has no parent directory")?; + let repo_dir = get_repo_dir(&file)?; let repo = open_repo(repo_dir) .context("failed to open git repo")? .to_thread_local(); let head = repo.head_commit()?; - let file_oid = find_file_in_commit(&repo, &head, file)?; + let file_oid = find_file_in_commit(&repo, &head, &file)?; let file_object = repo.find_object(file_oid)?; let data = file_object.detach().data; @@ -56,7 +62,9 @@ pub fn get_diff_base(file: &Path) -> Result<Vec<u8>> { pub fn get_current_head_name(file: &Path) -> Result<Arc<ArcSwap<Box<str>>>> { debug_assert!(!file.exists() || file.is_file()); debug_assert!(file.is_absolute()); - let repo_dir = file.parent().context("file has no parent directory")?; + let file = gix::path::realpath(file).context("resolve symlinks")?; + + let repo_dir = get_repo_dir(&file)?; let repo = open_repo(repo_dir) .context("failed to open git repo")? .to_thread_local(); diff --git a/helix-vcs/src/git/test.rs b/helix-vcs/src/git/test.rs index 95ff10b2..164040f5 100644 --- a/helix-vcs/src/git/test.rs +++ b/helix-vcs/src/git/test.rs @@ -98,9 +98,13 @@ fn directory() { assert!(git::get_diff_base(&dir).is_err()); } -/// Test that `get_file_head` does not return content for a symlink. -/// This is important to correctly cover cases where a symlink is removed and replaced by a file. -/// If the contents of the symlink object were returned a diff between a path and the actual file would be produced (bad ui). +/// Test that `get_diff_base` resolves symlinks so that the same diff base is +/// used as the target file. +/// +/// This is important to correctly cover cases where a symlink is removed and +/// replaced by a file. If the contents of the symlink object were returned +/// a diff between a literal file path and the actual file content would be +/// produced (bad ui). #[cfg(any(unix, windows))] #[test] fn symlink() { @@ -108,14 +112,41 @@ fn symlink() { use std::os::unix::fs::symlink; #[cfg(not(unix))] use std::os::windows::fs::symlink_file as symlink; + let temp_git = empty_git_repo(); let file = temp_git.path().join("file.txt"); - let contents = b"foo".as_slice(); - File::create(&file).unwrap().write_all(contents).unwrap(); + let contents = Vec::from(b"foo"); + File::create(&file).unwrap().write_all(&contents).unwrap(); let file_link = temp_git.path().join("file_link.txt"); + symlink("file.txt", &file_link).unwrap(); + create_commit(temp_git.path(), true); + + assert_eq!(git::get_diff_base(&file_link).unwrap(), contents); + assert_eq!(git::get_diff_base(&file).unwrap(), contents); +} + +/// Test that `get_diff_base` returns content when the file is a symlink to +/// another file that is in a git repo, but the symlink itself is not. +#[cfg(any(unix, windows))] +#[test] +fn symlink_to_git_repo() { + #[cfg(unix)] + use std::os::unix::fs::symlink; + #[cfg(not(unix))] + use std::os::windows::fs::symlink_file as symlink; + + let temp_dir = tempfile::tempdir().expect("create temp dir"); + let temp_git = empty_git_repo(); + let file = temp_git.path().join("file.txt"); + let contents = Vec::from(b"foo"); + File::create(&file).unwrap().write_all(&contents).unwrap(); create_commit(temp_git.path(), true); - assert!(git::get_diff_base(&file_link).is_err()); - assert_eq!(git::get_diff_base(&file).unwrap(), Vec::from(contents)); + + let file_link = temp_dir.path().join("file_link.txt"); + symlink(&file, &file_link).unwrap(); + + assert_eq!(git::get_diff_base(&file_link).unwrap(), contents); + assert_eq!(git::get_diff_base(&file).unwrap(), contents); } |