Zooko gets an error when he does a 'darcs changes --xml-output' on NetBSD. The
message is:'darcs failed:  Time.toClockTime: invalid input'. It's in a buildbot
log that can be found on http://allmydata.org/buildbot/builders/MM netbsd4 i386
warp/builds/70/steps/build/logs/stdio .

It appears that this bug is triggered via the Darcs.Patch.Info.to_xml and
Darcs.Patch.Info.friendly_d functions in darcs. toClockTime is then called on a
data structure that is produced by readPatchDate. 

toClockTime has the following ominous bit of source code:

     -- FIXME: check, whether this works on other arch's than Linux, too...
     -- 
     -- so we set it to (-1) (means `unknown') and let `mktime' determine
     -- the real value...
    let isDst = -1 :: CInt in   -- if _isdst then (1::Int) else 0

    if psec < 0 || psec > 999999999999 then
        error "Time.toClockTime: picoseconds out of range"
    else if tz < -43200 || tz > 43200 then
        error "Time.toClockTime: timezone offset out of range"
    else
      unsafePerformIO $ do
      allocaBytes (44) $ \ p_tm -> do
{-# LINE 548 "System/Time.hsc" #-}
        ((\hsc_ptr -> pokeByteOff hsc_ptr 0)) p_tm	(fromIntegral sec  :: CInt)
{-# LINE 549 "System/Time.hsc" #-}
        ((\hsc_ptr -> pokeByteOff hsc_ptr 4)) p_tm	(fromIntegral minute :: CInt)
{-# LINE 550 "System/Time.hsc" #-}
        ((\hsc_ptr -> pokeByteOff hsc_ptr 8)) p_tm	(fromIntegral hour :: CInt)
{-# LINE 551 "System/Time.hsc" #-}
        ((\hsc_ptr -> pokeByteOff hsc_ptr 12)) p_tm	(fromIntegral mday :: CInt)
{-# LINE 552 "System/Time.hsc" #-}
        ((\hsc_ptr -> pokeByteOff hsc_ptr 16)) p_tm	(fromIntegral (fromEnum mon)
:: CInt)
{-# LINE 553 "System/Time.hsc" #-}
        ((\hsc_ptr -> pokeByteOff hsc_ptr 20)) p_tm	(fromIntegral year - 1900 ::
CInt)
{-# LINE 554 "System/Time.hsc" #-}
        ((\hsc_ptr -> pokeByteOff hsc_ptr 32)) p_tm	isDst
{-# LINE 555 "System/Time.hsc" #-}
	t <- throwIf (== -1) (\_ -> "Time.toClockTime: invalid input")
		(mktime p_tm)


Hence, I expect this problem will go away when we stop using toClockTime in
friendly_d.

When you say "stop using toClockTime", you mean "dump the oldtime package in
favour of the newer one?"

On Saturday 15 August 2009 23:53:56 Eric Kow wrote:
> When you say "stop using toClockTime", you mean "dump the oldtime package
> in favour of the newer one?"

Yes, sort of. At least in this particular case. And if NetBSD's mktime() isn't 
happy about what toClockTime feeds it here, it probably isn't in other cases 
either, so we may have to purge all toClockTime calls from darcs.

Maybe Zooko could try a darcs which only removes the "toCalendarTime . 
toClockTime" from this function, and if that solves this particular case, get 
rid of the old-time dependency altogether?

Reinier

It is actually someone else -- a contributor to Tahoe-LAFS -- who has installed
darcs-2.2.0 on their NetBSD machine and then I can see this issue on the
Tahoe-LAFS buildbot.  So we would need to ask that person -- midnightmagic -- to
try such an experiment.

OK Zooko, I'm assigning this to you until we hear back from midnightmagic about
the effects of replacing friendly_d as follows:

friendly_d :: B.ByteString -> String
--friendly_d d = calendarTimeToString . readPatchDate . d
friendly_d d = unsafePerformIO $ do
    ct <- readPatchDate d
    return $ calendarTimeToString ct

Reinier, please shout if this is not the correct request to make

Hey folks! I'm he who had the problem. I'll attempt to test this patch. Sorry I 
missed this: I checked this literally just a few days prior to the final note.

Urk..  I'm sorry my Haskell-fu sucks so hard. I tried to get the patch correct, 
and I end up with this when building darcs-2.2.0:

src/Darcs/Patch/Info.hs:155:10:
    Couldn't match expected type `IO t'
           against inferred type `CalendarTime'
    In a stmt of a 'do' expression: ct <- readPatchDate d
    In the second argument of `($)', namely
        `do ct <- readPatchDate d
              return $ calendarTimeToString ct'
    In the expression:
          unsafePerformIO
        $ do ct <- readPatchDate d
               return $ calendarTimeToString ct
gmake: *** [src/Darcs/Patch/Info.o] Error 1
*** Error code 2

Stop.

Here's the patch I used to modify the source:

--- src/Darcs/Patch/Info.hs~    2009-09-02 14:34:07.000000000 -0700
+++ src/Darcs/Patch/Info.hs     2009-09-02 14:34:07.000000000 -0700
@@ -152,8 +152,8 @@
 friendly_d :: B.ByteString -> String
 --friendly_d d = calendarTimeToString . readPatchDate . d
 friendly_d d = unsafePerformIO $ do
-    ct <- toCalendarTime $ toClockTime $ readPatchDate d
-    return $ calendarTimeToString ct
+    ct <- readPatchDate d
+        return $ calendarTimeToString ct
 
 to_xml :: PatchInfo -> Doc
 to_xml pi =

My version of ghc is 6.8.3, in case that matters. I have had no success yet 
trying to bootstrap 6.10.x on my systems. But I'm also attempting to do that.

Frustrating not being as good at this stuff as I am with other.

On Thu, Sep 03, 2009 at 00:37:48 +0000, midnightmagic wrote:
> Urk..  I'm sorry my Haskell-fu sucks so hard. I tried to get the patch correct, 
> and I end up with this when building darcs-2.2.0:

Dealing with IO can be tricky at first if you don't have a guide.  If
you're interested in learning more, I can point you to some Haskell
tutorials.

Meanwhile, here's the short answer:

friendly_d d = calendarTimeToString (readPatchDate d)

Long answer:

>  friendly_d :: B.ByteString -> String
>  friendly_d d = unsafePerformIO $ do
> -    ct <- toCalendarTime $ toClockTime $ readPatchDate d
> -    return $ calendarTimeToString ct
> +    ct <- readPatchDate d
> +        return $ calendarTimeToString ct

Here are the type signatures of the three functions we're
using

readPatchDate :: B.ByteString -> CalendarTime
toClockTime :: CalendarTime -> ClockTime
toCalendarTime :: ClockTime -> IO CalendarTime

If you follow them in order, we're reading a ByteString into a CalendarTime
which we then convert to a ClockTime which we then convert "back" into an IO
CalendarTime.

The conversion from ClockTime to CalendarTime requires IO because you
have to interact with the real world to get locale information (ie. the
timezone).

But if you're just doing readPatchDate, you don't need to do any IO so you
can get rid of all the bureaucracy above with the do and the return
and the unsafePerformIO to get

  friendly_d :: B.ByteString -> String
  friendly_d d =
     let ct = readPatchDate d
     in calendarTimeToString ct

Which can be simplified down into

  friendly_d :: B.ByteString -> String
  friendly_d d = calendarTimeToString (readPatchDate d)

Or even further into

  friendly_d :: B.ByteString -> String
  friendly_d = calendarTimeToString . readPatchDate

which appears to be wrong

I'm afraid that's not much of an explanation, but you're best off with
a Haskell tutorial for that.

On Thu, Sep 03, 2009 at 06:39:45 +0000, Eric Kow wrote:
>   friendly_d :: B.ByteString -> String
>   friendly_d = calendarTimeToString . readPatchDate
> 
> which appears to be wrong

Err, I somehow lost part of my message there.

which appeared to be wrong :-)

Sorry for the confusion!

On Thu, Sep 03, 2009 at 06:43:03 +0000, Eric Kow wrote:
> >   friendly_d :: B.ByteString -> String
> >   friendly_d = calendarTimeToString . readPatchDate
> > 
> > which appears to be wrong
> 
> Err, I somehow lost part of my message there.
> 
> which appeared to be wrong :-)

Fascinating.  The bug-tracker keeps removing part of my message.  Hopefully
armoring it with some quotes will make it pass through

| Note that there is a commented-out                                                                           
| >  friendly_d d = calendarTimeToString . readPatchDate . d                                                   
| which appears to be wrong

The following patch, as recommended by you kind folks, has worked perfectly:

--- src/Darcs/Patch/Info.hs.orig        2009-01-15 14:53:59.000000000 -0800
+++ src/Darcs/Patch/Info.hs     2009-09-03 14:36:58.000000000 -0700
@@ -150,10 +150,7 @@
           t = BC.pack "TAG "
 
 friendly_d :: B.ByteString -> String
---friendly_d d = calendarTimeToString . readPatchDate . d
-friendly_d d = unsafePerformIO $ do
-    ct <- toCalendarTime $ toClockTime $ readPatchDate d
-    return $ calendarTimeToString ct
+friendly_d d = calendarTimeToString (readPatchDate d)
 
 to_xml :: PatchInfo -> Doc
 to_xml pi =

This is a diff from plain darcs-2.2.0: that Haskell comment in there is in the 
darcs sources originally. I'm still not completely clear on why this works and 
the old one didn't. That is, I don't know what precisely was causing the 
failure: what was it passing to mktime() that NetBSD was choking on?

The reason I ask is that NetBSD mktime includes all the fields in the struct tm 
that Linux does, plus two more optional ones (long tm_gmtoff and char *tm_zone) 
that Linux apparently doesn't, so it shouldn't have any issue with values 
passed to it. mktime is standards-compliant version on both systems.

Also, I wanted to thank you very much, kowey, for the in-depth discussion. I 
really appreciate the time you took.

Hi Reinier,

Would you mind following up on this bug, now that midnightmagic has kindly done
your suggested tests?  Is there anything in particular for us to do except work
harder on switching to the new time package?  Thanks :-)

(Midnightmagic: thanks, and glad to help!)

OK, this needs a summary.

1. On NetBSD only, we were getting a Time.toClockTime: invalid input (see
msg8169 for details)

2. To debug this, midnightmagic removed a (toCalendarTime . toClockTime) from
our function, which was:

friendly_d :: B.ByteString -> String
friendly_d d = unsafePerformIO $ do
    ct <- toCalendarTime $ toClockTime $ readPatchDate d
    return $ calendarTimeToString ct

3. This made the problem go away, but apparently we still don't have a good idea
why.  In msg8692 midnightmagic wonders: I don't know what precisely was causing the 
failure: what was it passing to mktime() that NetBSD was choking on?

The reason I ask is that NetBSD mktime includes all the fields in the struct tm 
that Linux does, plus two more optional ones (long tm_gmtoff and char *tm_zone) 
that Linux apparently doesn't, so it shouldn't have any issue with values 
passed to it. mktime is standards-compliant version on both systems

Hey, midnightmagic:

I just checked with some GHC guru types for advice.  Here is their request. 
Could you save the following, compile it (ghc --make) and let us know if it
fails or what it outputs otherwise?

Thanks!

---------------------
import System.Time

-- this is one line
main = print . toClockTime $ CalendarTime 2009 October 8 17 0 0 0 Thursday 280
"BST" 3600 True
----------------------

I have two Haskell systems.. (for lack of a better term) on the machine in 
question. I used Hugs:

[Hugs]
Main> :load darcs_test.hs
Main> main
Thu Oct  8 09:00:00 PDT 2009

Main> ^D[Leaving Hugs]

... and then for the ghc compiled version, here are the results:

:draco:10:55:11 /v2$ ghc --make darcs_test.hs 
[1 of 1] Compiling Main             ( darcs_test.hs, darcs_test.o )
Linking darcs_test ...
:draco:10:55:30 /v2$ ls -liad *darcs*
6 -rwxr-xr-x  1 mm  wheel  3275012 Oct  8 10:55 darcs_test
3 -rw-r--r--  1 mm  wheel      256 Oct  8 10:55 darcs_test.hi
4 -rw-r--r--  1 mm  wheel      135 Oct  8 10:53 darcs_test.hs
5 -rw-r--r--  1 mm  wheel     4192 Oct  8 10:55 darcs_test.o
:draco:10:55:32 /v2$ ./darcs_test
Thu Oct  8 09:00:00 PDT 2009
:draco:10:55:35 /v2$

Hi again,

Could you give this one a try?  You might have to run it a few times as it's
based on the current clock time.  GHC should be fine.

import System.Time

main = do t <- toCalendarTime =<< getClockTime
          print t
          print . toClockTime $ t

I ran it thousands and thousands of times and it appears to have worked every 
time, returning something like:

CalendarTime {ctYear = 2009, ctMonth = October, ctDay = 12, ctHour = 22, ctMin 
= 10, ctSec = 19, ctPicosec = 140068000000, ctWDay = Monday, ctYDay = 284, 
ctTZName = "PDT", ctTZ = -25200, ctIsDST = True}
Mon Oct 12 22:10:19 PDT 2009

Thanks!  Could I have you try one more thing?

First, presumably you can still reproduce the crash, right (since it's doing
date conversion on a specific date)?

Second, how about printing out the date before converting it, for example by
replacing your friendly_d with the following:

friendly_d d = unsafePerformIO $ do
    let d2 = readPatchDate d
    putStrLn $ "Making friendly " ++ show d2
    ct <- toCalendarTime $ toClockTime $ d2
    return $ calendarTimeToString ct

Third, if you can make it crash *and* print out the date that's causing the
crash, could you plug that date into the script I provided in msg8930 ?

Got it: reproducing the crash (tough on such a slow machine where rebuilding 
these things is quite the process, yay me):

<patch author='zooko@zooko.com' date='20090308025039' local_date='Making 
friendly CalendarTime {ctYear = 2009, ctMonth = March, ctDay = 8, ctHour = 2, 
ctMin = 50, ctSec = 39, ctPicosec = 0, ctWDay = Sunday, ctYDay = 0, ctTZName = 
"GMT", ctTZ = 0, ctIsDST = False}

Plugging that into your first requested program:

import System.Time

-- this is one line
main = print . toClockTime $ CalendarTime 2009 March 8 2 50 39 0 Sunday 0 "GMT" 
0 False

... returns:

t: user error (Time.toClockTime: invalid input)

So..  This suggests to me there are two problems. First, the toClockTime 
function has trouble with certain input.

This fails:
-- this is one line
main = print . toClockTime $ CalendarTime 2009 March 8 2 59 59 1000 Thursday 
280 "BST" 3600 True

This succeeds:
-- this is one line
main = print . toClockTime $ CalendarTime 2009 March 8 3 0 0 0 Thursday 280 
"BST" 3600 True

Weird huh?

And then clearly, the second problem appears to be the conversion of the 
datestamp in the following partial log entry:

<patch author='zooko@zooko.com' date='20090308025039' local_date='Making 
friendly CalendarTime {ctYear = 2009, ctMonth = March, ctDay = 8, ctHour = 2, 
ctMin = 50, ctSec = 39, ctPicosec = 0, ctWDay = Sunday, ctYDay = 0, ctTZName = 
"GMT", ctTZ = 0, ctIsDST = False}

(This is with a modified darcs with the printf-equivalent in it.)

This is suspiciously near to the DST switchover. If you need a zdump of my 
current DST timings, I can provide the pre-zic entries.

Is there perhaps a DST switchover assumption in that logic?

Let me know if you'd like me to delve further. Hopefully this is enough to make 
the problem clear to you darcs folks.

Excellent!  I've forwarded this to Ian and am waiting for their reply.  Thanks
much :-)

For the benefit of those living near the international date line...
this code wrongly restricts time zone offsets to a maximum of +1200 hours.  It 
should allow at least +1345 hours for Chatham Islands during daylight savings 
and, I think, +1400 hours for Kiribati... I don't think they have DST.

This results in the "Time.toClockTime: timezone offset out of range" error when 
you do, for example, darcs changes --matches 'date "<d1>/<d2>"'.  This might 
start happening to you without warning when you enter DST.

This affects at least darcs version 2.2.1, but it looks like this code is no 
longer present in head, so some later version might work ok... haven't had a 
chance to check.

Thanks Daniel.  I haven't had a chance to think about (or understand) your
observation yet, but could I just superficially ask if it's something you
could/should forward to the appropriate library maintainer?  Just delegating as
much as humanly possible :-)  Please shout if this isn't realistic for you to do.

Yeah, I just left a note here for other sufferers because this is the only page 
that came up when searching for the error.

I've filed a bug, and patch, against old-time to support timezone offsets up to 
+1400:
http://hackage.haskell.org/trac/ghc/ticket/3793