Navigation Menu

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

Jump to bottom

Cpython "pystate.h" subdirectory wrong #84822

Closed
jpelizza mannequin opened this issue May 16, 2020 · 10 comments
Closed

Cpython "pystate.h" subdirectory wrong #84822

jpelizza mannequin opened this issue May 16, 2020 · 10 comments
Labels
3.8 only security fixes build The build process and cross-build topic-C-API

Comments

@jpelizza
Copy link
Mannequin

jpelizza mannequin commented May 16, 2020

BPO 40642
Nosy @vstinner, @ericvsmith, @xry111
PRs
  • bpo-40642: use file-relative include for initconfig.h in pystate.h #24744
    		•
    Superseder
  • bpo-39026: Include/cpython/pystate.h contains non-relative of initconfig.h include causing macOS Framework include failure
    		•

    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-09-29.09:27:52.279>
created_at = <Date 2020-05-16.07:40:57.525>
labels = ['expert-C-API', 'build', '3.8']
title = 'Cpython "pystate.h" subdirectory wrong'
updated_at = <Date 2021-09-29.09:27:52.278>
user = 'https://bugs.python.org/jpelizza'

    bugs.python.org fields:

    activity = <Date 2021-09-29.09:27:52.278>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2021-09-29.09:27:52.279>
closer = 'vstinner'
components = ['C API']
creation = <Date 2020-05-16.07:40:57.525>
creator = 'jpelizza'
dependencies = []
files = []
hgrepos = []
issue_num = 40642
keywords = ['patch']
message_count = 10.0
messages = ['369019', '369051', '369427', '369445', '387710', '387711', '387713', '391531', '402829', '402836']
nosy_count = 5.0
nosy_names = ['vstinner', 'eric.smith', 'xry111', 'jpelizza', 'andrewtomazos']
pr_nums = ['24744']
priority = 'normal'
resolution = 'duplicate'
stage = 'resolved'
status = 'closed'
superseder = '39026'
type = 'compile error'
url = 'https://bugs.python.org/issue40642'
versions = ['Python 3.8']
    @jpelizza
    Copy link
    Mannequin Author

    jpelizza mannequin commented May 16, 2020 

    Line 9 of pystate.h is:
#include "cpython/initconfig.h"
should be:
#include "./initconfig.h"
since pystate.h is already inside cpython dir.

    @jpelizza jpelizza mannequin added 3.8 only security fixes topic-C-API build The build process and cross-build labels May 16, 2020
    @ericvsmith
    Copy link
    Member

    Won't either one work, since "Include" is in the "search path"? Is this causing an actual problem? You have this marked as "compile error", but haven't shown any information about the error, such as what compiler, how it's being invoked, and what the error message is. We need some more information.

    If this were to be changed (which I'm not advocating unless there's a real problem), would it be better as "initconfig.h" or "./initconfig.h"? I think they're equivalent.

    @jpelizza
    Copy link
    Mannequin Author

    jpelizza mannequin commented May 20, 2020

    In hindsight I provided absolutely nothing, new to this, bound to make dumb mistakes.

    Compiler error:

    In file included from /usr/include/python3.8/pystate.h:129,
    from /usr/include/python3.8/genobject.h:11,
    from /usr/include/python3.8/Python.h:121,
    from cpppython.cpp:2:
    /usr/include/python3.8/cpython/pystate.h:9:10: fatal error: cpython/initconfig.h: No such file or directory
    9 | #include "cpython/initconfig.h"
    | ^~~~~~~~~~~~~~~~~~~~~~
    compilation terminated.

    Compiler version:
    gcc version 9.3.0 (Arch Linux 9.3.0-1)

    About the change, yeah "initconfig.h" and the other one are the same, if it is a real problem then there is no need for "./"

    After changing it to just "initconfig.h" the code compiled normally. From my understanding since initconfig.h is already inside the folder cpython using "cpython/initconfig.h" will try to look for another cpython folder inside the cpython folder and then look for initconfig thus the error.

    @ericvsmith
    Copy link
    Member

    Thanks for the info.

    It's weird that this is just showing up for you, and I assume works everywhere else. How are you invoking the compiler, via make, or something else?

    And, what's the gcc command line look like when this specific file fails?

    I grepped the source code, and didn't see anywhere else where we import something this way, so maybe that's a reason to change it.

    As for the "./" think: maybe using it would be a better hint as to what's going on.

    But I'd like to hear from other core devs and get their opinion on any change, first.

    @andrewtomazos
    Copy link
    Mannequin

    andrewtomazos mannequin commented Feb 26, 2021

    The problem occurs if you:

       #include <python3.8/Python.h>

    on *nix systems. Note /usr/include/ is in the system include path. One workaround is to -I/usr/include/python3.8 and then #include <Python.h>. Another workaround is to comment out:

    // #include "cpython/initconfig.h"

    in cpython/pystate.h.

    A fix would be appreciated, so that #include <python3.8/Python.h> would work out of the box (without having to add -I).

    @andrewtomazos
    Copy link
    Mannequin

    andrewtomazos mannequin commented Feb 26, 2021

    Related: https://stackoverflow.com/q/63211981/1131467

    @andrewtomazos
    Copy link
    Mannequin

    andrewtomazos mannequin commented Feb 26, 2021

    After studying the include directives in the Include folder, I think the best fix would be:

    In Include/cpython/state.h:

    - #include "cpython/initconfig.h"
+ #include "initconfig.h"

    this will mean that state.h will find initconfig.h using a file-relative include (as all the other include directives do), instead of relying on the installed Include directory being put in the header search path.

    As this include directive is the only one with this property, the benefit of this change would be that the Include folder would be able to be installed in any subdirectory of a search path, rather than requiring its own dedicated one.

    This would mean (for example) you could install different versions of the Python headers side by side and then select between them using preprocessor directives - rather than having to switch up global compiler options.

    @xry111
    Copy link
    Mannequin

    xry111 mannequin commented Apr 21, 2021

    Dup of bpo-39026. And the proposed fix is nack'ed in bpo-39026.

    @vstinner
    Copy link
    Member

    Having two header files with the same name in two directories which are part of the include directories is causing issues like this one :-(

    For the internal C API, I solve the issue by adding "pycore_" prefix to filenames.

    Maybe pystate.h should be renamed to cpython_pystate.h to avoid to fix the issue. It would include cpython_initconfig.h.

    @vstinner
    Copy link
    Member

    I mark this issue as a duplicate of bpo-39026.

    @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
    Assignees
    No one assigned
    Labels
    3.8 only security fixes build The build process and cross-build topic-C-API
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @vstinner @ericvsmith