I tested manually the fix on FreeBSD 12.0-RC2 and Fedora Rawhide with TZ=UTC: the change fix test_email.

@bitdancer, @abalkin, @pganssle: Would you mind to review this change?

This will still blow up outside of test environment, when I have UTC timezone:

>>> t0 = datetime.datetime(2012, 8, 12, 1, 1)
>>> t1 = utils.localtime(t0, isdst=1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib64/python3.7/email/utils.py", line 361, in localtime
    seconds = time.mktime(tm)
OverflowError: mktime argument out of range

Shouldn't the exception be nicer? E.g.: ValueError: You cannot have DST when you have UTC timezone.

This will still blow up outside of test environment, when I have UTC timezone: (...)

That's expected. Python exposes the behavior of the OS.

The PR only fix test_email, it doesn't touch time nor datetime modules.

Hm, I was not aware of this email.utils.localtime. It seems like a bad idea to have added this function in the first place, but probably the tests for it should be pinning to a specific time zone if they implicitly rely on that time zone's behavior.

Hm, I was not aware of this email.utils.localtime. It seems like a bad idea to have added this function in the first place, but probably the tests for it should be pinning to a specific time zone if they implicitly rely on that time zone's behavior.

Sorry, I don't know the origin of this helper and I don't know the function. Feel free to propose an enhancement :-)

I only care of fixing the test ;-)

Shouldn't the exception be nicer? E.g.: ValueError: You cannot have DST when you have UTC timezone.

Python is a thin wrapper to the C function mktime() which only sets errno to EOVERFLOW on error: "The result cannot be represented". The function doesn't tell where the error comes from.

@vstinner Yes, hopefully we can get a "local time" tzinfo singleton in Python 3.8 or 3.9 and this function can either be deprecated or turned into a wrapper for return dt.astimezone(LOCAL_TIME_ZONE)

@vstinner Yes, hopefully we can get a "local time" tzinfo singleton in Python 3.8 or 3.9 and this function can either be deprecated or turned into a wrapper for return dt.astimezone(LOCAL_TIME_ZONE)

Adding a "local timezone" feature directly to datetime would be great, but I have no idea how to do that :-)

@vstinner @hroncok I agree with Miro that OverflowError is not the right exception to raise there, it's quite opaque. I also think it's a different bug than "this specific test fails if you run it in the wrong locale".

Maybe we can open up a new bug on the issue tracker (if one doesn't exist) to address the exception raised in UTC.

I'm pretty confident that at the very least email.utils.localtime should not throw OverflowError for any valid datetime in any real time zone, regardless of whether time.mktime may occasionally throw OverflowError.

I recommend a new PR for a new issue that starts by adding a new test that is effectively the same as the one that throws an error, but pins the time zone to 'UTC' instead. Note that that tests something different from this test - if I understand this correctly, this test is checking that is_dst works correctly when the date given is already DST. That test would be meaningless or have a very different meaning when applied to UTC.

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒⛏🤖

GH-10741 is a backport of this pull request to the 3.7 branch.

GH-10742 is a backport of this pull request to the 3.6 branch.