-
-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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
Comments
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. |
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
... |
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. |
Can't we move the #define only in .c files where they are needed? Or in a private header (not included by Python.h)? |
That's probably an option, though it would break extensions that use 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 |
Hum, I don't remember that the "timezone" symbol of part of the Python |
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. |
For me, it's not the responsability of python.h to ensure that the |
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...) |
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. |
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 :) |
I'd still rather put the redefinitions in our files that need it, and leave end users untouched. |
I've created a PR for this issue. |
Thanks, Zackery! |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: