-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Change several deprecated symbols in _macosx.m #18128
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
Conversation
How far back to the non-deprecated symbols exist? With 3.3 (see #18066 and #17956 ) we accidentally put a floor on the version of OSX to support. What is below is a brain dump. I think that OSX SDK version should fall under the "system and C-dependencies" category of https://matplotlib.org/devel/min_dep_policy.html so we should should support "as old as practical". We should define what "practical" means in this case (as per #18066 ). Given discussion in macports/macports-ports@b07e0eb it seems at least some people are advocating for a pattern of building on new OSX but being able to run on older OSX (which fits with how we do the wheels), but leaves people who a) have very old macs b) want to build from source them selves in a bind. The built Python on python.org reportedly supports back to 10.9 (I suspect because they are not using much of the SDK surface particularly around GUI work the way we do), macports also tries to support back very far (but that is their jam). |
Right, the new symbols probably don't exist in 10.11. It looks like Travis can run builds in OSX 10.11 so once the minimum version is decided, perhaps we should test the build in that version: https://docs.travis-ci.com/user/reference/osx/#macos-version |
I think that second commit is the way to stay compatible, but I can't actually properly test myself. I recently had to upgrade my remaining 10.13 system, because various pieces of software were dropping support. |
Unfortunately, due to 👍 to the changes. |
16309ca
to
c97b09e
Compare
Converting to draft until I can figure out the CI build |
Given that macports is happy (for now) to patch us and are considering moving to build on newer systems to target older systems this seems like a good place to draw the line. |
The compiler warns that some of these have been deprecated in MacOS 10.12, some in 10.14.
c97b09e
to
ae316b4
Compare
Thanks @jkseppan ! |
This does not build wheels when targeting 10.9: https://github.com/matplotlib/matplotlib/actions/runs/221342241 |
I thought we weren't ourselves supporting building on 10.9? |
Hmm, well the MacPython wheels still specify 10.9, but maybe we determined that that had no effect? And now cibuildwheel is correctly passing that along? |
I'm very confused by this though. The PR is doing
but the error we get is:
which makes me worry I don't understand what |
or at least the order in which the version checking vs marco expansion happens... |
@@ -18,6 +18,40 @@ | |||
#define COMPILING_FOR_10_10 | |||
#endif | |||
|
|||
#if __MAC_OS_X_VERSION_MAX_ALLOWED < 101200 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should you not be checking for minimum required, like above?
So what do we want to do about this? Target a newer macOS version? It also breaks building on the shared Mac mini. |
I think we should make a decision about the minimum version of macOS that we support (#18066). Based on this comment by @tacaswell I would suggest making that 10.12:
Whichever way that's decided, we can build wheels targeting the supported version and up, and fix the build as needed. |
The compiler warns that some of these have been deprecated in
MacOS 10.12, some in 10.14.
PR Summary
PR Checklist