classification
Title: Regression: test_datetime fails on 3.5, Win 7, works on 3.4
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: steve.dower Nosy List: JohnLeitch, belopolsky, brycedarling, eric.smith, eryksun, python-dev, r.david.murray, skrah, steve.dower, terry.reedy, vstinner, zach.ware
Priority: normal Keywords: 3.5regression, patch

Created on 2015-09-14 02:54 by terry.reedy, last changed 2015-09-23 01:23 by steve.dower. This issue is now closed.

Files
File name Uploaded Description Edit
25092_1.patch steve.dower, 2015-09-14 22:13
Messages (25)
msg250600 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2015-09-14 02:54
PS C:\Users\Terry> py -3 -m test test_datetime
[1/1] test_datetime
test test_datetime failed -- Traceback (most recent call last):
  File "C:\Programs\Python 3.5\lib\test\datetimetester.py", line 1154, in test_strftime
    self.assertEqual(t.strftime(""), "") # SF bug #761337
ValueError: Invalid format string

Same result 3 times.
msg250607 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2015-09-14 04:00
Looks like something related to issue 24917.
msg250616 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2015-09-14 04:38
I can reproduce.  I'm bewildered as to why none of the buildbots are complaining about this.
msg250623 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2015-09-14 04:49
Interestingly, the re-run performed by regrtest's '-w' flag did not fail.
msg250659 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-09-14 13:08
Maybe errno needs to be explicitly cleared before calling strftime or else we're seeing someone else's EINVAL?
msg250660 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-09-14 13:08
(In the C code I mean, not in the test.)
msg250662 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2015-09-14 13:25
Yes, errno should always be cleared before use (in C99 at least,
not sure what the crt does).
msg250666 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2015-09-14 14:01
> errno should always be cleared before use (in C99 at least,
> not sure what the crt does).

The CRT's strftime doesn't clear errno. I suggested clearing it in issue 25029, msg250215.
msg250671 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-09-14 15:33
I guess when I said I'd done "exactly" what you suggested I misread that part... whoops.

This is a pretty nasty bug to workaround... do we have any way for a user to clear errno from their own code?
msg250673 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2015-09-14 16:26
Clearing is easy: errno = 0; :)


C library functions are not supposed to set errno to 0 by
themselves (C99: paragraph 7.5).
msg250674 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-09-14 16:27
I get that part.

Is there a way people can set errno to zero from Python code? Or do we need to ship 3.5.1 already?
msg250676 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-09-14 16:29
It's common in Modules/posixmodule.c to set errno to 0 before calling a C function, especially when "errno != 0" is checked after the call (because it's the only way to check if the function failed?).
msg250677 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-09-14 16:35
Given all the changes in the windows support in 3.5, I will not be at all surprised if we need to spin 3.5.1 much more quickly than we normally would in any case.
msg250678 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2015-09-14 16:39
Steve, sorry if I misunderstand you: My point (and eryksun's)
was that errno is currently not cleared before the call to
format_time() in

  Modules/timemodule.c:657


I didn't look at this issue in depth, just pointing out a
possible cause.
msg250679 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2015-09-14 16:41
Argh, finally I got it: You suggested that Python users work
around this in 3.5 by setting errno from the interpreter.

I think it's better to release 3.5.1 early.
msg250680 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2015-09-14 16:42
> Is there a way people can set errno to zero from Python code? 
> Or do we need to ship 3.5.1 already?

The only the thing that comes to mind is using ctypes based on the CRT's implementation of errno:

    import ctypes
    import errno
    import time

    ucrtbase = ctypes.CDLL('ucrtbase')
    ucrtbase._errno.restype = ctypes.POINTER(ctypes.c_int)
    c_errno = ucrtbase._errno().contents

    >>> time.strftime('')
    ''
    >>> c_errno.value = errno.EINVAL; time.strftime('')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    ValueError: Invalid format string
msg250682 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2015-09-14 16:49
But it should go without saying that clearing errno from Python isn't reliable considering how much code executes between Python statements. It needs to be cleared right before call strftime, and optionally the old value needs to be saved first in order to restore it, if you're concerned about that.
msg250683 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-09-14 16:58
I'm not worried about preserving it - strftime is supposed to be a very thin wrapper around the underlying strftime.

I think David's right and we'll be shipping 3.5.1 pretty soon regardless (though a lot of the issues seem to be due to changed installation locations and have existing for a long time, just that the people who regularly change the defaults apparently haven't reported them).
msg250708 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-09-14 22:13
Patch attached. I haven't been able to repro the issue locally without the fix, but it seems indisputably correct behavior.

We could also skip most of the function for an empty format string and save ourselves a lot of work. That would avoid the ambiguous returned "is it zero-length or is it an error" value.
msg250709 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-09-14 22:13
(The fix is indisputably correct, is what I meant.)
msg250710 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-09-14 22:14
+#if defined _MSC_VER && _MSC_VER >= 1400 && defined(__STDC_SECURE_LIB__)
+        errno = 0;
+#endif

This code is too complex. Please remove the #if and always set errno to 0, on any platform!
msg250714 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-09-14 23:18
We don't check errno on any other platform.
msg250742 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-09-15 08:45
> We don't check errno on any other platform.

Oh ok. The code becomes more and more verbose because of Windows :-(
msg251350 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-09-22 21:49
Arguably it's because of the platforms that don't reliably set errno. (I don't know exactly which those are, but I'm not about to enable it everywhere right now. If someone else wants to do it and deal with the fallout they're welcome.)
msg251365 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-09-23 00:01
New changeset aa6b9205c120 by Steve Dower in branch '3.5':
Issue #25092: Fix datetime.strftime() failure when errno was already set to EINVAL.
https://hg.python.org/cpython/rev/aa6b9205c120
History
Date User Action Args
2015-09-23 01:23:44steve.dowersetstatus: open -> closed
resolution: fixed
stage: needs patch -> resolved
2015-09-23 00:01:51python-devsetnosy: + python-dev
messages: + msg251365
2015-09-22 21:51:48steve.dowersetassignee: steve.dower
2015-09-22 21:49:50steve.dowersetmessages: + msg251350
2015-09-20 22:52:50zach.waresetkeywords: + 3.5regression
2015-09-20 22:42:16eric.smithsetnosy: + eric.smith
2015-09-15 08:45:59vstinnersetmessages: + msg250742
2015-09-14 23:18:47steve.dowersetmessages: + msg250714
2015-09-14 22:14:39vstinnersetmessages: + msg250710
2015-09-14 22:13:29steve.dowersetmessages: + msg250709
2015-09-14 22:13:10steve.dowersetfiles: + 25092_1.patch
keywords: + patch
messages: + msg250708
2015-09-14 16:58:40steve.dowersetmessages: + msg250683
2015-09-14 16:49:31eryksunsetmessages: + msg250682
2015-09-14 16:42:43eryksunsetmessages: + msg250680
2015-09-14 16:41:58skrahsetmessages: + msg250679
2015-09-14 16:39:28skrahsetmessages: + msg250678
2015-09-14 16:35:56r.david.murraysetnosy: + r.david.murray
messages: + msg250677
2015-09-14 16:29:22vstinnersetmessages: + msg250676
2015-09-14 16:27:52steve.dowersetmessages: + msg250674
2015-09-14 16:26:02skrahsetmessages: + msg250673
2015-09-14 15:33:22steve.dowersetmessages: + msg250671
2015-09-14 14:01:41eryksunsetnosy: + eryksun
messages: + msg250666
2015-09-14 13:25:58skrahsetnosy: + skrah
messages: + msg250662
2015-09-14 13:08:54vstinnersetnosy: + vstinner
2015-09-14 13:08:41steve.dowersetmessages: + msg250660
2015-09-14 13:08:11steve.dowersetmessages: + msg250659
2015-09-14 04:49:54zach.waresetmessages: + msg250623
2015-09-14 04:45:24terry.reedysetversions: + Python 3.5
2015-09-14 04:38:16zach.waresetnosy: + zach.ware
messages: + msg250616
2015-09-14 04:09:19JohnLeitchsetnosy: + brycedarling
2015-09-14 04:00:07belopolskysetnosy: + steve.dower, JohnLeitch
messages: + msg250607
2015-09-14 02:54:18terry.reedycreate