Re: [PATCH v2 11/16] rerere: let `rerere_path()` write paths into a caller-provided buffer

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> One side note: using timestamp_t here should get us the same behavior
> that the original had before this patch. But I'm not sure the original
> was entirely correct. st_mtime is a time_t, so we are assuming the
> implicit cast is OK. Our timestamp_t tries to be at least as long as
> time_t, so I think we are OK for the future. For very old timestamps, it
> is probably wrong (since time_t is usually signed, and timestamp_t is
> not yet).
>
> It's mostly academic, though, unless your filesystem has rerere files
> before 1970. So I think we can probably just ignore it (and I do still
> hope eventually to support negative values with timestamp_t).

True.

I do not think timestamp_t is appropriate for rerere records,
actually.  The reason why we have timestamp_t is for things like the
author dates that can be arbitrarily and deliberately set to any
historical times, e.g. long before the committer was born.

Unlike that, the timestamps we are dealing with with rerere records
are the times on the filesystem when these rerere records were
created and/or used so whatever stat() gives us for st_mtime
(i.e. time_t) is a lot more appropriate.

I'd probably leave a #leftoverbits here; we should vet our use of
timestamp_t to see if we are not overusing the type.  Roughly, the
timestamps we may record in the commit and the tag objects should be
timestamp_t, but the time we get from the filesystem and only
compared with another timestamp the same way should use appropriate
system-defined type, which is likely to be time_t, as not everybody
may have struct timespec, and file expiration should not need
nanoseconds precision.

Thanks.