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

CPU: add ability to check for Rosetta #7995

Merged
merged 2 commits into from
Sep 1, 2020

Conversation

mistydemeo
Copy link
Contributor

  • 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 successfully run brew style with your changes locally?
  • Have you successfully run brew tests with your changes locally?

When running within an Intel terminal, uname -m and friends return Intel-based values for compatibility. An Intel shell will also prefer to launch Intel slices of programs unless the program is ARM-only.

It's an open question how Homebrew should manage running in Intel mode. Should it continue to behave as though the Mac is Intel-based, like it does now? Should it recognize it's ARM-based? Either way, it's useful for us to be able to tell whether the Mac is running under Rosetta or whether it's a real Intel Mac.

@Bo98
Copy link
Member

Bo98 commented Jul 13, 2020

I think it would make sense to move the Linux-specific code to instead be in the generic OS case (Library/Homebrew/hardware.rb) and override on macOS only.

@mistydemeo
Copy link
Contributor Author

Good call, done

@mistydemeo
Copy link
Contributor Author

Simplified the patch slightly with a macOS-only ARM64 helper method. In the future we'll likely be able to go without that if we can check for Rosetta directly instead of indirectly.

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.

Nice work!

@lembacon
Copy link
Contributor

Wouldn't checking for sysctl.proc_translated be the preferred approach?

https://developer.apple.com/documentation/apple_silicon/about_the_rosetta_translation_environment

@Bo98
Copy link
Member

Bo98 commented Aug 13, 2020

When we last checked, sysctl.proc_translated only worked from a C sysctl call and not from a CLI sysctl query.

mistydemeo and others added 2 commits September 1, 2020 14:52
When running within an Intel terminal, `uname -m` and friends return Intel-based
values for compatibility. An Intel shell will also prefer to launch Intel slices of
programs unless the program is ARM-only.

It's an open question how Homebrew should manage running in Intel mode. Should it
continue to behave as though the Mac is Intel-based, like it does now? Should it
recognize it's ARM-based? Either way, it's useful for us to be able to tell whether
the Mac is running under Rosetta or whether it's a real Intel Mac.
@MikeMcQuaid MikeMcQuaid merged commit 3b8f1b7 into Homebrew:master Sep 1, 2020
@MikeMcQuaid
Copy link
Member

@mistydemeo Thanks for this!

ticky added a commit to buildkite/agent that referenced this pull request Sep 9, 2020
Following on from #1237, with new discoveries!

Turns out that in many cases, Apple Silicon Macs will in fact run Bash in amd64 mode, which in turn means the reported architecture is amd64, even though it's an Apple Silicon Mac. Homebrew has worked around this in Homebrew/brew#7995, and we more or less follow their lead here.

This makes it so that we _always_ show our little notice on Apple Silicon Macs, whether their shell is in amd64 or arm64 mode.
@k15tfu
Copy link

k15tfu commented Sep 14, 2020

btw, seems that it works now:

~ % /usr/bin/arch -arm64e sysctl sysctl.proc_translated
sysctl.proc_translated: 0
~ % /usr/bin/arch -x86_64 sysctl sysctl.proc_translated
sysctl.proc_translated: 1
~ % sysctl sysctl.proc_translated
sysctl.proc_translated: 0

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

Successfully merging this pull request may close these issues.

8 participants