Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include/cpython/pystate.h contains non-relative of initconfig.h include causing macOS Framework include failure #83207

Closed
gaige mannequin opened this issue Dec 11, 2019 · 23 comments
Labels
3.9 only security fixes 3.10 only security fixes 3.11 only security fixes OS-mac topic-C-API type-bug An unexpected behavior, bug, or error

Comments

@gaige
Copy link
Mannequin

gaige mannequin commented Dec 11, 2019

BPO 39026
Nosy @ronaldoussoren, @vstinner, @tiran, @ned-deily, @mcepl, @gaige, @knjmooney
PRs
  • bpo-39026: Allow relative include paths #20181
  • bpo-39026: Rename Include/cpython/ header files #28612
  • bpo-39026: Fix Python.h when building with Xcode #29488
  • [3.10] bpo-39026: Fix Python.h when building with Xcode (GH-29488) #29732
  • bpo-45873: Restore Python 3.6 compatibility (GH-29730) #29730
  • [3.9] bpo-39026: Fix Python.h when building with Xcode (GH-29488) #29776
  • 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 2021-11-26.10:01:46.102>
    created_at = <Date 2019-12-11.15:34:04.806>
    labels = ['OS-mac', 'type-bug', '3.9', '3.10', '3.11', 'expert-C-API']
    title = 'Include/cpython/pystate.h contains non-relative of initconfig.h include causing macOS Framework include failure'
    updated_at = <Date 2021-11-26.10:01:46.099>
    user = 'https://github.com/gaige'

    bugs.python.org fields:

    activity = <Date 2021-11-26.10:01:46.099>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-11-26.10:01:46.102>
    closer = 'vstinner'
    components = ['macOS', 'C API']
    creation = <Date 2019-12-11.15:34:04.806>
    creator = 'gaige'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 39026
    keywords = ['patch']
    message_count = 23.0
    messages = ['358266', '358267', '358269', '369242', '369261', '369297', '369309', '369353', '402837', '402839', '402840', '406025', '406044', '406380', '406466', '406868', '406871', '406872', '406874', '406878', '406994', '407035', '407036']
    nosy_count = 7.0
    nosy_names = ['ronaldoussoren', 'vstinner', 'christian.heimes', 'ned.deily', 'mcepl', 'gaige', 'Kevin Mooney']
    pr_nums = ['20181', '28612', '29488', '29732', '29730', '29776']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue39026'
    versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

    @gaige
    Copy link
    Mannequin Author

    gaige mannequin commented Dec 11, 2019

    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.

    @gaige gaige mannequin added 3.8 only security fixes OS-mac topic-C-API type-bug An unexpected behavior, bug, or error labels Dec 11, 2019
    @ronaldoussoren
    Copy link
    Contributor

    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>"?

    @gaige
    Copy link
    Mannequin Author

    gaige mannequin commented Dec 11, 2019

    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.

    @vstinner
    Copy link
    Member

    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.

    @knjmooney
    Copy link
    Mannequin

    knjmooney mannequin commented May 18, 2020

    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.

    @vstinner
    Copy link
    Member

    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.

    @knjmooney
    Copy link
    Mannequin

    knjmooney mannequin commented May 18, 2020

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


    @vstinner
    Copy link
    Member

    1. Remove cpython/initconfig.h from cpython/pystate.h

    Let's start with that and see if it's enough to fix the issue.

    @vstinner
    Copy link
    Member

    I marked bpo-40642 as a duplicate of this issue.

    @vstinner vstinner changed the 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 Sep 29, 2021
    @vstinner
    Copy link
    Member

    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.

    @vstinner
    Copy link
    Member

    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.

    @vstinner
    Copy link
    Member

    vstinner commented Nov 9, 2021

    I wrote a different fix: PR 29488. Can someone please check if it fix the issue with Xcode?

    @gaige
    Copy link
    Mannequin Author

    gaige mannequin commented Nov 9, 2021

    I'll try to check that in the next day or two.

    @vstinner
    Copy link
    Member

    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?

    @gaige
    Copy link
    Mannequin Author

    gaige mannequin commented Nov 17, 2021

    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.

    @gaige
    Copy link
    Mannequin Author

    gaige mannequin commented Nov 23, 2021

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

    @vstinner
    Copy link
    Member

    New changeset 4ae26b9 by Victor Stinner in branch 'main':
    bpo-39026: Fix Python.h when building with Xcode (GH-29488)
    4ae26b9

    @vstinner
    Copy link
    Member

    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.

    @gaige
    Copy link
    Mannequin Author

    gaige mannequin commented Nov 23, 2021

    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!

    @tiran
    Copy link
    Member

    tiran commented Nov 23, 2021

    I think #72798 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

    @vstinner
    Copy link
    Member

    New changeset ce5a646 by Victor Stinner in branch '3.10':
    bpo-39026: Fix Python.h when building with Xcode (GH-29488) (GH-29732)
    ce5a646

    @vstinner
    Copy link
    Member

    New changeset 92631a4 by Victor Stinner in branch '3.9':
    bpo-39026: Fix Python.h when building with Xcode (GH-29488) (GH-29776)
    92631a4

    @vstinner
    Copy link
    Member

    Thanks Gaige Paulsen for the bug report, it's now fixed.

    I think #72798 broke Windows builds:

    It was related to C++ build, it's now fixed by bpo-45893.

    @vstinner vstinner added 3.9 only security fixes 3.10 only security fixes labels Nov 26, 2021
    @vstinner vstinner added 3.11 only security fixes and removed 3.8 only security fixes labels Nov 26, 2021
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 only security fixes 3.10 only security fixes 3.11 only security fixes OS-mac topic-C-API type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants