bpo-35134: Create Include/cpython/ subdirectory#10624
Conversation
|
I rebased my change on top of master to retrieve PR #10626 bugfix. |
Sorry, something went wrong.
|
@ncoghlan, @serhiy-storchaka, @pitrou, @zooba, @ericsnowcurrently: Would you mind to review this PR? I created a new Include/unstable/ subdirectory for all "#ifndef Py_LIMITED_API" APIs. See https://bugs.python.org/issue35134 for the rationale. |
Sorry, something went wrong.
|
I plan to merge this PR next week. |
Sorry, something went wrong.
|
@ncoghlan: You approved the PR but proposed a different directory name. Are you ok with "unstable" name? https://bugs.python.org/issue35134#msg330261 |
Sorry, something went wrong.
|
Note: this PR is my second attempt, the first one was PR #10285 which wasn't properly designed. |
Sorry, something went wrong.
|
There's no real easy way to test it, but Better to make the change now and cause the installer build to break than to accidentally ship without necessary headers. If it doesn't work, I'll fix it up. |
Sorry, something went wrong.
|
@zooba: Aha, I was looking for the code of the Windows installer.
Right now, Include/internal/ is not installed (on Unix at least). I just proposed to install these headers as well: https://bugs.python.org/issue35296 Would it be possible to only update the MSI installer once this PR and PR #10665 are merged? |
Sorry, something went wrong.
|
I renamed the subdirectory from Include/unstable/ to Include/cpython/. |
Sorry, something went wrong.
|
@vstinner If you're willing to keep reminding me to do it once this is merged until it's done, sure. |
Sorry, something went wrong.
Include/*.h should be the "portable Python API", whereas Include/cpython/*.h should be the "CPython API": CPython implementation details. Changes: * Create Include/cpython/ subdirectory * "make install" now creates $prefix/include/cpython and copy Include/cpython/* to $prefix/include/cpython * Create Include/cpython/objimpl.h: move objimpl.h code surrounded by "#ifndef Py_LIMITED_API" to cpython/objimpl.h. * objimpl.h now includes cpython/objimpl.h * Windows installer (MSI) now also install Include/ subdirectories: Include/cpython/ and Include/internal/.
|
@vstinner While I preferred "cpython" as the directory name, I was OK with "unstable", hence the original approval. I'm happy you decided to switch the name anyway, though :) |
Sorry, something went wrong.
Include/.h should be the "portable Python API", whereas
Include/cpython/.h should be the "CPython API": CPython
implementation details.
Changes:
Include/cpython/* to $prefix/include/cpython
surrounded by "#ifndef Py_LIMITED_API" to cpython/objimpl.h.
https://bugs.python.org/issue35134