From e46328fe375fb0ada9cf0a33b02550c441d702ef Mon Sep 17 00:00:00 2001 From: smittie2000 Date: Tue, 3 Nov 2026 05:43:23 +0200 Subject: [PATCH] refactor(native): one PATH lookup (#784) * refactor(native): one PATH lookup for detect and process process.rs (`which_on_path`) and two places in detect.rs's Linux module each searched PATH the same way. They call `process::which` now. These all ask what the user can run, so PATH is the right place to look. install.rs is left alone: programs run as root come from the root-owned SYSTEM_BIN_DIRS, never from PATH. Co-Authored-By: Claude Opus 5.5 * Document the Linux detect, find_binary and binary_version functions Co-Authored-By: Claude Opus 5.5 * Say every case where binary_version gives up The 5-second limit matches the code. The doc also missed that the binary can fail to start or exit with an error, and that it is killed on timeout. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- crates/native/src/detect.rs | 13 +++++++------ crates/native/src/process.rs | 5 +++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/crates/native/src/detect.rs b/crates/native/src/detect.rs index 98dfaf07..2f17e1b8 100644 --- a/crates/native/src/detect.rs +++ b/crates/native/src/detect.rs @@ -507,11 +507,7 @@ pub mod linux { /// Where a package put the app's binary: on `PATH`, else `/usr/bin/`. fn binary_on_path(id: &str) -> String { - std::env::var_os("PATH") - .and_then(|path| std::env::split_paths(&path).map(|d| d.join(id)).find(|p| p.is_file())) - .unwrap_or_else(|| PathBuf::from("/usr/bin").join(id)) - .display() - .to_string() + crate::process::which(id).unwrap_or_else(|| PathBuf::from("/usr/bin").join(id)).display().to_string() } /// `~/Applications`, where the launcher puts AppImages (the AppImageLauncher convention). @@ -560,6 +556,8 @@ pub mod linux { appimages_in(app, &appimage_dirs()) } + /// How the app is installed, in this order: an AppImage the launcher put down, a package + /// (deb, rpm, pacman or Flatpak), else a binary found on disk. pub fn detect(app: &CraftApp, snapshot: &Snapshot) -> Option { if let Some((path, version)) = appimages(app).into_iter().next() { return Some(Installed { version, source: InstallSource::AppImage, location: path.display().to_string(), package: None }); @@ -582,12 +580,15 @@ pub mod linux { Some(Installed { version: binary_version(&bin, app), source: InstallSource::Binary, location: bin.display().to_string(), package: None }) } + /// The app's binary on `PATH`, else in `/usr/bin`, `/usr/local/bin` or `~/.local/bin`. fn find_binary(id: &str) -> Option { - let on_path = std::env::var_os("PATH").and_then(|path| std::env::split_paths(&path).map(|d| d.join(id)).find(|p| p.is_file())); + let on_path = crate::process::which(id); let usual = ["/usr/bin", "/usr/local/bin"].into_iter().map(PathBuf::from).chain(dirs::home_dir().map(|h| h.join(".local/bin"))); on_path.or_else(|| usual.map(|d| d.join(id)).find(|p| p.is_file())) } + /// The version `bin --version` prints, or `None` when it can't be started, doesn't finish + /// within 5 seconds (it is then killed), exits with an error, or prints no version. pub(crate) fn binary_version(bin: &Path, app: &CraftApp) -> Option { let out = crate::process::capture(bin.to_str()?, &["--version"], Duration::from_secs(5))?; parse_cli_version(out.trim(), app) diff --git a/crates/native/src/process.rs b/crates/native/src/process.rs index 6a86933f..a8c29744 100644 --- a/crates/native/src/process.rs +++ b/crates/native/src/process.rs @@ -159,7 +159,7 @@ pub fn is_flatpak_available() -> bool { if in_sandbox() { capture("flatpak-spawn", &["--host", "flatpak", "--version"], Duration::from_secs(3)).is_some() } else { - which_on_path("flatpak").is_some() + which("flatpak").is_some() } } @@ -290,7 +290,8 @@ fn detect_discord_scheme_handler() -> bool { false } -fn which_on_path(name: &str) -> Option { +/// Finds a program on `PATH`. +pub(crate) fn which(name: &str) -> Option { let path = std::env::var_os("PATH")?; std::env::split_paths(&path).map(|d| d.join(name)).find(|p| p.is_file()) }