-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
Don't relocate Apple Silicon bottles for default prefix #19384
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Don't relocate Apple Silicon bottles for default prefix #19384
Conversation
This change skips the relocation process for bottles on Apple Silicon Macs when using the default prefix (/opt/homebrew). Relocation was initially needed for Intel Macs to avoid wide-reaching replacements with false positives, but is unnecessary for Apple Silicon where the default prefix is unique. Benefits: - Simplifies and speeds up bottle pouring on Apple Silicon - Potentially enables code-signing of homebrew-core packages - Improves security by avoiding binary modification post-build A --force-bottle-relocation flag is added for edge cases where relocation might still be needed. Fixes Homebrew#19247
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @samuelarogbonlo! Looks good so far. I suspect a bit more work required here and may need to do a gradual rollout with e.g. ENV.fetch("HOMEBREW_BOTTLE_SKIP_RELOCATION_ARM64", false)
or something.
@cho-m and other @Homebrew/maintainers may be able to suggest some next steps here.
As discussed already: this should probably do something along the lines of:
- see if any binary files need relocation with RubyMacho. if so: can skip text relocation. if not: can do text relocation.
- store whether relocation happened or not in e.g. the bottle manifest or the tab
Shout if you have more questions or we can help more here. Thanks for the PR ❤️
Agree regarding more work here. My view is that what we really want for arm64 is to move bottle relocation from bottle-creation time to bottle-pour time. It'll be a no-op for default prefix installations, but still works largely the same way for users with alternative prefixes. |
…out, remove force flag
Based on the feedback, I've updated the implementation to:
For the next steps, I'd like to address the remaining suggestions:
I'd appreciate some guidance on how to best implement these remaining items, particularly around using RubyMacho for analyzing binaries, and the preferred approach for tracking relocation status in the bottle metadata. Are there examples in the existing codebase I should look at for reference? |
Hey @samuelarogbonlo, thanks for giving this a go! I don't think I'll have answers to all your questions, but I'll try to give you more detailed guidance in a few days. Feel free to ping me this weekend as a reminder. |
@carlocab This is a friendly reminder. Thanks |
@samuelarogbonlo You can look at the relocation logic in |
1. Add binary relocation detection using RubyMacho 2. Update skip_relocation_for_apple_silicon? to consider binary analysis 3. Track relocation status in bottle tab metadata 4. Update keg relocation methods to use new functionality This implements the remaining suggestions from issue Homebrew#19247.
I've updated the PR based on the feedback:
The implementation now:
I looked at the relocation logic in keg_relocate.rb and bottle.rb, and the tab logic in tab.rb and formula_installer.rb as suggested. Let me know if there are other improvements you'd like to see! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for the delayed response here; life/work gets in the way.
I think Mike gave you a few places to look at above for inspiration. I also have a suggested alternative approach:
Instead of making replace_locations_with_placeholders
and replace_placeholders_with_locations
do nothing on Apple Silicon:
- make them do nothing if you are on Apple Silicon AND using the default prefix of
/opt/homebrew
- adjust the placeholders so that on Apple Silicon, they refer to concrete paths (e.g.
/opt/homebrew
for@@HOMEBREW_PREFIX@@
, etc)
Doing the second should hopefully get you most of the way towards making it so that relocation effectively happens at bottle-pour time on Apple Silicon when in a non-default prefix (which is what I suggested here).
Thanks again for persevering here! Let me know if you have any questions. (I'll try to be more responsive next time 😅)
Seems sensible 👍🏻. The most important thing here is that the files are not rewritten/have checksums changed on Apple Silicon. |
Thank you for the feedback! I've updated the implementation to:
This approach allows for both performance benefits on default installations and proper relocation on custom prefix installations. |
This change skips the relocation process for bottles on Apple Silicon Macs when using the default prefix (/opt/homebrew). Relocation was initially needed for Intel Macs to avoid wide-reaching replacements with false positives, but is unnecessary for Apple Silicon where the default prefix is unique.
Benefits:
A --force-bottle-relocation flag is added for edge cases where relocation might still be needed.
Fixes #19247
brew style
with your changes locally?brew typecheck
with your changes locally?brew tests
with your changes locally?Note: This PR is modifying autogenerated files that differ between Apple Silicon and Linux environments. Please run
brew generate-man-completions
during merge to ensure proper file generation.