Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.3k
gh-130740: Move somestdbool.h includes afterPython.h#130738
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Except when "pyconfig.h" is included first, and for platform specificcode such as android/emscripten
I see that the original PR didn't have an issue number but I would have been comfortable with one. We can also add a NEWS for build as some people might patch the sources (at least they would be notified of the change even if it's not an important one). |
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 to be sure, butPython.h already includesstdbool.h right? So we can just outright remove it or is there a reason not to?
It doesn't, or at least not directly |
stdbool.h includes afterPython.hstdbool.h includes afterPython.hstdbool.h includes afterPython.hThere 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 doesn't, or at least not directly
Oh I thought it did. Ok then the changes look good to me. Also, it's still better to include it explicitly.
PEP-7 doesn't say which includes should come first in general; should we treat local imports before standard ones, namely:
#include"Python.h"// include glibc or others system libs// include pycore_*// others?// local includes
so I think we can merge this one, unless someone tells me "no!". Just to be sure, I'll run the build bots on it to see if it causes some compilation failure. Don't commit in the meantime.
I added a NEWS entry, although I used the PR number as issue as well. Let me know that's OK enough |
I don't think we can do it. We need an issue. I'll create one. |
Feel free to use this one:#130740 |
stdbool.h includes afterPython.hstdbool.h includes afterPython.hOnce you've updated the NEWS entry, I'll run the build bots (as Sam and I aren't sure which one is an issue/not an issue). |
Misc/NEWS.d/next/Build/2025-03-01-18-20-07.gh-issue-130738.nDFSHR.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
…SHR.rstCo-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
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.
You need to delete one of the two blurb entries
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'll run the build bots so don't touch the PR, TiA!
bedevere-bot commentedMar 1, 2025
🤖 New build scheduled with the buildbot fleet by@picnixz for commit6a6e5b1 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F130738%2Fmerge If you want to schedule another build, you need to add the🔨 test-with-buildbots label again. |
Misc/NEWS.d/next/Build/2025-03-01-18-27-42.gh-issue-130740.nDFSHR.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
214562e intopython:mainUh oh!
There was an error while loading.Please reload this page.
Thank you! |
Thanks@chouquette for the PR, and@picnixz for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12. |
Thanks@chouquette for the PR, and@picnixz for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13. |
Sorry,@chouquette and@picnixz, I could not cleanly backport this to |
Sorry,@chouquette and@picnixz, I could not cleanly backport this to |
GH-130756 is a backport of this pull request to the3.13 branch. |
…hon#130738)Move some `#include <stdbool.h>` after `#include "Python.h"` when `pyconfig.h` is notincluded first and when we are in a platform-agnostic context. This is to avoid havingfeatures defined by `stdbool.h` before those decided by `Python.h`.
GH-130757 is a backport of this pull request to the3.12 branch. |
…30738) (#130756)gh-130740: Move some `stdbool.h` includes after `Python.h` (#130738)Move some `#include <stdbool.h>` after `#include "Python.h"` when `pyconfig.h` is notincluded first and when we are in a platform-agnostic context. This is to avoid havingfeatures defined by `stdbool.h` before those decided by `Python.h` (this caused somebuild failures when compiling CPython with `zig cc`).(cherry-picked from commit214562e)---------Co-authored-by: Hugo Beauzée-Luyssen <hugo@beauzee.fr>
…30738) (#130757)gh-130740: Move some `stdbool.h` includes after `Python.h` (#130738)Move some `#include <stdbool.h>` after `#include "Python.h"` when `pyconfig.h` is notincluded first and when we are in a platform-agnostic context. This is to avoid havingfeatures defined by `stdbool.h` before those decided by `Python.h` (this caused somebuild failures when compiling CPython with `zig cc`).(cherry-picked from commit214562e)---------Co-authored-by: Hugo Beauzée-Luyssen <hugo@beauzee.fr>
…hon#130738)Move some `#include <stdbool.h>` after `#include "Python.h"` when `pyconfig.h` is notincluded first and when we are in a platform-agnostic context. This is to avoid havingfeatures defined by `stdbool.h` before those decided by `Python.h`.
Uh oh!
There was an error while loading.Please reload this page.
Except when "pyconfig.h" is included first, and for platform specific code such as android/emscripten
Following up from#130641 (comment)
stdbool.his included afterPython.h#130740