Date parsing

Date parsing

[Sverre Rabbelier]

Heya,

I noticed that we parse
GIT_COMMITTER_DATE="31-05-2008 11:30pm" as "31st of July at 23:30 +0200"
whereas we parse
GIT_COMMITTER_DATE="01-06-2008 11:30pm" as "6th of January at 23:30 +0100"

Not only is it strange that we accept both orders (since apparently we
don't resort to the latter order unless the first digit > 12), but
it's also weird that the timezone changes based on which option was
chosen! What does the list think about this? Is this "normal"
behavior? If not, what (if anything) should we do about it?

-- 
Cheers,

Sverre Rabbelier

Re: Date parsing

[Johannes Sixt]

Sverre Rabbelier schrieb:
> Heya,
> 
> I noticed that we parse
> GIT_COMMITTER_DATE="31-05-2008 11:30pm" as "31st of July at 23:30 +0200"
> whereas we parse
> GIT_COMMITTER_DATE="01-06-2008 11:30pm" as "6th of January at 23:30 +0100"
> 
> Not only is it strange that we accept both orders (since apparently we
> don't resort to the latter order unless the first digit > 12), but
> it's also weird that the timezone changes based on which option was
> chosen! What does the list think about this? Is this "normal"
> behavior? If not, what (if anything) should we do about it?

Since you did not give a timezone, your local timezone is taken,
presumably MET, which has daylight saving time in July, but not in
January. This is OK.

-- Hannes

Re: Date parsing

[Sverre Rabbelier]

On Tue, Jun 10, 2008 at 5:07 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Since you did not give a timezone, your local timezone is taken,
> presumably MET, which has daylight saving time in July, but not in
> January. This is OK.

Ah, then I agree the timezone change makes sense, although magical
ordering switching is still bothering me.

-- 
Cheers,

Sverre Rabbelier

Re: Date parsing

[Brandon Casey]

Sverre Rabbelier wrote:
> Heya,
> 
> I noticed that we parse
> GIT_COMMITTER_DATE="31-05-2008 11:30pm" as "31st of July at 23:30 +0200"
> whereas we parse
> GIT_COMMITTER_DATE="01-06-2008 11:30pm" as "6th of January at 23:30 +0100"
> 
> Not only is it strange that we accept both orders (since apparently we
> don't resort to the latter order unless the first digit > 12),

Take a look at match_multi_number in date.c
European ordering is preferred when the separator is '.'

> but
> it's also weird that the timezone changes based on which option was
> chosen!

The timezone didn't change. One date is in daylight saving time, the other
is not. The offset from GMT is different to reflect this.

> What does the list think about this? Is this "normal"
> behavior? If not, what (if anything) should we do about it?

There will be flaws in any system that tries to automatically guess the
format. If there is not already one, perhaps a statement should be added
to the appropriate documentation describing how to ensure that european
ordering is used to parse the date.

-brandon

Re: Date parsing

[Sverre Rabbelier]

On Tue, Jun 10, 2008 at 5:10 PM, Brandon Casey <casey@nrlssc.navy.mil> wrote:
> Take a look at match_multi_number in date.c
> European ordering is preferred when the separator is '.'

Ok, then I'll use . in the future, that's nice :).

> The timezone didn't change. One date is in daylight saving time, the other
> is not. The offset from GMT is different to reflect this.

As Hannes said, no bug there, my mistake.

>> What does the list think about this? Is this "normal"
>> behavior? If not, what (if anything) should we do about it?
>
> There will be flaws in any system that tries to automatically guess the
> format. If there is not already one, perhaps a statement should be added
> to the appropriate documentation describing how to ensure that european
> ordering is used to parse the date.

I think that it should bail out when it encounters "20-01-2008"
instead of automagically going for european notation. Even more
helpfull would be to inform the user that "20.01.2008" is the proper
notation.

-- 
Cheers,

Sverre Rabbelier

Re: Date parsing

[Linus Torvalds]

On Tue, 10 Jun 2008, Sverre Rabbelier wrote:
> On Tue, Jun 10, 2008 at 5:10 PM, Brandon Casey <casey@nrlssc.navy.mil> wrote:
> > Take a look at match_multi_number in date.c
> > European ordering is preferred when the separator is '.'
> 
> Ok, then I'll use . in the future, that's nice :).

Well, there are safer ways to give the date.

If you do it in strict rfc822 format, you'll never have any confusion 
what-so-ever. The "approxidate()" thing tries to parse any random input, 
but it *is* meant to be excessively liberal.

IOW, you can literally say "at tea-time two weeks ago" and get a date, and 
it will even work. But you can also say "my 3rd child was born in 
December", and it will also give you a date. The date will not make 
_sense_, but it will give you one (it will decide that what you meant 
is "Dec 3rd").

So if you want to be precise and safe, you should be precise. I'd 
personally suggest using yyyy-mm-dd, which is the ISO date format, 
although if that fails approxidate will still try the admittedly 
crazy yyyy-dd-mm.

And always set the timezone explicitly if you really care. Again, we try 
out best if you don't explicitly say which timezone to use, but if you 
don't want any guessing - even _informed_ guessing - you really should 
state things explicitly.

> I think that it should bail out when it encounters "20-01-2008"
> instead of automagically going for european notation. Even more
> helpfull would be to inform the user that "20.01.2008" is the proper
> notation.

See above. git approxidate() tries the exact reverse: it's extremely 
willing to turn absolutely any line noise into a date.

Which is really nice when you do

	git log @{last.week}..

but if you actually want to state an exact date it really means that the 
onus is on _you_ to be exact, and use a well-defined standard format.

			Linus

Re: Date parsing

[Sverre Rabbelier]

On Tue, Jun 10, 2008 at 6:55 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Tue, 10 Jun 2008, Sverre Rabbelier wrote:
>> On Tue, Jun 10, 2008 at 5:10 PM, Brandon Casey <casey@nrlssc.navy.mil> wrote:
>> > Take a look at match_multi_number in date.c
>> > European ordering is preferred when the separator is '.'
>>
>> Ok, then I'll use . in the future, that's nice :).
>
> Well, there are safer ways to give the date.
>
> If you do it in strict rfc822 format, you'll never have any confusion
> what-so-ever. The "approxidate()" thing tries to parse any random input,
> but it *is* meant to be excessively liberal.
>
> IOW, you can literally say "at tea-time two weeks ago" and get a date, and
> it will even work. But you can also say "my 3rd child was born in
> December", and it will also give you a date. The date will not make
> _sense_, but it will give you one (it will decide that what you meant
> is "Dec 3rd").

Ok, so it'll eat anything it can make a date of, that makes sense I
guess, considering what you refer to below.

> So if you want to be precise and safe, you should be precise. I'd
> personally suggest using yyyy-mm-dd, which is the ISO date format,
> although if that fails approxidate will still try the admittedly
> crazy yyyy-dd-mm.
>
> And always set the timezone explicitly if you really care. Again, we try
> out best if you don't explicitly say which timezone to use, but if you
> don't want any guessing - even _informed_ guessing - you really should
> state things explicitly.

I overlooked the fact that in Januari there is no DST, so that my
local timezone would indeed be +1, so it handled it as desired.

>> I think that it should bail out when it encounters "20-01-2008"
>> instead of automagically going for european notation. Even more
>> helpfull would be to inform the user that "20.01.2008" is the proper
>> notation.
>
> See above. git approxidate() tries the exact reverse: it's extremely
> willing to turn absolutely any line noise into a date.
>
> Which is really nice when you do
>
>        git log @{last.week}..
>
> but if you actually want to state an exact date it really means that the
> onus is on _you_ to be exact, and use a well-defined standard format.

Thank you, this explained why it works the way it does very well. I'll
go with ISO notation in the future :).

-- 
Cheers,

Sverre Rabbelier

Re: Date parsing

[Stephan Beyer]

Hi,

> > On Tue, Jun 10, 2008 at 5:10 PM, Brandon Casey <casey@nrlssc.navy.mil> wrote:
> > > Take a look at match_multi_number in date.c
> > > European ordering is preferred when the separator is '.'
> > 
> > Ok, then I'll use . in the future, that's nice :).
> 
> Well, there are safer ways to give the date.
> 
> If you do it in strict rfc822 format, you'll never have any confusion 
> what-so-ever. The "approxidate()" thing tries to parse any random input, 
> but it *is* meant to be excessively liberal.

Today I've been playing around with approxidate(), too, and I think I
found some bug in date parsing. I let copy&paste speak...

Correct:

$ ./test-date "2008-07-01 23:59:59 +0200"
2008-07-01 23:59:59 +0200 -> 1214949599 +0200 -> Tue Jul  1 23:59:59 2008
2008-07-01 23:59:59 +0200 -> Tue Jul  1 23:59:59 2008

And even:

$ ./test-date "2008-07-01 24:00:00 +0200"
2008-07-01 24:00:00 +0200 -> 1214949600 +0200 -> Wed Jul  2 00:00:00 2008
2008-07-01 24:00:00 +0200 -> Wed Jul  2 00:00:00 2008

But then there's a jump in time:

$ ./test-date "2008-07-02 00:00:00 +0200"
2008-07-02 00:00:00 +0200 -> 1202335200 +0200 -> Wed Feb  6 23:00:00 2008
2008-07-02 00:00:00 +0200 -> Wed Feb  6 23:00:00 2008

$ ./test-date "2008-07-02 01:00:00 +0200"
2008-07-02 01:00:00 +0200 -> 1202338800 +0200 -> Thu Feb  7 00:00:00 2008
2008-07-02 01:00:00 +0200 -> Thu Feb  7 00:00:00 2008


If we let test-date just print the timestamp...

diff --git a/test-date.c b/test-date.c
index 62e8f23..18d53c1 100644
--- a/test-date.c
+++ b/test-date.c
@@ -11,10 +11,10 @@ int main(int argc, char **argv)
 		memcpy(result, "bad", 4);
 		parse_date(argv[i], result, sizeof(result));
 		t = strtoul(result, NULL, 0);
-		printf("%s -> %s -> %s", argv[i], result, ctime(&t));
+		printf("%s -> %lu\n", argv[i], t);
 
 		t = approxidate(argv[i]);
-		printf("%s -> %s\n", argv[i], ctime(&t));
+		printf("%s -> %lu\n", argv[i], t);
 	}
 	return 0;
 }
-- -- -- --

... then we get:

$ ./test-date "2008-07-01 23:59:59 +0200"
2008-07-01 23:59:59 +0200 -> 1214949599
2008-07-01 23:59:59 +0200 -> 1214949599

$ ./test-date "2008-07-02 00:00:00 +0200"
2008-07-02 00:00:00 +0200 -> 1202335200
2008-07-02 00:00:00 +0200 -> 1202335200

Also, with another timezone, we get:

$ ./test-date "2008-07-01 23:59:59 +0000"
2008-07-01 23:59:59 +0000 -> 1214956799
2008-07-01 23:59:59 +0000 -> 1214956799

$ ./test-date "2008-07-02 00:00:00 +0000"
2008-07-02 00:00:00 +0000 -> 1202342400
2008-07-02 00:00:00 +0000 -> 1202342400

Is something wrong in my format or is there a bug?

Providing timestamps works, of course ;-)

Regards,
  Stephan

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F

Re: Date parsing

[Linus Torvalds]

On Sat, 21 Jun 2008, Stephan Beyer wrote:
> 
> Today I've been playing around with approxidate(), too, and I think I
> found some bug in date parsing. I let copy&paste speak...
> 
> Correct:
> 
> $ ./test-date "2008-07-01 23:59:59 +0200"
> 2008-07-01 23:59:59 +0200 -> 1214949599 +0200 -> Tue Jul  1 23:59:59 2008
> 2008-07-01 23:59:59 +0200 -> Tue Jul  1 23:59:59 2008
> 
> And even:
> 
> $ ./test-date "2008-07-01 24:00:00 +0200"
> 2008-07-01 24:00:00 +0200 -> 1214949600 +0200 -> Wed Jul  2 00:00:00 2008
> 2008-07-01 24:00:00 +0200 -> Wed Jul  2 00:00:00 2008
> 
> But then there's a jump in time:
> 
> $ ./test-date "2008-07-02 00:00:00 +0200"
> 2008-07-02 00:00:00 +0200 -> 1202335200 +0200 -> Wed Feb  6 23:00:00 2008
> 2008-07-02 00:00:00 +0200 -> Wed Feb  6 23:00:00 2008

Heh.

What you're seeing is that approxidate() does not like dates in the 
future. You're hitting this case:

                /* Be it commit time or author time, it does not make
                 * sense to specify timestamp way into the future.  Make
                 * sure it is not later than ten days from now...
                 */
                if (now + 10*24*3600 < specified)
                        return 0;

so approxidate() refuses to think that "2008-07-02" is a valid date in 
July, because it is more than ten days in the future. So it decides that 
if somebody tried to feed it a date like that, it must be the seventh of 
February instead of July.

So the refusal to look at future dates is part of trying to disambiguate 
the "dd.mm" form from the "mm.dd" form, where it will decide that if it's 
far in the future (where "far" is 10 days), it cannot be right.

Remember: the git date handling was _not_ meant to be a generic date 
library. It is very much meant to be a *git* date library. This is why you 
can say things like "7 days ago", and it will return something sane, but 
if you say "7 days from now", it will still think you're talking about 
seven days ago - it simply doesn't have the concept of "future date".

That said, I think that in this case the thing just doesn't make sense. 
For the specific case of iso time format, we perhaps shouldn't even try to 
refuse future dates. Does anybody use the insane yyyy-dd-mm format? 

To avoid the future check, you could try something like the appended.

Of course, the whole parser was really designed to parse email dates from 
the beginning, and rfc2822 actually ends up being totally unambiguous and 
also won't ever hit the "refuse future" case.

So I was wrong. Rather than using the European ISO format (yyyy-mm-dd), 
the really safest format is the "English month name spelled out" format, 
ie

	./test-date "12 Jul 2010"

parses correctly, but

	./test-date 2010-07-12

does not, because the latter is found suspect due to being in the future.

		Linus

---
 date.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/date.c b/date.c
index 1a4eb87..870419a 100644
--- a/date.c
+++ b/date.c
@@ -374,7 +374,7 @@ static int match_multi_number(unsigned long num, char c, const char *date, char
 
 		if (num > 70) {
 			/* yyyy-mm-dd? */
-			if (is_date(num, num2, num3, refuse_future, now, tm))
+			if (is_date(num, num2, num3, NULL, now, tm))
 				break;
 			/* yyyy-dd-mm? */
 			if (is_date(num, num3, num2, refuse_future, now, tm))

Re: Date parsing

[Linus Torvalds]

On Sat, 21 Jun 2008, Linus Torvalds wrote:
> 
> Remember: the git date handling was _not_ meant to be a generic date 
> library. It is very much meant to be a *git* date library.

Btw, I'm not proud of that. The whole of "date.c" was a huge hack to 
overcome the fact that

 - standard library date parsing is pure and utter shit, even for standard 
   date formats.

 - Never mind the standard date formats, very few seem to do the relative 
   dateslike "10 days ago".

 - So I couldn't find anybody elses code to "borrow" that did a reasonable 
   job and handled the nice relative dates too.

The GNU "date" command has pretty good support for doing things like "5 
minutes ago", but I think I tried to look at the code, and quicky decided 
that it was not an option (when I say "I think I tried", I have to admit 
that I don't have strong memories from it, but I suspect it was all so 
utterly horrid that I repressed the episode).

I don't mind using other peoples code (the whole xdiff library and the 
SHA1 stuff was certainly all from outside), but it needs to be in a format 
that doesn't cause projectile vomiting.

If somebody has a good date parser, I think we could drop the git-specific 
one in an instant.

Or if somebody wants to just improve on it... Hint, hint.

		Linus

Re: Date parsing

[Stephan Beyer]

Hi,

On Sat, Jun 21, 2008 at 03:28:56PM -0700,
Linus Torvalds <torvalds@linux-foundation.org>:
[...]
>                 /* Be it commit time or author time, it does not make
>                  * sense to specify timestamp way into the future.  Make
>                  * sure it is not later than ten days from now...
>                  */
>                 if (now + 10*24*3600 < specified)
>                         return 0;
[...]

Great.
So it really turns out that this bug is a nifty feature. (Except if we receive
patches from the future with a numeric date format.)

Thanks for the explanation,
  Stephan

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F