From f8faec2c9b93e70fdbeebfd3ff2287f347e01b8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C5=ABdolfs=20O=C5=A1i=C5=86=C5=A1?= Date: Wed, 26 Aug 2026 12:28:09 +0200 Subject: [PATCH] Resolve git outside the session PATH A desktop launch inherits a `PATH` that holds none of the user's own install locations, so `Command::new("git")` finds nothing usable, or on macOS finds Apple's shim and only prompts to install the developer tools. Resolve git to an absolute path instead, searching `PATH` first and the usual install locations after, and cache the result for the process. `rad` is looked for the same way, which also fixes the CLI check for a Homebrew or Nix install. --- Cargo.lock | 10 + crates/radicle-tauri/src/commands/startup.rs | 39 +-- crates/radicle-types/Cargo.toml | 1 + crates/radicle-types/src/binaries.rs | 258 +++++++++++++++++++ crates/radicle-types/src/lib.rs | 1 + crates/radicle-types/src/traits/repo.rs | 95 ++++--- 6 files changed, 316 insertions(+), 88 deletions(-) create mode 100644 crates/radicle-types/src/binaries.rs diff --git a/Cargo.lock b/Cargo.lock index a39000ec6..be9165cbe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4080,6 +4080,7 @@ dependencies = [ "thiserror 2.0.19", "ts-rs", "url", + "which", ] [[package]] @@ -6605,6 +6606,15 @@ version = "0.1.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a28ac98ddc8b9274cb41bb4d9d4d5c425b6020c50c46f25559911905610b4a88" +[[package]] +name = "which" +version = "8.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8f3ef584124b911bcc3875c2f1472e80f24361ceb789bd1c62b3e9a3df9ff43c" +dependencies = [ + "libc", +] + [[package]] name = "winapi" version = "0.3.9" diff --git a/crates/radicle-tauri/src/commands/startup.rs b/crates/radicle-tauri/src/commands/startup.rs index e2faa79d7..832657656 100644 --- a/crates/radicle-tauri/src/commands/startup.rs +++ b/crates/radicle-tauri/src/commands/startup.rs @@ -16,50 +16,15 @@ pub(crate) fn version(app: AppHandle) -> Result { }) } -/// Check whether a binary can be found in the most common paths on Unix-like systems. -/// We don't bother checking the `$PATH` variable, as we're only looking for very standard tools -/// and prefer not to make this too complex. -#[cfg(unix)] -fn exists(cmd: &str) -> bool { - // Some common paths where system-installed binaries are found. - const PATHS: &[&str] = &["/usr/local/bin", "/usr/bin", "/bin"]; - - for dir in PATHS { - if std::path::Path::new(dir).join(cmd).exists() { - return true; - } - } - false -} - -/// Check whether a binary can be found on `$PATH`. -/// See: -/// - -/// - -#[cfg(windows)] -fn exists(cmd: &str) -> bool { - use std::os::windows::process::CommandExt; - - // See . - const CREATE_NO_WINDOW: u32 = 0x08000000; - - std::process::Command::new("where.exe") - .arg("/q") - .arg("$PATH:".to_owned() + cmd) - .creation_flags(CREATE_NO_WINDOW) - .output() - .map(|output| output.status.success()) - .unwrap_or_default() -} - #[tauri::command] pub(crate) fn check_radicle_cli(ctx: tauri::State) -> Result<(), Error> { + // Where the official installer puts it, even if it never reached `PATH`. let rad = ctx.profile().home().path().join("bin").join("rad"); if rad.exists() { return Ok(()); } - if exists("rad") { + if radicle_types::binaries::rad().is_some() { return Ok(()); } diff --git a/crates/radicle-types/Cargo.toml b/crates/radicle-types/Cargo.toml index e0f4569d6..41c0a227d 100644 --- a/crates/radicle-types/Cargo.toml +++ b/crates/radicle-types/Cargo.toml @@ -33,6 +33,7 @@ ts-rs = { version = "12.0.1", features = [ "format", ] } url = { version = "2.5.4", features = ["serde"] } +which = { version = "8.0.5" } [dev-dependencies] radicle = { version = "0.25", features = ["test", "tor", "i2p"] } diff --git a/crates/radicle-types/src/binaries.rs b/crates/radicle-types/src/binaries.rs new file mode 100644 index 000000000..8da01457e --- /dev/null +++ b/crates/radicle-types/src/binaries.rs @@ -0,0 +1,258 @@ +//! Locating the external binaries the app shells out to. +//! +//! A desktop launch inherits its environment from the OS session manager +//! rather than from a shell, so `PATH` holds none of the user's own install +//! locations: on macOS launchd hands out `/usr/bin:/bin:/usr/sbin:/sbin`, +//! which covers neither Homebrew nor Nix nor anything under the home +//! directory. So we look at `PATH` first and the usual install locations +//! after it. +//! +//! Everything the app runs `git` for it can also do through libgit2, only slower. + +use std::path::{Path, PathBuf}; +use std::sync::OnceLock; + +#[cfg(windows)] +use std::os::windows::process::CommandExt; +// See . +#[cfg(windows)] +const CREATE_NO_WINDOW: u32 = 0x08000000; + +/// Environment variable that pins the git binary, skipping detection. +pub const GIT_ENV: &str = "RAD_DESKTOP_GIT"; + +/// How long a candidate gets to answer `--version`. Only here so that a +/// candidate which never exits does not block resolution forever. +const PROBE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(3); + +/// Apple's `git` is a shim present on every macOS install whether or not git +/// is. Without the Command Line Developer Tools, running it only opens a +/// dialog offering to install them. See [`command_line_tools_installed`]. +#[cfg(target_os = "macos")] +const APPLE_GIT_SHIM: &str = "/usr/bin/git"; + +/// `None` means "searched and found nothing", cached so that a machine without git does not rescan +/// on every diff. +static GIT: OnceLock> = OnceLock::new(); + +pub fn git() -> Option { + GIT.get_or_init(|| { + let git = search(); + match &git { + Some(path) => log::info!("Using git binary {}", path.display()), + None => log::warn!( + "No working git binary found; falling back to libgit2 for diff stats, \ + commit counts and file history, which is slower on large repositories" + ), + } + + git + }) + .clone() +} + +/// A [`std::process::Command`] for the resolved binary. Spawning git anywhere +/// else risks forgetting `CREATE_NO_WINDOW` and flashing up a console window +/// on Windows. +pub fn git_command() -> Option { + #[cfg_attr(not(windows), allow(unused_mut))] + let mut command = std::process::Command::new(git()?); + #[cfg(windows)] + command.creation_flags(CREATE_NO_WINDOW); + + Some(command) +} + +/// The `rad` CLI, if it is installed. +/// +/// Uncached: the app polls for it so that installing the CLI while the app is +/// running lifts the "Radicle CLI not installed" warning without a restart. +pub fn rad() -> Option { + candidates("rad").find(|path| path.is_file()) +} + +/// Everywhere a binary called `name` might be, `PATH` first so that the user's +/// own choice wins. +fn candidates(name: &str) -> impl Iterator { + // Homebrew on Apple silicon and Intel, MacPorts, Nix profiles, and the + // directory the Radicle installer uses. + #[cfg(unix)] + const DIRS: &[&str] = &[ + "/opt/homebrew/bin", + "/usr/local/bin", + "/opt/local/bin", + "~/.nix-profile/bin", + "/nix/var/nix/profiles/default/bin", + "/run/current-system/sw/bin", + "~/.local/bin", + "~/.radicle/bin", + ]; + #[cfg(windows)] + const DIRS: &[&str] = &[]; + + let name = name.to_owned(); + // Collected because the iterator `which_all` hands back borrows the name. + let path = which::which_all(&name) + .into_iter() + .flatten() + .collect::>(); + let known = DIRS.iter().filter_map(move |dir| { + let dir = match dir.strip_prefix("~/") { + Some(rest) => PathBuf::from(std::env::var_os("HOME")?).join(rest), + None => PathBuf::from(dir), + }; + + Some(dir.join(&name)) + }); + + path.into_iter().chain(known) +} + +fn search() -> Option { + if let Some(pinned) = std::env::var_os(GIT_ENV) { + let pinned = PathBuf::from(pinned); + match validate(&pinned) { + Some(version) => { + log::info!("{GIT_ENV} pins git to {}: {version}", pinned.display()); + + return Some(pinned); + } + None => log::warn!( + "{GIT_ENV} is set to {}, which does not run git; detecting instead", + pinned.display() + ), + } + } + + detect() +} + +/// Also looks in the 64-bit, 32-bit and per-user program files directories. +/// `Git\cmd\git.exe` is the wrapper Git for Windows means for others to call, +/// as opposed to the bare executable under `Git\mingw64\bin`. +#[cfg(windows)] +fn detect() -> Option { + const PROGRAM_FILES: &[&str] = &["ProgramW6432", "ProgramFiles(x86)", "ProgramFiles"]; + + let program_files = PROGRAM_FILES + .iter() + .filter_map(|var| Some(PathBuf::from(std::env::var_os(var)?).join(r"Git\cmd\git.exe"))); + let local = std::env::var_os("LocalAppData") + .map(|base| PathBuf::from(base).join(r"Programs\Git\cmd\git.exe")); + + candidates("git.exe") + .chain(program_files) + .chain(local) + .find(|candidate| validate(candidate).is_some()) +} + +#[cfg(not(windows))] +fn detect() -> Option { + for candidate in candidates("git") { + // Skipping the shim has to leave a git further down the list still + // reachable, hence a list of candidates rather than just the first + // `git` on `PATH`. + #[cfg(target_os = "macos")] + if candidate == Path::new(APPLE_GIT_SHIM) && !command_line_tools_installed() { + log::info!( + "Skipping {APPLE_GIT_SHIM}: the Command Line Developer Tools are not installed, \ + so running it would only prompt to install them" + ); + continue; + } + if validate(&candidate).is_some() { + return Some(candidate); + } + } + + None +} + +/// Whether Apple's `git` shim has a real git to hand off to. +/// +/// `xcode-select -p` answers with the active developer directory, which ships git either way. It +/// exits with 2 when there is none, which is the case where the shim prompts instead of running +/// git. The directory it names can also have been deleted since, so it has to still exist. +/// `xcode-select` is a real binary rather than a shim, so asking has no side effects. +#[cfg(target_os = "macos")] +fn command_line_tools_installed() -> bool { + let Ok(output) = std::process::Command::new("/usr/bin/xcode-select") + .arg("-p") + .output() + .inspect_err(|e| log::warn!("Could not run xcode-select: {e}")) + else { + return false; + }; + if !output.status.success() { + return false; + } + let developer_dir = String::from_utf8_lossy(&output.stdout).trim().to_owned(); + + !developer_dir.is_empty() && Path::new(&developer_dir).is_dir() +} + +/// The version a candidate reports, or `None` if it is not a git we can use. +fn validate(path: &Path) -> Option { + if !path.is_file() { + return None; + } + let version = probe(path)?; + + if !version.starts_with("git version ") { + log::warn!( + "{} answers `--version` with {version:?} rather than a git version", + path.display() + ); + + return None; + } + + Some(version) +} + +/// Asks a candidate for its version, giving up after [`PROBE_TIMEOUT`]. +fn probe(path: &Path) -> Option { + use std::io::Read; + + let mut command = std::process::Command::new(path); + command + .arg("--version") + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::null()); + #[cfg(windows)] + command.creation_flags(CREATE_NO_WINDOW); + + let mut child = command.spawn().ok()?; + + // Waiting for the exit before reading: a candidate that prints more than + // the pipe holds would block on us while we block on it, and the deadline + // breaks that too. + let deadline = std::time::Instant::now() + PROBE_TIMEOUT; + loop { + match child.try_wait() { + Ok(Some(status)) if status.success() => break, + Ok(Some(_)) | Err(_) => return None, + Ok(None) => {} + } + if std::time::Instant::now() >= deadline { + log::warn!( + "{} did not answer `--version` within {}s, taking it as not being git", + path.display(), + PROBE_TIMEOUT.as_secs() + ); + let _ = child.kill(); + let _ = child.wait(); + + return None; + } + std::thread::sleep(std::time::Duration::from_millis(10)); + } + + // The process is gone and its output is a single line well within the + // pipe's capacity, so this reads what is already buffered. + let mut version = Vec::new(); + child.stdout.take()?.read_to_end(&mut version).ok()?; + + Some(String::from_utf8_lossy(&version).trim().to_string()) +} diff --git a/crates/radicle-types/src/lib.rs b/crates/radicle-types/src/lib.rs index 479849740..7c65d16f7 100644 --- a/crates/radicle-types/src/lib.rs +++ b/crates/radicle-types/src/lib.rs @@ -6,6 +6,7 @@ use traits::patch::{Patches, PatchesMut}; use traits::repo::Repo; use traits::thread::Thread; +pub mod binaries; pub mod cobs; pub mod config; pub mod diff; diff --git a/crates/radicle-types/src/traits/repo.rs b/crates/radicle-types/src/traits/repo.rs index f5a515c9d..9479529fd 100644 --- a/crates/radicle-types/src/traits/repo.rs +++ b/crates/radicle-types/src/traits/repo.rs @@ -21,12 +21,6 @@ use crate::repo; use crate::source; use crate::traits::Profile; -#[cfg(windows)] -use std::os::windows::process::CommandExt; -// See . -#[cfg(windows)] -const CREATE_NO_WINDOW: u32 = 0x08000000; - pub const MAX_BLOB_SIZE: usize = 10_485_760; #[derive(Serialize, Deserialize, PartialEq)] @@ -100,7 +94,7 @@ fn resolve_revision( /// `None` if git is unavailable or its output can't be parsed, so the caller /// can fall back to the (slower) radicle-surf diff. fn numstat(repo_dir: &std::path::Path, base: git::Oid, head: git::Oid) -> Option { - let mut command = std::process::Command::new("git"); + let mut command = crate::binaries::git_command()?; command .current_dir(repo_dir) // Porcelain `git diff` honours user configuration (diff.renames, @@ -114,8 +108,7 @@ fn numstat(repo_dir: &std::path::Path, base: git::Oid, head: git::Oid) -> Option .arg("--numstat") .arg(base.to_string()) .arg(head.to_string()); - #[cfg(windows)] - command.creation_flags(CREATE_NO_WINDOW); + let output = command .output() .ok() @@ -156,29 +149,29 @@ fn last_path_commit( head: git::Oid, path: &std::path::Path, ) -> Result { - let mut command = std::process::Command::new("git"); - command - .current_dir(repo_path) - .arg("rev-list") - .arg("-1") - .arg(head.to_string()) - .arg("--") - // `:(literal)` disables pathspec glob matching so file names - // containing `[`, `*` or `?` (e.g. `src/pages/[id].ts`) are looked - // up verbatim instead of being treated as wildcard patterns. - .arg(format!(":(literal){}", path.display())); - #[cfg(windows)] - command.creation_flags(CREATE_NO_WINDOW); - let fast = command - .output() - .ok() - .filter(|output| output.status.success()) - .and_then(|output| { - String::from_utf8_lossy(&output.stdout) - .trim() - .parse::() - .ok() - }); + let fast = crate::binaries::git_command().and_then(|mut command| { + command + .current_dir(repo_path) + .arg("rev-list") + .arg("-1") + .arg(head.to_string()) + .arg("--") + // `:(literal)` disables pathspec glob matching so file names + // containing `[`, `*` or `?` (e.g. `src/pages/[id].ts`) are looked + // up verbatim instead of being treated as wildcard patterns. + .arg(format!(":(literal){}", path.display())); + + command + .output() + .ok() + .filter(|output| output.status.success()) + .and_then(|output| { + String::from_utf8_lossy(&output.stdout) + .trim() + .parse::() + .ok() + }) + }); let commit = match fast { Some(oid) => surf_repo.commit(oid)?, @@ -875,25 +868,25 @@ pub trait Repo: Profile { // tamper-evident; the bitmap/commit-graph are local derived indexes // over those same objects, not a new trust input. Falls back to the // walk below if the git binary is unavailable or errors. - let mut command = std::process::Command::new("git"); - command.current_dir(repo.backend.path()).args([ - "rev-list", - "--count", - "--use-bitmap-index", - &head.to_string(), - ]); - #[cfg(windows)] - command.creation_flags(CREATE_NO_WINDOW); - let count = command - .output() - .ok() - .filter(|output| output.status.success()) - .and_then(|output| { - String::from_utf8_lossy(&output.stdout) - .trim() - .parse::() - .ok() - }); + let count = crate::binaries::git_command().and_then(|mut command| { + command.current_dir(repo.backend.path()).args([ + "rev-list", + "--count", + "--use-bitmap-index", + &head.to_string(), + ]); + + command + .output() + .ok() + .filter(|output| output.status.success()) + .and_then(|output| { + String::from_utf8_lossy(&output.stdout) + .trim() + .parse::() + .ok() + }) + }); if let Some(count) = count { return Ok(count); }