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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 15, 2015, 14:42 UTC
Message-ID: <xmqqk2xdmp0e.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20150415072223.GA1389@flurp.local>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 22 quoted lines

> 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.