Skip to content
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

gh-108220: Internal header files require Py_BUILD_CORE to be defined #108221

Merged
merged 2 commits into from
Aug 21, 2023

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Aug 21, 2023

  • pycore_intrinsics.h does nothing if included twice (add #ifndef and #define).
  • Update Tools/cases_generator/generate_cases.py to generate the Py_BUILD_CORE test.

…fined

* pycore_intrinsics.h does nothing if included twice
  (add #ifndef and #define).
* Update Tools/cases_generator/generate_cases.py to generate the
  Py_BUILD_CORE test.
_bz2, _lzma, _opcode and zlib extensions now define the
Py_BUILD_CORE_MODULE macro to use internal headers (pycore_code.h,
pycore_intrinsics.h and pycore_blocks_output_buffer.h).
@vstinner vstinner merged commit 21c0844 into python:main Aug 21, 2023
17 checks passed
@vstinner vstinner deleted the py_build_core branch August 21, 2023 17:15
Comment on lines +407 to +411
self.out.emit("\n" + textwrap.dedent("""
#ifndef Py_BUILD_CORE
# error "this header requires Py_BUILD_CORE define"
#endif
""").strip())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why bother asking for a review if you're going to merge it while I'm getting coffee? What response time do you expect for reviews?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why bother asking for a review

Hmm, I requested the review to you.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was venting at @vstinner who ignored that and merged eight minutes later. :-(

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't ask for a review on such "cleanup" change. I didn't see that @corona10 asked for a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants