From af630be7ca5ced87b889d45787444e0ce0d3d6fd Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 2 Oct 2025 14:22:10 -0400 Subject: [PATCH] git: Use environment from login shell to search for system git binary, and prefer it to the bundled binary (#39302) Closes #38571 Release Notes: - git: Fixed git features not working when git was installed in an unusual location. --------- Co-authored-by: Lukas Wirth --- crates/fs/src/fake_git_repo.rs | 4 +- crates/fs/src/fs.rs | 29 +++++-- crates/git/src/repository.rs | 124 ++++++++++++++++++---------- crates/project/src/git_store.rs | 5 +- crates/project/src/project_tests.rs | 4 +- 5 files changed, 112 insertions(+), 54 deletions(-) diff --git a/crates/fs/src/fake_git_repo.rs b/crates/fs/src/fake_git_repo.rs index 91c7113214adc6af722739e946588838b7bdd7a6..2c6db5b53987013d24a3a922e8f3b67adc9d43f5 100644 --- a/crates/fs/src/fake_git_repo.rs +++ b/crates/fs/src/fake_git_repo.rs @@ -608,7 +608,9 @@ mod tests { .await; fs.with_git_state(Path::new("/foo/.git"), true, |_git| {}) .unwrap(); - let repository = fs.open_repo(Path::new("/foo/.git")).unwrap(); + let repository = fs + .open_repo(Path::new("/foo/.git"), Some("git".as_ref())) + .unwrap(); let checkpoint_1 = repository.checkpoint().await.unwrap(); fs.write(Path::new("/foo/b"), b"IPSUM").await.unwrap(); diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 1c74584b9b6bf47a563ce20f20b0516a75cc071f..c7e7380febceac365e72a59f1609507caee22a81 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -134,7 +134,11 @@ pub trait Fs: Send + Sync { Arc, ); - fn open_repo(&self, abs_dot_git: &Path) -> Option>; + fn open_repo( + &self, + abs_dot_git: &Path, + system_git_binary_path: Option<&Path>, + ) -> Option>; async fn git_init(&self, abs_work_directory: &Path, fallback_branch_name: String) -> Result<()>; async fn git_clone(&self, repo_url: &str, abs_work_directory: &Path) -> Result<()>; @@ -247,7 +251,7 @@ impl From for proto::Timestamp { } pub struct RealFs { - git_binary_path: Option, + bundled_git_binary_path: Option, executor: BackgroundExecutor, } @@ -325,7 +329,7 @@ pub struct RealWatcher {} impl RealFs { pub fn new(git_binary_path: Option, executor: BackgroundExecutor) -> Self { Self { - git_binary_path, + bundled_git_binary_path: git_binary_path, executor, } } @@ -828,10 +832,15 @@ impl Fs for RealFs { ) } - fn open_repo(&self, dotgit_path: &Path) -> Option> { + fn open_repo( + &self, + dotgit_path: &Path, + system_git_binary_path: Option<&Path>, + ) -> Option> { Some(Arc::new(RealGitRepository::new( dotgit_path, - self.git_binary_path.clone(), + self.bundled_git_binary_path.clone(), + system_git_binary_path.map(|path| path.to_path_buf()), self.executor.clone(), )?)) } @@ -2425,7 +2434,11 @@ impl Fs for FakeFs { ) } - fn open_repo(&self, abs_dot_git: &Path) -> Option> { + fn open_repo( + &self, + abs_dot_git: &Path, + _system_git_binary: Option<&Path>, + ) -> Option> { use util::ResultExt as _; self.with_git_state_and_paths( @@ -3077,7 +3090,7 @@ mod tests { // With the file handle still open, the file should be replaced // https://github.com/zed-industries/zed/issues/30054 let fs = RealFs { - git_binary_path: None, + bundled_git_binary_path: None, executor, }; let temp_dir = TempDir::new().unwrap(); @@ -3095,7 +3108,7 @@ mod tests { #[gpui::test] async fn test_realfs_atomic_write_non_existing_file(executor: BackgroundExecutor) { let fs = RealFs { - git_binary_path: None, + bundled_git_binary_path: None, executor, }; let temp_dir = TempDir::new().unwrap(); diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index 34c17da8ec96948898fedee2a6ef12c471ff25f4..2e132d4eaca55c9307bf3368c412f77ed6726df2 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -545,21 +545,25 @@ impl std::fmt::Debug for dyn GitRepository { pub struct RealGitRepository { pub repository: Arc>, - pub git_binary_path: PathBuf, + pub system_git_binary_path: Option, + pub any_git_binary_path: PathBuf, executor: BackgroundExecutor, } impl RealGitRepository { pub fn new( dotgit_path: &Path, - git_binary_path: Option, + bundled_git_binary_path: Option, + system_git_binary_path: Option, executor: BackgroundExecutor, ) -> Option { + let any_git_binary_path = system_git_binary_path.clone().or(bundled_git_binary_path)?; let workdir_root = dotgit_path.parent()?; let repository = git2::Repository::open(workdir_root).log_err()?; Some(Self { repository: Arc::new(Mutex::new(repository)), - git_binary_path: git_binary_path.unwrap_or_else(|| PathBuf::from("git")), + system_git_binary_path, + any_git_binary_path, executor, }) } @@ -640,11 +644,12 @@ impl GitRepository for RealGitRepository { } fn show(&self, commit: String) -> BoxFuture<'_, Result> { + let git_binary_path = self.any_git_binary_path.clone(); let working_directory = self.working_directory(); self.executor .spawn(async move { let working_directory = working_directory?; - let output = new_smol_command("git") + let output = new_smol_command(git_binary_path) .current_dir(&working_directory) .args([ "--no-optional-locks", @@ -681,8 +686,9 @@ impl GitRepository for RealGitRepository { else { return future::ready(Err(anyhow!("no working directory"))).boxed(); }; + let git_binary_path = self.any_git_binary_path.clone(); cx.background_spawn(async move { - let show_output = util::command::new_smol_command("git") + let show_output = util::command::new_smol_command(&git_binary_path) .current_dir(&working_directory) .args([ "--no-optional-locks", @@ -705,7 +711,7 @@ impl GitRepository for RealGitRepository { let parent_sha = lines.next().unwrap().trim().trim_end_matches('\0'); let changes = parse_git_diff_name_status(lines.next().unwrap_or("")); - let mut cat_file_process = util::command::new_smol_command("git") + let mut cat_file_process = util::command::new_smol_command(&git_binary_path) .current_dir(&working_directory) .args(["--no-optional-locks", "cat-file", "--batch=%(objectsize)"]) .stdin(Stdio::piped()) @@ -809,7 +815,7 @@ impl GitRepository for RealGitRepository { ResetMode::Soft => "--soft", }; - let output = new_smol_command(&self.git_binary_path) + let output = new_smol_command(&self.any_git_binary_path) .envs(env.iter()) .current_dir(&working_directory?) .args(["reset", mode_flag, &commit]) @@ -832,7 +838,7 @@ impl GitRepository for RealGitRepository { env: Arc>, ) -> BoxFuture<'_, Result<()>> { let working_directory = self.working_directory(); - let git_binary_path = self.git_binary_path.clone(); + let git_binary_path = self.any_git_binary_path.clone(); async move { if paths.is_empty() { return Ok(()); @@ -909,7 +915,7 @@ impl GitRepository for RealGitRepository { env: Arc>, ) -> BoxFuture<'_, anyhow::Result<()>> { let working_directory = self.working_directory(); - let git_binary_path = self.git_binary_path.clone(); + let git_binary_path = self.any_git_binary_path.clone(); self.executor .spawn(async move { let working_directory = working_directory?; @@ -972,10 +978,11 @@ impl GitRepository for RealGitRepository { fn revparse_batch(&self, revs: Vec) -> BoxFuture<'_, Result>>> { let working_directory = self.working_directory(); + let git_binary_path = self.any_git_binary_path.clone(); self.executor .spawn(async move { let working_directory = working_directory?; - let mut process = new_smol_command("git") + let mut process = new_smol_command(&git_binary_path) .current_dir(&working_directory) .args([ "--no-optional-locks", @@ -1030,7 +1037,7 @@ impl GitRepository for RealGitRepository { } fn status(&self, path_prefixes: &[RepoPath]) -> Task> { - let git_binary_path = self.git_binary_path.clone(); + let git_binary_path = self.any_git_binary_path.clone(); let working_directory = match self.working_directory() { Ok(working_directory) => working_directory, Err(e) => return Task::ready(Err(e)), @@ -1054,7 +1061,7 @@ impl GitRepository for RealGitRepository { } fn stash_entries(&self) -> BoxFuture<'_, Result> { - let git_binary_path = self.git_binary_path.clone(); + let git_binary_path = self.any_git_binary_path.clone(); let working_directory = self.working_directory(); self.executor .spawn(async move { @@ -1076,7 +1083,7 @@ impl GitRepository for RealGitRepository { fn branches(&self) -> BoxFuture<'_, Result>> { let working_directory = self.working_directory(); - let git_binary_path = self.git_binary_path.clone(); + let git_binary_path = self.any_git_binary_path.clone(); self.executor .spawn(async move { let fields = [ @@ -1145,7 +1152,7 @@ impl GitRepository for RealGitRepository { fn change_branch(&self, name: String) -> BoxFuture<'_, Result<()>> { let repo = self.repository.clone(); let working_directory = self.working_directory(); - let git_binary_path = self.git_binary_path.clone(); + let git_binary_path = self.any_git_binary_path.clone(); let executor = self.executor.clone(); let branch = self.executor.spawn(async move { let repo = repo.lock(); @@ -1193,7 +1200,7 @@ impl GitRepository for RealGitRepository { } fn rename_branch(&self, branch: String, new_name: String) -> BoxFuture<'_, Result<()>> { - let git_binary_path = self.git_binary_path.clone(); + let git_binary_path = self.any_git_binary_path.clone(); let working_directory = self.working_directory(); let executor = self.executor.clone(); @@ -1209,7 +1216,7 @@ impl GitRepository for RealGitRepository { fn blame(&self, path: RepoPath, content: Rope) -> BoxFuture<'_, Result> { let working_directory = self.working_directory(); - let git_binary_path = self.git_binary_path.clone(); + let git_binary_path = self.any_git_binary_path.clone(); let remote_url = self .remote_url("upstream") @@ -1231,7 +1238,7 @@ impl GitRepository for RealGitRepository { fn diff(&self, diff: DiffType) -> BoxFuture<'_, Result> { let working_directory = self.working_directory(); - let git_binary_path = self.git_binary_path.clone(); + let git_binary_path = self.any_git_binary_path.clone(); self.executor .spawn(async move { let args = match diff { @@ -1262,7 +1269,7 @@ impl GitRepository for RealGitRepository { env: Arc>, ) -> BoxFuture<'_, Result<()>> { let working_directory = self.working_directory(); - let git_binary_path = self.git_binary_path.clone(); + let git_binary_path = self.any_git_binary_path.clone(); self.executor .spawn(async move { if !paths.is_empty() { @@ -1290,7 +1297,7 @@ impl GitRepository for RealGitRepository { env: Arc>, ) -> BoxFuture<'_, Result<()>> { let working_directory = self.working_directory(); - let git_binary_path = self.git_binary_path.clone(); + let git_binary_path = self.any_git_binary_path.clone(); self.executor .spawn(async move { @@ -1320,9 +1327,10 @@ impl GitRepository for RealGitRepository { 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"); + let mut cmd = new_smol_command(&git_binary_path); cmd.current_dir(&working_directory?) .envs(env.iter()) .args(["stash", "push", "--quiet"]) @@ -1348,9 +1356,10 @@ impl GitRepository for RealGitRepository { 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"); + let mut cmd = new_smol_command(git_binary_path); let mut args = vec!["stash".to_string(), "pop".to_string()]; if let Some(index) = index { args.push(format!("stash@{{{}}}", index)); @@ -1377,9 +1386,10 @@ impl GitRepository for RealGitRepository { 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"); + let mut cmd = new_smol_command(git_binary_path); let mut args = vec!["stash".to_string(), "apply".to_string()]; if let Some(index) = index { args.push(format!("stash@{{{}}}", index)); @@ -1406,9 +1416,10 @@ impl GitRepository for RealGitRepository { 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"); + let mut cmd = new_smol_command(git_binary_path); let mut args = vec!["stash".to_string(), "drop".to_string()]; if let Some(index) = index { args.push(format!("stash@{{{}}}", index)); @@ -1437,9 +1448,10 @@ impl GitRepository for RealGitRepository { 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"); + let mut cmd = new_smol_command(git_binary_path); cmd.current_dir(&working_directory?) .envs(env.iter()) .args(["commit", "--quiet", "-m"]) @@ -1481,9 +1493,11 @@ impl GitRepository for RealGitRepository { ) -> BoxFuture<'_, Result> { let working_directory = self.working_directory(); let executor = cx.background_executor().clone(); + let git_binary_path = self.system_git_binary_path.clone(); async move { + let git_binary_path = git_binary_path.context("git not found on $PATH, can't push")?; let working_directory = working_directory?; - let mut command = new_smol_command("git"); + let mut command = new_smol_command(git_binary_path); command .envs(env.iter()) .current_dir(&working_directory) @@ -1513,8 +1527,10 @@ impl GitRepository for RealGitRepository { ) -> BoxFuture<'_, Result> { let working_directory = self.working_directory(); let executor = cx.background_executor().clone(); + let git_binary_path = self.system_git_binary_path.clone(); async move { - let mut command = new_smol_command("git"); + let git_binary_path = git_binary_path.context("git not found on $PATH, can't pull")?; + let mut command = new_smol_command(git_binary_path); command .envs(env.iter()) .current_dir(&working_directory?) @@ -1538,9 +1554,11 @@ impl GitRepository for RealGitRepository { ) -> BoxFuture<'_, Result> { let working_directory = self.working_directory(); let remote_name = format!("{}", fetch_options); + let git_binary_path = self.system_git_binary_path.clone(); let executor = cx.background_executor().clone(); async move { - let mut command = new_smol_command("git"); + let git_binary_path = git_binary_path.context("git not found on $PATH, can't fetch")?; + let mut command = new_smol_command(git_binary_path); command .envs(env.iter()) .current_dir(&working_directory?) @@ -1555,7 +1573,7 @@ impl GitRepository for RealGitRepository { fn get_remotes(&self, branch_name: Option) -> BoxFuture<'_, Result>> { let working_directory = self.working_directory(); - let git_binary_path = self.git_binary_path.clone(); + let git_binary_path = self.any_git_binary_path.clone(); self.executor .spawn(async move { let working_directory = working_directory?; @@ -1601,7 +1619,7 @@ impl GitRepository for RealGitRepository { fn check_for_pushed_commit(&self) -> BoxFuture<'_, Result>> { let working_directory = self.working_directory(); - let git_binary_path = self.git_binary_path.clone(); + let git_binary_path = self.any_git_binary_path.clone(); self.executor .spawn(async move { let working_directory = working_directory?; @@ -1660,7 +1678,7 @@ impl GitRepository for RealGitRepository { fn checkpoint(&self) -> BoxFuture<'static, Result> { let working_directory = self.working_directory(); - let git_binary_path = self.git_binary_path.clone(); + let git_binary_path = self.any_git_binary_path.clone(); let executor = self.executor.clone(); self.executor .spawn(async move { @@ -1693,7 +1711,7 @@ impl GitRepository for RealGitRepository { fn restore_checkpoint(&self, checkpoint: GitRepositoryCheckpoint) -> BoxFuture<'_, Result<()>> { let working_directory = self.working_directory(); - let git_binary_path = self.git_binary_path.clone(); + let git_binary_path = self.any_git_binary_path.clone(); let executor = self.executor.clone(); self.executor @@ -1732,7 +1750,7 @@ impl GitRepository for RealGitRepository { right: GitRepositoryCheckpoint, ) -> BoxFuture<'_, Result> { let working_directory = self.working_directory(); - let git_binary_path = self.git_binary_path.clone(); + let git_binary_path = self.any_git_binary_path.clone(); let executor = self.executor.clone(); self.executor @@ -1770,7 +1788,7 @@ impl GitRepository for RealGitRepository { target_checkpoint: GitRepositoryCheckpoint, ) -> BoxFuture<'_, Result> { let working_directory = self.working_directory(); - let git_binary_path = self.git_binary_path.clone(); + let git_binary_path = self.any_git_binary_path.clone(); let executor = self.executor.clone(); self.executor @@ -1791,7 +1809,7 @@ impl GitRepository for RealGitRepository { fn default_branch(&self) -> BoxFuture<'_, Result>> { let working_directory = self.working_directory(); - let git_binary_path = self.git_binary_path.clone(); + let git_binary_path = self.any_git_binary_path.clone(); let executor = self.executor.clone(); self.executor @@ -2259,8 +2277,13 @@ mod tests { let file_path = repo_dir.path().join("file"); smol::fs::write(&file_path, "initial").await.unwrap(); - let repo = - RealGitRepository::new(&repo_dir.path().join(".git"), None, cx.executor()).unwrap(); + let repo = RealGitRepository::new( + &repo_dir.path().join(".git"), + None, + Some("git".into()), + cx.executor(), + ) + .unwrap(); repo.stage_paths(vec![repo_path("file")], Arc::new(HashMap::default())) .await .unwrap(); @@ -2335,8 +2358,13 @@ mod tests { let repo_dir = tempfile::tempdir().unwrap(); git2::Repository::init(repo_dir.path()).unwrap(); - let repo = - RealGitRepository::new(&repo_dir.path().join(".git"), None, cx.executor()).unwrap(); + let repo = RealGitRepository::new( + &repo_dir.path().join(".git"), + None, + Some("git".into()), + cx.executor(), + ) + .unwrap(); smol::fs::write(repo_dir.path().join("foo"), "foo") .await @@ -2374,8 +2402,13 @@ mod tests { let repo_dir = tempfile::tempdir().unwrap(); git2::Repository::init(repo_dir.path()).unwrap(); - let repo = - RealGitRepository::new(&repo_dir.path().join(".git"), None, cx.executor()).unwrap(); + let repo = RealGitRepository::new( + &repo_dir.path().join(".git"), + None, + Some("git".into()), + cx.executor(), + ) + .unwrap(); smol::fs::write(repo_dir.path().join("file1"), "content1") .await @@ -2418,8 +2451,13 @@ mod tests { .await .unwrap(); - let repo = - RealGitRepository::new(&repo_dir.path().join(".git"), None, cx.executor()).unwrap(); + let repo = RealGitRepository::new( + &repo_dir.path().join(".git"), + None, + Some("git".into()), + cx.executor(), + ) + .unwrap(); // initial commit repo.stage_paths(vec![repo_path("main.rs")], Arc::new(HashMap::default())) @@ -2490,7 +2528,7 @@ mod tests { /// Force a Git garbage collection on the repository. fn gc(&self) -> BoxFuture<'_, Result<()>> { let working_directory = self.working_directory(); - let git_binary_path = self.git_binary_path.clone(); + let git_binary_path = self.any_git_binary_path.clone(); let executor = self.executor.clone(); self.executor .spawn(async move { diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index 5b718ce68984a82fcbe207c67e37b374caa3417a..81a028638201bfb5e73ea8e402241bd658a2b288 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -4607,9 +4607,12 @@ impl Repository { log::error!("failed to get working directory environment for repository {work_directory_abs_path:?}"); HashMap::default() }); + let search_paths = environment.get("PATH").map(|val| val.to_owned()); let backend = cx .background_spawn(async move { - fs.open_repo(&dot_git_abs_path) + let system_git_binary_path = search_paths.and_then(|search_paths| which::which_in("git", Some(search_paths), &work_directory_abs_path).ok()) + .or_else(|| which::which("git").ok()); + fs.open_repo(&dot_git_abs_path, system_git_binary_path.as_deref()) .with_context(|| format!("opening repository at {dot_git_abs_path:?}")) }) .await?; diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index 226332d739eedd452fccc4e2714924bf7aec76b3..8d7da92f6b988a21c3879a0f0cbdcd9b2a616e66 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -7797,7 +7797,9 @@ async fn test_staging_random_hunks( path!("/dir/.git").as_ref(), &[("file.txt", index_text.clone())], ); - let repo = fs.open_repo(path!("/dir/.git").as_ref()).unwrap(); + let repo = fs + .open_repo(path!("/dir/.git").as_ref(), Some("git".as_ref())) + .unwrap(); let project = Project::test(fs.clone(), [path!("/dir").as_ref()], cx).await; let buffer = project