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

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

Closed
terryjreedy opened this issue Sep 14, 2015 · 25 comments
Closed

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

terryjreedy opened this issue Sep 14, 2015 · 25 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@terryjreedy
Copy link
Member

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
    Copy link
    Member Author

    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.

    @terryjreedy terryjreedy added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Sep 14, 2015
    @abalkin
    Copy link
    Member

    abalkin commented Sep 14, 2015

    Looks like something related to bpo-24917.

    @zware
    Copy link
    Member

    zware commented Sep 14, 2015

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

    @zware
    Copy link
    Member

    zware commented Sep 14, 2015

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

    @zooba
    Copy link
    Member

    zooba commented Sep 14, 2015

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

    @zooba
    Copy link
    Member

    zooba commented Sep 14, 2015

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

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Sep 14, 2015

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

    @eryksun
    Copy link
    Contributor

    eryksun commented Sep 14, 2015

    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
    Copy link
    Member

    zooba commented Sep 14, 2015

    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
    Copy link
    Mannequin

    skrah mannequin commented Sep 14, 2015

    Clearing is easy: errno = 0; :)

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

    @zooba
    Copy link
    Member

    zooba commented Sep 14, 2015

    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
    Copy link
    Member

    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
    Copy link
    Member

    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
    Copy link
    Mannequin

    skrah mannequin commented Sep 14, 2015

    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
    Copy link
    Mannequin

    skrah mannequin commented Sep 14, 2015

    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
    Copy link
    Contributor

    eryksun commented Sep 14, 2015

    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
    Copy link
    Contributor

    eryksun commented Sep 14, 2015

    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
    Copy link
    Member

    zooba commented Sep 14, 2015

    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
    Copy link
    Member

    zooba commented Sep 14, 2015

    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
    Copy link
    Member

    zooba commented Sep 14, 2015

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

    @vstinner
    Copy link
    Member

    +#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
    Copy link
    Member

    zooba commented Sep 14, 2015

    We don't check errno on any other platform.

    @vstinner
    Copy link
    Member

    We don't check errno on any other platform.

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

    @zooba
    Copy link
    Member

    zooba commented Sep 22, 2015

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

    @zooba zooba self-assigned this Sep 22, 2015
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 23, 2015

    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

    @zooba zooba closed this as completed Sep 23, 2015
    @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
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    7 participants