gh-85277: Fix building without stropts.h or empty stropts.h#143521
gh-85277: Fix building without stropts.h or empty stropts.h#143521vstinner merged 7 commits intopython:mainfrom
stropts.h or empty stropts.h#143521Conversation
shadchin
requested review from
AA-Turner,
corona10,
emmatyping and
erlend-aasland
as code owners
| #if !defined(I_PUSH) | ||
| #error I_PUSH | ||
| #endif |
There was a problem hiding this comment.
Use AC_CHECK_DECL instead of a program.
| AC_COMPILE_IFELSE([ | ||
| AC_LANG_PROGRAM([[ | ||
| AC_CHECK_DECL([I_PUSH], [ | ||
| AC_DEFINE([HAVE_STROPTS_H], [1], [Define to 1 if you have the <stropts.h> header file.]) |
There was a problem hiding this comment.
Please properly wrap the calls. E.g., use the 2-space indents for inner blocks. Have a look at https://github.com/python/cpython/pull/142380/files#r2615800453 and the code I commented.
picnixz
left a comment
picnixz
left a comment
There was a problem hiding this comment.
Please add a NEWS entry as well.
|
There is still some style issue. I will write a suggestion when I am on a laptop. |
| AC_CHECK_DECL([I_PUSH], [AC_DEFINE([HAVE_STROPTS_H], [1], [Define to 1 if you have the <stropts.h> header file.])], [], [ | ||
| #ifdef HAVE_SYS_TYPES_H | ||
| #include <sys/types.h> | ||
| #endif | ||
| #include <stropts.h> | ||
| ]) |
There was a problem hiding this comment.
| AC_CHECK_DECL([I_PUSH], [AC_DEFINE([HAVE_STROPTS_H], [1], [Define to 1 if you have the <stropts.h> header file.])], [], [ | |
| #ifdef HAVE_SYS_TYPES_H | |
| #include <sys/types.h> | |
| #endif | |
| #include <stropts.h> | |
| ]) | |
| AC_CHECK_DECL([I_PUSH], [ | |
| AC_DEFINE( | |
| [HAVE_STROPTS_H], [1], | |
| [Define to 1 if you have the <stropts.h> header file.] | |
| )], [], [ | |
| #ifdef HAVE_SYS_TYPES_H | |
| # include <sys/types.h> | |
| #endif | |
| #include <stropts.h> | |
| ]) |
Hopefully, I got my brackets right. Otherwise, put them when they are needed. m4 has some weird syntax and the way I put my blank lines may also be not supported.
|
@picnixz I've fixed everything. Is there anything else I need to do? |
picnixz
left a comment
picnixz
left a comment
There was a problem hiding this comment.
Sorry I forgot about it. I will run the build bots. That is, do NOT commit as otherwise we lose the CI status.
picnixz
added
awaiting review
🔨 test-with-buildbots
|
🤖 New build scheduled with the buildbot fleet by @picnixz for commit ba8f793 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F143521%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
bedevere-bot
removed
the
🔨 test-with-buildbots
|
There's nothing going on here. Can I help? |
emmatyping
added
the
🔨 test-with-buildbots
|
🤖 New build scheduled with the buildbot fleet by @emmatyping for commit ba8f793 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F143521%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
bedevere-bot
removed
the
🔨 test-with-buildbots
There was a problem hiding this comment.
I wanted to take a look at another buildbot run since Android failed in the last one but barring any other concerns by @picnixz I think this looks OK to merge
|
The change LGTM. I merged it. Thanks for the fix. |
5 participants
Added a check that not only do we have
stropts.h, butI_PUSHis also defined.I used
I_PUSHbecause it is used in two places (Modules/posixmodule.c,Modules/fcntlmodule.c), the rest of the definitions are only inModules/fcntlmodule.c.Also fix for gh-81541 if building without OpenPTY and without stropts.h.
Other related issues: