-
Notifications
You must be signed in to change notification settings - Fork 1
Improve error handling for missing native binaries #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
fec9b01
8be0dfb
7aee161
2822176
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -395,6 +395,13 @@ pub async fn build_binary(progress_cb: impl Fn(&str) + Send + 'static) -> Result | |
| return Ok(()); | ||
| } | ||
|
|
||
| if !cmd_exists("git") { | ||
| return Err("git is not installed. Install Xcode Command Line Tools: xcode-select --install".into()); | ||
| } | ||
| if !cmd_exists("cmake") { | ||
| return Err("cmake is not installed. Install it with: brew install cmake".into()); | ||
| } | ||
|
|
||
| let tmp_dir = std::env::temp_dir().join("echo-whisper-build"); | ||
| fs::create_dir_all(bin_dir()).map_err(|e| e.to_string())?; | ||
| fs::create_dir_all(&tmp_dir).map_err(|e| e.to_string())?; | ||
|
|
@@ -443,15 +450,27 @@ pub async fn build_binary(progress_cb: impl Fn(&str) + Send + 'static) -> Result | |
| Ok(()) | ||
| } | ||
|
|
||
| fn run_cmd(cmd: &str, args: &[&str], cwd: &Path) -> Result<(), String> { | ||
| let path_env = format!( | ||
| fn build_path_env() -> String { | ||
| format!( | ||
| "{}:/opt/homebrew/bin:/usr/local/bin", | ||
| std::env::var("PATH").unwrap_or_default() | ||
| ); | ||
| ) | ||
| } | ||
|
|
||
| fn cmd_exists(name: &str) -> bool { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocker — this ignores the PATH augmentation that every other subprocess spawn in the codebase does, so the new guard will falsely block working builds.
Hoist the PATH the file already builds: fn build_path_env() -> String {
format!("{}:/opt/homebrew/bin:/usr/local/bin", std::env::var("PATH").unwrap_or_default())
}
fn cmd_exists(name: &str) -> bool {
Command::new(name)
.arg("--version")
.env("PATH", build_path_env())
.output()
.map(|o| o.status.success())
.unwrap_or(false)
}…and have Nit while here: on a Mac without CLT, |
||
| Command::new(name) | ||
| .arg("--version") | ||
| .env("PATH", build_path_env()) | ||
| .output() | ||
| .map(|o| o.status.success()) | ||
| .unwrap_or(false) | ||
| } | ||
|
|
||
| fn run_cmd(cmd: &str, args: &[&str], cwd: &Path) -> Result<(), String> { | ||
| let output = Command::new(cmd) | ||
| .args(args) | ||
| .current_dir(cwd) | ||
| .env("PATH", &path_env) | ||
| .env("PATH", build_path_env()) | ||
| .output() | ||
| .map_err(|e| format!("{} failed: {}", cmd, e))?; | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor / FYI — right call for the log spam, but note the behavior change:
check_dependenciesno longer compiles, so on first launch it reportsfalsefor the window whileensure_swift_binary_async(lib.rs:2203) is still building the helper. Transient false negative in the health panel / tray until the async compile lands. Acceptable as-is, just shouldn't be a surprise later.