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

Handle macOS Homebrew on ARM #9117

Merged
merged 3 commits into from Nov 12, 2020
Merged

Conversation

MikeMcQuaid
Copy link
Member

@MikeMcQuaid MikeMcQuaid commented Nov 12, 2020

  • Output brew doctor and brew install messages noting this configuration is (currently) unsupported and encourage use of Rosetta instead
  • Output Rosetta 2 usage in brew config on ARM (whether in Rosetta 2 or not)
  • Check the architecture of (newly installed) dependencies and ensure they are using the correct architecture.
  • Don't allow installing macOS Intel Homebrew in macOS ARM Homebrew default prefix (and vice versa
  • Actually write out the architecture of dependencies to the tab rather than generating and throwing them away
  • Set and document the expected default prefix for macOS Intel Homebrew, macOS ARM Homebrew (/opt/homebrew) and Homebrew on Linux

While we're here:

  • Don't say Big Sur is a prerelease version but still make it clear we don't support it (yet).
  • Don't reference non-existent IRC channel

This needs to be merged and in a tagged release before Big Sur is released so please prioritise testing and reviewing this ASAP CC @claui @Bo98 @fxcoudert @mistydemeo

- Output `brew doctor` and `brew install` messages noting this configuration is (currently) unsupported and encourage use of Rosetta instead
- Output Rosetta 2 usage in `brew config` on ARM (whether in Rosetta 2 or not)
- Check the architecture of (newly installed) dependencies and ensure they are using the correct architecture.
- Don't allow installing macOS Intel Homebrew in macOS ARM Homebrew default prefix (and vice versa
- Actually write out the architecture of dependencies to the tab rather than generating and throwing them away
- Set and document the expected default prefix for macOS Intel Homebrew, macOS ARM Homebrew (`/opt/homebrew`) and Homebrew on Linux

While we're here:
- Don't say Big Sur is a prerelease version but still make it clear we
  don't support it (yet).
- Don't reference non-existent IRC channel
Copy link
Member

@Bo98 Bo98 left a comment

Choose a reason for hiding this comment

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

(I've not tested the FormulaInstaller changes yet.)

Library/Homebrew/install.rb Outdated Show resolved Hide resolved
Library/Homebrew/install.rb Outdated Show resolved Hide resolved
elsif Hardware::CPU.arm? && HOMEBREW_PREFIX == HOMEBREW_DEFAULT_PREFIX
odie "Cannot install in Homebrew on ARM processor in Intel default prefix (#{HOMEBREW_PREFIX})!"
Copy link
Member

@Bo98 Bo98 Nov 12, 2020

Choose a reason for hiding this comment

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

How do existing DTK users migrate? This error will appear for everyone who has tested ARM so far so it perhaps would be useful to point them somewhere.

I'm not entirely sure what this restriction does to help us. We already discourage using non-default prefixes so I'm not sure how this is any different? We can block mixed installations through other, better means (like the FormulaInstaller changes).

Copy link
Member Author

Choose a reason for hiding this comment

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

How do existing DTK users migrate? This error will appear for everyone who has tested ARM so far so it perhaps would be useful to point them somewhere.

@Bo98 They'll need to create and setup a new installation in a different location. We recommend one in the installation documentation. Should we do so here, too?

I'm not entirely sure what this restriction does to help us.

It avoids having ARM installs and Intel installs mixing. We don't have a way of enforcing that otherwise as the arch only starts being written to the tab as-of this PR.

We already discourage using non-default prefixes so I'm not sure how this is any different?

I think it's better to actively forbid it because it allows side-by-side installations. I can see the use-case for having two side-by-side installations on ARM until we have complete bottle coverage (which will take years).

Copy link
Member Author

Choose a reason for hiding this comment

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

@Bo98 Have added migration instructions in 2378a0b.

MikeMcQuaid and others added 2 commits November 12, 2020 18:58
@MikeMcQuaid MikeMcQuaid merged commit b43c0fe into Homebrew:master Nov 12, 2020
1 of 2 checks passed
@MikeMcQuaid MikeMcQuaid deleted the handle-arm branch November 12, 2020 20:14
@sjackman
Copy link
Member

Thanks for this necessary work, Mike, and on a tight deadline.

# since the `sysctl` flags don't change behaviour under Rosetta.
def in_rosetta?
# since the `sysctl` flags don't change behaviour under Rosetta 2.
def in_rosetta2?
intel? && physical_cpu_arm64?
Copy link

Choose a reason for hiding this comment

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

It seems Apple's recommended way to detect Rosetta 2 is to check sysctl.proc_translated.

@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Dec 14, 2020
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Dec 14, 2020
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.

None yet

5 participants