Trying to use AUTHOR_DATE

Trying to use AUTHOR_DATE

[tony.luck]

I'm using cogito-0.8 (036bb73c6dd1871101ca19557298684ab9832f81) and trying
to set AUTHOR_DATE based on the "Date:" from the patch e-mail.  But, it
appears that commit-tree is munging this based on my timezone.

Here's what I set in the environment before invoking cg-commit:

  AUTHOR_DATE="29 Apr 2005 02:02:00 -0700"

and here's what cg-log reports on the "author" line:

  Fri, 29 Apr 2005 10:02:00 -0700

My /etc/localtime is set for "US/Pacific" ... which is where the 8 hours
comes from (I think).  If I set "TZ=GMT0BST" as well in the environment of
cg-commit to override /etc/localtime, then the author time comes out ok,
but then the "committer" time gets messed up.

-Tony

Re: Trying to use AUTHOR_DATE

[H. Peter Anvin]

tony.luck@intel.com wrote:
> I'm using cogito-0.8 (036bb73c6dd1871101ca19557298684ab9832f81) and trying
> to set AUTHOR_DATE based on the "Date:" from the patch e-mail.  But, it
> appears that commit-tree is munging this based on my timezone.
> 
> Here's what I set in the environment before invoking cg-commit:
> 
>   AUTHOR_DATE="29 Apr 2005 02:02:00 -0700"
> 
> and here's what cg-log reports on the "author" line:
> 
>   Fri, 29 Apr 2005 10:02:00 -0700
> 
> My /etc/localtime is set for "US/Pacific" ... which is where the 8 hours
> comes from (I think).  If I set "TZ=GMT0BST" as well in the environment of
> cg-commit to override /etc/localtime, then the author time comes out ok,
> but then the "committer" time gets messed up.
> 

There was a time-parsing bug somewhere, where mktime() got invoked on a 
UTC date.  I proposed changing it to curl_gettime() instead.

	-hpa

Re: Trying to use AUTHOR_DATE

[tony.luck]

> There was a time-parsing bug somewhere, where mktime() got invoked on a 
> UTC date.  I proposed changing it to curl_gettime() instead.

Here's a patch to switch to using curl_getdate():

Signed-off-by: Tony Luck <tony.luck@intel.com>

---

 Makefile      |    1 
 commit-tree.c |  143 ++++------------------------------------------------------
 2 files changed, 11 insertions(+), 133 deletions(-)

Makefile: d73bea1cbb9451a89b03d6066bf2ed7fec32fd31
--- k/Makefile
+++ l/Makefile
@@ -92,6 +92,7 @@ $(LIB_FILE): $(LIB_OBJS)
 rpush: rsh.c
 rpull: rsh.c
 http-pull: LIBS += -lcurl
+commit-tree: LIBS += -lcurl
 
 
 ifneq (,$(wildcard .git))
commit-tree.c: 23de13361944ad7ba7c5320cf7cdd04e81842c60
--- k/commit-tree.c
+++ l/commit-tree.c
@@ -10,6 +10,7 @@
 #include <string.h>
 #include <ctype.h>
 #include <time.h>
+#include <curl/curl.h>
 
 #define BLOCKING (1ul << 14)
 
@@ -80,146 +81,22 @@ static void remove_special(char *p)
 	}
 }
 
-static const char *month_names[] = {
-        "Jan", "Feb", "Mar", "Apr", "May", "Jun",
-        "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"
-};
-
-static const char *weekday_names[] = {
-        "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"
-};
-
-
-static char *skipfws(char *str)
-{
-	while (isspace(*str))
-		str++;
-	return str;
-}
-
-	
 /* Gr. strptime is crap for this; it doesn't have a way to require RFC2822
    (i.e. English) day/month names, and it doesn't work correctly with %z. */
-static void parse_rfc2822_date(char *date, char *result, int maxlen)
+static void parse_date(char *date, time_t *now, char *result, int maxlen)
 {
-	struct tm tm;
 	char *p;
-	int i, offset;
 	time_t then;
 
-	memset(&tm, 0, sizeof(tm));
-
-	/* Skip day-name */
-	p = skipfws(date);
-	if (!isdigit(*p)) {
-		for (i=0; i<7; i++) {
-			if (!strncmp(p,weekday_names[i],3) && p[3] == ',') {
-				p = skipfws(p+4);
-				goto day;
-			}
-		}
-		return;
-	}					
-
-	/* day */
- day:
-	tm.tm_mday = strtoul(p, &p, 10);
-
-	if (tm.tm_mday < 1 || tm.tm_mday > 31)
-		return;
-
-	if (!isspace(*p))
-		return;
-
-	p = skipfws(p);
-
-	/* month */
-
-	for (i=0; i<12; i++) {
-		if (!strncmp(p, month_names[i], 3) && isspace(p[3])) {
-			tm.tm_mon = i;
-			p = skipfws(p+strlen(month_names[i]));
-			goto year;
-		}
-	}
-	return; /* Error -- bad month */
-
-	/* year */
- year:	
-	tm.tm_year = strtoul(p, &p, 10);
-
-	if (!tm.tm_year && !isspace(*p))
-		return;
-
-	if (tm.tm_year > 1900)
-		tm.tm_year -= 1900;
-		
-	p=skipfws(p);
-
-	/* hour */
-	if (!isdigit(*p))
-		return;
-	tm.tm_hour = strtoul(p, &p, 10);
-	
-	if (!tm.tm_hour > 23)
-		return;
-
-	if (*p != ':')
-		return; /* Error -- bad time */
-	p++;
-
-	/* minute */
-	if (!isdigit(*p))
-		return;
-	tm.tm_min = strtoul(p, &p, 10);
-	
-	if (!tm.tm_min > 59)
+	if ((then = curl_getdate(date, now)) == 0)
 		return;
 
-	if (isspace(*p))
-		goto zone;
-
-	if (*p != ':')
-		return; /* Error -- bad time */
-	p++;
-
-	/* second */
-	if (!isdigit(*p))
-		return;
-	tm.tm_sec = strtoul(p, &p, 10);
-	
-	if (!tm.tm_sec > 59)
-		return;
-
-	if (!isspace(*p))
-		return;
-
- zone:
-	p = skipfws(p);
-
-	if (*p == '-')
-		offset = -60;
-	else if (*p == '+')
-		offset = 60;
-	else
-	       return;
-
-	if (!isdigit(p[1]) || !isdigit(p[2]) || !isdigit(p[3]) || !isdigit(p[4]))
-		return;
-
-	i = strtoul(p+1, NULL, 10);
-	offset *= ((i % 100) + ((i / 100) * 60));
-
-	if (*(skipfws(p + 5)))
-		return;
-
-	then = mktime(&tm); /* mktime appears to ignore the GMT offset, stupidly */
-	if (then == -1)
-		return;
-
-	then -= offset;
-
-	snprintf(result, maxlen, "%lu %5.5s", then, p);
+	/* find the timezone at the end */
+	p = date + strlen(date);
+	while (p > date && isdigit(*--p))
+		;
+	if ((*p == '+' || *p == '-') && strlen(p) == 5)
+		snprintf(result, maxlen, "%lu %5.5s", then, p);
 }
 
 static void check_valid(unsigned char *sha1, const char *expect)
@@ -298,7 +175,7 @@ int main(int argc, char **argv)
 	email = getenv("AUTHOR_EMAIL") ? : realemail;
 	audate = getenv("AUTHOR_DATE");
 	if (audate)
-		parse_rfc2822_date(audate, date, sizeof(date));
+		parse_date(audate, &now, date, sizeof(date));
 
 	remove_special(gecos); remove_special(realgecos); remove_special(commitgecos);
 	remove_special(email); remove_special(realemail); remove_special(commitemail);

Re: Trying to use AUTHOR_DATE

[Edgar Toernig]

tony.luck@intel.com wrote:
>
> > There was a time-parsing bug somewhere, where mktime() got invoked on a 
> > UTC date.  I proposed changing it to curl_gettime() instead.
> 
> Here's a patch to switch to using curl_getdate():

Another dependency :-(   I can live without http-pull but not
without commit-tree.

What's wrong with the patch I sent to fix this:

	http://marc.theaimsgroup.com/?m=111446501003389

> +	/* find the timezone at the end */
> +	p = date + strlen(date);
> +	while (p > date && isdigit(*--p))
> +		;
> +	if ((*p == '+' || *p == '-') && strlen(p) == 5)
> +		snprintf(result, maxlen, "%lu %5.5s", then, p);

This will choke on dates from Linus which have a trailing comment:

	Date: Fri, 29 Apr 2005 15:26:14 -0700 (PDT)

Ciao, ET.

Re: Trying to use AUTHOR_DATE

[H. Peter Anvin]

Edgar Toernig wrote:
> 
> Another dependency :-(   I can live without http-pull but not
> without commit-tree.
> 

Then feel free to rip curl_getdate out of the libcurl sources and making 
them standalone.

	-hpa

RE: Trying to use AUTHOR_DATE

[Luck, Tony]

>Another dependency :-(   I can live without http-pull but not
>without commit-tree.

Yes, the extra dependency sucks ... libcurl is missing from one of
the systems that I'd like to use GIT on ... so I'd prefer a solution
that doesn't involve libcurl.

>What's wrong with the patch I sent to fix this:
>
>	http://marc.theaimsgroup.com/?m=111446501003389
>

I missed it ... there is a problem that you drop the timezone.  When I
used this patch, I ended up with a commit that said:

author Keith Owens <kaos@sgi.com> 1114239900
committer Tony Luck <tony.luck@intel.com> 1114832076 -0700

See the missing timezone on the author line :-)  This is most upsetting
to cg-log.  It prints "expr: syntax error" and then

author Keith Owens <kaos@sgi.com> Thu, 01 Jan 1970 00:00:01

>> +	/* find the timezone at the end */
>> +	p = date + strlen(date);
>> +	while (p > date && isdigit(*--p))
>> +		;
>> +	if ((*p == '+' || *p == '-') && strlen(p) == 5)
>> +		snprintf(result, maxlen, "%lu %5.5s", then, p);
>
>This will choke on dates from Linus which have a trailing comment:
>
>	Date: Fri, 29 Apr 2005 15:26:14 -0700 (PDT)

You are right ... that's what comes from only looking at one e-mail
message to determine that pattern to match :-)

I'd much rather see your version fixed up to preserve the timezone
than have the libcurl dependency.

-Tony

Re: Trying to use AUTHOR_DATE

[H. Peter Anvin]

Luck, Tony wrote:
>>Another dependency :-(   I can live without http-pull but not
>>without commit-tree.
> 
> Yes, the extra dependency sucks ... libcurl is missing from one of
> the systems that I'd like to use GIT on ... so I'd prefer a solution
> that doesn't involve libcurl.

...

> I'd much rather see your version fixed up to preserve the timezone
> than have the libcurl dependency.

For gawd's sake people, just grab a copy of the working code in libcurl, 
and turn it into a standalone .c file.  It'll even let you merge in 
future fixes, and you could even use autoconf to use libcurl or the 
standalone code depending on what's available.

	-hpa

Re: Trying to use AUTHOR_DATE

[Linus Torvalds]

On Fri, 29 Apr 2005, H. Peter Anvin wrote:
> 
> For gawd's sake people, just grab a copy of the working code in libcurl, 
> and turn it into a standalone .c file.  It'll even let you merge in 
> future fixes, and you could even use autoconf to use libcurl or the 
> standalone code depending on what's available.

I'll happily depend on libcurl, but I put my foot down on that tool of the 
devil called "autoconf".

Any package that starts using autoconf eventually becomes a total mess. 
Don't do it. 

		Linus

Re: Trying to use AUTHOR_DATE

[Linus Torvalds]

On Fri, 29 Apr 2005, Linus Torvalds wrote:
> 
> I'll happily depend on libcurl, but I put my foot down on that tool of the 
> devil called "autoconf".

Btw, looking at curl's "getdate.c", it doesn't seem to be _that_ much more 
different from the date parsing we used to have. In particular, it 
actually uses "mktime()" twice and subtracts out the difference.

It also seems to do so in a particularly stupid way, and David Woodhouses 
suggestion of just using mktime() on Jan 1st, 1970, seems to be much 
simpler than what curl does.

(Actually, it might make sense to modify David's version to use "Jan 2nd,
1970" and subtract 24 hours, in case some mktime() implementation decides
that underflow is a problem...)

Of course, I think we might as well go with Edgars version after all. 

Edgar, willing to create a separate "parse-date.c" with your "my_mktime()" 
thing and move the old date parsing there? That way we'll just use that 
instead of libcurl..

		Linus

Re: Trying to use AUTHOR_DATE

[Russ Allbery]

Linus Torvalds <torvalds@osdl.org> writes:

> It also seems to do so in a particularly stupid way, and David
> Woodhouses suggestion of just using mktime() on Jan 1st, 1970, seems to
> be much simpler than what curl does.

Because of daylight savings time, this doesn't actually work.  I know from
personal experience; this is the tactic that I took at first when writing
INN's date parser and was educated by test failures.

-- 
Russ Allbery (rra@stanford.edu)             <http://www.eyrie.org/~eagle/>

Re: Trying to use AUTHOR_DATE

[David Woodhouse]

On Fri, 2005-04-29 at 21:32 -0700, Russ Allbery wrote:
> Linus Torvalds <torvalds@osdl.org> writes:
> > It also seems to do so in a particularly stupid way, and David
> > Woodhouses suggestion of just using mktime() on Jan 1st, 1970, seems to
> > be much simpler than what curl does.
> 
> Because of daylight savings time, this doesn't actually work.  I know from
> personal experience; this is the tactic that I took at first when writing
> INN's date parser and was educated by test failures.

Eww. The time functions we have to play with _really_ suck, don't they?
How about this...

Signed-off-by: David Woodhouse <dwmw2@infradead.org>

commit-tree.c: needs update
Index: commit-tree.c
===================================================================
--- c3aa1e6b53cc59d5fbe261f3f859584904ae3a63/commit-tree.c  (mode:100644 sha1:23de13361944ad7ba7c5320cf7cdd04e81842c60)
+++ uncommitted/commit-tree.c  (mode:100644)
@@ -213,10 +213,18 @@
 	if (*(skipfws(p + 5)))
 		return;
 
-	then = mktime(&tm); /* mktime appears to ignore the GMT offset, stupidly */
+	tm.tm_gmtoff = 0;
+	tm.tm_isdst = -1;
+
+	then = mktime(&tm);
 	if (then == -1)
 		return;
 
+	/* mktime always uses localtime, regardless of the tm_gmtoff field.
+	   It does, however, honour 'tm_isdst'; stupidly. Thankfully, it does
+	   at least tell us the offset it decided to use, so we can compensate
+	   for it */
+	then += tm.tm_gmtoff;
 	then -= offset;
 
 	snprintf(result, maxlen, "%lu %5.5s", then, p);

-- 
dwmw2

Re: Trying to use AUTHOR_DATE

[Edgar Toernig]

David Woodhouse wrote:
>
> Eww. The time functions we have to play with _really_ suck, don't they?
> How about this...
> 
> +	then += tm.tm_gmtoff;

tm_gmtoff is not available everywhere - POSIX doesn't even mention it (BSD?).

Oh btw, when we are about sucking time functions: the %s and %z strftime-
sequences used further down are also non-standard (POSIX has no %s, old
libc has neither %s nor %z).

A possible workaround:

void make_datestamp(char *buf)
{
	time_t now;
	struct tm *tm;
	int tz;

	time(&now);

	tm = localtime(&now); /* get timezone and tm_isdst */
	tz = -timezone / 60;
	if (tm->tm_isdst > 0)
		tz += 60;

	sprintf(buf, "%lu %+05d", now, tz/60*100+tz%60);
}

That *should* work on any POSIX system but who knows ...

Ciao, ET.

Re: Trying to use AUTHOR_DATE

[Russ Allbery]

Edgar Toernig <froese@gmx.de> writes:

> Oh btw, when we are about sucking time functions: the %s and %z
> strftime- sequences used further down are also non-standard (POSIX has
> no %s, old libc has neither %s nor %z).

> A possible workaround:

[...]

> 	tm = localtime(&now); /* get timezone and tm_isdst */
> 	tz = -timezone / 60;
> 	if (tm->tm_isdst > 0)
> 		tz += 60;

The global timezone variable isn't available on all systems.  :)

You really cannot get portable behavior in this area without something
akin to Autoconf probes, unfortunately.  Oh, and you can't assume daylight
savings time is an hour; it is sometimes two hours.  You have to instead
use the altzone variable to get the offset when you're in daylight savings
time, but this again isn't available on all systems.

I posted a pointer to the INN source a while back; I'm really not sure
that anything less is sufficient to get full portability, although I
certainly trust Paul Eggart's implementation.

BTW, the yacc-based thing is exactly what I wrote the INN code to get rid
of, since I didn't want a yacc dependency.

-- 
Russ Allbery (rra@stanford.edu)             <http://www.eyrie.org/~eagle/>

Re: Trying to use AUTHOR_DATE

[Linus Torvalds]

On Sat, 30 Apr 2005, Russ Allbery wrote:
> 
> You really cannot get portable behavior in this area without something
> akin to Autoconf probes, unfortunately. 

Ok, since this only really matters for AUTHOR_DATE, which we pass in as a
random string anyway, and which comes from various mail programs which may
or may not follow all RFC's, I just rewrote it to give "almost correct 
results" for "pretty much any crap you throw at it".

As a test-bed, a "test-date" program that parses a date and then prints 
it out in git format _and_ in the local timezone format, here's a few 
examples:

	./test-date "$(date)" "April 4th, 1992 at 13:45" "13:04:09 +0100 2004 Yesterday, Friday 13th, December"

results in

	Sat Apr 30 13:26:52 PDT 2005 -> 1114892812 -0700 -> Sat Apr 30 13:26:52 2005

	April 4th, 1992 at 13:45 -> 702395100 +0000 -> Sat Apr  4 05:45:00 1992

	13:04:09 +0100 2004 Yesterday, Friday 13th, December -> 1102939449 +0100 -> Mon Dec 13 04:04:09 2004

which is just because it really doesn't check a hell of a lot.

For example, if you say

	"I caught 14 fishes in December 1998"

test-date will happily parse this as

	Sun Dec 13 16:00:00 1998

(That's "0:00:00 Dec 14th, 1998 UTC" shown in the local timezone ;). Or:

	./test-date  "12:15 4/17/2009"

	12:15 4/17/2009 -> 1239970500 +0000 -> Fri Apr 17 05:15:00 2009

ie it just greedily tries to make _some_ sense of the random strings you 
throw at it.

It doesn't even try getting timezones right - it doesn't know about 
summertime or anything. Besides, I probably used the wrong timezone info 
anyway.

I'll probably tweak it a bit more (make "no timezone means local 
timezone", for example, rather than UTC like it is now).

		Linus

Re: Trying to use AUTHOR_DATE

[Juliusz Chroboczek]

Hi,

Here's the code I'm using in darcs-git (copied from Polipo, another
project of mine).  You're welcome to use it in any way you see fit.

sprintf_a is defined as strdup of sprintf.

                                        Juliusz
#if defined __GLIBC__
#define HAVE_TM_GMTOFF
#define HAVE_SETENV
#ifndef __UCLIBC__
#define HAVE_TIMEGM
#endif
#endif

#if defined(__linux__) && (__GNU_LIBRARY__ == 1)
/* Linux libc 5 */
#define HAVE_TIMEGM
#define HAVE_SETENV
#endif

#ifdef BSD
#define HAVE_TM_GMTOFF
#define HAVE_SETENV
#endif

#ifdef __CYGWIN__
#define HAVE_SETENV
#endif

#if _POSIX_VERSION >= 200112L
#define HAVE_SETENV
#endif

#define HAVE_TZSET

/* Like mktime(3), but UTC rather than local time */
#if defined(HAVE_TIMEGM)
time_t
mktime_gmt(struct tm *tm)
{
    return timegm(tm);
}
#elif defined(HAVE_TM_GMTOFF)
time_t
mktime_gmt(struct tm *tm)
{
    time_t t;
    struct tm *ltm;

    t = mktime(tm);
    if(t < 0)
        return -1;
    ltm = localtime(&t);
    if(ltm == NULL)
        return -1;
    return t + ltm->tm_gmtoff;
}
#elif defined(HAVE_TZSET)
#ifdef HAVE_SETENV
/* Taken from the Linux timegm(3) man page. */
time_t
mktime_gmt(struct tm *tm)
{
    time_t t;
    char *tz;

    tz = getenv("TZ");
    setenv("TZ", "", 1);
    tzset();
    t = mktime(tm);
    if(tz)
        setenv("TZ", tz, 1);
    else
        unsetenv("TZ");
    tzset();
    return t;
}
#else
time_t
mktime_gmt(struct tm *tm)
{
    time_t t;
    char *tz;
    static char *old_tz = NULL;

    tz = getenv("TZ");
    putenv("TZ=");
    tzset();
    t = mktime(tm);
    if(old_tz)
        free(old_tz);
    if(tz)
        old_tz = sprintf_a("TZ=%s", tz);
    else
        old_tz = strdup("TZ");  /* XXX - non-portable? */
    if(old_tz)
        putenv(old_tz);
    tzset();
    return t;
}
#endif
#else
#error no mktime_gmt implementation on this platform
#endif

Re: Trying to use AUTHOR_DATE

[Edgar Toernig]

Linus Torvalds wrote:
>
> [...] I just rewrote it to give "almost correct 
> results" for "pretty much any crap you throw at it".

And I had the impression the strict checks in the original
version were intentionally ;-)

> I'll probably tweak it a bit more (make "no timezone means local 
> timezone", for example, rather than UTC like it is now).

Here's my try on that.  But whether it works everywhere ...

Btw, your %+03d%02d printf gave wrong results for i.e. -0130 (-01-30).



--- k/date.c  (mode:100644)
+++ l/date.c  (mode:100644)
@@ -10,7 +10,9 @@
 #include <ctype.h>
 #include <time.h>
 
-static time_t my_mktime(struct tm *tm)
+#define NO_TZ	11111
+
+static time_t utc_mktime(struct tm *tm)
 {
 	static const int mdays[] = {
 	    0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334
@@ -23,12 +25,19 @@ static time_t my_mktime(struct tm *tm)
 		return -1;
 	if (month < 0 || month > 11) /* array bounds */
 		return -1;
+	if (day < 1 || day > 31)
+		return -1;
 	if (month < 2 || (year + 2) % 4)
 		day--;
 	return (year * 365 + (year + 1) / 4 + mdays[month] + day) * 24*60*60UL +
 		tm->tm_hour * 60*60 + tm->tm_min * 60 + tm->tm_sec;
 }
 
+static int local_offset(time_t *when)
+{
+	return (utc_mktime(localtime(when)) - *when) / 60;
+}
+
 static const char *month_names[] = {
 	"January", "February", "March", "April", "May", "June",
 	"July", "August", "September", "October", "November", "December"
@@ -138,7 +147,8 @@ static int match_alpha(const char *date,
 	for (i = 0; i < NR_TZ; i++) {
 		int match = match_string(date, timezone_names[i].name);
 		if (match >= 3) {
-			*offset = 60*timezone_names[i].offset;
+			if (*offset == NO_TZ)
+				*offset = 60*timezone_names[i].offset;
 			return match;
 		}
 	}
@@ -245,7 +255,7 @@ void parse_date(char *date, char *result
 	tm.tm_year = -1;
 	tm.tm_mon = -1;
 	tm.tm_mday = -1;
-	offset = 0;
+	offset = NO_TZ;
 
 	for (;;) {
 		int match = 0;
@@ -270,13 +280,20 @@ void parse_date(char *date, char *result
 		date += match;
 	}
 
-	then = my_mktime(&tm); /* mktime uses local timezone */
-	if (then == -1)
-		return;
-
-	then -= offset * 60;
+	if (offset == NO_TZ) {
+		tm.tm_isdst = -1;
+		then = mktime(&tm);
+		if (then == -1)
+			return;
+		offset = local_offset(&then);
+	} else {
+		then = utc_mktime(&tm);
+		if (then == -1)
+			return;
+		then -= offset * 60;
+	}
 
-	snprintf(result, maxlen, "%lu %+03d%02d", then, offset/60, offset % 60);
+	snprintf(result, maxlen, "%lu %+05d", then, offset/60*100 + offset%60);
 }
 
 void datestamp(char *buf, int bufsize)
@@ -285,9 +302,7 @@ void datestamp(char *buf, int bufsize)
 	int offset;
 
 	time(&now);
-
-	offset = my_mktime(localtime(&now)) - now;
-	offset /= 60;
+	offset = local_offset(&now);
 
 	snprintf(buf, bufsize, "%lu %+05d", now, offset/60*100 + offset%60);
 }

Ciao, ET.

Re: Trying to use AUTHOR_DATE

[Linus Torvalds]

On Sun, 1 May 2005, Edgar Toernig wrote:
> 
> > I'll probably tweak it a bit more (make "no timezone means local 
> > timezone", for example, rather than UTC like it is now).
> 
> Here's my try on that.  But whether it works everywhere ...

I already did that part.

> Btw, your %+03d%02d printf gave wrong results for i.e. -0130 (-01-30).

Dang. Oh, well. That's what I get for not testing.

I don't much like that "%+05s" trick either, since that one also depends 
on the direction of rounding for negative division (it just gets it right 
for the normal case, and I guess C90 finally specified it precisely).

Let's just do unsigned arithmetic and check the sign specially. I already 
did that on input, just not on output.

		Linus

Re: Trying to use AUTHOR_DATE

[Linus Torvalds]

On Sun, 1 May 2005, Edgar Toernig wrote:
> 
> And I had the impression the strict checks in the original
> version were intentionally ;-)

Btw, here's my test of every single email in my email archive (which is 
not that big any more - after the SCO subpoena, I decided that I never 
want to go through with that kind of crap ever again, so now it's only a 
month or two of things). 

Almost everything seems to follow the RFC's or at least be close enough
that my "accept anything" ends up doing something sane, except for three
emails:

	Date: Fri, 08 Apr 2005 02:20:10 0200 -> bad
	Date: Mon, 18 Apr 05 15:05:29 Hora oficial do Brasil -> bad
	Date: 2002/04/11 18:29:07 -> bad

That first one doesn't have a sign in front of the timezone (I'll fix
things up - right now I end up believing that it's "year 200"), and the
third one has the sane European date order that sorts nicely (and which
I'll also fix up).

The second one is funny. Not just the "Hora oficial do Brasil" (hey, I 
could add it as a real timezone and my parser would do the right thing ;) 
but also because my parser decides that "05" is not a year, but the day in 
the month, so it doesn't see the year.

I can fake out that year thing pretty easily ("if it starts with '0' it's 
not a day of the month"), but it does show just how _strange_ stuff 
there's out there.

("Hora" is also Swedish for "whore", so that timezone does end up being
mentally parsed _quite_ the wrong way for somebody like me who doesn't
speak spanish).

			Linus

Re: Trying to use AUTHOR_DATE

[Randy.Dunlap]

On Sun, 1 May 2005 09:46:52 -0700 (PDT) Linus Torvalds wrote:

| 
| 
| On Sun, 1 May 2005, Edgar Toernig wrote:
| > 
| > And I had the impression the strict checks in the original
| > version were intentionally ;-)
| 
| Btw, here's my test of every single email in my email archive (which is 
| not that big any more - after the SCO subpoena, I decided that I never 
| want to go through with that kind of crap ever again, so now it's only a 
| month or two of things). 
| 
| Almost everything seems to follow the RFC's or at least be close enough
| that my "accept anything" ends up doing something sane, except for three
| emails:
| 
| 	Date: Fri, 08 Apr 2005 02:20:10 0200 -> bad
| 	Date: Mon, 18 Apr 05 15:05:29 Hora oficial do Brasil -> bad
| 	Date: 2002/04/11 18:29:07 -> bad
| 
| That first one doesn't have a sign in front of the timezone (I'll fix
| things up - right now I end up believing that it's "year 200"), and the
| third one has the sane European date order that sorts nicely (and which
| I'll also fix up).

Third one is almost ISO 8601 standard date format, except that
ISO uses hyphens, e.g., 2002-04-11, so I hope that the
punctation is a little flexible...

| The second one is funny. Not just the "Hora oficial do Brasil" (hey, I 
| could add it as a real timezone and my parser would do the right thing ;) 
| but also because my parser decides that "05" is not a year, but the day in 
| the month, so it doesn't see the year.
| 
| I can fake out that year thing pretty easily ("if it starts with '0' it's 
| not a day of the month"), but it does show just how _strange_ stuff 
| there's out there.
| 
| ("Hora" is also Swedish for "whore", so that timezone does end up being
| mentally parsed _quite_ the wrong way for somebody like me who doesn't
| speak spanish).


---
~Randy

Re: Trying to use AUTHOR_DATE

[Edgar Toernig]

Linus Torvalds wrote:
>
> 	Date: Fri, 08 Apr 2005 02:20:10 0200 -> bad
> 	Date: Mon, 18 Apr 05 15:05:29 Hora oficial do Brasil -> bad
> 	Date: 2002/04/11 18:29:07 -> bad
> 
> The second one is funny. Not just the "Hora oficial do Brasil" (hey, I 
> could add it as a real timezone and my parser would do the right thing ;) 
> but also because my parser decides that "05" is not a year, but the day in 
> the month, so it doesn't see the year.
> 
> I can fake out that year thing pretty easily ("if it starts with '0' it's 
> not a day of the month"), but it does show just how _strange_ stuff 
> there's out there.

And what happens then with the first example?  2008 Apr 2005?


I thought about missing timezones once more.  Don't you think it's
better to default to -0000?  Afaics, it was defined for just these
cases.  Simply appending an arbitrary timezone seems wrong.

Ciao, ET.

Re: Trying to use AUTHOR_DATE

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> On Fri, 29 Apr 2005, Linus Torvalds wrote:
>> 
>> I'll happily depend on libcurl, but I put my foot down on that tool of the 
>> devil called "autoconf".

LT> Btw, looking at curl's "getdate.c", it doesn't seem to be _that_ much more 
LT> different from the date parsing we used to have. In particular, it 
LT> actually uses "mktime()" twice and subtracts out the difference.

If we are going to lift code from somewhere, why don't we steal
from a pro who knows what he is doing?  

One careful implementation of my-mktime() I know of is the one
by Paul Eggert, found in patch (maketime.c::tm2time).

Re: Trying to use AUTHOR_DATE

[Edgar Toernig]

Linus Torvalds wrote:
>
> Edgar, willing to create a separate "parse-date.c" with your "my_mktime()" 
> thing and move the old date parsing there? That way we'll just use that 
> instead of libcurl..

Here it is.  I moved the strftime stuff too (workaround for non-standard
%s %z sequence).

--- k/Makefile  (mode:100644)
+++ l/Makefile  (mode:100644)
@@ -28,7 +28,8 @@ all: $(PROG)
 install: $(PROG) $(SCRIPTS)
 	install $(PROG) $(SCRIPTS) $(HOME)/bin/
 
-LIB_OBJS=read-cache.o sha1_file.o usage.o object.o commit.o tree.o blob.o tag.o
+LIB_OBJS=read-cache.o sha1_file.o usage.o object.o commit.o tree.o blob.o \
+	 tag.o date.o
 LIB_FILE=libgit.a
 LIB_H=cache.h object.h blob.h tree.h commit.h tag.h
 
--- k/cache.h  (mode:100644)
+++ l/cache.h  (mode:100644)
@@ -147,6 +160,9 @@ extern void *read_object_with_reference(
 					const unsigned char *required_type,
 					unsigned long *size,
 					unsigned char *sha1_ret);
+
+void parse_date(char *date, char *buf, int bufsize);
+void datestamp(char *buf, int bufsize);
 
 static inline void *xmalloc(int size)
 {
--- k/commit-tree.c  (mode:100644)
+++ l/commit-tree.c  (mode:100644)
@@ -10,7 +10,6 @@
 #include <string.h>
 #include <ctype.h>
 #include <time.h>
-#include <curl/curl.h>
 
 #define BLOCKING (1ul << 14)
 
@@ -81,24 +80,6 @@ static void remove_special(char *p)
 	}
 }
 
-/* Gr. strptime is crap for this; it doesn't have a way to require RFC2822
-   (i.e. English) day/month names, and it doesn't work correctly with %z. */
-static void parse_date(char *date, time_t *now, char *result, int maxlen)
-{
-	char *p;
-	time_t then;
-
-	if ((then = curl_getdate(date, now)) == 0)
-		return;
-
-	/* find the timezone at the end */
-	p = date + strlen(date);
-	while (p > date && isdigit(*--p))
-		;
-	if ((*p == '+' || *p == '-') && strlen(p) == 5)
-		snprintf(result, maxlen, "%lu %5.5s", then, p);
-}
-
 static void check_valid(unsigned char *sha1, const char *expect)
 {
 	void *buf;
@@ -132,8 +113,6 @@ int main(int argc, char **argv)
 	char *audate;
 	char comment[1000];
 	struct passwd *pw;
-	time_t now;
-	struct tm *tm;
 	char *buffer;
 	unsigned int size;
 
@@ -163,10 +142,8 @@ int main(int argc, char **argv)
 		strcat(realemail, ".");
 		getdomainname(realemail+strlen(realemail), sizeof(realemail)-strlen(realemail)-1);
 	}
-	time(&now);
-	tm = localtime(&now);
 
-	strftime(realdate, sizeof(realdate), "%s %z", tm);
+	datestamp(realdate, sizeof(realdate));
 	strcpy(date, realdate);
 
 	commitgecos = getenv("COMMIT_AUTHOR_NAME") ? : realgecos;
@@ -175,7 +152,7 @@ int main(int argc, char **argv)
 	email = getenv("AUTHOR_EMAIL") ? : realemail;
 	audate = getenv("AUTHOR_DATE");
 	if (audate)
-		parse_date(audate, &now, date, sizeof(date));
+		parse_date(audate, date, sizeof(date));
 
 	remove_special(gecos); remove_special(realgecos); remove_special(commitgecos);
 	remove_special(email); remove_special(realemail); remove_special(commitemail);
--- /dev/null
+++ l/date.c  (mode:100644)
@@ -0,0 +1,187 @@
+/*
+ * GIT - The information manager from hell
+ *
+ * Copyright (C) Linus Torvalds, 2005
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <ctype.h>
+#include <time.h>
+
+static time_t my_mktime(struct tm *tm)
+{
+	static const int mdays[] = {
+	    0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334
+	};
+	int year = tm->tm_year - 70;
+	int month = tm->tm_mon;
+	int day = tm->tm_mday;
+
+	if (year < 0 || year > 129) /* algo only works for 1970-2099 */
+		return -1;
+	if (month < 0 || month > 11) /* array bounds */
+		return -1;
+	if (month < 2 || (year + 2) % 4)
+		day--;
+	return (year * 365 + (year + 1) / 4 + mdays[month] + day) * 24*60*60UL +
+		tm->tm_hour * 60*60 + tm->tm_min * 60 + tm->tm_sec;
+}
+
+static const char *month_names[] = {
+        "Jan", "Feb", "Mar", "Apr", "May", "Jun",
+        "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"
+};
+
+static const char *weekday_names[] = {
+        "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"
+};
+
+
+static char *skipfws(char *str)
+{
+	while (isspace(*str))
+		str++;
+	return str;
+}
+
+	
+/* Gr. strptime is crap for this; it doesn't have a way to require RFC2822
+   (i.e. English) day/month names, and it doesn't work correctly with %z. */
+void parse_date(char *date, char *result, int maxlen)
+{
+	struct tm tm;
+	char *p, *tz;
+	int i, offset;
+	time_t then;
+
+	memset(&tm, 0, sizeof(tm));
+
+	/* Skip day-name */
+	p = skipfws(date);
+	if (!isdigit(*p)) {
+		for (i=0; i<7; i++) {
+			if (!strncmp(p,weekday_names[i],3) && p[3] == ',') {
+				p = skipfws(p+4);
+				goto day;
+			}
+		}
+		return;
+	}					
+
+	/* day */
+ day:
+	tm.tm_mday = strtoul(p, &p, 10);
+
+	if (tm.tm_mday < 1 || tm.tm_mday > 31)
+		return;
+
+	if (!isspace(*p))
+		return;
+
+	p = skipfws(p);
+
+	/* month */
+
+	for (i=0; i<12; i++) {
+		if (!strncmp(p, month_names[i], 3) && isspace(p[3])) {
+			tm.tm_mon = i;
+			p = skipfws(p+strlen(month_names[i]));
+			goto year;
+		}
+	}
+	return; /* Error -- bad month */
+
+	/* year */
+ year:	
+	tm.tm_year = strtoul(p, &p, 10);
+
+	if (!tm.tm_year && !isspace(*p))
+		return;
+
+	if (tm.tm_year > 1900)
+		tm.tm_year -= 1900;
+		
+	p=skipfws(p);
+
+	/* hour */
+	if (!isdigit(*p))
+		return;
+	tm.tm_hour = strtoul(p, &p, 10);
+	
+	if (tm.tm_hour > 23)
+		return;
+
+	if (*p != ':')
+		return; /* Error -- bad time */
+	p++;
+
+	/* minute */
+	if (!isdigit(*p))
+		return;
+	tm.tm_min = strtoul(p, &p, 10);
+	
+	if (tm.tm_min > 59)
+		return;
+
+	if (*p != ':')
+		goto zone;
+	p++;
+
+	/* second */
+	if (!isdigit(*p))
+		return;
+	tm.tm_sec = strtoul(p, &p, 10);
+	
+	if (tm.tm_sec > 59)
+		return;
+
+ zone:
+	if (!isspace(*p))
+		return;
+
+	p = skipfws(p);
+
+	if (*p == '-')
+		offset = -60;
+	else if (*p == '+')
+		offset = 60;
+	else
+	       return;
+
+	if (!isdigit(p[1]) || !isdigit(p[2]) || !isdigit(p[3]) || !isdigit(p[4]))
+		return;
+
+	tz = p;
+	i = strtoul(p+1, NULL, 10);
+	offset *= ((i % 100) + ((i / 100) * 60));
+
+	p = skipfws(p + 5);
+	if (*p && *p != '(') /* trailing comment like (EDT) is ok */
+		return;
+
+	then = my_mktime(&tm); /* mktime uses local timezone */
+	if (then == -1)
+		return;
+
+	then -= offset;
+
+	snprintf(result, maxlen, "%lu %5.5s", then, tz);
+}
+
+void datestamp(char *buf, int bufsize)
+{
+	time_t now;
+	struct tm *tm;
+	int offset;
+
+	time(&now);
+
+	tm = localtime(&now); /* get timezone and tm_isdst */
+	offset = -timezone / 60;
+	if (tm->tm_isdst > 0)
+		offset += 60;
+
+	snprintf(buf, bufsize, "%lu %+05d", now, offset/60*100 + offset%60);
+}

Ciao, ET.

Re: Trying to use AUTHOR_DATE

[David Woodhouse]

On Sat, 2005-04-30 at 12:53 +0200, Edgar Toernig wrote:
> +       tm = localtime(&now); /* get timezone and tm_isdst */
> +       offset = -timezone / 60;
> +       if (tm->tm_isdst > 0)
> +               offset += 60;

Some locales have DST offsets which aren't 60 minutes, don't they?

-- 
dwmw2

Re: Trying to use AUTHOR_DATE

[Kay Sievers]

On Sat, 2005-04-30 at 12:13 +0100, David Woodhouse wrote:
> On Sat, 2005-04-30 at 12:53 +0200, Edgar Toernig wrote:
> > +       tm = localtime(&now); /* get timezone and tm_isdst */
> > +       offset = -timezone / 60;
> > +       if (tm->tm_isdst > 0)
> > +               offset += 60;
> 
> Some locales have DST offsets which aren't 60 minutes, don't they?

Yes, some have half-hour offsets:
  http://www.timeanddate.com/library/abbreviations/timezones/au/nft.html

Kay

Re: Trying to use AUTHOR_DATE

[David Woodhouse]

On Sat, 2005-04-30 at 14:08 +0200, Kay Sievers wrote:
> Yes, some have half-hour offsets:
> http://www.timeanddate.com/library/abbreviations/timezones/au/nft.html

That doesn't count -- that timezone is honoured all year round. We're
talking about the difference between wintertime and summertime in any
given locale.

TBH I think I'd rather just put a gmt_mktime() which uses my trick of
looking at tm.tm_gmtoff after the mktime call into a separate file
wrapped in #ifdef GLIBC and let anyone else who really cares about their
own non-BSD-compatible system worry about whether it works there and fix
it up accordingly.

-- 
dwmw2

Re: Trying to use AUTHOR_DATE

[Edgar Toernig]

David Woodhouse wrote:
>
> > +       if (tm->tm_isdst > 0)
> > +               offset += 60;
> 
> Some locales have DST offsets which aren't 60 minutes, don't they?

Oh shit :-/

If grepped through the tz-database and it seems there's one
"country" left that has non-60-minute DST: Lord Howe Island.
All others dropped that before 1970.

Ok, here's a new version of the patch.

--- k/Makefile  (mode:100644)
+++ l/Makefile  (mode:100644)
@@ -28,7 +28,8 @@ all: $(PROG)
 install: $(PROG) $(SCRIPTS)
 	install $(PROG) $(SCRIPTS) $(HOME)/bin/
 
-LIB_OBJS=read-cache.o sha1_file.o usage.o object.o commit.o tree.o blob.o tag.o
+LIB_OBJS=read-cache.o sha1_file.o usage.o object.o commit.o tree.o blob.o \
+	 tag.o date.o
 LIB_FILE=libgit.a
 LIB_H=cache.h object.h blob.h tree.h commit.h tag.h
 
@@ -91,7 +92,6 @@ git-diff-tree-helper: diff-tree-helper.c
 git-tar-tree: tar-tree.c
 
 git-http-pull: LIBS += -lcurl
-git-commit-tree: LIBS += -lcurl
 
 # Library objects..
 blob.o: $(LIB_H)
--- k/cache.h  (mode:100644)
+++ l/cache.h  (mode:100644)
@@ -148,6 +148,9 @@ extern void *read_object_with_reference(
 					unsigned long *size,
 					unsigned char *sha1_ret);
 
+void parse_date(char *date, char *buf, int bufsize);
+void datestamp(char *buf, int bufsize);
+
 static inline void *xmalloc(int size)
 {
 	void *ret = malloc(size);
--- k/commit-tree.c  (mode:100644)
+++ l/commit-tree.c  (mode:100644)
@@ -10,7 +10,6 @@
 #include <string.h>
 #include <ctype.h>
 #include <time.h>
-#include <curl/curl.h>
 
 #define BLOCKING (1ul << 14)
 
@@ -81,24 +80,6 @@ static void remove_special(char *p)
 	}
 }
 
-/* Gr. strptime is crap for this; it doesn't have a way to require RFC2822
-   (i.e. English) day/month names, and it doesn't work correctly with %z. */
-static void parse_date(char *date, time_t *now, char *result, int maxlen)
-{
-	char *p;
-	time_t then;
-
-	if ((then = curl_getdate(date, now)) == 0)
-		return;
-
-	/* find the timezone at the end */
-	p = date + strlen(date);
-	while (p > date && isdigit(*--p))
-		;
-	if ((*p == '+' || *p == '-') && strlen(p) == 5)
-		snprintf(result, maxlen, "%lu %5.5s", then, p);
-}
-
 static void check_valid(unsigned char *sha1, const char *expect)
 {
 	void *buf;
@@ -132,8 +113,6 @@ int main(int argc, char **argv)
 	char *audate;
 	char comment[1000];
 	struct passwd *pw;
-	time_t now;
-	struct tm *tm;
 	char *buffer;
 	unsigned int size;
 
@@ -163,10 +142,8 @@ int main(int argc, char **argv)
 		strcat(realemail, ".");
 		getdomainname(realemail+strlen(realemail), sizeof(realemail)-strlen(realemail)-1);
 	}
-	time(&now);
-	tm = localtime(&now);
 
-	strftime(realdate, sizeof(realdate), "%s %z", tm);
+	datestamp(realdate, sizeof(realdate));
 	strcpy(date, realdate);
 
 	commitgecos = getenv("COMMIT_AUTHOR_NAME") ? : realgecos;
@@ -175,7 +152,7 @@ int main(int argc, char **argv)
 	email = getenv("AUTHOR_EMAIL") ? : realemail;
 	audate = getenv("AUTHOR_DATE");
 	if (audate)
-		parse_date(audate, &now, date, sizeof(date));
+		parse_date(audate, date, sizeof(date));
 
 	remove_special(gecos); remove_special(realgecos); remove_special(commitgecos);
 	remove_special(email); remove_special(realemail); remove_special(commitemail);
--- /dev/null
+++ l/date.c  (mode:100644)
@@ -0,0 +1,184 @@
+/*
+ * GIT - The information manager from hell
+ *
+ * Copyright (C) Linus Torvalds, 2005
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <ctype.h>
+#include <time.h>
+
+static time_t my_mktime(struct tm *tm)
+{
+	static const int mdays[] = {
+	    0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334
+	};
+	int year = tm->tm_year - 70;
+	int month = tm->tm_mon;
+	int day = tm->tm_mday;
+
+	if (year < 0 || year > 129) /* algo only works for 1970-2099 */
+		return -1;
+	if (month < 0 || month > 11) /* array bounds */
+		return -1;
+	if (month < 2 || (year + 2) % 4)
+		day--;
+	return (year * 365 + (year + 1) / 4 + mdays[month] + day) * 24*60*60UL +
+		tm->tm_hour * 60*60 + tm->tm_min * 60 + tm->tm_sec;
+}
+
+static const char *month_names[] = {
+        "Jan", "Feb", "Mar", "Apr", "May", "Jun",
+        "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"
+};
+
+static const char *weekday_names[] = {
+        "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"
+};
+
+
+static char *skipfws(char *str)
+{
+	while (isspace(*str))
+		str++;
+	return str;
+}
+
+	
+/* Gr. strptime is crap for this; it doesn't have a way to require RFC2822
+   (i.e. English) day/month names, and it doesn't work correctly with %z. */
+void parse_date(char *date, char *result, int maxlen)
+{
+	struct tm tm;
+	char *p, *tz;
+	int i, offset;
+	time_t then;
+
+	memset(&tm, 0, sizeof(tm));
+
+	/* Skip day-name */
+	p = skipfws(date);
+	if (!isdigit(*p)) {
+		for (i=0; i<7; i++) {
+			if (!strncmp(p,weekday_names[i],3) && p[3] == ',') {
+				p = skipfws(p+4);
+				goto day;
+			}
+		}
+		return;
+	}					
+
+	/* day */
+ day:
+	tm.tm_mday = strtoul(p, &p, 10);
+
+	if (tm.tm_mday < 1 || tm.tm_mday > 31)
+		return;
+
+	if (!isspace(*p))
+		return;
+
+	p = skipfws(p);
+
+	/* month */
+
+	for (i=0; i<12; i++) {
+		if (!strncmp(p, month_names[i], 3) && isspace(p[3])) {
+			tm.tm_mon = i;
+			p = skipfws(p+strlen(month_names[i]));
+			goto year;
+		}
+	}
+	return; /* Error -- bad month */
+
+	/* year */
+ year:	
+	tm.tm_year = strtoul(p, &p, 10);
+
+	if (!tm.tm_year && !isspace(*p))
+		return;
+
+	if (tm.tm_year > 1900)
+		tm.tm_year -= 1900;
+		
+	p=skipfws(p);
+
+	/* hour */
+	if (!isdigit(*p))
+		return;
+	tm.tm_hour = strtoul(p, &p, 10);
+	
+	if (tm.tm_hour > 23)
+		return;
+
+	if (*p != ':')
+		return; /* Error -- bad time */
+	p++;
+
+	/* minute */
+	if (!isdigit(*p))
+		return;
+	tm.tm_min = strtoul(p, &p, 10);
+	
+	if (tm.tm_min > 59)
+		return;
+
+	if (*p != ':')
+		goto zone;
+	p++;
+
+	/* second */
+	if (!isdigit(*p))
+		return;
+	tm.tm_sec = strtoul(p, &p, 10);
+	
+	if (tm.tm_sec > 59)
+		return;
+
+ zone:
+	if (!isspace(*p))
+		return;
+
+	p = skipfws(p);
+
+	if (*p == '-')
+		offset = -60;
+	else if (*p == '+')
+		offset = 60;
+	else
+	       return;
+
+	if (!isdigit(p[1]) || !isdigit(p[2]) || !isdigit(p[3]) || !isdigit(p[4]))
+		return;
+
+	tz = p;
+	i = strtoul(p+1, NULL, 10);
+	offset *= ((i % 100) + ((i / 100) * 60));
+
+	p = skipfws(p + 5);
+	if (*p && *p != '(') /* trailing comment like (EDT) is ok */
+		return;
+
+	then = my_mktime(&tm); /* mktime uses local timezone */
+	if (then == -1)
+		return;
+
+	then -= offset;
+
+	snprintf(result, maxlen, "%lu %5.5s", then, tz);
+}
+
+void datestamp(char *buf, int bufsize)
+{
+	time_t now;
+	int offset;
+
+	time(&now);
+
+	offset = my_mktime(localtime(&now)) - now;
+	offset /= 60;
+
+	snprintf(buf, bufsize, "%lu %+05d", now, offset/60*100 + offset%60);
+}

Ciao, ET.

Re: Trying to use AUTHOR_DATE

[David Woodhouse]

On Sat, 2005-04-30 at 14:49 +0200, Edgar Toernig wrote:
> +       if (tm.tm_sec > 59)
> +               return;

During a leap second, won't tm_sec be 60? And in fact you don't seem to
handle leap seconds at all, so isn't my_mktime going to be out by one
second for every leap second which has occurred since 1970?

There's a reason I'd rather just let glibc handle it :)

It's not as if tm_gmtoff is particularly esoteric -- we inherited it
from BSD. Let's just use it and let both remaining HPUX users worry
about it themselves if they ever want to use git on their systems.

-- 
dwmw2

Re: Trying to use AUTHOR_DATE

[Edgar Toernig]

David Woodhouse wrote:
>
> On Sat, 2005-04-30 at 14:49 +0200, Edgar Toernig wrote:
> > +       if (tm.tm_sec > 59)
> > +               return;
> 
> During a leap second, won't tm_sec be 60? And in fact you don't seem to
> handle leap seconds at all, so isn't my_mktime going to be out by one
> second for every leap second which has occurred since 1970?

There are no leap-seconds on POSIX systems.  They allow tm_sec
to be 60 but thats all - 00:00:60 is the same as 00:01:00.

Whether the check should be against 59 or 60?  I don't care.
It's Linus decision.

> There's a reason I'd rather just let glibc handle it :)

Good joke.

Ciao, ET.

Re: Trying to use AUTHOR_DATE

[Krzysztof Halasa]

David Woodhouse <dwmw2@infradead.org> writes:

> During a leap second, won't tm_sec be 60?

You could rather have two 59th seconds. Or the "seconds" could be, say,
0.1% longer for 1000 s. Depends on synchronization mechanism.

I think 60th second could only be possible with leap-seconds aware
things (NTP, GPS, reference radio clocks etc.).

> And in fact you don't seem to
> handle leap seconds at all, so isn't my_mktime going to be out by one
> second for every leap second which has occurred since 1970?

No, actually the system time (i.e., the number of seconds since 1970)
is already corrected (minutes are seconds/60, hrs = minutes/60 etc.)
You are off calculating time deltas, but I guess if you need such
accuracy your software already knows about leap seconds.
-- 
Krzysztof Halasa

Re: Trying to use AUTHOR_DATE

[H. Peter Anvin]

Krzysztof Halasa wrote:
> David Woodhouse <dwmw2@infradead.org> writes:
> 
>>During a leap second, won't tm_sec be 60?
> 
> You could rather have two 59th seconds. Or the "seconds" could be, say,
> 0.1% longer for 1000 s. Depends on synchronization mechanism.
>  
> I think 60th second could only be possible with leap-seconds aware
> things (NTP, GPS, reference radio clocks etc.).
> 

It is, but you can't assume you don't have that.  Either way, you just 
treat it the same as the following second.

	-hpa

Re: Trying to use AUTHOR_DATE

[Krzysztof Halasa]

"H. Peter Anvin" <hpa@zytor.com> writes:

> It is, but you can't assume you don't have that.

Yes, if you use NTP time (directly - not the system time) you can get
second=60 (and, in theory, even 61 - not to be expected soon).

>  Either way, you just
> treat it the same as the following second.

Sure, that's the safe way.
-- 
Krzysztof Halasa

Re: Trying to use AUTHOR_DATE

[H. Peter Anvin]

Krzysztof Halasa wrote:
> "H. Peter Anvin" <hpa@zytor.com> writes:
> 
> 
>>It is, but you can't assume you don't have that.
> 
> 
> Yes, if you use NTP time (directly - not the system time) you can get
> second=60 (and, in theory, even 61 - not to be expected soon).
> 

No.  You cannot get 61.  You can, however, get jumps from 58 to 00.

> 
>> Either way, you just
>>treat it the same as the following second.
> 
> Sure, that's the safe way.

Re: Trying to use AUTHOR_DATE

[Krzysztof Halasa]

"H. Peter Anvin" <hpa@zytor.com> writes:

> No.  You cannot get 61.

I was told it would be possible if two leap seconds were needed in some
point of time. Have never occured yet, and maybe never will.

Well, it seems it would need two seconds a month (at least 13 leap seconds
a year) -> not in this century if ever, and it wouldn't be UTC anymore.

> You can, however, get jumps from 58 to 00.

Correct, that would be a deletion. Not yet tried, either, but they say
it's possible.
-- 
Krzysztof Halasa

Re: Trying to use AUTHOR_DATE

[H. Peter Anvin]

Krzysztof Halasa wrote:
> "H. Peter Anvin" <hpa@zytor.com> writes:
> 
> 
>>No.  You cannot get 61.
> 
> I was told it would be possible if two leap seconds were needed in some
> point of time. Have never occured yet, and maybe never will.
> 
> Well, it seems it would need two seconds a month (at least 13 leap seconds
> a year) -> not in this century if ever, and it wouldn't be UTC anymore.
> 

It's certainly not permitted by the current UTC definition, which only 
allows 4 leap seconds per year.  61 comes from a typo in an old version 
of the POSIX standard.

>>You can, however, get jumps from 58 to 00.
> 
> Correct, that would be a deletion. Not yet tried, either, but they say
> it's possible.

... and permitted by the current UTC standard.

	-hpa

Re: Trying to use AUTHOR_DATE

[H. Peter Anvin]

David Woodhouse wrote:
> On Sat, 2005-04-30 at 12:53 +0200, Edgar Toernig wrote:
> 
>>+       tm = localtime(&now); /* get timezone and tm_isdst */
>>+       offset = -timezone / 60;
>>+       if (tm->tm_isdst > 0)
>>+               offset += 60;
> 
> 
> Some locales have DST offsets which aren't 60 minutes, don't they?
> 

Correct.  I believe South Australia is among them.

	-hpa

Re: Trying to use AUTHOR_DATE

[Edgar Toernig]

Luck, Tony wrote:
>
> >What's wrong with the patch I sent to fix this:
>
> I missed it ... there is a problem that you drop the timezone.

Upps, sorry.

> I'd much rather see your version fixed up to preserve the timezone
> than have the libcurl dependency.

Fixed version below.

--- x/commit-tree.c	Thu Apr 21 19:58:47 2005
+++ y/commit-tree.c	Sat Apr 30 06:24:19 2005
@@ -113,6 +113,25 @@
 	}
 }
 
+static time_t my_mktime(struct tm *tm)
+{
+	static const int mdays[] = {
+	    0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334
+	};
+	int year = tm->tm_year - 70;
+	int month = tm->tm_mon;
+	int day = tm->tm_mday;
+
+	if (year < 0 || year > 129) /* algo only works for 1970-2099 */
+		return -1;
+	if (month < 0 || month > 11) /* array bounds */
+		return -1;
+	if (month < 2 || (year + 2) % 4)
+		day--;
+	return (year * 365 + (year + 1) / 4 + mdays[month] + day) * 24*60*60UL +
+		tm->tm_hour * 60*60 + tm->tm_min * 60 + tm->tm_sec;
+}
+
 static const char *month_names[] = {
         "Jan", "Feb", "Mar", "Apr", "May", "Jun",
         "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"
@@ -136,7 +155,7 @@
 static void parse_rfc2822_date(char *date, char *result, int maxlen)
 {
 	struct tm tm;
-	char *p;
+	char *p, *tz;
 	int i, offset;
 	time_t then;
 
@@ -194,7 +213,7 @@
 		return;
 	tm.tm_hour = strtoul(p, &p, 10);
 	
-	if (!tm.tm_hour > 23)
+	if (tm.tm_hour > 23)
 		return;
 
 	if (*p != ':')
@@ -206,14 +225,11 @@
 		return;
 	tm.tm_min = strtoul(p, &p, 10);
 	
-	if (!tm.tm_min > 59)
+	if (tm.tm_min > 59)
 		return;
 
-	if (isspace(*p))
-		goto zone;
-
 	if (*p != ':')
-		return; /* Error -- bad time */
+		goto zone;
 	p++;
 
 	/* second */
@@ -221,13 +237,13 @@
 		return;
 	tm.tm_sec = strtoul(p, &p, 10);
 	
-	if (!tm.tm_sec > 59)
+	if (tm.tm_sec > 59)
 		return;
 
+ zone:
 	if (!isspace(*p))
 		return;
 
- zone:
 	p = skipfws(p);
 
 	if (*p == '-')
@@ -240,19 +256,21 @@
 	if (!isdigit(p[1]) || !isdigit(p[2]) || !isdigit(p[3]) || !isdigit(p[4]))
 		return;
 
+	tz = p;
 	i = strtoul(p+1, NULL, 10);
 	offset *= ((i % 100) + ((i / 100) * 60));
 
-	if (*(skipfws(p + 5)))
+	p = skipfws(p + 5);
+	if (*p && *p != '(') /* trailing comment like (EDT) is ok */
 		return;
 
-	then = mktime(&tm); /* mktime appears to ignore the GMT offset, stupidly */
+	then = my_mktime(&tm); /* mktime uses local timezone */
 	if (then == -1)
 		return;
 
 	then -= offset;
 
-	snprintf(result, maxlen, "%lu %5.5s", then, p);
+	snprintf(result, maxlen, "%lu %5.5s", then, tz);
 }
 
 static void check_valid(unsigned char *sha1, const char *expect)


Ciao, ET.

RE: Trying to use AUTHOR_DATE

[Luck, Tony]

>Fixed version below.

Yup ... that fixes it for my initial test cases.  Thanks.

-Tony

P.S. The libcurl that I found (curl-7.12.1-3.src.rpm) has curl_getdate()
implemented as >1000 lines of yacc.  Which seems like overkill (unless
I really need to set AUTHOR_DATE="a week ago last tuesday" :-)

Re: Trying to use AUTHOR_DATE

[H. Peter Anvin]

Luck, Tony wrote:
>>Fixed version below.
> 
> 
> Yup ... that fixes it for my initial test cases.  Thanks.
> 
> -Tony
> 
> P.S. The libcurl that I found (curl-7.12.1-3.src.rpm) has curl_getdate()
> implemented as >1000 lines of yacc.  Which seems like overkill (unless
> I really need to set AUTHOR_DATE="a week ago last tuesday" :-)

That one is obsolete.

	-hpa