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

Add intel cpu_arch on MacOS as universal-compatible #361

Merged
merged 2 commits into from
Nov 30, 2020

Conversation

pradyunsg
Copy link
Member

As noted in #319 (comment)

Identified in pypa/pip#9170.

@@ -410,7 +410,7 @@ def _mac_binary_formats(version, cpu_arch):
if cpu_arch in {"arm64", "x86_64"}:
formats.append("universal2")

if cpu_arch in {"x86_64", "i386", "ppc64", "ppc"}:
if cpu_arch in {"x86_64", "i386", "ppc64", "ppc", "intel"}:

Choose a reason for hiding this comment

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

Why do we add only "universal" and not "universal2" for "intel"? (I'm not sure when this arch name is used)

Copy link
Member Author

Choose a reason for hiding this comment

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

See https://github.com/python/cpython/blob/41761933c1c30bb6003b65eef1ba23a83db4eae4/Mac/README.rst#21-flavors-of-universal-binaries

intel refers to a universal binary that works on i386 and x86_64 -- which are both supported by universal.

@pradyunsg
Copy link
Member Author

pradyunsg commented Nov 29, 2020

The slightly broader question here is whether we want to allow universal architectures (i.e. architectures used to refer to binaries that work on multiple ISAs) to be passed into this function as cpu_arch.

As on today, there's only 1 special case involved here: intel and it was (unintentionally?) permitted in earlier releases -> universal wheels would be OK if you pass in intel as the cpu_arch.

Looking at the downstream pip implementation of allowing users to pass in custom tags, and given that there's only 1 special case we allow today, I feel like it is only natural to conclude "yes, we should allow this". :)

@brettcannon
Copy link
Member

@ned-deily @ronaldoussoren does this change make sense to you?

@pradyunsg I think you're right, but I just want the Mac expert's opinion. But if you don't hear from anyone within a day and need to get this out for pip then I'm fine merging it.

@pradyunsg
Copy link
Member Author

Sure! I don't mind waiting here and am 100% on board for waiting on a Mac expert's opinion.

As noted in pypa/pip#9170 (comment), I don't think a large group of people are gonna hit this issue, but it'd still be nice to have a fix sometime reasonably soon.

@ronaldoussoren
Copy link
Contributor

Assuming _mac_binary_formats(.., cpu_arch) returns the list of cpu tags compatible with cpu_arch adding "universal" to the result when cpu_arch is "intel" is fine as "universal" is a 4-way fat binary including both architectures present in the 2-way fat binaries denoted by "intel".

That said, it is highly unlikely that you'll ever run into these. AFAIK we've never shipped "universal" installers for CPython.

@pradyunsg
Copy link
Member Author

AFAIK we've never shipped "universal" installers for CPython.

LOL

Seems like this is purely theoretical use case then? I genuinely don't think anyone other than CPython devs are building CPython's MacOS Installers.

@pradyunsg pradyunsg merged commit 5037e4c into pypa:master Nov 30, 2020
@pradyunsg pradyunsg deleted the intel-macos-arch branch November 30, 2020 22:40
@ned-deily
Copy link

LGTM, too. Thanks!

BTW, there are builders and users of universal macOS binaries other than just for python.org downloadable installers. For example, third-party apps written in Python generally need to include a copy of Python that may be built by the app developer and there are probably still some apps being shipped that still support the intel variant of universal builds (i.e. intel-32 and intel-64). But it is true, as Ronald points out, that many of the defined variants are likely barely used anymore (and in particular the universal universal (4-way) variant's time came and went pretty quickly). OTOH, the cost to keep supporting them seems very low; easier than trying to find out who might still may be using them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants