Skip to content

Move internal headers to Include/internal/ #79262

Closed
Closed
Move internal headers to Include/internal/#79262
Labels
3.8 (EOL)end of lifeinterpreter-core(Objects, Python, Grammar, and Parser dirs)

Description

@vstinner
vstinner
opened on Oct 27, 2018
BPO 35081
Nosy @malemburg, @tim-one, @mdickinson, @scoder, @vstinner, @ericsnowcurrently, @serhiy-storchaka, @izbyshev
PRs
  • [WIP] bpo-35081: Include/internal/ becomes Include/pycore/ #10143
  • bpo-35081: Remove Py_BUILD_CORE from datetime.h #10238
  • bpo-35081: Move Include/pyatomic.c to Include/internal/ #10239
  • bpo-35081: Cleanup pystate.c and pystate.h #10240
  • bpo-35081: Move Py_BUILD_CORE code to internal/mem.h #10249
  • bpo-35081: Add pycore_ prefix to internal header files #10263
  • bpo-35081: Add _PyThreadState_GET() internal macro #10266
  • bpo-35081: Move accu.h to Include/internal/pycore_accu.h #10271
  • bpo-35081: Move _PyObject_GC_TRACK() to pycore_object.h #10272
  • bpo-35081: And pycore_lifecycle.h and pycore_pathconfig.h #10273
  • [WIP] bpo-35081: _testcapi is now compiled with Py_BUILD_CORE #10274
  • bpo-35081: Rename internal headers #10275
  • bpo-35081: Move _PyObject_GC_TRACK() to pycore_object.h #10276
  • bpo-35081: Enhance Python-ast.h and ast.h #10277
  • bpo-35081: Add _PyCoreConfig_AsDict() #10362
  • bpo-35081: Internal headers require Py_BUILD_CORE #10363
  • bpo-35081: Add pycore_fileutils.h #10371
  • bpo-35081: Remove Py_BUILD_CORE from datetime.h #10416
  • bpo-35081: Make some _PyGC macros internal #10507
  • bpo-35081: Move _PyGC_FINALIZED() back to C API #10626
  • bpo-35081: Add Include/internal/pycore_object.h #10640
  • bpo-35081: Add Include/internal/pycore_object.h #10640
  • bpo-35081: Add Include/internal/pycore_object.h #10640
  • bpo-35081: add NEWS entry for new Include/internal/pycore_*.h files #10666
  • bpo-35081: Add new internal headers to Makefile #10670
  • bpo-35081: Add _PyTuple_CAST() #10704
  • bpo-35081: Add Include/internal/pycore_tupleobject.h #10705
  • bpo-37194: Add a new public PyObject_CallNoArgs() function #13890
  • bpo-35081: Move dtoa.h header to the internal C API #18489
  • bpo-35081: Move bytes_methods.h to the internal C API #18492
  • bpo-35081: Move interpreteridobject.h to Include/internal/ #28969
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2022-01-30.23:33:22.200>
created_at = <Date 2018-10-27.00:42:23.909>
labels = ['interpreter-core', '3.8']
title = 'Move internal headers to Include/internal/'
updated_at = <Date 2022-01-30.23:33:22.199>
user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2022-01-30.23:33:22.199>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2022-01-30.23:33:22.200>
closer = 'vstinner'
components = ['Interpreter Core']
creation = <Date 2018-10-27.00:42:23.909>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 35081
keywords = ['patch']
message_count = 49.0
messages = ['328619', '328620', '328623', '328854', '328920', '328921', '328922', '328923', '328942', '329006', '329009', '329026', '329029', '329030', '329031', '329035', '329037', '329062', '329082', '329086', '329362', '329398', '329509', '329608', '329693', '329739', '329812', '329814', '329819', '329830', '329955', '330045', '330067', '330068', '330163', '330164', '330187', '330218', '330304', '330326', '330328', '330334', '330399', '330400', '361917', '361920', '403979', '403997', '404057']
nosy_count = 9.0
nosy_names = ['lemburg', 'tim.peters', 'mark.dickinson', 'atuining', 'scoder', 'vstinner', 'eric.snow', 'serhiy.storchaka', 'izbyshev']
pr_nums = ['10143', '10238', '10239', '10240', '10249', '10263', '10266', '10271', '10272', '10273', '10274', '10275', '10276', '10277', '10362', '10363', '10371', '10416', '10507', '10626', '10640', '10640', '10640', '10666', '10670', '10704', '10705', '13890', '18489', '18492', '28969']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue35081'
versions = ['Python 3.8']

    Activity

    1. vstinner commented on Oct 27, 2018

      @vstinner
      vstinner
      on Oct 27, 2018
      MemberAuthor

      Currently, #include "pymem.h" may include Include/pymem.h or Include/internal/pymem.h depending where is the C file (.c) and if Include/internal/ is in the header search path or not.

      I propose to:

      • Rename Include/internal/ to Include/pycore/
      • In this subdirectory, rename xxx.h to pycore_xxx.h to avoid any risk of confusion
      • Automatically include pycore_xxx.h in xxx.h if Py_BUILD_CORE is defined: this should avoid the need of explicit #include "internal/xxx.h" in C files

      For example, Include/internal/pystate.h becomes Include/pycore/pycore_pystate.h.

      Attached PR implements this idea.

      I chose to rename "internal" subdirectory to "pycore" to prepare the addition of other subdirectories. See:

      Next steps:

      This change should be backward compatible since Include/internal/ must not be used outside CPython core. If someone does that, well, be ready for breakage :-) It's not supported.

    2. vstinner
      added
      3.8 (EOL)end of life
      interpreter-core(Objects, Python, Grammar, and Parser dirs)
      on Oct 27, 2018

    3. vstinner commented on Oct 27, 2018

      @vstinner
      vstinner
      on Oct 27, 2018
      MemberAuthor

      I decided to work on this issue while trying to convert the _PyObject_GC_TRACK() macro into a "static inline" function. Currently, the macro uses _PyGC_generation0 which is defined earlier by "extern PyGC_Head *_PyGC_generation0;".

      Problem: _PyGC_generation0 no longer exists... Include/internal/mem.h now defines: "#define _PyGC_generation0 _PyRuntime.gc.generation0".

      Include/internal/mem.h includes Include/objimpl.h, but Include/objimpl.h requires Include/internal/mem.h. The include order matters here, many header files are inter-dependent, and have two header files with the same name in Include/ and Include/internal/ causes issues depending where the #include is done...

      My PR renames mem.h to pycore_objimpl.h and include pycore_objimpl.h at "the right place" in objimpl.h. Since objimpl.h controls where pycore_objimpl.h is imported, it's simpler to handle inter-dependencies simpler.

      Some inter-dependencies issues are hidden by the fact the C macros don't really require functions, macros and variables in the right order. They "just work". But converting C macros to proper "static inline" functions expose many issues.

    4. vstinner commented on Oct 27, 2018

      @vstinner
      vstinner
      on Oct 27, 2018
      MemberAuthor

      See also bpo-35059 which converts C macros to static inline functions.

    5. vstinner commented on Oct 29, 2018

      @vstinner
      vstinner
      on Oct 29, 2018
      MemberAuthor

      Discussion on python-dev:
      https://mail.python.org/pipermail/python-dev/2018-October/155587.html

    6. vstinner commented on Oct 30, 2018

      @vstinner
      vstinner
      on Oct 30, 2018
      MemberAuthor

      New changeset 9204fb8 by Victor Stinner in branch 'master':
      bpo-35081: Cleanup pystate.c and pystate.h (GH-10240)
      9204fb8

    7. vstinner commented on Oct 30, 2018

      @vstinner
      vstinner
      on Oct 30, 2018
      MemberAuthor

      New changeset 31368a4 by Victor Stinner in branch 'master':
      bpo-35081: Move Include/pyatomic.c to Include/internal/ (GH-10239)
      31368a4

    8. vstinner commented on Oct 30, 2018

      @vstinner
      vstinner
      on Oct 30, 2018
      MemberAuthor

      Serhiy Storchaka asked: "Would not be better to move files with the content fully surrounded by #ifdef Py_BUILD_CORE out of the Include/ directory?"

      See his comment and my answer in the PR 10239:
      #10239 (comment)

    9. vstinner commented on Oct 30, 2018

      @vstinner
      vstinner
      on Oct 30, 2018
      MemberAuthor

      My changes move Py_BUILD_CORE to Include/internal/. I'm not sure of the effect on the backward compatibility.

      Since Python 3.7, many "Py_BUILD_CORE" functions rely on Include/internal/, like PyThreadState_GET() which uses _PyRuntime.gilstate.tstate_current.

      On my Fedora 28, the python3-devel package doesn't proide Include/internal/ headers, only Include/*.h in /usr/include/python3.7m/. It seems like Include/internal/ is not usable by 3rd party modules on Fedora at least.

      I understand that even if a 3rd party C extension used the Py_BUILD_CORE API, Python 3.7 already broke these extensions.

      I don't want C extensions to use Py_BUILD_CORE: Py_BUILD_CORE API is really designed to only be used inside Python. If this API is used outside Python, we cannot modify the API anymore since it would break extensions. But I want to make sure that we can break this API for different reasons.

      In Python 3.7, pyatomic.h is included by Python.h. In Python 3.7.0, pyatomic.h content wasn't surrounded by Py_BUILD_CORE and this header file caused multiple compilation issues: see bpo-23644 and bpo-25150. The content is now restricted to Py_BUILD_CORE since Python 3.7.1. It allows us to more easily change the implementation.

    10. vstinner commented on Oct 30, 2018

      @vstinner
      vstinner
      on Oct 30, 2018
      MemberAuthor

      Include/internal/pystate.h uses #include "pystate.h" to include Include/pystate.h, but it tries to include itself (Include/internal/pystate.h) which does nothing because of "#ifndef Py_INTERNAL_PYSTATE_H #define Py_INTERNAL_PYSTATE_H ... #endif".

      Remove the #ifndef #define to see the bug:

      diff --git a/Include/internal/pystate.h b/Include/internal/pystate.h
index 38845d32ec..2ef023a9a5 100644
--- a/Include/internal/pystate.h
+++ b/Include/internal/pystate.h
@@ -1,5 +1,3 @@
-#ifndef Py_INTERNAL_PYSTATE_H
-#define Py_INTERNAL_PYSTATE_H
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -222,4 +220,3 @@ PyAPI_FUNC(void) _PyInterpreterState_DeleteExceptMain(void);
 #ifdef __cplusplus
 }
 #endif
-#endif /* !Py_INTERNAL_PYSTATE_H */

      Compilation fails with:

      In file included from ./Include/internal/pystate.h:5,
                 from ./Include/internal/pystate.h:5,
                 from ./Include/internal/pystate.h:5,
                 from ./Include/internal/pystate.h:5,
                 from ./Include/internal/pystate.h:5,
                 from ./Include/internal/pystate.h:5,
                 from ./Include/internal/pystate.h:5,
                 ...
./Include/internal/pystate.h:5:21: error: #include nested too deeply
 #include "pystate.h"

    11. vstinner commented on Oct 31, 2018

      @vstinner
      vstinner
      on Oct 31, 2018
      MemberAuthor

      New changeset 2be00d9 by Victor Stinner in branch 'master':
      bpo-35081: Move Py_BUILD_CORE code to internal/mem.h (GH-10249)
      2be00d9

    12. vstinner commented on Oct 31, 2018

      @vstinner
      vstinner
      on Oct 31, 2018
      MemberAuthor

      Include/internal/pystate.h uses #include "pystate.h" to include Include/pystate.h, but it tries to include itself (Include/internal/pystate.h) which does nothing because of "#ifndef Py_INTERNAL_PYSTATE_H #define Py_INTERNAL_PYSTATE_H ... #endif".

      I proposed to rename internal header files, add an "internal_" prefix, to avoid this issue: PR 10263.

    13. vstinner commented on Oct 31, 2018

      @vstinner
      vstinner
      on Oct 31, 2018
      MemberAuthor

      New changeset 27e2d1f by Victor Stinner in branch 'master':
      bpo-35081: Add pycore_ prefix to internal header files (GH-10263)
      27e2d1f

    14. vstinner commented on Nov 1, 2018

      @vstinner
      vstinner
      on Nov 1, 2018
      MemberAuthor

      New changeset 50b4857 by Victor Stinner in branch 'master':
      bpo-35081: Add _PyThreadState_GET() internal macro (GH-10266)
      50b4857

    15. 26 remaining items

    16. vstinner commented on Nov 23, 2018

      @vstinner
      vstinner
      on Nov 23, 2018
      MemberAuthor

      New changeset 984061e by Victor Stinner in branch 'master':
      bpo-35081: Add new internal headers to Makefile (GH-10670)
      984061e

    17. vstinner commented on Nov 23, 2018

      @vstinner
      vstinner
      on Nov 23, 2018
      MemberAuthor

      New changeset 4ac5328 by Victor Stinner in branch 'master':
      bpo-35081: add NEWS entry for new Include/internal/pycore_*.h files (GH-10666)
      4ac5328

    18. vstinner commented on Nov 23, 2018

      @vstinner
      vstinner
      on Nov 23, 2018
      MemberAuthor

      I close the issue, it seems like all sub-tasks have been completed!

      Summary of the change:

      • Header files of Include/internal/ are now all prefixed by "pycore_" and the directory has been added to the header search path. For example, #include "pycore_pystate.h" is now used in C code, instead of #include "internal/pystate.h".
      • All "#ifdef Py_BUILD_CORE" code has been moved from Include/.h to Include/internal/.h (except of the _PyTuple_ITEMS() macro)
      • I created many new header files in Include/internal/
      • _json and _locale modules are now built with Py_BUILD_CORE define
      • _PyObject_GC_TRACK() and _PyObject_GC_UNTRACK() have been removed from the public C API and moved to the internal API

      There are other issues to track following steps:

    19. vstinner
      closed this as completedon Nov 23, 2018

    20. vstinner commented on Nov 23, 2018

      @vstinner
      vstinner
      on Nov 23, 2018
      MemberAuthor

      In bpo-35296, I modified "make install" to also install Include/internal/.

    21. vstinner commented on Nov 25, 2018

      @vstinner
      vstinner
      on Nov 25, 2018
      MemberAuthor

      New changeset 8ac6539 by Victor Stinner in branch 'master':
      bpo-35081: Add _PyTuple_CAST() (GH-10704)
      8ac6539

    22. vstinner commented on Nov 25, 2018

      @vstinner
      vstinner
      on Nov 25, 2018
      MemberAuthor

      New changeset ec13b93 by Victor Stinner in branch 'master':
      bpo-35081: Add Include/internal/pycore_tupleobject.h (GH-10705)
      ec13b93

    23. vstinner commented on Feb 12, 2020

      @vstinner
      vstinner
      on Feb 12, 2020
      MemberAuthor

      New changeset 45876a9 by Victor Stinner in branch 'master':
      bpo-35081: Move bytes_methods.h to the internal C API (GH-18492)
      45876a9

    24. vstinner commented on Feb 12, 2020

      @vstinner
      vstinner
      on Feb 12, 2020
      MemberAuthor

      New changeset e9e7d28 by Victor Stinner in branch 'master':
      bpo-35081: Move dtoa.h header to the internal C API (GH-18489)
      e9e7d28

    25. vstinner commented on Oct 15, 2021

      @vstinner
      vstinner
      on Oct 15, 2021
      MemberAuthor

      I reopen the issue since there is still activity on it.

    26. vstinner
      reopened this on Oct 15, 2021

    27. vstinner commented on Oct 15, 2021

      @vstinner
      vstinner
      on Oct 15, 2021
      MemberAuthor

      New changeset 063abd9 by Victor Stinner in branch 'main':
      bpo-35081: Move interpreteridobject.h to Include/internal/ (GH-28969)
      063abd9

    28. ericsnowcurrently commented on Oct 15, 2021

      @ericsnowcurrently
      ericsnowcurrently
      on Oct 15, 2021
      Member

      On Fri, Oct 15, 2021 at 3:56 AM STINNER Victor <report@bugs.python.org> wrote:

      New changeset 063abd9 by Victor Stinner in branch 'main':
      bpo-35081: Move interpreteridobject.h to Include/internal/ (GH-28969)

      FYI, _xxsubinterpretersmodule is supposed to be an extension module
      (not builtin), so it's not meant to use internal APIs. At some point
      that happened and I only noticed now. I've created bpo-45486 to
      address that.

    29. vstinner
      closed this as completedon Jan 30, 2022
    30. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      3.8 (EOL)end of lifeinterpreter-core(Objects, Python, Grammar, and Parser dirs)

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions