patch, 2 partst0006: test timezone parsing

Previously, test-date simply ignored the parsed timezone and told show_date() to use UTC. Instead, let's print out what we actually parsed.

While we're at it, let's make it easy for tests to work in a specific timezone.

Signed-off-by: Jeff King <peff@peff.net>
---
This is a good idea anyway, but it also makes it easy to write a test
for the next patch.

 t/t0006-date.sh |    5 +++--
 test-date.c     |    9 ++++++---
 2 files changed, 9 insertions(+), 5 deletions(-)

When no timezone is specified, we deduce the offset by subtracting the result of mktime from our calculated timestamp.

However, our timestamp is stored as an unsigned integer, meaning we perform the subtraction as unsigned. For a negative offset, this means we wrap to a very high number, and our numeric timezone is in the millions of hours. You can see this bug by doing:

   $ TZ=EST \
     GIT_AUTHOR_DATE='2010-06-01 10:00' \
     git commit -a -m foo
   $ git cat-file -p HEAD | grep author
   author Jeff King <peff@peff.net> 1275404416 +119304128

Instead, we should perform this subtraction as a time_t, the same type that mktime returns.

Signed-off-by: Jeff King <peff@peff.net>
---
Are there any platforms with an unsigned time_t? In that case, we would
need to use a regular "long" to get signedness. Presumably on such
platforms "long" will grow with time_t to 64-bits so we don't create a
Y2038 problem (though most of the rest of the code uses "unsigned long"
interchangeably with time_t, so we have a Y2106 problem in that case
anyway).

 date.c          |    2 +-
 t/t0006-date.sh |    1 +
 2 files changed, 2 insertions(+), 1 deletions(-)

Thanks; both patches make sense to me.

From: Brandon Casey <drafnel@gmail.com>

POSIX requires that both the timezone "standard" and "offset" be specified in the TZ environment variable. This causes a problem on IRIX which does not understand the timezone 'EST'.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---

I guess 'EST' is an alias? for EST5EDT? Linux and Solaris both grok EST just fine. POSIX says the offset is required. I don't see any mention of aliases.

-brandon

 t/t0006-date.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

On Wed, Jul 7, 2010 at 01:34, Brandon Casey <casey@nrlssc.navy.mil> wrote:

Same on the Cygwin installation I happen to have here: it doesn't recongnize EST, but does know about EST5.

On Tue, Jul 06, 2010 at 06:34:20PM -0500, Brandon Casey wrote:

Yeah, I believe it is the same as EST5EDT. Which time zone isn't really important. The main thing is that it be a negative offset from UTC, and that we be able to convert the zone name into a numeric offset for the date specified in the test (which is totally arbitrary and can be tweaked if need be to avoid daylight saving issues).

So I think your patch is fine, but we can look harder if somebody finds a platform that doesn't understand EST5.

-Peff

Brandon Casey <casey@nrlssc.navy.mil> writes:

It's a backward-compatible alias in the Olsen database.

Andreas.

Jeff King <peff@peff.net> writes:

Gah...

On FreeBSD 8.0, we now see this.

   cc1: warnings being treated as errors
   test-date.c: In function 'parse_dates':
   test-date.c:28: warning: format '%ld' expects type 'long int *', but argument 3 has type 'time_t *'

On Tue, Jul 06, 2010 at 12:01:35AM -0700, Junio C Hamano wrote:

Meh. I was worried about that when I used sscanf. I think we can just do this:

On Tue, Jul 06, 2010 at 03:28:49AM -0400, Jeff King wrote:

Oh, I see this is already on maint. :) So here is my fixup in a nicer form:

-- >8 --
Subject: [PATCH] test-date: fix sscanf type conversion

Reading into a time_t isn't portable, since we don't know the exact type. Instead, use an unsigned long, which is what show_date wants, anyway.

Signed-off-by: Jeff King <peff@peff.net>
---
 test-date.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Jeff King <peff@peff.net> writes:

Thanks.