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

PATH_MAX already defined on some Windows compilers #64796

Closed
JeffreyArmstrong mannequin opened this issue Feb 11, 2014 · 15 comments
Closed

PATH_MAX already defined on some Windows compilers #64796

JeffreyArmstrong mannequin opened this issue Feb 11, 2014 · 15 comments
Labels
build The build process and cross-build interpreter-core (Objects, Python, Grammar, and Parser dirs) OS-windows

Comments

@JeffreyArmstrong
Copy link
Mannequin

JeffreyArmstrong mannequin commented Feb 11, 2014

BPO 20597
Nosy @loewis, @vstinner, @larryhastings, @serhiy-storchaka
Files
  • path_max.default.patch: Patch to check for PATH_MAX definition before defining it on Windows
  • remove_path_max.default.patch: Removes PATH_MAX defines entirely
  • 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 2014-11-05.14:18:35.449>
    created_at = <Date 2014-02-11.13:11:15.430>
    labels = ['interpreter-core', 'build', 'OS-windows']
    title = 'PATH_MAX already defined on some Windows compilers'
    updated_at = <Date 2014-11-05.14:18:35.448>
    user = 'https://bugs.python.org/JeffreyArmstrong'

    bugs.python.org fields:

    activity = <Date 2014-11-05.14:18:35.448>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2014-11-05.14:18:35.449>
    closer = 'vstinner'
    components = ['Interpreter Core', 'Windows']
    creation = <Date 2014-02-11.13:11:15.430>
    creator = 'Jeffrey.Armstrong'
    dependencies = []
    files = ['34039', '34044']
    hgrepos = []
    issue_num = 20597
    keywords = ['patch']
    message_count = 15.0
    messages = ['210936', '210962', '210966', '210971', '210985', '210986', '210997', '211000', '211004', '226598', '230662', '230673', '230679', '230681', '230682']
    nosy_count = 6.0
    nosy_names = ['loewis', 'vstinner', 'larry', 'python-dev', 'serhiy.storchaka', 'Jeffrey.Armstrong']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = 'compile error'
    url = 'https://bugs.python.org/issue20597'
    versions = ['Python 3.3', 'Python 3.4']

    @JeffreyArmstrong
    Copy link
    Mannequin Author

    JeffreyArmstrong mannequin commented Feb 11, 2014

    On some Windows compilers, the constant PATH_MAX may already be defined, which will cause compile errors on non-MSVC compilers (notably Open Watcom and MinGW). Rather than assume it is not available and define it in all "#ifdef MS_WINDOWS" cases, it should first be checked for existence.

    This patch adds said check to Modules/main.c and Python/pythonrun.c. This patch should not affect any existing platforms (including MSVC builds).

    @JeffreyArmstrong JeffreyArmstrong mannequin added interpreter-core (Objects, Python, Grammar, and Parser dirs) OS-windows build The build process and cross-build labels Feb 11, 2014
    @vstinner
    Copy link
    Member

    The patch is simple and looks safe. I'm in favor of applying it on Python 3.3 and 3.4. (Python 2.7 is not affect.)

    @larry: ok for Python 3.4?

    @larryhastings
    Copy link
    Contributor

    Let's be a little smarter. PATH_MAX isn't used anymore. Just remove the #defines entirely.

    @JeffreyArmstrong
    Copy link
    Mannequin Author

    JeffreyArmstrong mannequin commented Feb 11, 2014

    Here's an additional patch removing PATH_MAX from Modules/main.c and Python/pythonrun.c. This solution works fine for me.

    @vstinner
    Copy link
    Member

    Let's be a little smarter. PATH_MAX isn't used anymore. Just remove the #defines entirely.

    Oh I remember that I replaced PATH_MAX with MAXPATHLEN or maybe something else when I tried to fix Python compilation issue on our IRIX buildbot :-)

    remove_path_max.default.patch looks good to me. Larry: can it be applied on Python 3.4?

    path_max.default.patch is still needed for Python 3.3.

    @vstinner
    Copy link
    Member

    I found it:

    changeset: 87113:159e51e5fc2c
    branch: 3.3
    parent: 87102:46fc4fb2c8c5
    user: Victor Stinner <victor.stinner@gmail.com>
    date: Fri Nov 15 17:09:24 2013 +0100
    files: Python/pythonrun.c
    description:
    pythonrun.c: fix Py_GetPythonHome(), use Py_ARRAY_LENGTH() to get the size of
    the env_home buffer, not PATH_MAX+1. env_home is declared using MAXPATHLEN+1,
    and PATH_MAX is not declared on IRIX.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Feb 11, 2014

    Jeffrey: Which compilers specifically?

    It's probably not MSVC, hence it's not a "supported" compiler, and IMO shouldn't go in after the release candidate for 3.4.

    @JeffreyArmstrong
    Copy link
    Mannequin Author

    JeffreyArmstrong mannequin commented Feb 11, 2014

    PATH_MAX is defined in both Open Watcom and MinGW's GCC toolchain. Neither is a "supported" compiler, I suppose. I hadn't meant to suggest that it be included in 3.4's release candidate, only that the problem exists on current versions.

    @larryhastings
    Copy link
    Contributor

    Assuming you keep an eye on the buildbots, this has my permission to go in.

    Martin: While we don't officially support those compilers, I don't see the harm of removing unused #defines, so I'm willing to accept it for rc2.

    @JeffreyArmstrong
    Copy link
    Mannequin Author

    JeffreyArmstrong mannequin commented Sep 8, 2014

    Was this ever accepted?

    @JeffreyArmstrong JeffreyArmstrong mannequin closed this as completed Nov 4, 2014
    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Nov 5, 2014

    Reopening. I still don't understand the issue for 3.4, especially in the light of bpo-21274

    @loewis loewis mannequin reopened this Nov 5, 2014
    @JeffreyArmstrong
    Copy link
    Mannequin Author

    JeffreyArmstrong mannequin commented Nov 5, 2014

    What's to understand? Some compilers, particularly MinGW and Open Watcom, already define a PATH_MAX macro on Windows, and it's not necessarily the same as Python's redefinition of it, possibly causing a compiler error. That's all.

    Given the time frame that this bug has existed (9 months!?!) and its trivial fix, which would involve adding an "#ifndef PATH_MAX" right before its declaration, I think "won't fix" is an appropriate resolution.

    Leave it open or don't, it makes little difference to me as I'm no longer interested.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 5, 2014

    New changeset 6aaa0aab1e93 by Victor Stinner in branch 'default':
    Issue bpo-20597: Remove unused definition of PATH_MAX on Windows, MAXPATHLEN is
    https://hg.python.org/cpython/rev/6aaa0aab1e93

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 5, 2014

    New changeset d6fb87972dee by Victor Stinner in branch 'default':
    Issue bpo-20597, bpo-21274: Remove unused definition of PATH_MAX on GNU/Hurd,
    https://hg.python.org/cpython/rev/d6fb87972dee

    @vstinner
    Copy link
    Member

    vstinner commented Nov 5, 2014

    Reopening. I still don't understand the issue for 3.4, especially in the light of bpo-21274

    In Python 3.5, PATH_MAX is no more used in Modules/main.c nor Python/pythonrun.c. I removed the "#define PATH_MAX ..." on Windows on Hurd.

    This issue is about supporting OpenWatcom which is not officially supported to compile Python on Windows, so I don't want to change Python 2.7 nor 3.4. If anyone disagree, please complain :-)

    PATH_MAX is still used in posixmodule.c, but it looks to work on all platforms:

    #ifndef MAXPATHLEN
    #if defined(PATH_MAX) && PATH_MAX > 1024
    #define MAXPATHLEN PATH_MAX
    #else
    #define MAXPATHLEN 1024
    #endif
    #endif /* MAXPATHLEN */

    I'm now closing this issue.

    @vstinner vstinner closed this as completed Nov 5, 2014
    @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
    build The build process and cross-build interpreter-core (Objects, Python, Grammar, and Parser dirs) OS-windows
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants