classification
Title: Include/cpython/pystate.h contains non-relative of initconfig.h include causing macOS Framework include failure
Type: behavior Stage: resolved
Components: C API, macOS Versions: Python 3.11, Python 3.10, Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Kevin Mooney, christian.heimes, gaige, mcepl, ned.deily, ronaldoussoren, vstinner
Priority: normal Keywords: patch

Created on 2019-12-11 15:34 by gaige, last changed 2021-11-26 10:01 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 20181 closed Kevin Mooney, 2020-05-18 15:05
PR 28612 closed vstinner, 2021-09-29 09:29
PR 29488 merged vstinner, 2021-11-09 14:43
PR 29732 merged vstinner, 2021-11-23 18:02
PR 29730 christian.heimes, 2021-11-23 20:36
PR 29776 merged vstinner, 2021-11-25 12:41
Messages (23)
msg358266 - (view) Author: Gaige Paulsen (gaige) Date: 2019-12-11 15:34
The cpython/pystate.h includes cpython/initconfig.h using the relative path "cpython/initconfig.h", which probably works fine if your include path explicitly contains the top of the python directory, however when developing with a framework in macOS, the framework's root path cannot be referred to relatively.

Since cpython/pystate.h and cpython/initconfig.h live in the same directory, any C compiler should include them correctly, regardless of include path if the cpython/pystate.h includes "initconfig.h" instead of "cpython/initconfig.h", since I believe the very first path is always relative to the file including the next file.  In this case, cpython is the parent of pystate.h and thus including initconfig.h directly should work fine.

Previous 3.x versions worked fine, but the cpython directory wasn't in Headers for macOS.

Although I wasn't able to exhaustively test this on all platforms and with all compilers, changing the include in cpython/pystate.h to "initconfig.h" solved the compilation/include problem.
msg358267 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2019-12-11 15:42
In what way does it not work when using a framework build? Is this when you add the python framework to an (Xcode) project and include the main python header using "#include <Python/Python.h>"?
msg358269 - (view) Author: Gaige Paulsen (gaige) Date: 2019-12-11 15:57
I should have been more specific. Sorry about that.

Yes, when including <Python/Python.h> in Xcode, incorporating the framework. In particular, I'm embedding 3.8 in an application (previously used the system Framework, which was 2.7, which worked but Apple has warned that it may disappear....).  

The Xcode error is:

In file included from /Users/gaige/MyApp/Code/PythonImporter.m:10:
In file included from ../python-framework/Python.framework/Headers/Python.h:121:
In file included from ../python-framework/Python.framework/Headers/genobject.h:11:
In file included from ../python-framework/Python.framework/Headers/pystate.h:129:
../python-framework/Python.framework/Headers/cpython/pystate.h:10:10: fatal error: 'cpython/initconfig.h' file not found
#include "cpython/initconfig.h"
         ^~~~~~~~~~~~~~~~~~~~~~
1 error generated.

python-framework is in the framework search path.

Let me know if there's something further I can clarify.
msg369242 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-05-18 15:16
IMO a better approach would be to add "cpython_" prefix to all header files living in Include/cpython/. It would prevent bad surprises.

I tried to avoid adding a prefix when I added Include/internal/ but I got many practical issues. Depending where the #include is done, I got different errors. Adding a prefix prevents this issue.
msg369261 - (view) Author: Kevin Mooney (Kevin Mooney) * Date: 2020-05-18 16:14
Ok, I think I see. Is there a concern that removing the cpython/ prefix might lead to the wrong initconfig.h being included?

So your proposal is all includes in the root will do 

    #include "cpython/cpython_foo.h"

And any includes done in the cpython dir will be

    #include "cpython_foo.h"

Currently there's only one file in the cpython dir that includes another, which would explain why this has never come up before.
msg369297 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-05-18 22:08
I propose to rename "cpython/initconfig.h" to "cpython/cpython_initconfig.h".

#include "cpython/initconfig.h"

would become:

#include "cpython/cpython_initconfig.h"

So it becomes possible to include a cpython_xxx.h header from another cpython_xxx.h header (as done py cpython/pystate.h which includes cpython/initconfig.h).

--

Maybe a simpler change is simply to remove #include "cpython/initconfig.h" from cpython/pystate.h. Currently, cpython/pystate.h is included indirectly by Python.h at line 137, whereas cpython/initconfig.h is included by Python.h at line 135 (two lines before).

C extensions must include "Python.h" first and when "Python.h" is used, pystate.h gets the cpython/initconfig.h definitions as expected.
msg369309 - (view) Author: Kevin Mooney (Kevin Mooney) * Date: 2020-05-18 23:25
Ok, so there are three potential solutions

1. Add cpython_ prefix to cpython headers
2. Remove cpython/initconfig.h from cpython/pystate.h
3. Include initconfig.h rather than cpython/initconfig.h

1 intoduces verbosity and touches more than the other solutions. 2
introduces a dependency on the include order. 3 adds potential nasty
surprises. Doing nothing means C API users need to adapt their include
paths.

I'd appreciate your guidance on which path is preferable.

On Mon, 18 May 2020, 23:08 STINNER Victor, <report@bugs.python.org> wrote:

>
> STINNER Victor <vstinner@python.org> added the comment:
>
> I propose to rename "cpython/initconfig.h" to
> "cpython/cpython_initconfig.h".
>
> #include "cpython/initconfig.h"
>
> would become:
>
> #include "cpython/cpython_initconfig.h"
>
> So it becomes possible to include a cpython_xxx.h header from another
> cpython_xxx.h header (as done py cpython/pystate.h which includes
> cpython/initconfig.h).
>
> --
>
> Maybe a simpler change is simply to remove #include "cpython/initconfig.h"
> from cpython/pystate.h. Currently, cpython/pystate.h is included indirectly
> by Python.h at line 137, whereas cpython/initconfig.h is included by
> Python.h at line 135 (two lines before).
>
> C extensions must include "Python.h" first and when "Python.h" is used,
> pystate.h gets the cpython/initconfig.h definitions as expected.
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue39026>
> _______________________________________
>
msg369353 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-05-19 13:21
> 2. Remove cpython/initconfig.h from cpython/pystate.h

Let's start with that and see if it's enough to fix the issue.
msg402837 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-09-29 09:28
I marked bpo-40642 as a duplicate of this issue.
msg402839 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-09-29 09:36
I'm not sure why Include/cpython/ is part of the compiler include directories. I am not sure why Include/ is *not* part of the compiler include directory.

Anyway, it's hard to control how people build Python and there are always legit use cases where you fall into such issue.

That's why I suggest to fix any risk of confusion by renaming Include/cpython/ header files to add a "cpython_" prefix. For example, rename Include/cpython/pystate.h to Include/cpython/cpython_pystate.h.

In the Include/internal/ directory, header files already have a prefix for the exact same reason. For example, there is a pycore_pystate.h file there.
msg402840 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-09-29 09:38
I propose to only rename header files in Python 3.11.

For Python 3.9 and 3.10, I suggest to modify Include/cpython/pystate.h by replacing:
  #include "cpython/initconfig.h"
with:
  #include "initconfig.h"

Like the PR 20181.
msg406025 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-11-09 14:44
I wrote a different fix: PR 29488. Can someone please check if it fix the issue with Xcode?
msg406044 - (view) Author: Gaige Paulsen (gaige) Date: 2021-11-09 19:41
I'll try to check that in the next day or two.
msg406380 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-11-16 00:01
Gaige Paulsen: "I'll try to check that in the next day or two."

Thanks. Did you have the opportunity to give it a try?
msg406466 - (view) Author: Gaige Paulsen (gaige) Date: 2021-11-17 11:44
Short version is not yet. I spent time on it but ran into an issue getting the full build on the machine I was building on. I expect to try it again before the weekend.

Is there a binary copy of the framework from the CI somewhere? If so, I can certainly do a much faster check against building with the framework, saving me the step I'm having problem with right now.
msg406868 - (view) Author: Gaige Paulsen (gaige) Date: 2021-11-23 17:42
Sorry about the delay.
Finally fixed my workflow for building from scratch.

Result: I can confirm that the compilation problem is resolved with this change (and was failing with the main branch prior to the change).
msg406871 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-11-23 17:59
New changeset 4ae26b9c1d0c33e3db92c6f305293f9240dea358 by Victor Stinner in branch 'main':
bpo-39026: Fix Python.h when building with Xcode (GH-29488)
https://github.com/python/cpython/commit/4ae26b9c1d0c33e3db92c6f305293f9240dea358
msg406872 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-11-23 18:05
Gaige Paulsen: "Result: I can confirm that the compilation problem is resolved with this change (and was failing with the main branch prior to the change)."

Thank you for the confirmation. I merged my change and I'm backporting it to Python 3.9 and 3.10.
msg406874 - (view) Author: Gaige Paulsen (gaige) Date: 2021-11-23 18:21
I can test the backports if you like. In that case, let me know when the backports are done, I should be able to test those rapidly, now that I have my build environment working for building from source.

Thanks again!
msg406878 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2021-11-23 19:45
I think GH-28612 broke Windows builds:

  python_uwp.cpp
python_uwp.obj : error LNK2001: unresolved external symbol "__declspec(dllimport) struct PyStatus __cdecl PyConfig_SetArgv(struct PyConfig *,__int64,wchar_t * const *)" (__imp_?PyConfig_SetArgv@@YA?AUPyStatus@@PEAUPyConfig@@_JPEBQEA_W@Z) [D:\a\1\s\PCbuild\python_uwp.vcxproj]
python_uwp.obj : error LNK2001: unresolved external symbol "__declspec(dllimport) int __cdecl PyStatus_Exception(struct PyStatus)" (__imp_?PyStatus_Exception@@YAHUPyStatus@@@Z) [D:\a\1\s\PCbuild\python_uwp.vcxproj]
python_uwp.obj : error LNK2001: unresolved external symbol "__declspec(dllimport) void __cdecl PyConfig_InitPythonConfig(struct PyConfig *)" (__imp_?PyConfig_InitPythonConfig@@YAXPEAUPyConfig@@@Z) [D:\a\1\s\PCbuild\python_uwp.vcxproj]
python_uwp.obj : error LNK2001: unresolved external symbol "__declspec(dllimport) int __cdecl PyStatus_IsExit(struct PyStatus)" (__imp_?PyStatus_IsExit@@YAHUPyStatus@@@Z) [D:\a\1\s\PCbuild\python_uwp.vcxproj]
python_uwp.obj : error LNK2001: unresolved external symbol "__declspec(dllimport) struct PyStatus __cdecl PyConfig_SetString(struct PyConfig *,wchar_t * *,wchar_t const *)" (__imp_?PyConfig_SetString@@YA?AUPyStatus@@PEAUPyConfig@@PEAPEA_WPEB_W@Z) [D:\a\1\s\PCbuild\python_uwp.vcxproj]
python_uwp.obj : error LNK2001: unresolved external symbol "__declspec(dllimport) void __cdecl PyConfig_Clear(struct PyConfig *)" (__imp_?PyConfig_Clear@@YAXPEAUPyConfig@@@Z) [D:\a\1\s\PCbuild\python_uwp.vcxproj]
python_uwp.obj : error LNK2001: unresolved external symbol "__declspec(dllimport) void __cdecl PyPreConfig_InitPythonConfig(struct PyPreConfig *)" (__imp_?PyPreConfig_InitPythonConfig@@YAXPEAUPyPreConfig@@@Z) [D:\a\1\s\PCbuild\python_uwp.vcxproj]
python_uwp.obj : error LNK2001: unresolved external symbol "__declspec(dllimport) struct PyStatus __cdecl PyStatus_Ok(void)" (__imp_?PyStatus_Ok@@YA?AUPyStatus@@XZ) [D:\a\1\s\PCbuild\python_uwp.vcxproj]
D:\a\1\s\PCbuild\amd64\python_uwp.exe : fatal error LNK1120: 8 unresolved externals [D:\a\1\s\PCbuild\python_uwp.vcxproj]

https://dev.azure.com/Python/cpython/_build/results?buildId=92032&view=results
msg406994 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-11-25 12:35
New changeset ce5a6460aebdc51a810d2755782fe8f0d7918ec2 by Victor Stinner in branch '3.10':
bpo-39026: Fix Python.h when building with Xcode (GH-29488) (GH-29732)
https://github.com/python/cpython/commit/ce5a6460aebdc51a810d2755782fe8f0d7918ec2
msg407035 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-11-26 09:59
New changeset 92631a4144fba041a5fb6578620db784821dfdc5 by Victor Stinner in branch '3.9':
bpo-39026: Fix Python.h when building with Xcode (GH-29488) (GH-29776)
https://github.com/python/cpython/commit/92631a4144fba041a5fb6578620db784821dfdc5
msg407036 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-11-26 10:01
Thanks Gaige Paulsen for the bug report, it's now fixed.

> I think GH-28612 broke Windows builds:

It was related to C++ build, it's now fixed by bpo-45893.
History
Date User Action Args
2021-11-26 10:01:46vstinnersetstatus: open -> closed
versions: + Python 3.9, Python 3.10, Python 3.11, - Python 3.8
messages: + msg407036

resolution: fixed
stage: patch review -> resolved
2021-11-26 09:59:40vstinnersetmessages: + msg407035
2021-11-25 12:41:23vstinnersetpull_requests: + pull_request28013
2021-11-25 12:35:30vstinnersetmessages: + msg406994
2021-11-23 20:36:00christian.heimessetpull_requests: + pull_request27971
2021-11-23 19:45:33christian.heimessetnosy: + christian.heimes
messages: + msg406878
2021-11-23 18:21:38gaigesetmessages: + msg406874
2021-11-23 18:05:41vstinnersetmessages: + msg406872
2021-11-23 18:02:03vstinnersetpull_requests: + pull_request27969
2021-11-23 17:59:08vstinnersetmessages: + msg406871
2021-11-23 17:42:17gaigesetmessages: + msg406868
2021-11-17 11:44:14gaigesetmessages: + msg406466
2021-11-16 00:01:27vstinnersetmessages: + msg406380
2021-11-09 19:41:54gaigesetmessages: + msg406044
2021-11-09 14:44:35vstinnersetmessages: + msg406025
2021-11-09 14:43:02vstinnersetpull_requests: + pull_request27739
2021-09-29 09:38:47vstinnersetmessages: + msg402840
2021-09-29 09:36:53vstinnersetmessages: + msg402839
2021-09-29 09:30:51vstinnersettitle: pystate.h contains non-relative of initconfig.h include causing macOS Framework include failure -> Include/cpython/pystate.h contains non-relative of initconfig.h include causing macOS Framework include failure
2021-09-29 09:29:12vstinnersetpull_requests: + pull_request26982
2021-09-29 09:28:17vstinnersetmessages: + msg402837
2021-09-29 09:27:52vstinnerlinkissue40642 superseder
2020-05-19 13:21:12vstinnersetmessages: + msg369353
2020-05-18 23:25:00Kevin Mooneysetmessages: + msg369309
2020-05-18 22:08:32vstinnersetmessages: + msg369297
2020-05-18 16:14:35Kevin Mooneysetmessages: + msg369261
2020-05-18 15:16:16vstinnersetmessages: + msg369242
2020-05-18 15:05:07Kevin Mooneysetkeywords: + patch
nosy: + Kevin Mooney

pull_requests: + pull_request19482
stage: patch review
2020-02-05 20:20:05mceplsetnosy: + mcepl
2019-12-11 15:57:00gaigesetmessages: + msg358269
2019-12-11 15:42:00ronaldoussorensetmessages: + msg358267
2019-12-11 15:36:46vstinnersetnosy: + vstinner
2019-12-11 15:34:04gaigecreate