Skip to content
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

bin/brew: re-exec self under Rosetta if needed #9287

Conversation

mistydemeo
Copy link
Member

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew tests with your changes locally?
  • Have you successfully run brew man locally and committed any changes?

As discussed in #9177, we've reserved /usr/local as the prefix to use on Apple Silicon Macs for an Intel installation of Homebrew to be run under Rosetta.

If a user has a /usr/local installation on an Apple Silicon Mac, and executes /usr/local/bin/brew, we should make sure we're running that copy of brew as Intel under emulation in order to avoid confusion. This makes that change.

cc #9126

cc @claui, @MikeMcQuaid

As discussed in Homebrew#9177, we've reserved /usr/local as
the prefix to use on Apple Silicon Macs for an Intel
installation of Homebrew to be run under Rosetta.

If a user has a /usr/local installation on an Apple
Silicon Mac, and executes /usr/local/bin/brew, we
should make sure we're running that copy of brew as
Intel under emulation in order to avoid confusion.
This makes that change.
@BrewTestBot
Copy link
Member

Review period will end on 2020-11-26 at 01:48:37 UTC.

@BrewTestBot BrewTestBot added the waiting for feedback Merging is blocked until sufficient time has passed for review label Nov 25, 2020
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

A few concerns:

  • Will this succeed if Rosetta isn't installed?
  • This will avoid the existing prefix messaging from ever being displayed for /usr/local which is undesirable.
  • If the existing /usr/local had some ARM software installed in it this will produce confusing errors later on.
  • I'd rather we avoided hardcoding /usr/local in another place here.
  • It feels like there's a bit of overlap here with Check flavour to prevent mixed architectures #9126.

An alternate approach I'd rather (which overlaps with #9126) is changing or tweaking the existing error messages to make it more clear what needs to be done and how to do it. Doing so automatically without prompt or notification seems dangerous. Perhaps having an environment variable which has this behaviour (in brew.sh not bin/brew) would be an acceptable workaround after #9126 is finished and merged (or closed)

@BrewTestBot BrewTestBot removed the waiting for feedback Merging is blocked until sufficient time has passed for review label Nov 26, 2020
@BrewTestBot
Copy link
Member

Review period ended.

@MikeMcQuaid
Copy link
Member

Closing in favour of #9418 ❤️

@MikeMcQuaid MikeMcQuaid closed this Dec 4, 2020
@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Jan 4, 2021
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Jan 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants