bpo-32705: Current Android does not have posix_spawn #5413
Conversation
|
I think a NEWS entry is not necessary as this patch is a fix to a new feature landed today (6c6ddf9). From https://devguide.python.org/committing/#what-s-new-and-news-entries:
|
Modules/posixmodule.c
Outdated
There was a problem hiding this comment.
Isn't there a way to check the Android API level at build time? Per your comments on the issue, when running on P (API 28) it should be available.
If not, at least mention that P (API 28) contains the API in a comment here so someone can revisit this in the future.
There was a problem hiding this comment.
Technically the macro __ANDROID_API__ can be used, while I prefer to wait until Android P is publicly available so that test_posix_spawn can be run before enabling it.
There was a problem hiding this comment.
I'm fine with always skipping, and "revisit" the code once API 28 is released.
|
A better fix would be to check for posix_spawn() in configure, but it can be done later. |
I found there's already one: 6c6ddf9#diff-67e997bcfdac55191033d57a16d1408aR3434. Maybe the HAVE_POSIX_SPAWN define in posixmodule.c is not necessary? |
|
@vstinner: Please replace |
configure checks for posix_spawn() availability but then posixmodule.c defines "#define HAVE_POSIX_SPAWN 1"? I'm not sure that "#define HAVE_POSIX_SPAWN 1" makes sense. Would you like to propose a patch to remove this #define? I merged your PR to get a working Python 3.7beta1 on Android. I suggest to wait after the beta1 to revisit (remove) the #define. |
https://bugs.python.org/issue32705