fix(walltime): use brew-installed bash for samply on macOS#347
Merged
not-matthias merged 2 commits intoMay 13, 2026
Conversation
Merging this PR will not alter performance
|
Repository was transferred from AvalancheHQ/samply to CodSpeedHQ/samply-codspeed. Same commit, same branch.
not-matthias
force-pushed
the
cod-2649-fix-samply-crashing-due-to-signed-bash
branch
2 times, most recently
from
e5cc90c to
b3c84c5
Compare
GuillaumeLagrange
approved these changes
May 13, 2026
samply can't profile macOS's Apple-signed /bin/bash (and Python launched
through it), so it needs an unsigned (ad-hoc-signed) bash. On macOS we now:
- Probe the `bash` first on PATH with `codesign -dv` and only intervene
when it lacks `Signature=adhoc` (i.e. system bash or another signed
build). If a compatible bash is already on PATH (e.g. Homebrew's), we
skip the brew dance entirely and leave PATH untouched.
- When intervention is needed:
- Require Homebrew to be installed (bail with a pointer to brew.sh
otherwise); installing it ourselves would be too invasive.
- Skip the install 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.
not-matthias
force-pushed
the
cod-2649-fix-samply-crashing-due-to-signed-bash
branch
from
b3c84c5 to
e26d369
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
samply currently crashes on macOS because it ends up profiling the system's signed
/bin/bash. SIP blocks the instrumentation samply needs (e.g.DYLD_INSERT_LIBRARIES), and anything launched through that bash — including Python — inherits the same restriction. We need an unsigned bash earlier on PATH for the samply child process.Fixes COD-2649