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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 24, 2025, 22:50 UTC
Message-ID: <xmqqh64j3qkn.fsf@gitster.g>
In-Reply-To: <20250224221914.GA193356@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 11 quoted lines

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