Skip to content

Commit c3dce3a

Browse files
committed
fix(walltime): use brew-installed bash for samply on macOS
samply can't profile macOS's signed /bin/bash (and Python launched through it), so it needs an unsigned bash. On macOS we now: - Require Homebrew to be installed (bail with a pointer to brew.sh otherwise); installing it ourselves would be too invasive. - Skip the install entirely if `brew list --formula --quiet bash` reports bash is already present. - In a TTY, prompt the user before running `brew install bash` (default Y, accepts y/yes/Enter). In CI (non-TTY), install silently. - Suppress brew's stdio unless CODSPEED_LOG=debug; on failure the captured output is included in the error message. - Prepend `$(brew --prefix)/bin` to the spawned samply command's PATH so brew's unsigned bash wins over other `bash` entries earlier on PATH (e.g. nix-profile's signed bash). Only the samply child process tree sees this PATH — the parent shell is untouched. Adds: - `executor/helpers/homebrew` module with `ensure_installed`, `is_installed`, `prefix`, and `install`. - `CommandBuilder::env()` to set env vars on the spawned process.
1 parent b5c800c commit c3dce3a

4 files changed

Lines changed: 152 additions & 1 deletion

File tree

src/executor/helpers/command.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,17 @@ impl CommandBuilder {
4848
self
4949
}
5050

51+
#[allow(dead_code)] // currently only used behind cfg(target_os = "macos")
52+
pub fn env<K, V>(&mut self, key: K, value: V) -> &mut Self
53+
where
54+
K: AsRef<OsStr>,
55+
V: AsRef<OsStr>,
56+
{
57+
self.envs
58+
.insert(key.as_ref().to_owned(), value.as_ref().to_owned());
59+
self
60+
}
61+
5162
pub fn current_dir<D>(&mut self, dir: D)
5263
where
5364
D: AsRef<OsStr>,

src/executor/helpers/homebrew.rs

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
//! Thin wrappers around the `brew` CLI for macOS-only setup paths.
2+
3+
use crate::executor::helpers::env::is_codspeed_debug_enabled;
4+
use crate::prelude::*;
5+
use std::path::PathBuf;
6+
use std::process::{Command, Stdio};
7+
8+
/// Fail unless `brew` is on `PATH`. We intentionally do not install Homebrew
9+
/// ourselves — that's too invasive a side effect for a profiler setup step.
10+
pub fn ensure_installed() -> Result<()> {
11+
let installed = Command::new("which")
12+
.arg("brew")
13+
.output()
14+
.is_ok_and(|o| o.status.success());
15+
if !installed {
16+
bail!("Homebrew is required but was not found on PATH");
17+
}
18+
Ok(())
19+
}
20+
21+
/// Return Homebrew's install prefix (`/opt/homebrew` on Apple Silicon,
22+
/// `/usr/local` on Intel). Shells out to `brew --prefix` rather than hardcoding
23+
/// so we don't have to guess the architecture.
24+
pub fn prefix() -> Result<PathBuf> {
25+
let output = Command::new("brew")
26+
.arg("--prefix")
27+
.output()
28+
.context("failed to spawn `brew --prefix`")?;
29+
if !output.status.success() {
30+
bail!("`brew --prefix` exited with status {}", output.status);
31+
}
32+
let path = String::from_utf8(output.stdout)
33+
.context("`brew --prefix` returned non-UTF-8 output")?
34+
.trim()
35+
.to_owned();
36+
Ok(PathBuf::from(path))
37+
}
38+
39+
/// Check whether a brew formula is already installed. Uses `brew list <pkg>`,
40+
/// which is local-only (no network/API hit) and returns non-zero when missing.
41+
pub fn is_installed(package: &str) -> bool {
42+
Command::new("brew")
43+
.args(["list", "--formula", "--quiet", package])
44+
.output()
45+
.is_ok_and(|o| o.status.success())
46+
}
47+
48+
/// Run `brew install <package>`. Idempotent: brew exits 0 when the formula
49+
/// is already installed, so callers don't need to pre-check.
50+
pub fn install(package: &str) -> Result<()> {
51+
// Bypass the logger here: `info!` goes through the spinner-suspend path
52+
// which buffers until the spinner ticks, so the message would only show
53+
// up after brew returns. We want the user to see it before brew starts.
54+
eprintln!("Installing {package} via Homebrew...");
55+
56+
// Check the user-facing debug knob rather than log::max_level(); the
57+
// latter is forced to Trace by the runner's file logger and can't
58+
// distinguish "user wants debug output" from "captured to runner.log".
59+
let stdio = || {
60+
if is_codspeed_debug_enabled() {
61+
Stdio::inherit()
62+
} else {
63+
Stdio::piped()
64+
}
65+
};
66+
let output = Command::new("brew")
67+
.args(["install", package])
68+
.stdout(stdio())
69+
.stderr(stdio())
70+
.output()
71+
.with_context(|| format!("failed to spawn `brew install {package}`"))?;
72+
if !output.status.success() {
73+
bail!(
74+
"`brew install {package}` exited with status {}\nstdout:\n{}\nstderr:\n{}",
75+
output.status,
76+
String::from_utf8_lossy(&output.stdout),
77+
String::from_utf8_lossy(&output.stderr),
78+
);
79+
}
80+
Ok(())
81+
}

src/executor/helpers/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ pub mod detect_executable;
44
pub mod env;
55
pub mod get_bench_command;
66
pub mod harvest_perf_maps_for_pids;
7+
#[cfg(target_os = "macos")]
8+
pub mod homebrew;
79
pub mod introspected_golang;
810
pub mod introspected_nodejs;
911
pub mod profile_folder;

src/executor/wall_time/profiler/samply/mod.rs

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,20 @@ impl Profiler for SamplyProfiler {
4242
_system_info: &SystemInfo,
4343
_setup_cache_dir: Option<&Path>,
4444
) -> anyhow::Result<()> {
45-
ensure_linux_profiling_sysctls()
45+
ensure_linux_profiling_sysctls()?;
46+
47+
// samply can't profile macOS's signed /bin/bash
48+
#[cfg(target_os = "macos")]
49+
{
50+
use crate::executor::helpers::homebrew;
51+
homebrew::ensure_installed()?;
52+
if !homebrew::is_installed("bash") {
53+
confirm_bash_install()?;
54+
homebrew::install("bash")?;
55+
}
56+
}
57+
58+
Ok(())
4659
}
4760

4861
async fn wrap_command(
@@ -71,6 +84,23 @@ impl Profiler for SamplyProfiler {
7184
.get_command_builder()?;
7285

7386
cmd_builder.wrap_with(samply_builder);
87+
88+
// Ensure the brew-installed bash (and any other brew binary) wins over
89+
// other `bash` entries on PATH (e.g. nix-profile's signed bash), which
90+
// samply can't profile.
91+
#[cfg(target_os = "macos")]
92+
{
93+
use crate::executor::helpers::homebrew;
94+
let brew_bin = homebrew::prefix()?.join("bin");
95+
let existing = std::env::var_os("PATH").unwrap_or_default();
96+
let mut new_path = std::ffi::OsString::from(brew_bin);
97+
if !existing.is_empty() {
98+
new_path.push(":");
99+
new_path.push(&existing);
100+
}
101+
cmd_builder.env("PATH", new_path);
102+
}
103+
74104
self.output_path = Some(output_path);
75105
Ok(cmd_builder)
76106
}
@@ -114,3 +144,30 @@ impl Profiler for SamplyProfiler {
114144
Ok(())
115145
}
116146
}
147+
148+
#[cfg(target_os = "macos")]
149+
fn confirm_bash_install() -> anyhow::Result<()> {
150+
use crate::local_logger::IS_TTY;
151+
use console::Term;
152+
153+
// Non-interactive (CI): just install
154+
if !*IS_TTY {
155+
return Ok(());
156+
}
157+
158+
eprintln!(
159+
"CodSpeed depends on bash for benchmark execution, but can't use /bin/bash because system executables are signed in a way that prevents profiling. Because of this, we need to install bash with Homebrew. This is a one-time setup, your system bash is untouched."
160+
);
161+
eprint!("\nRun `brew install bash` now? [Y/n] ");
162+
let line = Term::stderr().read_line().unwrap_or_default();
163+
let answer = line.trim();
164+
165+
// Default to yes on empty input (just pressing Enter).
166+
if !(answer.is_empty()
167+
|| answer.eq_ignore_ascii_case("y")
168+
|| answer.eq_ignore_ascii_case("yes"))
169+
{
170+
bail!("Declined; cannot continue without an unsigned bash");
171+
}
172+
Ok(())
173+
}

0 commit comments

Comments
 (0)