Re: [RFC PATCH] rerere: fix overeager gc

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 2, 2010, 17:25 UTC
Message-ID: <7vmxu923up.fsf@alter.siamese.dyndns.org>
In-Reply-To: <4C2D7DF7.8030408@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

Show 14 quoted lines

> Am 7/1/2010 18:27, schrieb Junio C Hamano:
>> Johannes Sixt <j.sixt@viscovery.net> writes:
>> 
>>> Would it be possible to update the timestamp of preimage every time it is
>>> used (e.g., in rerere.c:merge()), and check for that?
>> 
>> It would be _possible_, but we are _not_ modifying the file at that point,
>> so somehow that solution feels very wrong, no?
>
> rr-cache is basically a static database. The fact that we have a file
> named 'thisimage' is just an abuse - to put a temporary file somewhere.
> Therefore, depending on lockfile infrastructure to change timestamps for
> us while manipulating 'thisimage' should feel no less wrong, don't you
> think so?

Not really.

"This" in "thisimage" stands for "image involved in conflict we are looking at _this moment_", and using the per-conflict-id directory to house that file was a conscious design decision. Because you are trying to record a new data item (namely "the last time we saw this conflict") somewhere in the rr-cache database, using the timestamp of that directory to tell when was the last time that _this moment_ was makes sense, at least to me.

If anything, "preimage" that has newer timestamp than "postimage" feels wrong, as you would record pre, think for a while how the corresponding post should look like, and then record post.

If we for whatever reason trust placing an extra timestamp on a regular file more than using directory timestamp (which I think may be a valid concern from portability point of view), I'd rather see "preimage" timestamp to keep track of the time when we _first_ encountered the particular conflict, and "postimage" used for recording the time when we saw the conflict most recently.