Skip to content

Harden build prefix relocation of poured bottles - #23667

Open
MikeMcQuaid wants to merge 1 commit into
relocate-build-prefix-hardeningfrom
relocation-observability
Open

Harden build prefix relocation of poured bottles#23667
MikeMcQuaid wants to merge 1 commit into
relocate-build-prefix-hardeningfrom
relocation-observability

Conversation

@MikeMcQuaid

Copy link
Copy Markdown
Member
  • Keg#relocate_build_prefix NUL-pads raw prefix strings in pinned bottles at pour time but had no tests beyond one happy path; the hardening cases in the plan each get a spec, and three found bugs.
  • Patching wrote the file atomically, giving it a new inode, so any hardlinked names kept the unpatched content. Files are now grouped by inode, patched once and re-linked afterwards, as text relocation already does.
  • A prefix longer than the bottled one failed midway with a size mismatch after earlier files had already been rewritten; it is now refused up front.
  • Linkers suffix-merge strings, so a dynamic string table entry can be referenced from its interior (libfoo.so inside /old/prefix/libfoo.so); trailing NUL padding shifted such references onto garbage. Strings with interior loader references are now padded with extra path separators after the prefix, which path resolution ignores, keeping every reference's offset valid. Other strings keep NUL padding.
  • Also covered: several occurrences per string and several strings per file, the sharball skip, re-signing of patched files and prefix strings inside Mach-O load commands.

This change is part of plans/relocatable-bottles.md


  • Have you followed our Contributing guidelines?
  • Have you checked for other open Pull Requests for the same change?
  • Have you explained what your changes do? Performance claims (e.g. "this is faster") must include brew benchmark results.
  • Have you explained why you'd like these changes included, not just what they do?
  • For bug fixes, have you given step-by-step brew commands to reproduce the bug?
  • Have you written new tests (excluding integration tests)? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) locally?

  • I did not use AI/LLM to create this PR, or I disclosed the tool/model below and reviewed its output; I did not attribute commits to AI and will answer maintainer questions and review comments myself without AI/LLM.

Claude Fable 5 high with local review and testing.


- `HOMEBREW_RELOCATE_BUILD_PREFIX` was read straight from `ENV` in two
  places; it is now a hidden `env_config` boolean so it is typed and
  testable while staying undocumented until the feature is hardened.
- A bottle poured with its build prefix patched in place now records
  `relocated_build_prefix` and `relocated_files` in its tab, so a
  relocated keg can be told apart from a pristine one when diagnosing
  bug reports. `Keg#relocate_build_prefix` returns the patched files
  to make that possible.
- The "building from source" warning for a bottle built for another
  prefix now states how many characters longer or shorter the local
  prefix is than the bottle's, which decides whether in-place
  patching could ever apply, without mentioning the hidden variable.

This change is part of [`plans/relocatable-bottles.md`](https://github.com/Homebrew/brew/blob/HEAD/Library/Homebrew/plans/relocatable-bottles.md)
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