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) * |
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) * |
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) * |
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) * |
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) * |
Date: 2021-09-29 09:28 |
I marked bpo-40642 as a duplicate of this issue.
|
msg402839 - (view) |
Author: STINNER Victor (vstinner) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:59:24 | admin | set | github: 83207 |
2021-11-26 10:01:46 | vstinner | set | status: 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:40 | vstinner | set | messages:
+ msg407035 |
2021-11-25 12:41:23 | vstinner | set | pull_requests:
+ pull_request28013 |
2021-11-25 12:35:30 | vstinner | set | messages:
+ msg406994 |
2021-11-23 20:36:00 | christian.heimes | set | pull_requests:
+ pull_request27971 |
2021-11-23 19:45:33 | christian.heimes | set | nosy:
+ christian.heimes messages:
+ msg406878
|
2021-11-23 18:21:38 | gaige | set | messages:
+ msg406874 |
2021-11-23 18:05:41 | vstinner | set | messages:
+ msg406872 |
2021-11-23 18:02:03 | vstinner | set | pull_requests:
+ pull_request27969 |
2021-11-23 17:59:08 | vstinner | set | messages:
+ msg406871 |
2021-11-23 17:42:17 | gaige | set | messages:
+ msg406868 |
2021-11-17 11:44:14 | gaige | set | messages:
+ msg406466 |
2021-11-16 00:01:27 | vstinner | set | messages:
+ msg406380 |
2021-11-09 19:41:54 | gaige | set | messages:
+ msg406044 |
2021-11-09 14:44:35 | vstinner | set | messages:
+ msg406025 |
2021-11-09 14:43:02 | vstinner | set | pull_requests:
+ pull_request27739 |
2021-09-29 09:38:47 | vstinner | set | messages:
+ msg402840 |
2021-09-29 09:36:53 | vstinner | set | messages:
+ msg402839 |
2021-09-29 09:30:51 | vstinner | set | title: 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:12 | vstinner | set | pull_requests:
+ pull_request26982 |
2021-09-29 09:28:17 | vstinner | set | messages:
+ msg402837 |
2021-09-29 09:27:52 | vstinner | link | issue40642 superseder |
2020-05-19 13:21:12 | vstinner | set | messages:
+ msg369353 |
2020-05-18 23:25:00 | Kevin Mooney | set | messages:
+ msg369309 |
2020-05-18 22:08:32 | vstinner | set | messages:
+ msg369297 |
2020-05-18 16:14:35 | Kevin Mooney | set | messages:
+ msg369261 |
2020-05-18 15:16:16 | vstinner | set | messages:
+ msg369242 |
2020-05-18 15:05:07 | Kevin Mooney | set | keywords:
+ patch nosy:
+ Kevin Mooney
pull_requests:
+ pull_request19482 stage: patch review |
2020-02-05 20:20:05 | mcepl | set | nosy:
+ mcepl
|
2019-12-11 15:57:00 | gaige | set | messages:
+ msg358269 |
2019-12-11 15:42:00 | ronaldoussoren | set | messages:
+ msg358267 |
2019-12-11 15:36:46 | vstinner | set | nosy:
+ vstinner
|
2019-12-11 15:34:04 | gaige | create | |