Skip to content

fix(brew): relocate shebang executables with long prefixes - #11632

Merged
jdx merged 1 commit into
jdx:mainfrom
Marukome0743:pr11632
Aug 2, 2026
Merged

fix(brew): relocate shebang executables with long prefixes#11632
jdx merged 1 commit into
jdx:mainfrom
Marukome0743:pr11632

Conversation

@Marukome0743

@Marukome0743 Marukome0743 commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix Linux Homebrew bottle relocation for shebang executables that contain binary data, such as Watchman's watchman-diag Python zipapp.

On Linux, brew:watchman failed while pouring the bottle with:

mise ERROR cannot relocate /home/linuxbrew/.linuxbrew/Cellar/watchman/.mise-tmp-2026.07.27.00/bin/watchman-diag: replacement for @@HOMEBREW_PREFIX@@ does not fit (64 > 61 bytes)

Root cause

watchman-diag starts with a Python shebang but contains a ZIP payload and NUL bytes. mise therefore classified it as an ordinary fixed-width binary. The Linuxbrew prefix is longer than the placeholder slot, so relocation failed.

Homebrew treats shebang executables as text executables before checking for binary contents and performs variable-length replacement. Inspection of the original Linux arm64 Watchman bottle confirmed that the Homebrew prefix placeholder occurs only in the shebang. Extending that shebang preserves both ZIP integrity and executable behavior.

Changes

  • Detect shebang executables before using the NUL-based binary fallback.
  • Apply variable-length placeholder replacement to shebang executables, including zipapps.
  • Preserve the existing Mach-O, ELF, and fixed-width binary relocation paths.
  • Keep rejecting overlong replacements in ordinary non-shebang binaries.
  • Document the shebang/zipapp relocation behavior.
  • Add a synthetic zipapp regression test that verifies the longer Linuxbrew prefix, ZIP readability, and unchanged payload.

Validation

  • mise exec -- cargo test --all-features system::packages::brew::relocate::tests
  • mise exec -- cargo check --all-features
  • mise exec -- cargo clippy --workspace --all-features --all-targets -- -D warnings
  • mise exec -- cargo fmt --all -- --check
  • Prettier and markdownlint for docs/bootstrap/packages/brew.md
  • Linux arm64 Docker with Ubuntu 24.04:
    • brew:watchman apply and final status succeeded
    • watchman --version returned 2026.07.27.00
    • watchman-diag retained a valid ZIP payload
    • its shebang was relocated to /home/linuxbrew/.linuxbrew/opt/python@3.14/bin/python3.14
    • the zipapp launched successfully through the relocated shebang
  • macOS arm64 with MISE_SYSTEM_BREW_PREFIX=/tmp/hb-macos:
    • brew:fswatch and brew:watchman both installed successfully
    • Watchman returned 2026.07.27.00
    • watchman-diag remained a valid ZIP archive with the relocated test prefix

mise run format and mise run lint could not start because hk requires a Git working copy while this checkout is managed by Sapling. The applicable Rust, documentation, check, and Clippy commands above passed individually.

Addresses #5655

AI-assisted — Tool: Codex; model: OpenAI/GPT-5; version: unavailable.

Summary by CodeRabbit

  • Bug Fixes

    • Improved relocation of executable scripts packaged as zipapps.
    • Shebang paths are now updated correctly without modifying the packaged payload.
    • Existing relocation behavior for text files, Mach-O binaries, and Linux ELF files remains unchanged.
  • Tests

    • Added coverage for zipapp relocation and shebang validation.

@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Brew relocation now detects valid shebang executables, including binary-containing zipapps. It applies length-changing path replacements while retaining existing Mach-O and Linux ELF handling.

Changes

Shebang relocation

Layer / File(s) Summary
Detect and classify shebang executables
src/system/packages/brew/relocate.rs
The relocation logic validates shebangs within the first 1024 bytes, preserves binary payloads, and applies shebang-only replacements to binary-containing executables.
Validate zipapp relocation
src/system/packages/brew/relocate.rs, docs/bootstrap/packages/brew.md
Tests cover longer shebang replacements, archive preservation, length growth, and invalid interpreters. Documentation describes shebang relocation.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

A rabbit found a zipapp bright,
With shebang paths that grew in flight.
The archive stayed whole and true,
While tests checked every cue.
“Hop, relocate!” the rabbit sings.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 88.89% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the Homebrew relocation fix for shebang executables with long prefixes.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR fixes relocation of NUL-backed shebang executables by replacing placeholders only in their shebang preamble while preserving the binary payload.

  • Adds line-bounded validation for shebang interpreters.
  • Keeps Mach-O, ELF, and ordinary binary relocation behavior separate.
  • Adds a synthetic zipapp regression test covering a longer Linuxbrew prefix, unchanged archive bytes, and archive readability.
  • Documents binary-backed shebang relocation behavior.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; both previously reported relocation issues are addressed by line-bounded shebang validation and payload-preserving replacement.

Important Files Changed

Filename Overview
src/system/packages/brew/relocate.rs Detects valid shebangs before the generic NUL-binary fallback and relocates only the shebang of binary-backed executables, with regression coverage confirming the payload remains intact.
docs/bootstrap/packages/brew.md Documents that binary-backed shebang executables receive preamble-only relocation while their payload remains unchanged.

Reviews (2): Last reviewed commit: "fix(brew): relocate shebang executables ..." | Re-trigger Greptile

Comment thread src/system/packages/brew/relocate.rs Outdated
Comment thread src/system/packages/brew/relocate.rs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/system/packages/brew/relocate.rs`:
- Around line 89-97: Update is_text_executable to inspect only the first shebang
line, stopping at \n or \r, and require a non-empty interpreter with no NUL
bytes. Preserve the existing prefix limit and ensure relocate_keg does not
classify binary payloads such as "#!\n\0payload" as text; add a regression
covering that input.
- Around line 206-212: Restrict relocation replacements for ZIP apps to the
prepended shebang, or rebuild the ZIP while updating entry data, CRCs, sizes,
filename and extra-field lengths, and central-directory offsets. Update the
relevant relocation logic near the macho/ELF branch and extend its test with a
stored entry containing a placeholder, confirming the resulting archive remains
readable; verify any Brew guarantee that placeholders only occur in the shebang
before relying on it.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: f8a11702-cd0a-418e-a19b-0e32cc528f96

📥 Commits

Reviewing files that changed from the base of the PR and between 832623e and 335bf2f.

📒 Files selected for processing (2)
  • docs/bootstrap/packages/brew.md
  • src/system/packages/brew/relocate.rs

Comment thread src/system/packages/brew/relocate.rs Outdated
Comment thread src/system/packages/brew/relocate.rs Outdated
@jdx
jdx merged commit f24659a into jdx:main Aug 2, 2026
30 checks passed
@Marukome0743
Marukome0743 deleted the pr11632 branch August 2, 2026 13:14
donbeave pushed a commit to donbeave/mise that referenced this pull request Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants