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

VS 2015 pyconfig.h #define timezone _timezone conflicts with timeb.h #68831

Closed
JamesSalter mannequin opened this issue Jul 16, 2015 · 16 comments
Closed

VS 2015 pyconfig.h #define timezone _timezone conflicts with timeb.h #68831

JamesSalter mannequin opened this issue Jul 16, 2015 · 16 comments
Assignees
Labels
3.7 (EOL) end of life 3.8 only security fixes build The build process and cross-build OS-windows type-bug An unexpected behavior, bug, or error

Comments

@JamesSalter
Copy link
Mannequin

JamesSalter mannequin commented Jul 16, 2015

BPO 24643
Nosy @pfmoore, @tjguk, @zware, @zooba, @pganssle, @ZackerySpytz, @miss-islington
PRs
  • bpo-24643: Fix "#define timezone _timezone" clashes on Windows #12019
  • [3.7] bpo-24643: Fix "GH-define timezone _timezone" clashes on Windows (GH-12019) #12040
  • 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 = 'https://github.com/zooba'
    closed_at = <Date 2019-02-26.16:54:34.464>
    created_at = <Date 2015-07-16.13:51:20.041>
    labels = ['3.8', 'type-bug', '3.7', 'OS-windows', 'build']
    title = 'VS 2015 pyconfig.h #define timezone _timezone conflicts with timeb.h'
    updated_at = <Date 2019-02-26.16:54:34.463>
    user = 'https://bugs.python.org/JamesSalter'

    bugs.python.org fields:

    activity = <Date 2019-02-26.16:54:34.463>
    actor = 'steve.dower'
    assignee = 'steve.dower'
    closed = True
    closed_date = <Date 2019-02-26.16:54:34.464>
    closer = 'steve.dower'
    components = ['Build', 'Windows']
    creation = <Date 2015-07-16.13:51:20.041>
    creator = 'James Salter'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 24643
    keywords = ['patch']
    message_count = 16.0
    messages = ['246803', '246804', '246805', '246806', '246807', '246817', '246820', '246821', '246823', '246826', '325250', '325254', '336473', '336583', '336586', '336696']
    nosy_count = 8.0
    nosy_names = ['paul.moore', 'tim.golden', 'zach.ware', 'steve.dower', 'James Salter', 'p-ganssle', 'ZackerySpytz', 'miss-islington']
    pr_nums = ['12019', '12040']
    priority = 'high'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue24643'
    versions = ['Python 3.7', 'Python 3.8']

    @JamesSalter
    Copy link
    Mannequin Author

    JamesSalter mannequin commented Jul 16, 2015

    For python 3.5, PC/pyconfig.h contains the following for vs2015 support:

    /* VS 2015 defines these names with a leading underscore */
    #if _MSC_VER >= 1900
    #define timezone _timezone
    #define daylight _daylight
    #define tzname _tzname
    #endif

    This breaks any python extension code that includes pyconfig.h and then defines any function or variable called 'timezone', 'daylight', or 'tzname'.

    My breaking case is a conflict with <ucrt/sys/timeb.h> from the Windows 10 kit (for me: c:\program files (x86)\Windows Kits\10\include\10.0.10056.0\ucrt\sys\timeb.h). This is used during compilation of gevent, which includes that file via libev/ev_win32.c. timeb.h contains this innocent structure:

    struct __timeb32
    {
        __time32_t     time;
        unsigned short millitm;
        short          timezone;
        short          dstflag;
    };

    I think we need a different approach that doesn't conflict with common english variable names and in particular other windows SDK includes.

    @JamesSalter JamesSalter mannequin added extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error labels Jul 16, 2015
    @zware
    Copy link
    Member

    zware commented Jul 16, 2015

    I suppose we'll have to resort to

    #ifndef _Py_timezone
    #if _MSC_VER >= 1900
    #define _Py_timezone _timezone
    #else
    #define _Py_timezone timezone
    #endif
    #endif
    ...

    @zware zware added build The build process and cross-build OS-windows and removed extension-modules C modules in the Modules dir labels Jul 16, 2015
    @zooba
    Copy link
    Member

    zooba commented Jul 16, 2015

    Or we could define _timezone on those platforms that don't have the underscore.

    I'm not hugely fussed either way. We need to fix this though.

    @vstinner
    Copy link
    Member

    Can't we move the #define only in .c files where they are needed? Or in a private header (not included by Python.h)?

    @zooba
    Copy link
    Member

    zooba commented Jul 16, 2015

    That's probably an option, though it would break extensions that use timezone expecting it to work. But it seems like any change is going to cause that.

    I prefer defining _Py_timezone, since at least we can offer something that is portable for all Python 3.5 platforms (that support timezones), unlike timezone/_timezone (MSVC deprecated timezone a few versions ago and has just removed it completely, hence the breakage).

    @vstinner
    Copy link
    Member

    That's probably an option, though it would break extensions that use timezone expecting it to work.

    Hum, I don't remember that the "timezone" symbol of part of the Python
    public API. Which extensions?

    @zooba
    Copy link
    Member

    zooba commented Jul 16, 2015

    It's not, but "#include <python.h>" in any extension will make it available for you, so it's very likely that extensions have simply used it without adding their own conditional compilation for the various interpretations of whether timezone is standard or not.

    Bit of a strawman, granted. Maybe working at Microsoft has made me overly cautious about changes that could break *anyone* - it's a big deal in many groups here.

    @vstinner
    Copy link
    Member

    For me, it's not the responsability of python.h to ensure that the
    timezone symbol is available.

    @zooba
    Copy link
    Member

    zooba commented Jul 16, 2015

    Agreed. However, I also don't want extensions to stop building because of a change we make. But since that's inevitable here, let's go with Zach's original suggestion and use a name that won't conflict. (IIRC, I originally put the #ifdefs in each file and was told to move them to pyconfig.h...)

    @zware
    Copy link
    Member

    zware commented Jul 16, 2015

    (IIRC, I originally put the #ifdefs in each file and was told to move
    them to pyconfig.h...)

    Yeah, I think I did suggest that, to match what we do with hypot/_hypot for MSC 1600+. We should probably also change that one while fixing timezone, daylight, and tzname.

    @zware
    Copy link
    Member

    zware commented Sep 13, 2018

    This was just reported again in bpo-34657. We should go ahead and define _Py_timezone et al., which I will try to do today if nobody beats me to it :)

    @zware zware added 3.7 (EOL) end of life 3.8 only security fixes labels Sep 13, 2018
    @zooba
    Copy link
    Member

    zooba commented Sep 13, 2018

    I'd still rather put the redefinitions in our files that need it, and leave end users untouched.

    @ZackerySpytz
    Copy link
    Mannequin

    ZackerySpytz mannequin commented Feb 24, 2019

    I've created a PR for this issue.

    @zooba
    Copy link
    Member

    zooba commented Feb 25, 2019

    New changeset 6673dec by Steve Dower (Zackery Spytz) in branch 'master':
    bpo-24643: Fix "#define timezone _timezone" clashes on Windows (GH-12019)
    6673dec

    @miss-islington
    Copy link
    Contributor

    New changeset 0b3019a by Miss Islington (bot) in branch '3.7':
    bpo-24643: Fix "GH-define timezone _timezone" clashes on Windows (GH-12019)
    0b3019a

    @zooba
    Copy link
    Member

    zooba commented Feb 26, 2019

    Thanks, Zackery!

    @zooba zooba closed this as completed Feb 26, 2019
    @zooba zooba self-assigned this Feb 26, 2019
    @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.7 (EOL) end of life 3.8 only security fixes build The build process and cross-build OS-windows type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants