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

vendor-install: handle native ARM running under Rosetta #12417

Merged
merged 6 commits into from Nov 11, 2021

Conversation

carlocab
Copy link
Member

@carlocab carlocab commented Nov 11, 2021

Since HOMEBREW_PROCESSOR is populated using uname -m, this will
register as Intel even when a native ARM install is running under
Rosetta.

Let's work around this by checking sysctl -n machdep.cpu.brand_string.
On my Intel machine:

❯ sysctl -n machdep.cpu.brand_string
Intel(R) Core(TM) i3-1000NG4 CPU @ 1.10GHz

On Apple Silicon:

brew@HMBRW-A-001-M1-001 ~ % sysctl -n machdep.cpu.brand_string
Apple M1
brew@HMBRW-A-001-M1-001 ~ % arch -x86_64 sysctl -n machdep.cpu.brand_string
Apple M1

The case of a Rosetta installation of Homebrew is already handled below
the proposed change.

Fixes Homebrew/discussions#2434.

Since `HOMEBREW_PROCESSOR` is populated using `uname -m`, this will
register as `Intel` even when a native ARM install is running under
Rosetta.

Let's work around this by checking `sysctl -n machdep.cpu.brand_string`.
On my Intel machine:

    ❯ sysctl -n machdep.cpu.brand_string
    Intel(R) Core(TM) i3-1000NG4 CPU @ 1.10GHz

On Apple Silicon:

    brew@HMBRW-A-001-M1-001 ~ % sysctl -n machdep.cpu.brand_string
    Apple M1

The case of a Rosetta installation of Homebrew is already handled below
the proposed change.

Fixes Homebrew/discussions#2434.
@carlocab carlocab added the critical Critical change which should be shipped as soon as possible. label Nov 11, 2021
@BrewTestBot
Copy link
Member

Review period skipped due to critical label.

The previous commit only prevented the installation of an Intel Portable
Ruby into `/opt/homebrew` prefix. Let's actually install an ARM64
Portable Ruby there too.
Whenever the `sysctl` call does not match `"Apple"*`, we can be sure
that `HOMEBREW_PROCESSOR` is `Intel`, so there's no need for this
additional check.
This condition is always true when we've reached this branch.
@MikeMcQuaid
Copy link
Member

Thanks for the PR @carlocab, just want to tweak the approach here a bit.

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.

This looks great, nice work!

# used in vendor-install.sh
# shellcheck disable=SC2034
HOMEBREW_PHYSICAL_PROCESSOR="arm64"
HOMEBREW_ROSETTA="$(sysctl -n sysctl.proc_translated)"
Copy link
Member

Choose a reason for hiding this comment

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

Could potentially be used in Library/Homebrew/extend/os/mac/hardware/cpu.rb but not sure if it's worth the effort 🤷🏻

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, I thought about that, but decided doing

ENV["HOMEBREW_ROSETTA"].present? && ENV["HOMEBREW_ROSETTA"] == "1"

was less nice.

@carlocab carlocab merged commit 3b41be9 into Homebrew:master Nov 11, 2021
@carlocab carlocab deleted the rosetta-arm branch November 11, 2021 10:04
@github-actions github-actions bot added the outdated PR was locked due to age label Dec 12, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
critical Critical change which should be shipped as soon as possible. outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants