From 359160c8b15a276d11af5d269d9fb1f0ebcd1765 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Mon, 10 Nov 2025 08:57:50 +0100 Subject: [PATCH] git: Add askpass delegate to git-commit handlers (#42239) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In my local setup, I always enforce git-commit signing with GPG/SSH which automatically enforces `git commit -S` when committing. This changeset will now show a modal to the user for them to specify the passphrase (if any) so that they can unlock their private key for signing when committing in Zed. Screenshot 2025-11-07 at 11 09 09 PM Release Notes: - Handle automatic git-commit signing by presenting the user with an askpass modal --- crates/fs/src/fake_git_repo.rs | 1 + crates/git/src/repository.rs | 70 +++++++++++++++++++++------------ crates/git_ui/src/git_panel.rs | 5 ++- crates/project/src/git_store.rs | 21 +++++++++- crates/proto/proto/git.proto | 1 + 5 files changed, 69 insertions(+), 29 deletions(-) diff --git a/crates/fs/src/fake_git_repo.rs b/crates/fs/src/fake_git_repo.rs index aeaed1d6fc2947e55551026d518da18952cc051a..e3d58f3001c407f8d4deea115b460000bc666574 100644 --- a/crates/fs/src/fake_git_repo.rs +++ b/crates/fs/src/fake_git_repo.rs @@ -526,6 +526,7 @@ impl GitRepository for FakeGitRepository { _message: gpui::SharedString, _name_and_email: Option<(gpui::SharedString, gpui::SharedString)>, _options: CommitOptions, + _askpass: AskPassDelegate, _env: Arc>, ) -> BoxFuture<'_, Result<()>> { unimplemented!() diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index 6fcf285e384f4a03a0f3fe8d2a613a56ace4666e..1ad21d993607c75777302ddb6f4ec1964a916ad0 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -491,6 +491,7 @@ pub trait GitRepository: Send + Sync { message: SharedString, name_and_email: Option<(SharedString, SharedString)>, options: CommitOptions, + askpass: AskPassDelegate, env: Arc>, ) -> BoxFuture<'_, Result<()>>; @@ -1630,41 +1631,39 @@ impl GitRepository for RealGitRepository { message: SharedString, name_and_email: Option<(SharedString, SharedString)>, options: CommitOptions, + ask_pass: AskPassDelegate, env: Arc>, ) -> BoxFuture<'_, Result<()>> { let working_directory = self.working_directory(); let git_binary_path = self.any_git_binary_path.clone(); - self.executor - .spawn(async move { - let mut cmd = new_smol_command(git_binary_path); - cmd.current_dir(&working_directory?) - .envs(env.iter()) - .args(["commit", "--quiet", "-m"]) - .arg(&message.to_string()) - .arg("--cleanup=strip"); + let executor = self.executor.clone(); + async move { + let mut cmd = new_smol_command(git_binary_path); + cmd.current_dir(&working_directory?) + .envs(env.iter()) + .args(["commit", "--quiet", "-m"]) + .arg(&message.to_string()) + .arg("--cleanup=strip") + .stdout(smol::process::Stdio::piped()) + .stderr(smol::process::Stdio::piped()); - if options.amend { - cmd.arg("--amend"); - } + if options.amend { + cmd.arg("--amend"); + } - if options.signoff { - cmd.arg("--signoff"); - } + if options.signoff { + cmd.arg("--signoff"); + } - if let Some((name, email)) = name_and_email { - cmd.arg("--author").arg(&format!("{name} <{email}>")); - } + if let Some((name, email)) = name_and_email { + cmd.arg("--author").arg(&format!("{name} <{email}>")); + } - let output = cmd.output().await?; + run_git_command(env, ask_pass, cmd, &executor).await?; - anyhow::ensure!( - output.status.success(), - "Failed to commit:\n{}", - String::from_utf8_lossy(&output.stderr) - ); - Ok(()) - }) - .boxed() + Ok(()) + } + .boxed() } fn push( @@ -2469,8 +2468,17 @@ mod tests { use super::*; use gpui::TestAppContext; + fn disable_git_global_config() { + unsafe { + std::env::set_var("GIT_CONFIG_GLOBAL", ""); + std::env::set_var("GIT_CONFIG_SYSTEM", ""); + } + } + #[gpui::test] async fn test_checkpoint_basic(cx: &mut TestAppContext) { + disable_git_global_config(); + cx.executor().allow_parking(); let repo_dir = tempfile::tempdir().unwrap(); @@ -2486,6 +2494,7 @@ mod tests { cx.executor(), ) .unwrap(); + repo.stage_paths(vec![repo_path("file")], Arc::new(HashMap::default())) .await .unwrap(); @@ -2493,6 +2502,7 @@ mod tests { "Initial commit".into(), None, CommitOptions::default(), + AskPassDelegate::new(&mut cx.to_async(), |_, _, _| {}), Arc::new(checkpoint_author_envs()), ) .await @@ -2519,6 +2529,7 @@ mod tests { "Commit after checkpoint".into(), None, CommitOptions::default(), + AskPassDelegate::new(&mut cx.to_async(), |_, _, _| {}), Arc::new(checkpoint_author_envs()), ) .await @@ -2556,6 +2567,8 @@ mod tests { #[gpui::test] async fn test_checkpoint_empty_repo(cx: &mut TestAppContext) { + disable_git_global_config(); + cx.executor().allow_parking(); let repo_dir = tempfile::tempdir().unwrap(); @@ -2600,6 +2613,8 @@ mod tests { #[gpui::test] async fn test_compare_checkpoints(cx: &mut TestAppContext) { + disable_git_global_config(); + cx.executor().allow_parking(); let repo_dir = tempfile::tempdir().unwrap(); @@ -2639,6 +2654,8 @@ mod tests { #[gpui::test] async fn test_checkpoint_exclude_binary_files(cx: &mut TestAppContext) { + disable_git_global_config(); + cx.executor().allow_parking(); let repo_dir = tempfile::tempdir().unwrap(); @@ -2669,6 +2686,7 @@ mod tests { "Initial commit".into(), None, CommitOptions::default(), + AskPassDelegate::new(&mut cx.to_async(), |_, _, _| {}), Arc::new(checkpoint_author_envs()), ) .await diff --git a/crates/git_ui/src/git_panel.rs b/crates/git_ui/src/git_panel.rs index cb292a62bf6ebf4ae3e3c90f269bd839b971e22e..aec36e1730b282e94e5dba6847eb55ab464beb00 100644 --- a/crates/git_ui/src/git_panel.rs +++ b/crates/git_ui/src/git_panel.rs @@ -1585,6 +1585,7 @@ impl GitPanel { return; } + let askpass = self.askpass_delegate("git commit", window, cx); let commit_message = self.custom_or_suggested_commit_message(window, cx); let Some(mut message) = commit_message else { @@ -1599,7 +1600,7 @@ impl GitPanel { let task = if self.has_staged_changes() { // Repository serializes all git operations, so we can just send a commit immediately let commit_task = active_repository.update(cx, |repo, cx| { - repo.commit(message.into(), None, options, cx) + repo.commit(message.into(), None, options, askpass, cx) }); cx.background_spawn(async move { commit_task.await? }) } else { @@ -1621,7 +1622,7 @@ impl GitPanel { cx.spawn(async move |_, cx| { stage_task.await?; let commit_task = active_repository.update(cx, |repo, cx| { - repo.commit(message.into(), None, options, cx) + repo.commit(message.into(), None, options, askpass, cx) })?; commit_task.await? }) diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index 90d76f51be27c66894519ea22ddcaa19baedc9c4..b4b353faadcaa359f85d263e63d4e370aaec1e4a 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -1957,6 +1957,15 @@ impl GitStore { ) -> Result { let repository_id = RepositoryId::from_proto(envelope.payload.repository_id); let repository_handle = Self::repository_for_request(&this, repository_id, &mut cx)?; + let askpass_id = envelope.payload.askpass_id; + + let askpass = make_remote_delegate( + this, + envelope.payload.project_id, + repository_id, + askpass_id, + &mut cx, + ); let message = SharedString::from(envelope.payload.message); let name = envelope.payload.name.map(SharedString::from); @@ -1972,6 +1981,7 @@ impl GitStore { amend: options.amend, signoff: options.signoff, }, + askpass, cx, ) })? @@ -4216,9 +4226,12 @@ impl Repository { message: SharedString, name_and_email: Option<(SharedString, SharedString)>, options: CommitOptions, + askpass: AskPassDelegate, _cx: &mut App, ) -> oneshot::Receiver> { let id = self.id; + let askpass_delegates = self.askpass_delegates.clone(); + let askpass_id = util::post_inc(&mut self.latest_askpass_id); self.send_job(Some("git commit".into()), move |git_repo, _cx| async move { match git_repo { @@ -4228,10 +4241,15 @@ impl Repository { .. } => { backend - .commit(message, name_and_email, options, environment) + .commit(message, name_and_email, options, askpass, environment) .await } RepositoryState::Remote { project_id, client } => { + askpass_delegates.lock().insert(askpass_id, askpass); + let _defer = util::defer(|| { + let askpass_delegate = askpass_delegates.lock().remove(&askpass_id); + debug_assert!(askpass_delegate.is_some()); + }); let (name, email) = name_and_email.unzip(); client .request(proto::Commit { @@ -4244,6 +4262,7 @@ impl Repository { amend: options.amend, signoff: options.signoff, }), + askpass_id, }) .await .context("sending commit request")?; diff --git a/crates/proto/proto/git.proto b/crates/proto/proto/git.proto index d8ce1d2a75633eb4f31378ee574ffe043f956e05..efbd7f616f9e75c4e0409f4dc73c67f9eb1836e0 100644 --- a/crates/proto/proto/git.proto +++ b/crates/proto/proto/git.proto @@ -347,6 +347,7 @@ message Commit { string message = 6; optional CommitOptions options = 7; reserved 8; + uint64 askpass_id = 9; message CommitOptions { bool amend = 1;