classification
Title: VS 2015 pyconfig.h #define timezone _timezone conflicts with timeb.h
Type: behavior Stage: resolved
Components: Build, Windows Versions: Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: steve.dower Nosy List: James Salter, ZackerySpytz, miss-islington, p-ganssle, paul.moore, steve.dower, tim.golden, zach.ware
Priority: high Keywords: patch

Created on 2015-07-16 13:51 by James Salter, last changed 2019-02-26 16:54 by steve.dower. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 12019 merged ZackerySpytz, 2019-02-24 14:55
PR 12040 merged miss-islington, 2019-02-25 23:56
Messages (16)
msg246803 - (view) Author: James Salter (James Salter) * Date: 2015-07-16 13:51
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.
msg246804 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2015-07-16 14:14
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
...
msg246805 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-07-16 14:26
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.
msg246806 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-07-16 15:05
Can't we move the #define only in .c files where they are needed? Or in a private header (not included by Python.h)?
msg246807 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-07-16 15:37
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).
msg246817 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-07-16 20:08
> 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?
msg246820 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-07-16 20:26
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.
msg246821 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-07-16 20:29
For me, it's not the responsability of python.h to ensure that the
timezone symbol is available.
msg246823 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-07-16 20:38
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...)
msg246826 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2015-07-16 20:50
> (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.
msg325250 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2018-09-13 14:43
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 :)
msg325254 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2018-09-13 15:14
I'd still rather put the redefinitions in our files that need it, and leave end users untouched.
msg336473 - (view) Author: Zackery Spytz (ZackerySpytz) * (Python triager) Date: 2019-02-24 15:01
I've created a PR for this issue.
msg336583 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-02-25 23:56
New changeset 6673decfa0fb078f60587f5cb5e98460eea137c2 by Steve Dower (Zackery Spytz) in branch 'master':
bpo-24643: Fix "#define timezone _timezone" clashes on Windows (GH-12019)
https://github.com/python/cpython/commit/6673decfa0fb078f60587f5cb5e98460eea137c2
msg336586 - (view) Author: miss-islington (miss-islington) Date: 2019-02-26 00:15
New changeset 0b3019a02e60171e9b7edb261e1234109001819c by Miss Islington (bot) in branch '3.7':
bpo-24643: Fix "GH-define timezone _timezone" clashes on Windows (GH-12019)
https://github.com/python/cpython/commit/0b3019a02e60171e9b7edb261e1234109001819c
msg336696 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-02-26 16:54
Thanks, Zackery!
History
Date User Action Args
2019-02-26 16:54:34steve.dowersetstatus: open -> closed
messages: + msg336696

assignee: steve.dower
resolution: fixed
stage: patch review -> resolved
2019-02-26 00:15:08miss-islingtonsetnosy: + miss-islington
messages: + msg336586
2019-02-25 23:56:56miss-islingtonsetpull_requests: + pull_request12066
2019-02-25 23:56:47steve.dowersetmessages: + msg336583
2019-02-24 15:08:13p-gansslesetnosy: + p-ganssle
2019-02-24 15:01:08ZackerySpytzsetnosy: + ZackerySpytz
messages: + msg336473
2019-02-24 14:55:49ZackerySpytzsetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request12050
2018-09-13 18:44:33vstinnersetnosy: - vstinner
2018-09-13 15:14:29steve.dowersetmessages: + msg325254
2018-09-13 14:43:28zach.waresetpriority: normal -> high

messages: + msg325250
versions: + Python 3.7, Python 3.8, - Python 3.5, Python 3.6
2018-09-13 14:40:35zach.warelinkissue34657 superseder
2015-07-16 20:50:33zach.waresetmessages: + msg246826
2015-07-16 20:38:52steve.dowersetmessages: + msg246823
2015-07-16 20:29:34vstinnersetmessages: + msg246821
2015-07-16 20:26:44steve.dowersetmessages: + msg246820
2015-07-16 20:08:58vstinnersetmessages: + msg246817
2015-07-16 15:37:22steve.dowersetmessages: + msg246807
2015-07-16 15:05:54vstinnersetnosy: + vstinner
messages: + msg246806
2015-07-16 14:26:30steve.dowersetmessages: + msg246805
2015-07-16 14:14:53zach.waresetversions: + Python 3.6
nosy: + paul.moore, tim.golden, zach.ware, steve.dower

messages: + msg246804

components: + Build, Windows, - Extension Modules
stage: needs patch
2015-07-16 13:51:20James Saltercreate