Subject: Re: Odd broken "--date=now" behavior in current git

From: Junio C Hamano <[email-masked]>
Date: Wed, 15 Apr 2015 07:42:41 -0700

Eric Sunshine <[email-masked]> writes:

> Later parse_date_basic() computes the offset from GMT by comparing
> the values returned by tm_to_time_t() and mktime(). The existing 'tm'
> is passed to mktime() with the tm_isdst field already set to 0 by
> gmtime_r(), and mktime() respects that as a statement that DST is not
> in effect, rather than determining it dynamically.
>
> The fix seems to be simply:
>
> ---- >8 ----
> diff --git a/date.c b/date.c
> index 3eba2df..99ad2a0 100644
> --- a/date.c
> +++ b/date.c
> @@ -707,6 +707,7 @@ int parse_date_basic(const char *date, unsigned long *timestamp, int *offset)
>  	/* mktime uses local timezone */
>  	*timestamp = tm_to_time_t(&tm);
>  	if (*offset == -1) {
> +		tm.tm_isdst = -1;
>  		time_t temp_time = mktime(&tm);
>  		if ((time_t)*timestamp > temp_time) {
>  			*offset = ((time_t)*timestamp - temp_time) / 60;
> ---- >8 ----

I briefly wondered if the caller of gmtime_r() in match_digit()
should be preserving the tm_isdst, though, as that codepath knows
that it is handling a bare number without GMT offset.

But resetting it to -1 here makes it even less error prone (we may
gain other code that stomp on tm.tm_isdst before we get here, and
having -1 in *offset is a sign that nobody saw GMT offset in the
input).

I think I see a decl-after-statment, but other than that, this looks
like a good fix.

Thanks.