git: Handle git pre-commit hooks separately (#43285)

Jakub Konka and Cole Miller created

We now run git pre-commit hooks before we commit. This ensures we don't
run into timeout issues with askpass delegate and report invalid error
to the user.

Closes #43157

Release Notes:

- Fixed long running pre-commit hooks causing committing from Zed to
fail.

Co-authored-by: Cole Miller <cole@zed.dev>

Change summary

crates/collab/src/rpc.rs        |  1 
crates/fs/src/fake_git_repo.rs  | 10 ++++++
crates/git/src/git.rs           | 25 +++++++++++++++++
crates/git/src/repository.rs    | 29 +++++++++++++++++++
crates/project/src/git_store.rs | 51 +++++++++++++++++++++++++++++++++-
crates/proto/proto/git.proto    | 10 ++++++
crates/proto/proto/zed.proto    |  4 ++
crates/proto/src/proto.rs       |  3 ++
8 files changed, 128 insertions(+), 5 deletions(-)

Detailed changes

crates/collab/src/rpc.rs 🔗

@@ -453,6 +453,7 @@ impl Server {
             .add_request_handler(forward_mutating_project_request::<proto::StashPop>)
             .add_request_handler(forward_mutating_project_request::<proto::StashDrop>)
             .add_request_handler(forward_mutating_project_request::<proto::Commit>)
+            .add_request_handler(forward_mutating_project_request::<proto::RunGitHook>)
             .add_request_handler(forward_mutating_project_request::<proto::GitInit>)
             .add_request_handler(forward_read_only_project_request::<proto::GetRemotes>)
             .add_request_handler(forward_read_only_project_request::<proto::GitShow>)

crates/fs/src/fake_git_repo.rs 🔗

@@ -3,7 +3,7 @@ use anyhow::{Context as _, Result, bail};
 use collections::{HashMap, HashSet};
 use futures::future::{self, BoxFuture, join_all};
 use git::{
-    Oid,
+    Oid, RunHook,
     blame::Blame,
     repository::{
         AskPassDelegate, Branch, CommitDetails, CommitOptions, FetchOptions, GitRepository,
@@ -532,6 +532,14 @@ impl GitRepository for FakeGitRepository {
         unimplemented!()
     }
 
+    fn run_hook(
+        &self,
+        _hook: RunHook,
+        _env: Arc<HashMap<String, String>>,
+    ) -> BoxFuture<'_, Result<()>> {
+        unimplemented!()
+    }
+
     fn push(
         &self,
         _branch: String,

crates/git/src/git.rs 🔗

@@ -225,3 +225,28 @@ impl From<Oid> for usize {
         u64::from_ne_bytes(u64_bytes) as usize
     }
 }
+
+#[repr(i32)]
+#[derive(Copy, Clone, Debug)]
+pub enum RunHook {
+    PreCommit,
+}
+
+impl RunHook {
+    pub fn as_str(&self) -> &str {
+        match self {
+            Self::PreCommit => "pre-commit",
+        }
+    }
+
+    pub fn to_proto(&self) -> i32 {
+        *self as i32
+    }
+
+    pub fn from_proto(value: i32) -> Option<Self> {
+        match value {
+            0 => Some(Self::PreCommit),
+            _ => None,
+        }
+    }
+}

crates/git/src/repository.rs 🔗

@@ -1,7 +1,7 @@
 use crate::commit::parse_git_diff_name_status;
 use crate::stash::GitStash;
 use crate::status::{DiffTreeType, GitStatus, StatusCode, TreeDiff};
-use crate::{Oid, SHORT_SHA_LENGTH};
+use crate::{Oid, RunHook, SHORT_SHA_LENGTH};
 use anyhow::{Context as _, Result, anyhow, bail};
 use collections::HashMap;
 use futures::future::BoxFuture;
@@ -485,6 +485,12 @@ pub trait GitRepository: Send + Sync {
         env: Arc<HashMap<String, String>>,
     ) -> BoxFuture<'_, Result<()>>;
 
+    fn run_hook(
+        &self,
+        hook: RunHook,
+        env: Arc<HashMap<String, String>>,
+    ) -> BoxFuture<'_, Result<()>>;
+
     fn commit(
         &self,
         message: SharedString,
@@ -1643,6 +1649,7 @@ impl GitRepository for RealGitRepository {
                 .args(["commit", "--quiet", "-m"])
                 .arg(&message.to_string())
                 .arg("--cleanup=strip")
+                .arg("--no-verify")
                 .stdout(smol::process::Stdio::piped())
                 .stderr(smol::process::Stdio::piped());
 
@@ -2037,6 +2044,26 @@ impl GitRepository for RealGitRepository {
             })
             .boxed()
     }
+
+    fn run_hook(
+        &self,
+        hook: RunHook,
+        env: Arc<HashMap<String, String>>,
+    ) -> BoxFuture<'_, Result<()>> {
+        let working_directory = self.working_directory();
+        let git_binary_path = self.any_git_binary_path.clone();
+        let executor = self.executor.clone();
+        self.executor
+            .spawn(async move {
+                let working_directory = working_directory?;
+                let git = GitBinary::new(git_binary_path, working_directory, executor)
+                    .envs(HashMap::clone(&env));
+                git.run(&["hook", "run", "--ignore-missing", hook.as_str()])
+                    .await?;
+                Ok(())
+            })
+            .boxed()
+    }
 }
 
 fn git_status_args(path_prefixes: &[RepoPath]) -> Vec<OsString> {

crates/project/src/git_store.rs 🔗

@@ -25,7 +25,7 @@ use futures::{
     stream::FuturesOrdered,
 };
 use git::{
-    BuildPermalinkParams, GitHostingProviderRegistry, Oid,
+    BuildPermalinkParams, GitHostingProviderRegistry, Oid, RunHook,
     blame::Blame,
     parse_git_remote_url,
     repository::{
@@ -433,6 +433,7 @@ impl GitStore {
         client.add_entity_request_handler(Self::handle_stash_apply);
         client.add_entity_request_handler(Self::handle_stash_drop);
         client.add_entity_request_handler(Self::handle_commit);
+        client.add_entity_request_handler(Self::handle_run_hook);
         client.add_entity_request_handler(Self::handle_reset);
         client.add_entity_request_handler(Self::handle_show);
         client.add_entity_request_handler(Self::handle_load_commit_diff);
@@ -1982,6 +1983,22 @@ impl GitStore {
         Ok(proto::Ack {})
     }
 
+    async fn handle_run_hook(
+        this: Entity<Self>,
+        envelope: TypedEnvelope<proto::RunGitHook>,
+        mut cx: AsyncApp,
+    ) -> Result<proto::Ack> {
+        let repository_id = RepositoryId::from_proto(envelope.payload.repository_id);
+        let repository_handle = Self::repository_for_request(&this, repository_id, &mut cx)?;
+        let hook = RunHook::from_proto(envelope.payload.hook).context("invalid hook")?;
+        repository_handle
+            .update(&mut cx, |repository_handle, cx| {
+                repository_handle.run_hook(hook, cx)
+            })?
+            .await??;
+        Ok(proto::Ack {})
+    }
+
     async fn handle_commit(
         this: Entity<Self>,
         envelope: TypedEnvelope<proto::Commit>,
@@ -4262,19 +4279,49 @@ impl Repository {
         })
     }
 
+    pub fn run_hook(&mut self, hook: RunHook, _cx: &mut App) -> oneshot::Receiver<Result<()>> {
+        let id = self.id;
+        self.send_job(
+            Some(format!("git hook {}", hook.as_str()).into()),
+            move |git_repo, _cx| async move {
+                match git_repo {
+                    RepositoryState::Local {
+                        backend,
+                        environment,
+                    } => backend.run_hook(hook, environment.clone()).await,
+                    RepositoryState::Remote { project_id, client } => {
+                        client
+                            .request(proto::RunGitHook {
+                                project_id: project_id.0,
+                                repository_id: id.to_proto(),
+                                hook: hook.to_proto(),
+                            })
+                            .await?;
+
+                        Ok(())
+                    }
+                }
+            },
+        )
+    }
+
     pub fn commit(
         &mut self,
         message: SharedString,
         name_and_email: Option<(SharedString, SharedString)>,
         options: CommitOptions,
         askpass: AskPassDelegate,
-        _cx: &mut App,
+        cx: &mut App,
     ) -> oneshot::Receiver<Result<()>> {
         let id = self.id;
         let askpass_delegates = self.askpass_delegates.clone();
         let askpass_id = util::post_inc(&mut self.latest_askpass_id);
 
+        let rx = self.run_hook(RunHook::PreCommit, cx);
+
         self.send_job(Some("git commit".into()), move |git_repo, _cx| async move {
+            rx.await??;
+
             match git_repo {
                 RepositoryState::Local {
                     backend,

crates/proto/proto/git.proto 🔗

@@ -531,3 +531,13 @@ message GitCreateWorktree {
     string directory = 4;
     optional string commit = 5;
 }
+
+message RunGitHook {
+    enum GitHook {
+        PRE_COMMIT = 0;
+    }
+
+    uint64 project_id = 1;
+    uint64 repository_id = 2;
+    GitHook hook = 3;
+}

crates/proto/proto/zed.proto 🔗

@@ -437,7 +437,9 @@ message Envelope {
         OpenImageResponse open_image_response = 392;
         CreateImageForPeer create_image_for_peer = 393;
 
-        ExternalExtensionAgentsUpdated external_extension_agents_updated = 394; // current max
+        ExternalExtensionAgentsUpdated external_extension_agents_updated = 394;
+
+        RunGitHook run_git_hook = 395; // current max
     }
 
     reserved 87 to 88;

crates/proto/src/proto.rs 🔗

@@ -49,6 +49,7 @@ messages!(
     (ChannelMessageUpdate, Foreground),
     (CloseBuffer, Foreground),
     (Commit, Background),
+    (RunGitHook, Background),
     (CopyProjectEntry, Foreground),
     (CreateBufferForPeer, Foreground),
     (CreateImageForPeer, Foreground),
@@ -349,6 +350,7 @@ request_messages!(
     (Call, Ack),
     (CancelCall, Ack),
     (Commit, Ack),
+    (RunGitHook, Ack),
     (CopyProjectEntry, ProjectEntryResponse),
     (CreateChannel, CreateChannelResponse),
     (CreateProjectEntry, ProjectEntryResponse),
@@ -547,6 +549,7 @@ entity_messages!(
     BufferSaved,
     CloseBuffer,
     Commit,
+    RunGitHook,
     GetColorPresentation,
     CopyProjectEntry,
     CreateBufferForPeer,