Skip to content

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

Merged
vstinner merged 2 commits intopython:mainfrom
vstinner:py_build_core
Aug 21, 2023
Merged

gh-108220: Internal header files require Py_BUILD_CORE to be defined#108221
vstinner merged 2 commits intopython:mainfrom
vstinner:py_build_core

Conversation

@vstinner
Copy link
Copy Markdown
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).
@corona10 corona10 requested a review from gvanrossum August 21, 2023 17:07
@vstinner vstinner merged commit 21c0844 into python:main Aug 21, 2023
@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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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