macOS: add a tarball fallback to INSTALL.sh - #3946
Open
moflwi wants to merge 4 commits into
Open
Conversation
added 4 commits
August 8, 2026 10:31
/usr is on the SIP-sealed system volume on modern macOS, but the codebase hardcoded /usr/share/opentelemetry_shell and /usr/bin/... as absolute paths for cross-file sourcing. Make the bootstrap (agent.sh, api.sh), curl/wget/ netcat instrumentation, the CI/GH-Action installers, and the test suite's package entrypoint resolve to /usr/local/... on Darwin instead, leaving Linux behavior untouched. Also: drop the dead macos-13 runner, use `ps` instead of the unreliable $SHELL fallback for shell detection, fix the Homebrew formula's post_install chicken-and-egg bug and deprecated python@3.9 dependency, template the formula's version/sha256 in build-brew, and restore the perl fallback that was dropped when the timeout fallback was added.
Extracting the whole tarball with --strip-components=2 into /usr/local also stripped the opt/ prefix, landing opt/opentelemetry_shell/requirements.txt at /usr/local/opentelemetry_shell/requirements.txt. The subsequent `pip3 install -r /opt/opentelemetry_shell/requirements.txt` would then fail, and api.sh looks for the SDK venv at /opt/opentelemetry_shell/venv too. /opt is not SIP-sealed (Homebrew itself lives at /opt/homebrew), so only the usr subtree needs relocating. Extract the two subtrees separately and create the venv, matching what the debian postinst does.
find -executable is a GNU extension. BSD find on macOS rejects it with "unknown primary or operator", and _otel_list_path_executables redirects stderr to /dev/null and swallows the exit code with `|| true`, so the failure was silent: the function returned nothing, _otel_list_path_commands returned nothing, and the auto-instrumentation loop had no commands to instrument. macOS would have appeared to run while instrumenting nothing. Select the predicate once at load time, using -perm +111 on Darwin, the same conditional-definition pattern already used elsewhere in the codebase. Linux keeps -executable and is unaffected.
The Darwin branch previously only offered `brew tap ... && brew install`, but that tap does not exist yet, so INSTALL.sh could not actually install anything on a Mac. Fall back to the release tarball -- the same mechanism the CI smoke test exercises -- when brew is missing or the tap install fails, so the tested path is what users get by default. The usr subtree goes to /usr/local (since /usr is SIP-sealed), while the opt subtree keeps its normal location because /opt is not sealed and api.sh expects the SDK venv at /opt/opentelemetry_shell/venv.
Owner
|
Hello, thanks for contributing for the first time! |
3 tasks
Collaborator
Author
|
The Homebrew tap this PR's fallback logic works around not having: tracked in #3952. |
Open
Contributor
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds a macOS-friendly installation path by falling back to a direct tarball install when Homebrew isn’t available or fails, aligning real-user behavior with the existing CI smoke-tested tarball extraction approach.
Changes:
- Add
tar.gzdirect-download fallback toINSTALL.shon Darwin when Homebrew isn’t present or install fails. - Adjust tarball extraction on macOS to avoid SIP-sealed
/usrand installusrsubtree into/usr/local, plus create the Python venv under/opt/.... - Make tests and runtime scripts configurable for macOS install locations (e.g.,
/usr/local/...) viaOTEL_TEST_ENTRYPOINT/OTEL_TEST_API_ENTRYPOINT, and update CI/workflows + Homebrew formula accordingly.
Reviewed changes
Copilot reviewed 71 out of 71 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_unit_escape_arg.sh | Source API script from an overrideable test entrypoint path. |
| tests/unit/test_unit_busybox_parses_otelapi.sh | Validate API script syntax using an overrideable path. |
| tests/sdk/test_sdk_traceparent.sh | Source API script from an overrideable test entrypoint path. |
| tests/sdk/test_sdk_span_event.sh | Source API script from an overrideable test entrypoint path. |
| tests/sdk/test_sdk_span_error.sh | Source API script from an overrideable test entrypoint path. |
| tests/sdk/test_sdk_span_attribute.sh | Source API script from an overrideable test entrypoint path. |
| tests/sdk/test_sdk_span.sh | Source API script from an overrideable test entrypoint path. |
| tests/sdk/test_sdk_resource_attributes.sh | Source API script from an overrideable test entrypoint path. |
| tests/sdk/test_sdk_observe_whitespaces.sh | Source API script from an overrideable test entrypoint path. |
| tests/sdk/test_sdk_observe_stdout.sh | Source API script from an overrideable test entrypoint path. |
| tests/sdk/test_sdk_observe_stderr.sh | Source API script from an overrideable test entrypoint path. |
| tests/sdk/test_sdk_observe_pipes_empty.sh | Source API script from an overrideable test entrypoint path. |
| tests/sdk/test_sdk_observe_pipes_detached_stdout.bash | Source API script from an overrideable test entrypoint path. |
| tests/sdk/test_sdk_observe_pipes.sh | Source API script from an overrideable test entrypoint path. |
| tests/sdk/test_sdk_observe_log.sh | Source API script from an overrideable test entrypoint path. |
| tests/sdk/test_sdk_observe_failed.sh | Source API script from an overrideable test entrypoint path. |
| tests/sdk/test_sdk_observe_exit.sh | Source API script from an overrideable test entrypoint path. |
| tests/sdk/test_sdk_observe.sh | Source API script from an overrideable test entrypoint path. |
| tests/sdk/test_sdk_metric.sh | Source API script from an overrideable test entrypoint path. |
| tests/sdk/test_sdk_init_shutdown.sh | Source API script from an overrideable test entrypoint path. |
| tests/sdk/test_log_sideeffects.sh | Source API script from an overrideable test entrypoint path. |
| tests/sdk/DISABLED_test_sdk_observe_pipes_detached_stdin.sh | Source API script from an overrideable test entrypoint path. |
| tests/run_tests.sh | Add macOS test entrypoint overrides; add lightweight fallbacks for perl/timeout. |
| tests/performance/test_performance_script.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/performance/test_performance_full.sh | Source instrumentation script from an overrideable test entrypoint path under timeout. |
| tests/integration/test_integration_debconf_second.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/integration/test_integration_debconf_first.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/integration/test_integration_debconf_arguments.bash | Use overrideable instrumentation script path in command construction and source calls. |
| tests/integration/test_integration__which__IFS.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/integration/test_integration__renovate.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/integration/test_integration__philbot_stop__multi_tier_injection.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/integration/test_integration__bash_ai__shebang_vs_alias_injection.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/wget2.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/wget.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/test_auto_traceparent.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/test_auto_minimal_instrumentation.sh | Source instrumentation script from an overrideable test entrypoint path (including subshell run). |
| tests/auto/test_auto_lifecycle.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/test_auto_instrument.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/test_auto_injection_xargs.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/test_auto_injection_whitespaces.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/test_auto_injection_timeout.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/test_auto_injection_time.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/test_auto_injection_sudo_whitespaces.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/test_auto_injection_sudo.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/test_auto_injection_stdin.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/test_auto_injection_shebang.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/test_auto_injection_parallel.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/test_auto_injection_flock.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/test_auto_injection_find.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/test_auto_injection_docker.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/test_auto_injection_c.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/test_auto_injection.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/test_auto_init_in_if.sh | Source instrumentation script from an overrideable test entrypoint path inside conditional. |
| tests/auto/test_auto_http_injection_cleanup.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/test_auto_alias.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/test_auto_[.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/exec.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/curl_subshell.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/curl.sh | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/DISABLED_test_auto_injection_with_script.bash | Source instrumentation script from an overrideable test entrypoint path. |
| tests/auto/DISABLED_test_auto_injection_with_path.bash | Source instrumentation script from an overrideable test entrypoint path. |
| src/usr/share/opentelemetry_shell/api.sh | Add Darwin-aware _otel_shell_home and use it for SDK and sourced files; improve shell detection without /proc. |
| src/usr/share/opentelemetry_shell/agent.sh | Source correct api.sh path on Darwin; make instrumentation file discovery and PATH scanning more portable. |
| src/usr/share/opentelemetry_shell/agent.instrumentation.wget.sh | Use _otel_shell_home for injected library path; add Darwin-specific dependency check attempt. |
| src/usr/share/opentelemetry_shell/agent.instrumentation.curl.sh | Use _otel_shell_home for injected library path; add Darwin-specific dependency check attempt. |
| src/usr/share/opentelemetry_shell/agent.injection.netcat.sh | Use _otel_shell_home for instrumentation file evaluation. |
| meta/homebrew/opentelemetry-shell.rb | Update Python dependency and venv creation; add caveat about fixed /usr/local/share lookup. |
| actions/instrument/shared/install.sh | Update macOS tarball extraction to install into /usr/local and /opt, and create venv. |
| INSTALL.sh | Add Darwin tar.gz fallback when Homebrew is missing/fails; implement macOS SIP-safe tar extraction and venv setup. |
| .github/workflows/test_shell.yml | Update macOS CI extraction to SIP-safe layout and new share path; adjust runner matrix. |
| .github/workflows/build.yml | Generate a ready-to-publish Homebrew formula artifact with version/sha256 substituted. |
Suppressed comments (3)
tests/run_tests.sh:1
function name() { ... }is not POSIX sh and will syntax-error under shells likedash(which this test harness appears to support). Use POSIX function syntax (perl() { ...; },timeout() { ...; }) sorun_tests.shremains runnable in non-bash shells.
tests/performance/test_performance_full.sh:1- Because the script passed to
sh -cis single-quoted,${OTEL_TEST_ENTRYPOINT:-...}will not expand and the.command will try to source a literal path containing${...}. Use quoting that allows expansion (and preferably quote the resulting path) before passing the string tosh -c.
INSTALL.sh:1 - For
tar.gz, selecting.[0]from.assetsis not guaranteed to be the tarball asset once releases include multiple file types. This can cause downloading a non-tarball artifact into a*.tar.gzfilename and then failing at extraction. Filter the asset list to select the.tar.gzby name (similar to how the action installer does).
#!/bin/sh
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+181
to
+182
| if \[ -d /proc ] && \[ "$(\readlink -f "$hint")" = "$(\readlink -f "/proc/$$/exe")" ]; then local _otel_hint_is_self=TRUE; else local _otel_hint_is_self=FALSE; fi | ||
| { \[ -f "$hint" ] && \[ "$_otel_hint_is_self" = FALSE ] && \[ "$(\readlink -f "$hint")" != "$_otel_shell_home"/agent.sh ] && \cat "$hint" || \echo "$hint"; } | \tr -s ' $=";(){}/\\!#~^'\' '\n' | _otel_filter_by_validity | \sort -u |
Comment on lines
+131
to
+135
| if \[ "$(\uname -s)" = Darwin ]; then | ||
| _otel_find_type_executable() { \find "$1" -maxdepth 1 -type "$2" -perm +111 2> /dev/null || \true; } | ||
| else | ||
| _otel_find_type_executable() { \find "$1" -maxdepth 1 -type "$2" -executable 2> /dev/null || \true; } | ||
| fi |
Comment on lines
9
to
+11
| depends_on "coreutils" | ||
| depends_on "findutils" | ||
| depends_on "python@3.9" => :recommended | ||
| depends_on "python@3" => :recommended |
Comment on lines
+26
to
28
| # during the build), not the venv's own not-yet-created python3 | ||
| system "python3", "-m", "venv", "#{prefix}/opt/opentelemetry_shell/venv" | ||
| system "#{prefix}/opt/opentelemetry_shell/venv/bin/pip3", "install", "-r", "#{prefix}/opt/opentelemetry_shell/requirements.txt" |
|
|
||
| . ./assert.sh | ||
| . /usr/bin/opentelemetry_shell.sh | ||
| . ${OTEL_TEST_ENTRYPOINT:-/usr/bin/opentelemetry_shell.sh} |
| local file="$_otel_shell_home"/agent.instrumentation.http/libinjecthttpheader.dylib | ||
| fi | ||
| if \[ -f "$file" ] && ! \ldd "$file" 2> /dev/null | \grep -q 'not found' && ! ( \[ "$_otel_shell" = 'busybox sh' ] && \help | \tail -n +3 | \grep -q curl ); then | ||
| if \[ -f "$file" ] && ! ( \[ "$(\uname -s)" = "Darwin" ] && \otool -L "$file" 2> /dev/null | \grep -q 'not found' ) && ! \ldd "$file" 2> /dev/null | \grep -q 'not found' && ! ( \[ "$_otel_shell" = 'busybox sh' ] && \help | \tail -n +3 | \grep -q curl ); then |
plengauer
requested changes
Aug 15, 2026
plengauer
left a comment
plengauer
left a comment
Owner
There was a problem hiding this comment.
as discussed:
*) lets rebase
*) lets not make sure they are not overlapping
*) i suggest finding a new solve for the dynamic path resolution everywhere - like rewriting paths when buillding the tarball
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
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.
Summary
`INSTALL.sh`'s Darwin branch only offered `brew tap ... && brew install` — and that tap doesn't exist yet, so a real user running `INSTALL.sh` on a Mac today gets a hard failure. The tarball + `--strip-components=2` extraction into `/usr/local` is the mechanism already fixed in #3942 and exercised by the CI smoke test.
Adds a tarball-download-and-extract fallback to `INSTALL.sh`'s Darwin branch, used when `brew` isn't present or the tap install fails — mirroring how the Linux branches already fall back between API-based and direct-download release fetching. This makes the tested path the one real users get by default, with Homebrew as a secondary option.
Depends on #3942 (base branch).
Test plan
🤖 Generated with Claude Code