[Python-Dev] Rename Include/internals/ to Include/pycore/

Victor Stinner vstinner at redhat.com
Wed Oct 31 15:57:07 EDT 2018


Ok, I proposed to rename internal header files, add an "internal_"
prefix, to avoid this issue:
https://github.com/python/cpython/pull/10263

My PR also adds Include/internal/ to search paths of header files.

Extract of the change:

diff --git a/Include/internal/pystate.h b/Include/internal/internal_pystate.h
--- a/Include/internal/pystate.h
+++ b/Include/internal/internal_pystate.h
@@ -7,9 +7,9 @@ extern "C" {

 #include "pystate.h"
 #include "pythread.h"

-#include "internal/mem.h"
-#include "internal/ceval.h"
-#include "internal/warnings.h"
+#include "internal_pymem.h"
+#include "internal_ceval.h"
+#include "internal_warnings.h"

With this PR, internal_pystate.h of Include/internal/ is able to
properly include pystate.h from Include/ (include the correct header
file).


For practical reasons, I added Include/internal/ to all projects of
the Visual Studio solution (I modified the properties common to all
projects).

Again, this is where I would like to replace "internal_" with
"pycore_": "internal" is too generic, there is a risk that a third
party project also the same header filename. "internal_hash.h" name is
quite general. There is a non-zero risk of conflict with a library. In
the past, we also had issues when compiling OpenSSL for example.

Maybe we can keep "Include/internal/" directory name, but add
"pycore_" rather than "internal_" to header filenames?

Victor

> > > Moreover, files of this subdirectory would have
> > > the prefix "pycore_". For example, Include/objimpl.h would have a
> > > twin: Include/pycore/pycore_objimpl.h which extend the public API with
> > > "core" APIs.
> >
> > I'm not sure why this is necessary.  (...)
> > https://github.com/python/cpython/blob/master/Include/internal/pystate.h#L11
> >     #include "internal/ceval.h"
> > Note that a few lines above in that file I include the public header file:
> >     #include "pystate.h"
>
> The last include doesn't work:
> https://bugs.python.org/issue35081#msg328942
>
> """
> 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:
> (...)
>
> 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"
> """
>
> Using <pystate.h> has no effect, it stills looks for Include/internal/pystate.h.
>
> IMHO the only solution is to use different names in Include/ and
> Include/internal/, at least for the header files of Include/internal/
> which also exist in Include/.
>
> Rename Include/internal/pystate.h to Include/internal/pyruntime.h,
> Include/internal/internal_pystate.h or
> Include/internal/pycore_pystate.h.
>
> If we add Include/internal/ to the header search path (gcc -I
> Include/internal/), we can avoid redundant
> "internal/internal_<name>.h" and just write "internal_<name>.h". I
> proposed to rename "internal" to "pycore" to get: #include
> "pycore_pystate.h" instead of #include "internal_pystate.h". But I
> have no strong preference for "pycore" or "internal", both work for
> me.


More information about the Python-Dev mailing list