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

Fix OS X platform detection for pep425tags #3232

Merged
merged 1 commit into from
Nov 7, 2015

Conversation

rmcgibbo
Copy link
Member

@rmcgibbo rmcgibbo commented Nov 7, 2015

For the purpose of detecting the user's platform to determine which wheels are compatible, this switches pep425tags.get_platform() to avoid distutils.util.get_platform() on OS X, because that method returns the release on which the interpreter was built (often but not always 10.5 or 10.6) as opposed to the platform on which the interpreter is running.

see also: https://mail.python.org/pipermail/distutils-sig/2015-November/027578.html

Review on Reviewable

For the purpose of detecting the user's platform to determine which
wheels are compatible, this switches pep425tags.get_platform() to
avoid distutils.util.get_platform() on OS X, because that method
returns the release on which the current interpreter was built (often
10.5 or 10.6) as opposed to the platform on which the interpreter is
running.
@dstufft
Copy link
Member

dstufft commented Nov 7, 2015

Just to be clear, currently we'll accept any wheel compiled against an SDK starting from 10.0 and up to and including the SDK version that Python was built against (10.6 for Python.org). This change will change that so it's up to the version of OSX that is currently running instead.

I'm not super familiar with the OSX binary ABI rules, so questions:

  • Can a Python compiled against the 10.6 SDK load a wheel that was compiled against the 10.10 SDK when running on OSX 10.10 or later?
  • Can a Python compiled against the 10.10 SDK load a wheel that was compiled against the 10.6 SDK when running on OSX 10.10 or later?
  • Can a Python compiled against the 10.10 SDK even run on a version of OSX < 10.10?

/cc @ned-deily @reaperhulk

@rgommers
Copy link

rgommers commented Nov 7, 2015

IIRC the answers are yes/yes/no, but if someone knows of a nice write-up of these rules that would be very useful.

@dstufft
Copy link
Member

dstufft commented Nov 7, 2015

Ok. So this is probably safe to merge then.

Sent from my iPhone

On Nov 7, 2015, at 10:03 AM, Ralf Gommers [email protected] wrote:

IIRC the answers are yes/yes/no, but if someone knows of a nice write-up of these rules that would be very useful.


Reply to this email directly or view it on GitHub.

dstufft added a commit that referenced this pull request Nov 7, 2015
Fix OS X platform detection for pep425tags
@dstufft dstufft merged commit e8bbf80 into pypa:develop Nov 7, 2015
@rmcgibbo rmcgibbo deleted the pep425tags branch November 7, 2015 21:07
@ghost
Copy link

ghost commented Feb 26, 2016

@dstufft: Doesn't this change mercilessly break shit?

Previously, I could just compile a python interpreter on the newest version of OSX and then all wheels would work, regardless of the platform that my interpreter was running on.

Now, I have to make sure that all of my wheels are built with an interpreter that was compiled on a version of OSX not newer than the version my interpreter is going to run on.

@rgommers
Copy link

Previously, I could just compile a python interpreter on the newest version of OSX and then all wheels would work, regardless of the platform that my interpreter was running on.

Are you really doing that? Building against an older SDK isn't that reliable, Apple's backwards compat is a lot better than its forward compat. IIRC the python.org installers are always built on the lowest supported OS X version.

@ghost
Copy link

ghost commented Feb 26, 2016

If your python interpreter is built on an ancient OS X version then you're fucked if someone is distributing wheels built on an interpreter that was built on a newer OS X version (previously this was so).

@rgommers
Copy link

And can you elaborate on the issue you're having, preferably with a concrete example?

@ghost
Copy link

ghost commented Feb 26, 2016

I built a python interpreter on OSX 10.11

>>> import distutils.util
>>> distutils.util.get_platform()
'macosx-10.11-intel'
>>> import platform
>>> platform.mac_ver()
('10.10.5', ('', '', ''), 'x86_64')

This was nice because I didn't need to worry about what interpreter wheel distributors were building stuff with because newer interpreter build system versions are considered compatible with older wheel build system versions.

For example, this was useful with cffi==1.3.0, https://pypi.python.org/pypi/cffi/1.3.0 because it only had wheels for 10.10. Those wheels weren't going to install with python.org or Apple distributions because those interpreters were built on 10.6/10.8.

At this point, the cat was out of the bag and I had a custom python build, so I started building other wheels against my 10.11 system. The interpreter and wheels then worked no problem on 10.10 and 10.11 systems.

But with the newer version of pip, these wheels won't work on 10.10 systems. Seems like a breaking change.

@rgommers
Copy link

At this point, the cat was out of the bag and I had a custom python build, so I started building other wheels against my 10.11 system. The interpreter and wheels then worked no problem on 10.10 and 10.11 systems.

This is unsupported usage and is not guaranteed to work. I'm surprised that the interpreter even runs, that's just luck I think. You need to set MACOSX_DEPLOYMENT_TARGET to 10.10 if you want to compile things on 10.11 and run them on 10.10. And I've even had problems with SDKs doing that; it works in the vast majority of cases, but the most robust way is compiling on the lowest OS version you need.

@rgommers
Copy link

But with the newer version of pip, these wheels won't work on 10.10 systems. Seems like a breaking change.

So those wheels are named ...-macosx_10_11_intel.whl right? Then it seems correct that they don't run on 10.10.

@ghost
Copy link

ghost commented Feb 26, 2016

This is unsupported usage and is not guaranteed to work.

Unsupported by Apple you mean?

...but the most robust way is compiling on the lowest OS version you need.

Right

So those wheels are named ...-macosx_10_11_intel.whl right? Then it seems correct that they don't run on 10.10.

I'm not saying that the old way was correct. Just that the new way is incompatible with the assumptions the old way allowed you to make. I'm going to rebuild all of my wheels/interpreter, but just want to point out that this definitely broke shit.

tl;dr You broke my hack that seemed inline with pip's policy but involved unsupported Apple behavior.

@rgommers
Copy link

This is unsupported usage and is not guaranteed to work.

Unsupported by Apple you mean?

indeed.

tl;dr You broke my hack that seemed inline with pip's policy but involved unsupported Apple behavior.

Fair enough, can be annoying if it used to make your life easier. Don't think it was "pip policy" though, more like a plain bug that was fixed by this PR.

@ghost
Copy link

ghost commented Feb 27, 2016

By policy, I mean this document.

The platform tag comes from the output of python -c "import distutils.util;
print(distutils.util.get_platform())"  on the platform building the wheel. I (MB) built
numpy-1.8.0-cp27-none-macosx_10_6_intel.whl with Python 2.7 from a Python.org
binary installer on an OSX 10.9 machine. The OSX platform tag further  breaks down
to:
    macosx
    10_6 (the version of the SDK used to compile Python)
    intel (short-hand for a fat binary containing x86_64 and i386 objects)

As you see, the SDK part of the tag is not from the OSX running on the machine I
(MB) was building from  but from the distutils configuration on the Python I was building
for. In this case I was building for a Python.org binary, and that Python.org binary gave
macosx-10.6-intel from distutils get_platform().

Up until pip 6.0, for pip to accept the wheel as matching its own platform, these values
have to match exactly. Meaning, that the Python running pip on the installing machine
has to have a value from distutils.util.get_platform() that matches macosx_10_6_intel
exactly (after converting dots and dashes to underscores).

Anyways, it's Friday, have a good weekend.

@rgommers
Copy link

Thanks @burrows-labs. That's not a pip document, but it does reverse engineer what pip used to do. So can't hurt to ping the author to sanity-check this PR and my comments above (and update the now outdated description). @matthew-brett ping

@matthew-brett
Copy link

I'm a bit confused I'm afraid - @burrows-labs - you are building wheels with an interpreter reporting itself as:

>>> distutils.util.get_platform()
'macosx-10.11-intel'

These wheels presumably have names like mypackage-0.1--cp27-cp27m-macosx_10_11_intel.whl ?

Then you are trying to install these wheels, via pip, on an interpreter reporting itself as something like:

>>> distutils.util.get_platform()
'macosx-10.10-intel'

?

@rgommers
Copy link

@matthew-brett no, his interpreter is built on 10.11 so gives 'macosx-10.11-intel' (also when running on 10.10), but this PR changed the logic pip uses so it doesn't use distutils.util.get_platform() anymore.

I pinged you because (a) you're one of the experts here so would be nice if you could verify that this PR and my answers above are correct, and (b) your description isn't quite correct anymore after this PR so you may want to update it.

@matthew-brett
Copy link

OK - sorry - I've now read the thread the Robert pointed to at the beginning, and this thread with more attention.

Explaining to myself:

Interpreter SDK = SDK with which the interpreter was built;
Wheel SDK = SDK with which the wheel was built;
Platform version = OSX version on which interpreter is running.

Initial pip rule: A) wheel SDK <= interpreter SDK.
New pip rule: B) wheel SDK <= platform version.

The change broke stuff for @burrows-labs because they are building with the 10.11 SDK (interpreter SDK) but running on an earlier platform version, so that wheels that appear compatible when running on 10.11 appear incompatible when running on 10.10.

The assumption of the initial rule A) is that wheels should be using an SDK compatible with the interpreter. The assumption of the new rule B) is that that the only thing that matters is the platform on which the wheel will run (we can assume SDKs of versions <= platform version are available).

We've got good reason to think that interpreter SDK can be > wheel SDK, as wheels built with the 10.6 SDK have been working well with homebrew / system Python, almost always built with higher SDKs.

So, the key question is, whether the interpreter SDK can be < wheel SDK (Robert's initial example was Interpreter SDK == 10.5, wheel SDK == 10.6, platform version == 10.10). I don't know the answer to that. @minrk, @ned-deily - do you know whether there is a risk of using extension code compiled with a later SDK than that of the interpreter?

@rmcgibbo
Copy link
Member Author

I don't know the answer to that.

I think it can be. IMO, the evidence to that effect comes from many people using interpreter SDK == 10.5/10.6 from Python.org or a re-distributor like Anaconda, and then either compiling or downloading wheels compiled with a more recent SDK, and things working fine.

@ghost
Copy link

ghost commented Feb 28, 2016

@matthew-brett That sums up my understanding of the issue.

@matthew-brett
Copy link

Robert - I don't think it's that common to download or compile wheels compiled for a higher SDK. By default the standard wheels are compiled against Python.org Python, and therefore SDK 10.6. If you build wheels against Python.org Python / Anaconda, you'll get 10.6 / 10.5 SDK.

@ned-deily
Copy link

I think there's some confusion here between SDK and Deployment Target. The value reported by distutils.util.get_platform() is the value of the Deployment Target, normally set implicitly or explicitly by the environment variable MACOSX_DEPLOYMENT_TARGET. The deployment target essentially describes a minimum OS X release level (or ABI) that the compiled and linked code will be able to run against. So if the deployment target is 10.6, the resultant binary should run on 10.6 and any higher releases (assuming the code makes no use of interfaces removed in later releases). The SDK version essentially defines the maximum known OS X release level (ABI) available for use during compilation and linking. distutils.util.get_platform() and, as far as I know, pip wheels do not contain information about what SDK was used. It can be assumed that the SDK release was greater than or equal to the deployment target release. If the deployment target is less than the SDK release and if your code tries to make use of OS X features released after the deployment target release, your code needs to be prepared to deal with missing symbols et al; you may also get compile time warnings about deprecated or removed features. The Apple Developer documentation goes into some more detail here.

@minrk
Copy link
Contributor

minrk commented Feb 29, 2016

I've actually yet to find a combination that doesn't work in terms of platform, Python, and package deployment targets, probably because the libraries I test with don't call APIs that have changed. It would perhaps be best to find some APIs that are added/deprecated/removed and build extensions that call those to see what actually works. Checking the current platform version makes sense to me, but I don't know what happens when you load two libraries built against different targets calling changed versions of the same API.

@matthew-brett
Copy link

@ned-deily - thanks for the clarification. From the doc you pointed to, Apple makes a distinction between "Deployment target" (specified by MACOSX_DEPLOYMENT_TARGET) and "Base SDK" (specified by the -isysroot flag). Setting the base SDK to be higher than the deployment target SDK allows the developer to write code to conditionally use later ABIs when available on the runtime platform. A Python extension builder would have to specifically add -isysroot to CFLAGS to change this, so I guess in practice the MACOSX_DEPLOYMENT_TARGET nearly always corresponds to the ABI level of the built extension. My Python.org Python interpreter also seems to have set the default -isysroot to the 10.6 SDK corresponding to the default MACOSX_DEPLOYMENT_TARGET (python -c 'import pprint; from distutils import sysconfig; pprint.pprint(sysconfig.get_config_vars())' | grep isysroot).

@matthew-brett
Copy link

Min - can you think of any ABI changes that we might run into, and could test against?

@dstufft
Copy link
Member

dstufft commented Feb 29, 2016

Let me just say with my pip developer hat on, that we probably want to do what is considered the "correct" thing to do wrt what we accept as a viable wheel not just whatever works.

Just to be clear though, there are two sides to this thing, one is what wheel sets (which I assume should be the deployment target, which I believe is what it is now) and the other is which wheels should pip itself support which I think is currently >= to the current platform we're running on, ignoring what platform the interpreter was built for. I think the assumption was that a wheel built for 10.11 may not run on 10.11 (missing symbols and the like) so we shouldn't attempt to use it. Perhaps this doesn't actually matter unless you're using one of the APIs provided by the SDK and we should offer a more generic version of the platform tag.

@minrk
Copy link
Contributor

minrk commented Mar 1, 2016

we probably want to do what is considered the "correct" thing to do wrt what we accept as a viable wheel not just whatever works.

Certainly. I only meant to convey that I haven't found examples that don't work, which we could use to double check that we are doing the right thing.

one is what wheel sets (which I assume should be the deployment target, which I believe is what it is now)

I think this is almost right. The wheel always stores the deployment target of Python itself, which will be the deployment target of the library unless MACOSX_DEPLOYMENT_TARGET is set explicitly. Changing the deployment target does not change the string stored in the wheel, which perhaps it should.

the other is which wheels should pip itself support which I think is currently >= to the current platform we're running on

After this PR, yes. I think that's the correct behavior, but haven't found an authoritative answer or test case to back it up. I'm confident that it's the right interpretation for whole programs. Where I haven't found a test case or confirmation is what happens when you have load libraries with different targets in the same process.

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 3, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants