If GIT_ASKPASS is already set, assume it will do the right thing (#27681)

Conrad Irwin created

Fixes running git push on a coder instance.

Closes #ISSUE

Release Notes:

- Zed will now use `GIT_ASKPASS` if you already have one set instead of
overriding with our own. Fixes `git push` in Coder.

Change summary

crates/fs/src/fake_git_repo.rs  | 10 ++--
crates/git/src/repository.rs    | 86 +++++++++++++++++++++-------------
crates/project/src/git_store.rs |  9 --
3 files changed, 61 insertions(+), 44 deletions(-)

Detailed changes

crates/fs/src/fake_git_repo.rs 🔗

@@ -5,8 +5,8 @@ use futures::future::{self, BoxFuture};
 use git::{
     blame::Blame,
     repository::{
-        AskPassSession, Branch, CommitDetails, GitRepository, GitRepositoryCheckpoint, PushOptions,
-        Remote, RepoPath, ResetMode,
+        AskPassDelegate, Branch, CommitDetails, GitRepository, GitRepositoryCheckpoint,
+        PushOptions, Remote, RepoPath, ResetMode,
     },
     status::{FileStatus, GitStatus, StatusCode, TrackedStatus, UnmergedStatus},
 };
@@ -370,7 +370,7 @@ impl GitRepository for FakeGitRepository {
         _branch: String,
         _remote: String,
         _options: Option<PushOptions>,
-        _askpass: AskPassSession,
+        _askpass: AskPassDelegate,
         _env: HashMap<String, String>,
         _cx: AsyncApp,
     ) -> BoxFuture<Result<git::repository::RemoteCommandOutput>> {
@@ -381,7 +381,7 @@ impl GitRepository for FakeGitRepository {
         &self,
         _branch: String,
         _remote: String,
-        _askpass: AskPassSession,
+        _askpass: AskPassDelegate,
         _env: HashMap<String, String>,
         _cx: AsyncApp,
     ) -> BoxFuture<Result<git::repository::RemoteCommandOutput>> {
@@ -390,7 +390,7 @@ impl GitRepository for FakeGitRepository {
 
     fn fetch(
         &self,
-        _askpass: AskPassSession,
+        _askpass: AskPassDelegate,
         _env: HashMap<String, String>,
         _cx: AsyncApp,
     ) -> BoxFuture<Result<git::repository::RemoteCommandOutput>> {

crates/git/src/repository.rs 🔗

@@ -27,7 +27,7 @@ use util::command::{new_smol_command, new_std_command};
 use util::ResultExt;
 use uuid::Uuid;
 
-pub use askpass::{AskPassResult, AskPassSession};
+pub use askpass::{AskPassDelegate, AskPassResult, AskPassSession};
 
 pub const REMOTE_CANCELLED_BY_USER: &str = "Operation cancelled by user";
 
@@ -249,7 +249,7 @@ pub trait GitRepository: Send + Sync {
         branch_name: String,
         upstream_name: String,
         options: Option<PushOptions>,
-        askpass: AskPassSession,
+        askpass: AskPassDelegate,
         env: HashMap<String, String>,
         // This method takes an AsyncApp to ensure it's invoked on the main thread,
         // otherwise git-credentials-manager won't work.
@@ -260,7 +260,7 @@ pub trait GitRepository: Send + Sync {
         &self,
         branch_name: String,
         upstream_name: String,
-        askpass: AskPassSession,
+        askpass: AskPassDelegate,
         env: HashMap<String, String>,
         // This method takes an AsyncApp to ensure it's invoked on the main thread,
         // otherwise git-credentials-manager won't work.
@@ -269,7 +269,7 @@ pub trait GitRepository: Send + Sync {
 
     fn fetch(
         &self,
-        askpass: AskPassSession,
+        askpass: AskPassDelegate,
         env: HashMap<String, String>,
         // This method takes an AsyncApp to ensure it's invoked on the main thread,
         // otherwise git-credentials-manager won't work.
@@ -868,20 +868,17 @@ impl GitRepository for RealGitRepository {
         branch_name: String,
         remote_name: String,
         options: Option<PushOptions>,
-        ask_pass: AskPassSession,
+        ask_pass: AskPassDelegate,
         env: HashMap<String, String>,
-        _cx: AsyncApp,
+        cx: AsyncApp,
     ) -> BoxFuture<Result<RemoteCommandOutput>> {
         let working_directory = self.working_directory();
+        let executor = cx.background_executor().clone();
         async move {
             let working_directory = working_directory?;
-
             let mut command = new_smol_command("git");
             command
-                .envs(env)
-                .env("GIT_ASKPASS", ask_pass.script_path())
-                .env("SSH_ASKPASS", ask_pass.script_path())
-                .env("SSH_ASKPASS_REQUIRE", "force")
+                .envs(&env)
                 .env("GIT_HTTP_USER_AGENT", "Zed")
                 .current_dir(&working_directory)
                 .args(["push"])
@@ -894,9 +891,8 @@ impl GitRepository for RealGitRepository {
                 .stdin(smol::process::Stdio::null())
                 .stdout(smol::process::Stdio::piped())
                 .stderr(smol::process::Stdio::piped());
-            let git_process = command.spawn()?;
 
-            run_remote_command(ask_pass, git_process).await
+            run_git_command(env, ask_pass, command, &executor).await
         }
         .boxed()
     }
@@ -905,52 +901,48 @@ impl GitRepository for RealGitRepository {
         &self,
         branch_name: String,
         remote_name: String,
-        ask_pass: AskPassSession,
+        ask_pass: AskPassDelegate,
         env: HashMap<String, String>,
-        _cx: AsyncApp,
+        cx: AsyncApp,
     ) -> BoxFuture<Result<RemoteCommandOutput>> {
         let working_directory = self.working_directory();
-        async {
+        let executor = cx.background_executor().clone();
+        async move {
             let mut command = new_smol_command("git");
             command
-                .envs(env)
-                .env("GIT_ASKPASS", ask_pass.script_path())
-                .env("SSH_ASKPASS", ask_pass.script_path())
-                .env("SSH_ASKPASS_REQUIRE", "force")
+                .envs(&env)
+                .env("GIT_HTTP_USER_AGENT", "Zed")
                 .current_dir(&working_directory?)
                 .args(["pull"])
                 .arg(remote_name)
                 .arg(branch_name)
                 .stdout(smol::process::Stdio::piped())
                 .stderr(smol::process::Stdio::piped());
-            let git_process = command.spawn()?;
 
-            run_remote_command(ask_pass, git_process).await
+            run_git_command(env, ask_pass, command, &executor).await
         }
         .boxed()
     }
 
     fn fetch(
         &self,
-        ask_pass: AskPassSession,
+        ask_pass: AskPassDelegate,
         env: HashMap<String, String>,
-        _cx: AsyncApp,
+        cx: AsyncApp,
     ) -> BoxFuture<Result<RemoteCommandOutput>> {
         let working_directory = self.working_directory();
-        async {
+        let executor = cx.background_executor().clone();
+        async move {
             let mut command = new_smol_command("git");
             command
-                .envs(env)
-                .env("GIT_ASKPASS", ask_pass.script_path())
-                .env("SSH_ASKPASS", ask_pass.script_path())
-                .env("SSH_ASKPASS_REQUIRE", "force")
+                .envs(&env)
+                .env("GIT_HTTP_USER_AGENT", "Zed")
                 .current_dir(&working_directory?)
                 .args(["fetch", "--all"])
                 .stdout(smol::process::Stdio::piped())
                 .stderr(smol::process::Stdio::piped());
-            let git_process = command.spawn()?;
 
-            run_remote_command(ask_pass, git_process).await
+            run_git_command(env, ask_pass, command, &executor).await
         }
         .boxed()
     }
@@ -1355,7 +1347,37 @@ struct GitBinaryCommandError {
     status: ExitStatus,
 }
 
-async fn run_remote_command(
+async fn run_git_command(
+    env: HashMap<String, String>,
+    ask_pass: AskPassDelegate,
+    mut command: smol::process::Command,
+    executor: &BackgroundExecutor,
+) -> Result<RemoteCommandOutput> {
+    if env.contains_key("GIT_ASKPASS") {
+        let git_process = command.spawn()?;
+        let output = git_process.output().await?;
+        if !output.status.success() {
+            Err(anyhow!("{}", String::from_utf8_lossy(&output.stderr)))
+        } else {
+            Ok(RemoteCommandOutput {
+                stdout: String::from_utf8_lossy(&output.stdout).to_string(),
+                stderr: String::from_utf8_lossy(&output.stderr).to_string(),
+            })
+        }
+    } else {
+        let ask_pass = AskPassSession::new(executor, ask_pass).await?;
+        let mut command = new_smol_command("git");
+        command
+            .env("GIT_ASKPASS", ask_pass.script_path())
+            .env("SSH_ASKPASS", ask_pass.script_path())
+            .env("SSH_ASKPASS_REQUIRE", "force");
+        let git_process = command.spawn()?;
+
+        run_askpass_command(ask_pass, git_process).await
+    }
+}
+
+async fn run_askpass_command(
     mut ask_pass: AskPassSession,
     git_process: smol::process::Child,
 ) -> std::result::Result<RemoteCommandOutput, anyhow::Error> {

crates/project/src/git_store.rs 🔗

@@ -6,7 +6,7 @@ use crate::{
     ProjectEnvironment, ProjectItem, ProjectPath,
 };
 use anyhow::{anyhow, bail, Context as _, Result};
-use askpass::{AskPassDelegate, AskPassSession};
+use askpass::AskPassDelegate;
 use buffer_diff::{BufferDiff, BufferDiffEvent};
 use client::ProjectId;
 use collections::HashMap;
@@ -3098,7 +3098,6 @@ impl Repository {
         askpass: AskPassDelegate,
         cx: &mut App,
     ) -> oneshot::Receiver<Result<RemoteCommandOutput>> {
-        let executor = cx.background_executor().clone();
         let askpass_delegates = self.askpass_delegates.clone();
         let askpass_id = util::post_inc(&mut self.latest_askpass_id);
         let env = self.worktree_environment(cx);
@@ -3106,7 +3105,6 @@ impl Repository {
         self.send_job(move |git_repo, cx| async move {
             match git_repo {
                 RepositoryState::Local(git_repository) => {
-                    let askpass = AskPassSession::new(&executor, askpass).await?;
                     let env = env.await;
                     git_repository.fetch(askpass, env, cx).await
                 }
@@ -3147,7 +3145,6 @@ impl Repository {
         askpass: AskPassDelegate,
         cx: &mut App,
     ) -> oneshot::Receiver<Result<RemoteCommandOutput>> {
-        let executor = cx.background_executor().clone();
         let askpass_delegates = self.askpass_delegates.clone();
         let askpass_id = util::post_inc(&mut self.latest_askpass_id);
         let env = self.worktree_environment(cx);
@@ -3156,7 +3153,7 @@ impl Repository {
             match git_repo {
                 RepositoryState::Local(git_repository) => {
                     let env = env.await;
-                    let askpass = AskPassSession::new(&executor, askpass).await?;
+                    // let askpass = AskPassSession::new(&executor, askpass).await?;
                     git_repository
                         .push(
                             branch.to_string(),
@@ -3209,7 +3206,6 @@ impl Repository {
         askpass: AskPassDelegate,
         cx: &mut App,
     ) -> oneshot::Receiver<Result<RemoteCommandOutput>> {
-        let executor = cx.background_executor().clone();
         let askpass_delegates = self.askpass_delegates.clone();
         let askpass_id = util::post_inc(&mut self.latest_askpass_id);
         let env = self.worktree_environment(cx);
@@ -3217,7 +3213,6 @@ impl Repository {
         self.send_job(move |git_repo, cx| async move {
             match git_repo {
                 RepositoryState::Local(git_repository) => {
-                    let askpass = AskPassSession::new(&executor, askpass).await?;
                     let env = env.await;
                     git_repository
                         .pull(branch.to_string(), remote.to_string(), askpass, env, cx)