Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.3k
bpo-41100: add runtime checks for MACOSX_DEPLOYMENT_TARGET=10.10#21577
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
bpo-41100: add runtime checks for MACOSX_DEPLOYMENT_TARGET=10.10#21577
Uh oh!
There was an error while loading.Please reload this page.
Conversation
In order to support a universal2 build, supporitng Mac OS 11 on arm64 and Mac OS onx86_64 going back to 10.10, we need to add in runtime checks for functions that willbe detected as present by autoconf, because they are in the SDK, but which did notexist in Mac OS 10.10. This fixes all the instances of -WWunguarded-availability-newwhen building with MACOSX_DEPLOYMENT_TARGET=10.10
ned-deily commentedJul 21, 2020
Thanks for the PR.@ronaldoussoren is working on a similar one at the moment. He can review and resolve differences. |
ronaldoussoren commentedJul 21, 2020
I have a similar patch in#21583, but that one is targeting macOS 10.9 instead of 10.10 and is incomplete and as of yet untested ("it compiles therefore it is correct"). I like your approach with HAVE_..._RUNTIME macro's, that reduces the amount of cruft compared with my approach. |
ronaldoussoren left a comment
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.
Just some comments at this time. The basic approach looks sane, but I haven't look at the changes in detail yet.
configure.ac Outdated
| AC_MSG_RESULT(yes) | ||
| ]) | ||
| AC_MSG_CHECKING([to see if the compiler supports __builtin_available]) |
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.
It is not necessary to test for this in a configure script, the following should work in a header file:
#ifdef __has_builtin#if __has_builtin(__builtin_available)#define HAVE_BUILTIN_AVAILABLE 1#endif#endifThere 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.
ok
Modules/posixmodule.c Outdated
| intasync_err=0; | ||
| structiovec*iov; | ||
| Py_buffer*buf; | ||
| if (HAVE_PWRITEV_RUNTIME) { |
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.
same as in os_preadv.
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.
ok
Modules/posixmodule.c Outdated
| intasync_err=0; | ||
| structiovec*iov; | ||
| Py_buffer*buf; | ||
| if (HAVE_PREADV_RUNTIME) { |
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.
I'd just ignore the compiler warning here, as this function will be removed from the module when HAVE_PREADV_RUNTIME is false.
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.
ok
Modules/posixmodule.c Outdated
| #defineHAVE_FUTIMENS_RUNTIME __builtin_available(macos 10.13, ios 11, tvos 11, watchos 4, *) | ||
| #defineHAVE_PREADV_RUNTIME __builtin_available(macos 10.16, ios 14, tvos 14, watchos 7, *) | ||
| #defineHAVE_PWRITEV_RUNTIME __builtin_available(macos 10.16, ios 14, tvos 14, watchos 7, *) | ||
| #defineHAVE_POSIX_SPAWN_SETSID_RUNTIME __builtin_available(macos 10.15, ios 13, tvos 13, watchos 6, *) |
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.
Is this (the introduction of POSIX_SPAWN_SETSID) documented somewhere?
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.
I couldn't find any proper documentation, I got 10.15 by looking up which version of xnu introduced it.
https://opensource.apple.com/source/xnu/xnu-4903.270.47/bsd/sys/spawn.h.auto.html
https://opensource.apple.com/source/xnu/xnu-6153.11.26/bsd/sys/spawn.h.auto.html
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.
It is 10.15. I know because I asked for it (cf. FB5361314 aka rdar://43247036).
ned-deily commentedOct 26, 2020
Thanks for the PR. The changes in the PR have been used as the basis for changes inGH-22855. |
Uh oh!
There was an error while loading.Please reload this page.
In order to support a universal2 build, supporitng Mac OS 11 on arm64 and Mac OS on
x86_64 going back to 10.10, we need to add in runtime checks for functions that will
be detected as present by autoconf, because they are in the SDK, but which did not
exist in Mac OS 10.10. This fixes all the instances of -WWunguarded-availability-new
when building with MACOSX_DEPLOYMENT_TARGET=10.10
https://bugs.python.org/issue41100