diff --git a/cli/src/merge_tools/external.rs b/cli/src/merge_tools/external.rs index 96dcf8fda..a1d53374a 100644 --- a/cli/src/merge_tools/external.rs +++ b/cli/src/merge_tools/external.rs @@ -212,15 +212,18 @@ fn new_utf8_temp_dir(prefix: &str) -> io::Result { fn set_readonly_recursively(path: &Path) -> Result<(), std::io::Error> { // Directory permission is unchanged since files under readonly directory cannot // be removed. - if path.is_dir() { + let metadata = path.symlink_metadata()?; + if metadata.is_dir() { for entry in path.read_dir()? { set_readonly_recursively(&entry?.path())?; } Ok(()) - } else { - let mut perms = std::fs::metadata(path)?.permissions(); + } else if metadata.is_file() { + let mut perms = metadata.permissions(); perms.set_readonly(true); std::fs::set_permissions(path, perms) + } else { + Ok(()) } } diff --git a/cli/tests/test_diff_command.rs b/cli/tests/test_diff_command.rs index aae29a34a..2b76ca58e 100644 --- a/cli/tests/test_diff_command.rs +++ b/cli/tests/test_diff_command.rs @@ -756,6 +756,49 @@ fn test_diff_external_tool() { "###); } +#[cfg(unix)] +#[test] +fn test_diff_external_tool_symlink() { + let mut test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + + let external_file_path = test_env.env_root().join("external-file"); + std::fs::write(&external_file_path, "").unwrap(); + let external_file_permissions = external_file_path.symlink_metadata().unwrap().permissions(); + + std::os::unix::fs::symlink("non-existent1", repo_path.join("dead")).unwrap(); + std::os::unix::fs::symlink(&external_file_path, repo_path.join("file")).unwrap(); + test_env.jj_cmd_ok(&repo_path, &["new"]); + std::fs::remove_file(repo_path.join("dead")).unwrap(); + std::os::unix::fs::symlink("non-existent2", repo_path.join("dead")).unwrap(); + std::fs::remove_file(repo_path.join("file")).unwrap(); + std::fs::write(repo_path.join("file"), "").unwrap(); + + let edit_script = test_env.set_up_fake_diff_editor(); + std::fs::write( + &edit_script, + "print-files-before\0print --\0print-files-after", + ) + .unwrap(); + + // Shouldn't try to change permission of symlinks + insta::assert_snapshot!( + test_env.jj_cmd_success(&repo_path, &["diff", "--tool=fake-diff-editor"]), @r###" + dead + file + -- + dead + file + "###); + + // External file should be intact + assert_eq!( + external_file_path.symlink_metadata().unwrap().permissions(), + external_file_permissions + ); +} + #[test] fn test_diff_stat() { let test_env = TestEnvironment::default();