[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

STINNER Victor  added the comment:

Copy of my comment on PR 10271:
https://github.com/python/cpython/pull/10271#issuecomment-434897408

I tried to enforce to require Py_BUILD_CORE in pycore_accu.h to be defined 
using:

#ifndef Py_BUILD_CORE
#  error "Py_BUILD_CORE must be defined to include this header"
#endif

But the compilation of the _json module failed, because it isn't compiled with 
Py_BUILD_CORE. Moreover, _json.c contains:

/* Core extension modules are built-in on some platforms (e.g. Windows). */
#ifdef Py_BUILD_CORE
#define Py_BUILD_CORE_BUILTIN
#undef Py_BUILD_CORE
#endif

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

STINNER Victor  added the comment:


New changeset e281f7d80ce2584a7e6a36acffb5a9cd796a0fe2 by Victor Stinner in 
branch 'master':
bpo-35081: Move accu.h to Include/internal/pycore_accu.h (GH-10271)
https://github.com/python/cpython/commit/e281f7d80ce2584a7e6a36acffb5a9cd796a0fe2


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +9583

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +9582

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 50b48572d9a90c5bb36e2bef6179548ea927a35a by Victor Stinner in 
branch 'master':
bpo-35081: Add _PyThreadState_GET() internal macro (GH-10266)
https://github.com/python/cpython/commit/50b48572d9a90c5bb36e2bef6179548ea927a35a


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 27e2d1f21975dfb8c0ddcb192fa0f45a51b7977e by Victor Stinner in 
branch 'master':
bpo-35081: Add pycore_ prefix to internal header files (GH-10263)
https://github.com/python/cpython/commit/27e2d1f21975dfb8c0ddcb192fa0f45a51b7977e


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[Eric Snow]

Change by Eric Snow :


--
nosy: +eric.snow

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +9577

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

STINNER Victor  added the comment:

> 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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +9574

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 2be00d987d37682a55db67c298e82c405d01b868 by Victor Stinner in 
branch 'master':
bpo-35081: Move Py_BUILD_CORE code to internal/mem.h (GH-10249)
https://github.com/python/cpython/commit/2be00d987d37682a55db67c298e82c405d01b868


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

STINNER Victor  added the comment:

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"

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +9563

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

STINNER Victor  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

STINNER Victor  added the comment:

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:
https://github.com/python/cpython/pull/10239#issuecomment-434289361

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 31368a4f0e531c19affe2a1becd25fc316bc7501 by Victor Stinner in 
branch 'master':
bpo-35081: Move Include/pyatomic.c to Include/internal/ (GH-10239)
https://github.com/python/cpython/commit/31368a4f0e531c19affe2a1becd25fc316bc7501


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 9204fb8623896cc5f68ae350784ee25e8a7b2184 by Victor Stinner in 
branch 'master':
bpo-35081: Cleanup pystate.c and pystate.h (GH-10240)
https://github.com/python/cpython/commit/9204fb8623896cc5f68ae350784ee25e8a7b2184


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +9553

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +9552

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +9551

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

STINNER Victor  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

STINNER Victor  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

STINNER Victor  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

Change by STINNER Victor :


--
keywords: +patch
pull_requests: +9470
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35081] Rename Include/internals/ to Include/pycore/

[STINNER Victor]

New submission from STINNER Victor :

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:

* https://pythoncapi.readthedocs.io/split_include.html
* https://pythoncapi.readthedocs.io/

Next steps:

* Move all code surrounded by #ifdef Py_BUILD_CORE from Include/*.h into 
Include/pycore/*.h: see the second commit of my PR for an example
* Move core surrounded by #ifndef Py_LIMITED_API from Include/*.h into 
Include/limited/ (example of filename: Include/limited/limited_object.h)


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.

--
components: Interpreter Core
messages: 328619
nosy: vstinner
priority: normal
severity: normal
status: open
title: Rename Include/internals/ to Include/pycore/
versions: Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com