[BUG] minor: wrong handling of GIT_AUTHOR_DATE

5 messages, 3 authors, 2016-06-15 · open the first message on its own page

[BUG] minor: wrong handling of GIT_AUTHOR_DATE

From: Hermann Gausterer <hidden>
Date: 2016-06-15 22:45:10

hi

i found a minor bug in the handling of the
environment variable GIT_AUTHOR_DATE

i used this variable to import an old project.

i used "stat" to get the timestamp of a file
and set the git history to this date with
this command:

GIT_AUTHOR_DATE=`stat -c '%y' "$FILE"`

old files (created with an older kernel)
produced this output.

2008-05-28 14:21:35.000000000 +0200

but new files return nanosecond resolution
timestamps.

2008-06-04 17:25:54.917476713 +0200

of course this resolution is NOT needed
for git, but git DOES NOT ignore this time-
stamps. it changes the date to something
completly wrong :-/

steps to reproduce:

$ git init
$ touch test
$ stat -c %y test
2008-08-16 22:25:45.491701924 +0200
$ export GIT_AUTHOR_DATE=`stat -c %y test`
$ git add test
$ git commit -a
$ git log
commit 56f92b8f6efc7bdaa5abdf03a8c5dbf79dd1fdff
Author: Hermann Gausterer [off-list ref]
Date:   Thu Aug 1 01:52:04 1985 +0200

    test
$

mfg hermann

Re: [BUG] minor: wrong handling of GIT_AUTHOR_DATE

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2016-06-15 22:45:10


On Sat, 16 Aug 2008, Hermann Gausterer wrote:
i used "stat" to get the timestamp of a file
and set the git history to this date with
this command:

GIT_AUTHOR_DATE=`stat -c '%y' "$FILE"`

old files (created with an older kernel)
produced this output.

2008-05-28 14:21:35.000000000 +0200

but new files return nanosecond resolution
timestamps.

2008-06-04 17:25:54.917476713 +0200

of course this resolution is NOT needed
for git, but git DOES NOT ignore this time-
stamps. it changes the date to something
completly wrong :-/
Git uses a fairly odd date parsing library, and it turns out that 
917476713 +0200 is actually a perfectly valid date in the git format, 
because one thing git allows is the "seconds since epoch" one. So doing

	[torvalds@nehalem git]$ ./test-date "917476713 +0200"
	917476713 +0200 -> 917476713 +0200 -> Wed Jan 27 14:38:33 1999
	917476713 +0200 -> Wed Jan 27 14:38:33 1999

and it turns out that git will totally ignore any other format date when i 
sees this standard format (yes, that is literally the format that git uses 
internally).

So because git date parsing doesn't even really understand fractional 
seconds, and thus doesn't parse it, it will take the fraction, and if it 
was larger than 100000000, it will assume it's a seconds-since-epoch date.

Unlucky.

Anyway, something like this should fix it.

Junio: we might also make the code that actually parses the 
seconds-per-epoch thing only trigger if we haven't already seen a date (ie 
it might check for "tm->tm_year < 0" etc before accepting that seconds 
format).

		Linus

---
Subject: Ignore fractional seconds in date parsing
From: Linus Torvalds <redacted>

.. otherwise a nanosecond resolution fractional second might be 
interpreted as a seconds-since-epoch date format string and overwrite the 
date we so carefully just parsed.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Noticed-by: Hermann Gausterer [off-list ref]
---
 date.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/date.c b/date.c
index 35a5257..5e502da 100644
--- a/date.c
+++ b/date.c
@@ -363,6 +363,11 @@ static int match_multi_number(unsigned long num, char c, const char *date, char
 			tm->tm_hour = num;
 			tm->tm_min = num2;
 			tm->tm_sec = num3;
+
+			/* Ignore any possible fractional seconds */
+			if (*end == '.')
+				(void) strtol(end+1, &end, 10);
+
 			break;
 		}
 		return 0;

Re: [BUG] minor: wrong handling of GIT_AUTHOR_DATE

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2016-06-15 22:45:10


On Sat, 16 Aug 2008, Linus Torvalds wrote:
Junio: we might also make the code that actually parses the 
seconds-per-epoch thing only trigger if we haven't already seen a date (ie 
it might check for "tm->tm_year < 0" etc before accepting that seconds 
format).
Here's a slightly expanded version of the previous patch, which will 
ignore those big integer if it has already seen any human-readable date 
format (either any time except 0:00:00 or any normal date).

It includes the fractional second parsing code from the previous patch 
too, since that's an independent thing and makes sense regardless.

Junio, your call. But this one gets the date right for strings that just 
randomly have some big number in them, ie

	[torvalds@nehalem git]$ ./test-date "17:25:54 917476713 2008-06-04 -0700"
	17:25:54 917476713 2008-06-04 -0700 -> 1212625554 -0700 -> Wed Jun  4 17:25:54 2008
	17:25:54 917476713 2008-06-04 -0700 -> Wed Jun  4 17:25:54 2008

because it will now see that "nodate()" is not true. I think it's a good 
idea to only accept the epoch format when there hasn't been any other time 
format visible.

			Linus

---
 date.c |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/date.c b/date.c
index 35a5257..e11e78e 100644
--- a/date.c
+++ b/date.c
@@ -363,6 +363,11 @@ static int match_multi_number(unsigned long num, char c, const char *date, char
 			tm->tm_hour = num;
 			tm->tm_min = num2;
 			tm->tm_sec = num3;
+
+			/* Ignore any possible fractional seconds */
+			if (*end == '.')
+				(void) strtol(end+1, &end, 10);
+
 			break;
 		}
 		return 0;
@@ -402,6 +407,15 @@ static int match_multi_number(unsigned long num, char c, const char *date, char
 	return end - date;
 }
 
+/* Have we filled in any part of the time/date yet? */
+static inline int nodate(struct tm *tm)
+{
+	return tm->tm_year < 0 &&
+		tm->tm_mon < 0 &&
+		tm->tm_mday < 0 &&
+		!(tm->tm_hour | tm->tm_min | tm->tm_sec);
+}
+
 /*
  * We've seen a digit. Time? Year? Date?
  */
@@ -418,7 +432,7 @@ static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt
 	 * more than 8 digits. This is because we don't want to rule out
 	 * numbers like 20070606 as a YYYYMMDD date.
 	 */
-	if (num >= 100000000) {
+	if (num >= 100000000 && nodate(tm)) {
 		time_t time = num;
 		if (gmtime_r(&time, tm)) {
 			*tm_gmt = 1;

Re: [BUG] minor: wrong handling of GIT_AUTHOR_DATE

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:45:10

Linus Torvalds [off-list ref] writes:
Junio, your call. But this one gets the date right for strings that just 
randomly have some big number in them, ie

	[torvalds@nehalem git]$ ./test-date "17:25:54 917476713 2008-06-04 -0700"
	17:25:54 917476713 2008-06-04 -0700 -> 1212625554 -0700 -> Wed Jun  4 17:25:54 2008
	17:25:54 917476713 2008-06-04 -0700 -> Wed Jun  4 17:25:54 2008
Being able to parse this is a very low priority.

You've taught people here and on the kernel list that the "date" can use
any non-digit-non-word as a word separator, and "git log --since 2.days"
is something you often do.

People who followed that advice would have gotten used to this already, e.g.

   $ git reflog delete master@{07.04.2005.15:15:00.-0700}

should not be broken.

I think your first hunk needs to distinguish between "very-long-precision
posint" (in which case we ignore because it is likely to be nanoseconds
fraction) and others.

Re: [BUG] minor: wrong handling of GIT_AUTHOR_DATE

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2016-06-15 22:45:10


On Sat, 16 Aug 2008, Junio C Hamano wrote:
People who followed that advice would have gotten used to this already, e.g.

   $ git reflog delete master@{07.04.2005.15:15:00.-0700}

should not be broken.
Hmm. Fair enough. In that case, just the "nodate()" approach is probably 
fine on its own. HOWEVER:
I think your first hunk needs to distinguish between "very-long-precision
posint" (in which case we ignore because it is likely to be nanoseconds
fraction) and others.
Well, that ignores nanosecond resolution seconds, but not microseconds, 
for example. Now, microseconds normally don't matter (because they won't 
trigger the 'seconds-since-epoch' case), but they _can_ trigger some other 
cases.

For example, let's assume that we have microseconds in the date specifier. 
Then try this one:

	./test-date "12:12:12.000001"

Notice what happens? Oops.

With my patch, you get

	12:12:12.0000001 -> Sat Aug 16 12:12:12 2008

and with your, you get

	12:12:12.000001 -> Fri Aug  1 12:12:12 2008

and yeah, it's odd, but I can explain it.

But you are definitely right about the case of doing

	"15:15:00.-0700"

and yes, my patch was crap too. 

			Linus
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help