Re: [PATCH 2/2] apply: notice creation/removal patches produced by GNU diff

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 11, 2009, 03:57 UTC
Message-ID: <7vws6foor7.fsf@alter.siamese.dyndns.org>
In-Reply-To: <alpine.LFD.2.01.0907102029570.3552@localhost.localdomain>

Linus Torvalds <torvalds@linux-foundation.org> writes:

Show 8 quoted lines

> On Fri, 10 Jul 2009, Junio C Hamano wrote:
>>
>> Unified context patch generated by GNU diff has UNIX epoch timestamp
>> on the side that does not exist when the patch is about a creation or
>> a deletion event.  Notice this convention when reading a non-git diff.
>
> Hmm. Do you really want to do a regex here? That seems overkill. Why not 
> just try to parse the date?

If parse_date() says "It is a date and it is UNIX epoch", we mark the patch as either creation or deletion, and that is used by various consistency checks. Deletion patch must remove the entire line, the file must not exist if it is the target of a creation patch, etc.

I found parse_date() to be a bit too forgiving for my taste for this
particular application; I wanted to be anal and only accept the format
specified by
 
  http://www.opengroup.org/onlinepubs/9699919799/utilities/diff.html#tag_20_34_10_07

By the way, the above does not mention anything about marking creation/deletion event with UNIX epoch; I am guessing it is a GNU extension.

Show 10 quoted lines

>> +	const char stamp_regexp[] =
>> +		"^[0-9][0-9][0-9][0-9]-[01][0-9]-[0-3][0-9]"
>> +		" "
>> +		"[0-2][0-9]:[0-5][0-9]:[0-6][0-9](\\.0+)?"
>> +		" "
>> +		"[-+][0-2][0-9][0-5][0-9]\n";
>
> Also, why are you apparently expecting micro-seconds to always be all 
> zeroes? Maybe that's the common case, but I'd expect that somebody has 
> non-zero microseconds on filesystems that support them..

As the comment before the quoted part of the patch says, the point of this function is not about detecting the presense of a timestamp (and parsing the time), but telling if the timestamp that represents UNIX epoch is there. The first iteration of my patch did not even use parse_date() but accepted only 1969-12-31 (west of GMT) or 1970-01-01 (east of GMT) and then checked timestamp with timezone manually. Perhaps that might be a better way to do this function? I dunno.