Revert "git: Intercept signing prompt from GPG when committing" (#34306)

Reverts zed-industries/zed#34096

This introduced a regression, because the unlocked key can't benefit
from caching.

Release Notes:
- N/A

Cargo.lock

@@ -608,7 +608,6 @@ dependencies = [
  "parking_lot",
  "smol",
  "tempfile",
- "unindent",
  "util",
  "workspace-hack",
 ]

crates/askpass/Cargo.toml

@@ -19,6 +19,5 @@ net.workspace = true
 parking_lot.workspace = true
 smol.workspace = true
 tempfile.workspace = true
-unindent.workspace = true
 util.workspace = true
 workspace-hack.workspace = true

crates/askpass/src/askpass.rs

@@ -40,21 +40,11 @@ impl AskPassDelegate {
         self.tx.send((prompt, tx)).await?;
         Ok(rx.await?)
     }
-
-    pub fn new_always_failing() -> Self {
-        let (tx, _rx) = mpsc::unbounded::<(String, oneshot::Sender<String>)>();
-        Self {
-            tx,
-            _task: Task::ready(()),
-        }
-    }
 }
 
 pub struct AskPassSession {
     #[cfg(not(target_os = "windows"))]
     script_path: std::path::PathBuf,
-    #[cfg(not(target_os = "windows"))]
-    gpg_script_path: std::path::PathBuf,
     #[cfg(target_os = "windows")]
     askpass_helper: String,
     #[cfg(target_os = "windows")]
@@ -69,9 +59,6 @@ const ASKPASS_SCRIPT_NAME: &str = "askpass.sh";
 #[cfg(target_os = "windows")]
 const ASKPASS_SCRIPT_NAME: &str = "askpass.ps1";
 
-#[cfg(not(target_os = "windows"))]
-const GPG_SCRIPT_NAME: &str = "gpg.sh";
-
 impl AskPassSession {
     /// This will create a new AskPassSession.
     /// You must retain this session until the master process exits.
@@ -85,8 +72,6 @@ impl AskPassSession {
         let temp_dir = tempfile::Builder::new().prefix("zed-askpass").tempdir()?;
         let askpass_socket = temp_dir.path().join("askpass.sock");
         let askpass_script_path = temp_dir.path().join(ASKPASS_SCRIPT_NAME);
-        #[cfg(not(target_os = "windows"))]
-        let gpg_script_path = temp_dir.path().join(GPG_SCRIPT_NAME);
         let (askpass_opened_tx, askpass_opened_rx) = oneshot::channel::<()>();
         let listener = UnixListener::bind(&askpass_socket).context("creating askpass socket")?;
         #[cfg(not(target_os = "windows"))]
@@ -150,20 +135,9 @@ impl AskPassSession {
             askpass_script_path.display()
         );
 
-        #[cfg(not(target_os = "windows"))]
-        {
-            let gpg_script = generate_gpg_script();
-            fs::write(&gpg_script_path, gpg_script)
-                .await
-                .with_context(|| format!("creating gpg wrapper script at {gpg_script_path:?}"))?;
-            make_file_executable(&gpg_script_path).await?;
-        }
-
         Ok(Self {
             #[cfg(not(target_os = "windows"))]
             script_path: askpass_script_path,
-            #[cfg(not(target_os = "windows"))]
-            gpg_script_path,
 
             #[cfg(target_os = "windows")]
             secret,
@@ -186,19 +160,6 @@ impl AskPassSession {
         &self.askpass_helper
     }
 
-    #[cfg(not(target_os = "windows"))]
-    pub fn gpg_script_path(&self) -> Option<impl AsRef<OsStr>> {
-        Some(&self.gpg_script_path)
-    }
-
-    #[cfg(target_os = "windows")]
-    pub fn gpg_script_path(&self) -> Option<impl AsRef<OsStr>> {
-        // TODO implement wrapping GPG on Windows. This is more difficult than on Unix
-        // because we can't use --passphrase-fd with a nonstandard FD, and both --passphrase
-        // and --passphrase-file are insecure.
-        None::<std::path::PathBuf>
-    }
-
     // This will run the askpass task forever, resolving as many authentication requests as needed.
     // The caller is responsible for examining the result of their own commands and cancelling this
     // future when this is no longer needed. Note that this can only be called once, but due to the
@@ -302,23 +263,3 @@ fn generate_askpass_script(zed_path: &std::path::Path, askpass_socket: &std::pat
         askpass_socket = askpass_socket.display(),
     )
 }
-
-#[inline]
-#[cfg(not(target_os = "windows"))]
-fn generate_gpg_script() -> String {
-    use unindent::Unindent as _;
-
-    r#"
-        #!/bin/sh
-        set -eu
-
-        unset GIT_CONFIG_PARAMETERS
-        GPG_PROGRAM=$(git config gpg.program || echo 'gpg')
-        PROMPT="Enter passphrase to unlock GPG key:"
-        PASSPHRASE=$(${GIT_ASKPASS} "${PROMPT}")
-
-        exec "${GPG_PROGRAM}" --batch --no-tty --yes --passphrase-fd 3 --pinentry-mode loopback "$@" 3<<EOF
-        ${PASSPHRASE}
-        EOF
-    "#.unindent()
-}

crates/fs/src/fake_git_repo.rs

@@ -375,10 +375,8 @@ impl GitRepository for FakeGitRepository {
         _message: gpui::SharedString,
         _name_and_email: Option<(gpui::SharedString, gpui::SharedString)>,
         _options: CommitOptions,
-        _ask_pass: AskPassDelegate,
         _env: Arc<HashMap<String, String>>,
-        _cx: AsyncApp,
+    ) -> BoxFuture<'_, Result<()>> {
-    ) -> BoxFuture<'static, Result<()>> {
         unimplemented!()
     }
 

crates/git/Cargo.toml

@@ -41,9 +41,9 @@ futures.workspace = true
 workspace-hack.workspace = true
 
 [dev-dependencies]
-gpui = { workspace = true, features = ["test-support"] }
 pretty_assertions.workspace = true
 serde_json.workspace = true
-tempfile.workspace = true
 text = { workspace = true, features = ["test-support"] }
 unindent.workspace = true
+gpui = { workspace = true, features = ["test-support"] }
+tempfile.workspace = true

crates/git/src/repository.rs

@@ -391,12 +391,8 @@ pub trait GitRepository: Send + Sync {
         message: SharedString,
         name_and_email: Option<(SharedString, SharedString)>,
         options: CommitOptions,
-        askpass: AskPassDelegate,
         env: Arc<HashMap<String, String>>,
-        // This method takes an AsyncApp to ensure it's invoked on the main thread,
+    ) -> BoxFuture<'_, Result<()>>;
-        // otherwise git-credentials-manager won't work.
-        cx: AsyncApp,
-    ) -> BoxFuture<'static, Result<()>>;
 
     fn push(
         &self,
@@ -1197,68 +1193,36 @@ impl GitRepository for RealGitRepository {
         message: SharedString,
         name_and_email: Option<(SharedString, SharedString)>,
         options: CommitOptions,
-        ask_pass: AskPassDelegate,
         env: Arc<HashMap<String, String>>,
-        cx: AsyncApp,
+    ) -> BoxFuture<'_, Result<()>> {
-    ) -> BoxFuture<'static, Result<()>> {
         let working_directory = self.working_directory();
-        let executor = cx.background_executor().clone();
+        self.executor
-        async move {
+            .spawn(async move {
-            let working_directory = working_directory?;
+                let mut cmd = new_smol_command("git");
-            let have_user_git_askpass = env.contains_key("GIT_ASKPASS");
+                cmd.current_dir(&working_directory?)
-            let mut command = new_smol_command("git");
+                    .envs(env.iter())
-            command.current_dir(&working_directory).envs(env.iter());
+                    .args(["commit", "--quiet", "-m"])
+                    .arg(&message.to_string())
+                    .arg("--cleanup=strip");
 
-            let ask_pass = if have_user_git_askpass {
+                if options.amend {
-                None
+                    cmd.arg("--amend");
-            } else {
+                }
-                Some(AskPassSession::new(&executor, ask_pass).await?)
-            };
 
-            if let Some(program) = ask_pass
+                if let Some((name, email)) = name_and_email {
-                .as_ref()
+                    cmd.arg("--author").arg(&format!("{name} <{email}>"));
-                .and_then(|ask_pass| ask_pass.gpg_script_path())
+                }
-            {
-                command.arg("-c").arg(format!(
-                    "gpg.program={}",
-                    program.as_ref().to_string_lossy()
-                ));
-            }
 
-            command
+                let output = cmd.output().await?;
-                .args(["commit", "-m"])
-                .arg(message.to_string())
-                .arg("--cleanup=strip")
-                .stdin(smol::process::Stdio::null())
-                .stdout(smol::process::Stdio::piped())
-                .stderr(smol::process::Stdio::piped());
 
-            if options.amend {
-                command.arg("--amend");
-            }
-
-            if let Some((name, email)) = name_and_email {
-                command.arg("--author").arg(&format!("{name} <{email}>"));
-            }
-
-            if let Some(ask_pass) = ask_pass {
-                command.env("GIT_ASKPASS", ask_pass.script_path());
-                let git_process = command.spawn()?;
-
-                run_askpass_command(ask_pass, git_process).await?;
-                Ok(())
-            } else {
-                let git_process = command.spawn()?;
-                let output = git_process.output().await?;
                 anyhow::ensure!(
                     output.status.success(),
-                    "{}",
+                    "Failed to commit:\n{}",
                     String::from_utf8_lossy(&output.stderr)
                 );
                 Ok(())
-            }
+            })
-        }
+            .boxed()
-        .boxed()
     }
 
     fn push(
@@ -2082,16 +2046,12 @@ mod tests {
         )
         .await
         .unwrap();
-        cx.spawn(|cx| {
+        repo.commit(
-            repo.commit(
+            "Initial commit".into(),
-                "Initial commit".into(),
+            None,
-                None,
+            CommitOptions::default(),
-                CommitOptions::default(),
+            Arc::new(checkpoint_author_envs()),
-                AskPassDelegate::new_always_failing(),
+        )
-                Arc::new(checkpoint_author_envs()),
-                cx,
-            )
-        })
         .await
         .unwrap();
 
@@ -2115,16 +2075,12 @@ mod tests {
         )
         .await
         .unwrap();
-        cx.spawn(|cx| {
+        repo.commit(
-            repo.commit(
+            "Commit after checkpoint".into(),
-                "Commit after checkpoint".into(),
+            None,
-                None,
+            CommitOptions::default(),
-                CommitOptions::default(),
+            Arc::new(checkpoint_author_envs()),
-                AskPassDelegate::new_always_failing(),
+        )
-                Arc::new(checkpoint_author_envs()),
-                cx,
-            )
-        })
         .await
         .unwrap();
 
@@ -2257,16 +2213,12 @@ mod tests {
         )
         .await
         .unwrap();
-        cx.spawn(|cx| {
+        repo.commit(
-            repo.commit(
+            "Initial commit".into(),
-                "Initial commit".into(),
+            None,
-                None,
+            CommitOptions::default(),
-                CommitOptions::default(),
+            Arc::new(checkpoint_author_envs()),
-                AskPassDelegate::new_always_failing(),
+        )
-                Arc::new(checkpoint_author_envs()),
-                cx,
-            )
-        })
         .await
         .unwrap();
 

crates/git_ui/src/git_panel.rs

@@ -1574,15 +1574,10 @@ impl GitPanel {
 
         let task = if self.has_staged_changes() {
             // Repository serializes all git operations, so we can just send a commit immediately
-            cx.spawn_in(window, async move |this, cx| {
+            let commit_task = active_repository.update(cx, |repo, cx| {
-                let askpass_delegate = this.update_in(cx, |this, window, cx| {
+                repo.commit(message.into(), None, options, cx)
-                    this.askpass_delegate("git commit", window, cx)
+            });
-                })?;
+            cx.background_spawn(async move { commit_task.await? })
-                let commit_task = active_repository.update(cx, |repo, cx| {
-                    repo.commit(message.into(), None, options, askpass_delegate, cx)
-                })?;
-                commit_task.await?
-            })
         } else {
             let changed_files = self
                 .entries
@@ -1599,13 +1594,10 @@ impl GitPanel {
 
             let stage_task =
                 active_repository.update(cx, |repo, cx| repo.stage_entries(changed_files, cx));
-            cx.spawn_in(window, async move |this, cx| {
+            cx.spawn(async move |_, cx| {
                 stage_task.await?;
-                let askpass_delegate = this.update_in(cx, |this, window, cx| {
-                    this.askpass_delegate("git commit".to_string(), window, cx)
-                })?;
                 let commit_task = active_repository.update(cx, |repo, cx| {
-                    repo.commit(message.into(), None, options, askpass_delegate, cx)
+                    repo.commit(message.into(), None, options, cx)
                 })?;
                 commit_task.await?
             })

crates/project/src/git_store.rs

@@ -1726,18 +1726,6 @@ impl GitStore {
         let repository_id = RepositoryId::from_proto(envelope.payload.repository_id);
         let repository_handle = Self::repository_for_request(&this, repository_id, &mut cx)?;
 
-        let askpass = if let Some(askpass_id) = envelope.payload.askpass_id {
-            make_remote_delegate(
-                this,
-                envelope.payload.project_id,
-                repository_id,
-                askpass_id,
-                &mut cx,
-            )
-        } else {
-            AskPassDelegate::new_always_failing()
-        };
-
         let message = SharedString::from(envelope.payload.message);
         let name = envelope.payload.name.map(SharedString::from);
         let email = envelope.payload.email.map(SharedString::from);
@@ -1751,7 +1739,6 @@ impl GitStore {
                     CommitOptions {
                         amend: options.amend,
                     },
-                    askpass,
                     cx,
                 )
             })?
@@ -3475,14 +3462,11 @@ impl Repository {
         message: SharedString,
         name_and_email: Option<(SharedString, SharedString)>,
         options: CommitOptions,
-        askpass: AskPassDelegate,
         _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);
 
-        self.send_job(Some("git commit".into()), move |git_repo, cx| async move {
+        self.send_job(Some("git commit".into()), move |git_repo, _cx| async move {
             match git_repo {
                 RepositoryState::Local {
                     backend,
@@ -3490,16 +3474,10 @@ impl Repository {
                     ..
                 } => {
                     backend
-                        .commit(message, name_and_email, options, askpass, environment, cx)
+                        .commit(message, name_and_email, options, 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 {
@@ -3511,9 +3489,9 @@ impl Repository {
                             options: Some(proto::commit::CommitOptions {
                                 amend: options.amend,
                             }),
-                            askpass_id: Some(askpass_id),
                         })
-                        .await?;
+                        .await
+                        .context("sending commit request")?;
 
                     Ok(())
                 }

crates/proto/proto/git.proto

@@ -294,7 +294,7 @@ message Commit {
     optional string email = 5;
     string message = 6;
     optional CommitOptions options = 7;
-    optional uint64 askpass_id = 8;
+    reserved 8;
 
     message CommitOptions {
         bool amend = 1;