[PATCH STABLE] do not crash on or store negative timestamps (issue2513)

[Adrian Buehlmann]

On 2010-11-23 02:50, Adrian Buehlmann wrote:
> On 2010-11-23 02:34, Benjamin Pollack wrote:
>> On Nov 22, 2010, at 5:48 PM, Adrian Buehlmann wrote:
>>>> diff --git a/mercurial/util.py b/mercurial/util.py
>>>> --- a/mercurial/util.py
>>>> +++ b/mercurial/util.py
>>>> @@ -1031,6 +1031,8 @@ def datestr(date=None, format='%a %b %d 
>>>>         minutes = abs(tz) // 60
>>>>         format = format.replace("%1", "%c%02d" % (sign, minutes // 60))
>>>>         format = format.replace("%2", "%02d" % (minutes % 60))
>>>> +    if t < 0:
>>>> +        t = 0
>>>>     s = time.strftime(format, time.gmtime(float(t) - tz))
>>>>     return s
>>>
>>> Doesn't t=0 still lead to negative arguments for time.gmtime() for
>>> timezones to the west of Greenwich (e.g. U.S.)?
>>
>> Correct; I realized that in the channel after I'd already set the patch. Near as I can tell, what matters is not that t < 0, but rather that t < 43200, which is the greatest negative offset that I know of in a timezone. (50400 would be the greatest positive, but because it's ahead of GMT, I don't think we care.)
>>
>> I'll do a second patch with this and with Nicolas' request for an argument to ValueError.
> 
> I found something interesting though, which could be somewhat of an
> argument for having the patch like you sent it above. See this:
> 
> $ python
> Python 2.6.5 (r265:79096, Mar 19 2010, 21:48:26) [MSC v.1500 32 bit
> (Intel)] on win32
> Type "help", "copyright", "credits" or "license" for more information.
>>>> import time
>>>> time.gmtime(0)
> time.struct_time(tm_year=1970, tm_mon=1, tm_mday=1, tm_hour=0, tm_min=0,
> tm_sec=0, tm_wday=3, tm_yday=1, tm_isdst=0)
>>>> time.gmtime(-1)
> time.struct_time(tm_year=1969, tm_mon=12, tm_mday=31, tm_hour=23,
> tm_min=59, tm_sec=59, tm_wday=3, tm_yday=365, tm_isdst=
> 0)
>>>> time.gmtime(-43200)
> time.struct_time(tm_year=1969, tm_mon=12, tm_mday=31, tm_hour=12,
> tm_min=0, tm_sec=0, tm_wday=3, tm_yday=365, tm_isdst=0)
>>>> time.gmtime(-43201)
> Traceback (most recent call last):
>   File "<stdin>", line 1, in <module>
> ValueError: (22, 'Invalid argument')
> 
> 
> In other words, time.gmtime(-1) doesn't crash neither does
> time.gmtime(-43200).
> 
> It needs to be -43201 or smaller to crash (which is not what I'd expect
> from reading the msdn page I linked).

Oh, I'm beginning to suspect we should do:


diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -1031,7 +1031,11 @@ def datestr(date=None, format='%a %b %d
         minutes = abs(tz) // 60
         format = format.replace("%1", "%c%02d" % (sign, minutes // 60))
         format = format.replace("%2", "%02d" % (minutes % 60))
-    s = time.strftime(format, time.gmtime(float(t) - tz))
+    if t < 0:
+        lt = 0  # time.gmtime(lt) fails on Windows for lt < -43200
+    else:
+        lt = float(t) - tz
+    s = time.strftime(format, time.gmtime(lt))
     return s

 def shortdate(date=None):


so we get time.gmtime(0) (the epoch) displayed in all timezones if the
limiter kicks in.

If somebody wants to detect if the limiter was hit by looking at
mercurial output, he/she can scan for the epoch (independent in what
timezone that user is) -- instead of the epoch translated into the local
timezone.

Does that make any sense?