git: Use correct file mode when staging

crates/fs/Cargo.toml

@@ -33,6 +33,7 @@ tempfile.workspace = true
 text.workspace = true
 time.workspace = true
 util.workspace = true
+is_executable = "1.0.5"
 
 [target.'cfg(target_os = "macos")'.dependencies]
 fsevent.workspace = true

crates/fs/src/fake_git_repo.rs

@@ -138,6 +138,7 @@ impl GitRepository for FakeGitRepository {
         path: RepoPath,
         content: Option<String>,
         _env: Arc<HashMap<String, String>>,
+        _is_executable: bool,
     ) -> BoxFuture<'_, anyhow::Result<()>> {
         self.with_state_async(true, move |state| {
             if let Some(message) = &state.simulated_index_write_error_message {

crates/fs/src/fs.rs

@@ -32,6 +32,7 @@ use std::mem::MaybeUninit;
 use async_tar::Archive;
 use futures::{AsyncRead, Stream, StreamExt, future::BoxFuture};
 use git::repository::{GitRepository, RealGitRepository};
+use is_executable::IsExecutable;
 use rope::Rope;
 use serde::{Deserialize, Serialize};
 use smol::io::AsyncWriteExt;
@@ -208,6 +209,7 @@ pub struct Metadata {
     pub is_dir: bool,
     pub len: u64,
     pub is_fifo: bool,
+    pub is_executable: bool,
 }
 
 /// Filesystem modification time. The purpose of this newtype is to discourage use of operations
@@ -895,13 +897,20 @@ impl Fs for RealFs {
         #[cfg(unix)]
         let is_fifo = metadata.file_type().is_fifo();
 
+        let path_buf = path.to_path_buf();
+        let is_executable = self
+            .executor
+            .spawn(async move { path_buf.is_executable() })
+            .await;
+
         Ok(Some(Metadata {
             inode,
             mtime: MTime(metadata.modified().unwrap_or(SystemTime::UNIX_EPOCH)),
             len: metadata.len(),
             is_symlink,
             is_dir: metadata.file_type().is_dir(),
             is_fifo,
+            is_executable,
         }))
     }
 
@@ -2602,6 +2611,7 @@ impl Fs for FakeFs {
                     is_dir: false,
                     is_symlink,
                     is_fifo: false,
+                    is_executable: false,
                 },
                 FakeFsEntry::Dir {
                     inode, mtime, len, ..
@@ -2612,6 +2622,7 @@ impl Fs for FakeFs {
                     is_dir: true,
                     is_symlink,
                     is_fifo: false,
+                    is_executable: false,
                 },
                 FakeFsEntry::Symlink { .. } => unreachable!(),
             }))

crates/git/src/repository.rs

@@ -400,6 +400,7 @@ pub trait GitRepository: Send + Sync {
         path: RepoPath,
         content: Option<String>,
         env: Arc<HashMap<String, String>>,
+        is_executable: bool,
     ) -> BoxFuture<'_, anyhow::Result<()>>;
 
     /// Returns the URL of the remote with the given name.
@@ -987,12 +988,15 @@ impl GitRepository for RealGitRepository {
         path: RepoPath,
         content: Option<String>,
         env: Arc<HashMap<String, String>>,
+        is_executable: bool,
     ) -> BoxFuture<'_, anyhow::Result<()>> {
         let working_directory = self.working_directory();
         let git_binary_path = self.any_git_binary_path.clone();
         self.executor
             .spawn(async move {
                 let working_directory = working_directory?;
+                let mode = if is_executable { "100755" } else { "100644" };
+
                 if let Some(content) = content {
                     let mut child = new_smol_command(&git_binary_path)
                         .current_dir(&working_directory)
@@ -1013,7 +1017,7 @@ impl GitRepository for RealGitRepository {
                     let output = new_smol_command(&git_binary_path)
                         .current_dir(&working_directory)
                         .envs(env.iter())
-                        .args(["update-index", "--add", "--cacheinfo", "100644", sha])
+                        .args(["update-index", "--add", "--cacheinfo", mode, sha])
                         .arg(path.as_unix_str())
                         .output()
                         .await?;

crates/project/src/git_store.rs

@@ -301,6 +301,7 @@ impl std::ops::Deref for Repository {
 #[derive(Clone)]
 pub enum RepositoryState {
     Local {
+        fs: Arc<dyn Fs>,
         backend: Arc<dyn GitRepository>,
         environment: Arc<HashMap<String, String>>,
     },
@@ -4288,6 +4289,7 @@ impl Repository {
                     RepositoryState::Local {
                         backend,
                         environment,
+                        ..
                     } => backend.run_hook(hook, environment.clone()).await,
                     RepositoryState::Remote { project_id, client } => {
                         client
@@ -4580,6 +4582,7 @@ impl Repository {
         let id = self.id;
         let this = cx.weak_entity();
         let git_store = self.git_store.clone();
+        let abs_path = self.snapshot.repo_path_to_abs_path(&path);
         self.send_keyed_job(
             Some(GitJobKey::WriteIndex(vec![path.clone()])),
             None,
@@ -4588,14 +4591,21 @@ impl Repository {
                     "start updating index text for buffer {}",
                     path.as_unix_str()
                 );
+
                 match git_repo {
                     RepositoryState::Local {
+                        fs,
                         backend,
                         environment,
                         ..
                     } => {
+                        let executable = match fs.metadata(&abs_path).await {
+                            Ok(Some(meta)) => meta.is_executable,
+                            Ok(None) => false,
+                            Err(_err) => false,
+                        };
                         backend
-                            .set_index_text(path.clone(), content, environment.clone())
+                            .set_index_text(path.clone(), content, environment.clone(), executable)
                             .await?;
                     }
                     RepositoryState::Remote { project_id, client } => {
@@ -5164,6 +5174,7 @@ impl Repository {
         cx: &mut Context<Self>,
     ) -> mpsc::UnboundedSender<GitJob> {
         let (job_tx, mut job_rx) = mpsc::unbounded::<GitJob>();
+        let fs_cloned = fs.clone();
 
         cx.spawn(async move |_, cx| {
             let environment = project_environment
@@ -5195,8 +5206,8 @@ impl Repository {
                     backend.clone(),
                 );
             }
-
             let state = RepositoryState::Local {
+                fs: fs_cloned,
                 backend,
                 environment: Arc::new(environment),
             };

crates/project/src/project_tests.rs

@@ -8174,6 +8174,91 @@ async fn test_single_file_diffs(cx: &mut gpui::TestAppContext) {
     });
 }
 
+// TODO: Should we test this on Windows also?
+#[gpui::test]
+#[cfg(not(windows))]
+async fn test_staging_hunk_preserve_executable_permission(cx: &mut gpui::TestAppContext) {
+    use std::os::unix::fs::PermissionsExt;
+    init_test(cx);
+    cx.executor().allow_parking();
+    let committed_contents = "bar\n";
+    let file_contents = "baz\n";
+    let root = TempTree::new(json!({
+        "project": {
+            "foo": committed_contents
+        },
+    }));
+
+    let work_dir = root.path().join("project");
+    let file_path = work_dir.join("foo");
+    let repo = git_init(work_dir.as_path());
+    let mut perms = std::fs::metadata(&file_path).unwrap().permissions();
+    perms.set_mode(0o755);
+    std::fs::set_permissions(&file_path, perms).unwrap();
+    git_add("foo", &repo);
+    git_commit("Initial commit", &repo);
+    std::fs::write(&file_path, file_contents).unwrap();
+
+    let project = Project::test(
+        Arc::new(RealFs::new(None, cx.executor())),
+        [root.path()],
+        cx,
+    )
+    .await;
+
+    let buffer = project
+        .update(cx, |project, cx| {
+            project.open_local_buffer(file_path.as_path(), cx)
+        })
+        .await
+        .unwrap();
+
+    let snapshot = buffer.read_with(cx, |buffer, _| buffer.snapshot());
+
+    let uncommitted_diff = project
+        .update(cx, |project, cx| {
+            project.open_uncommitted_diff(buffer.clone(), cx)
+        })
+        .await
+        .unwrap();
+
+    uncommitted_diff.update(cx, |diff, cx| {
+        let hunks = diff.hunks(&snapshot, cx).collect::<Vec<_>>();
+        diff.stage_or_unstage_hunks(true, &hunks, &snapshot, true, cx);
+    });
+
+    cx.run_until_parked();
+
+    let output = smol::process::Command::new("git")
+        .current_dir(&work_dir)
+        .args(["diff", "--staged"])
+        .output()
+        .await
+        .unwrap();
+
+    let staged_diff = String::from_utf8_lossy(&output.stdout);
+
+    assert!(
+        !staged_diff.contains("new mode 100644"),
+        "Staging should not change file mode from 755 to 644.\ngit diff --staged:\n{}",
+        staged_diff
+    );
+
+    let output = smol::process::Command::new("git")
+        .current_dir(&work_dir)
+        .args(["ls-files", "-s"])
+        .output()
+        .await
+        .unwrap();
+    let index_contents = String::from_utf8_lossy(&output.stdout);
+
+    assert!(
+        index_contents.contains("100755"),
+        "Index should show file as executable (100755).\ngit ls-files -s:\n{}",
+        index_contents
+    );
+}
+
 #[gpui::test]
 async fn test_repository_and_path_for_project_path(
     background_executor: BackgroundExecutor,