Re: [PATCH v3 14/21] read-cache: touch shared index files when used

From: Christian Couder <christian.couder@gmail.com>
Date: Jan 2, 2017, 14:09 UTC
Message-ID: <CAP8UFD1EgOxcPi=tpiosKkYMcCZe+b6gwW0CKt2sE1NZ7gQv=A@mail.gmail.com>
In-Reply-To: <xmqqa8bhb32x.fsf@gitster.mtv.corp.google.com>

On Tue, Dec 27, 2016 at 8:10 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 17 quoted lines

> Christian Couder <christian.couder@gmail.com> writes:
>
>> +/*
>> + * Signal that the shared index is used by updating its mtime.
>> + *
>> + * This way, shared index can be removed if they have not been used
>> + * for some time. It's ok to fail to update the mtime if we are on a
>> + * read only file system.
>> + */
>> +void freshen_shared_index(char *base_sha1_hex)
>> +{
>> +     const char *shared_index = git_path("sharedindex.%s", base_sha1_hex);
>> +     check_and_freshen_file(shared_index, 1);
>
> What happens when this call fails?  The function returns 0 if the
> file did not even exist.  It also returns 0 if you cannot update its
> timestamp.

Yeah and I don't think it's a problem in either case. If we cannot update its timestamp, it's not a problem, as we could be on a read-only file system, and you said in a previous iteration that we should not even warn in this case. If the file does not exist, it could be because it has just been deleted for a good reason, and anyway, if it is a problem that the shared index file has been deleted, it is better addressed when we will actually need the shared index file to read something into from it, rather than just to update its mtime.

> Shouldn't the series be exposing freshen_file() instead _and_ taking
> its error return value seriously?

So what should we do if freshen_file() returns 0 which means that the freshening failed?

>> +}