Regression: test_datetime fails on 3.5, Win 7, works on 3.4

[terryjreedy]

BPO	25092
Nosy	@terryjreedy, @abalkin, @vstinner, @ericvsmith, @bitdancer, @skrah, @zware, @eryksun, @zooba
Files	25092_1.patch

^{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 2015-09-23.01:23:44.666>
created_at = <Date 2015-09-14.02:54:18.671>
labels = ['type-bug', 'library']
title = 'Regression: test_datetime fails on 3.5, Win 7, works on 3.4'
updated_at = <Date 2015-09-23.01:23:44.665>
user = 'https://github.com/terryjreedy'

bugs.python.org fields:

activity = <Date 2015-09-23.01:23:44.665>
actor = 'steve.dower'
assignee = 'steve.dower'
closed = True
closed_date = <Date 2015-09-23.01:23:44.666>
closer = 'steve.dower'
components = ['Library (Lib)']
creation = <Date 2015-09-14.02:54:18.671>
creator = 'terry.reedy'
dependencies = []
files = ['40469']
hgrepos = []
issue_num = 25092
keywords = ['patch', '3.5regression']
message_count = 25.0
messages = ['250600', '250607', '250616', '250623', '250659', '250660', '250662', '250666', '250671', '250673', '250674', '250676', '250677', '250678', '250679', '250680', '250682', '250683', '250708', '250709', '250710', '250714', '250742', '251350', '251365']
nosy_count = 12.0
nosy_names = ['terry.reedy', 'belopolsky', 'vstinner', 'eric.smith', 'r.david.murray', 'skrah', 'python-dev', 'zach.ware', 'eryksun', 'steve.dower', 'JohnLeitch', 'brycedarling']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue25092'
versions = ['Python 3.5']

[terryjreedy]

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 python/cpython#38716
ValueError: Invalid format string

Same result 3 times.

[abalkin]

Looks like something related to bpo-24917.

[zware]

I can reproduce. I'm bewildered as to why none of the buildbots are complaining about this.

[zware]

Interestingly, the re-run performed by regrtest's '-w' flag did not fail.

[zooba]

Maybe errno needs to be explicitly cleared before calling strftime or else we're seeing someone else's EINVAL?

[zooba]

(In the C code I mean, not in the test.)

[skrah]

Yes, errno should always be cleared before use (in C99 at least,
not sure what the crt does).

[eryksun]

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 bpo-25029, msg250215.

[zooba]

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?

[skrah]

Clearing is easy: errno = 0; :)

C library functions are not supposed to set errno to 0 by
themselves (C99: paragraph 7.5).

[zooba]

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?

[vstinner]

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?).

[bitdancer]

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.

[skrah]

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.

[skrah]

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.

[eryksun]

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

[eryksun]

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.

[zooba]

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).

[zooba]

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.

[zooba]

(The fix is indisputably correct, is what I meant.)

[vstinner]

+#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!

[zooba]

We don't check errno on any other platform.

[vstinner]

We don't check errno on any other platform.

Oh ok. The code becomes more and more verbose because of Windows :-(

[zooba]

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.)

[python-dev]

New changeset aa6b9205c120 by Steve Dower in branch '3.5':
Issue bpo-25092: Fix datetime.strftime() failure when errno was already set to EINVAL.
https://hg.python.org/cpython/rev/aa6b9205c120