From 4a5c492188148acb2ab1e65e7ef6d4560d233666 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Fri, 28 Mar 2025 14:50:05 -0600 Subject: [PATCH] If GIT_ASKPASS is already set, assume it will do the right thing (#27681) 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. --- 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(-) diff --git a/crates/fs/src/fake_git_repo.rs b/crates/fs/src/fake_git_repo.rs index 365aeae383..6d1a19faf2 100644 --- a/crates/fs/src/fake_git_repo.rs +++ b/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, - _askpass: AskPassSession, + _askpass: AskPassDelegate, _env: HashMap, _cx: AsyncApp, ) -> BoxFuture> { @@ -381,7 +381,7 @@ impl GitRepository for FakeGitRepository { &self, _branch: String, _remote: String, - _askpass: AskPassSession, + _askpass: AskPassDelegate, _env: HashMap, _cx: AsyncApp, ) -> BoxFuture> { @@ -390,7 +390,7 @@ impl GitRepository for FakeGitRepository { fn fetch( &self, - _askpass: AskPassSession, + _askpass: AskPassDelegate, _env: HashMap, _cx: AsyncApp, ) -> BoxFuture> { diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index cdeacc7e97..d076725ca4 100644 --- a/crates/git/src/repository.rs +++ b/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, - askpass: AskPassSession, + askpass: AskPassDelegate, env: HashMap, // 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, // 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, // 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, - ask_pass: AskPassSession, + ask_pass: AskPassDelegate, env: HashMap, - _cx: AsyncApp, + cx: AsyncApp, ) -> BoxFuture> { 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, - _cx: AsyncApp, + cx: AsyncApp, ) -> BoxFuture> { 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, - _cx: AsyncApp, + cx: AsyncApp, ) -> BoxFuture> { 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, + ask_pass: AskPassDelegate, + mut command: smol::process::Command, + executor: &BackgroundExecutor, +) -> Result { + 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 { diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index e9b85fffff..32624206ea 100644 --- a/crates/project/src/git_store.rs +++ b/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> { - 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> { - 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> { - 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)