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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 31, 2017, 21:09 UTC
Message-ID: <xmqqbmunq6mg.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqqy3xrqbkr.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

> Yes, but you need to realize that "it is better not to bother users
> with a report of failure to touch in read-only repository" and "we
> ignore all failures".

Sorry about an unfinished sentence here. "need to realize that ... and ... are different things."

Show 18 quoted lines

> ... It is very similar
> to a situation where you ... run "status".
> The command first runs the equivalent of "update-index --refresh"
> only in-core, and it attempts to write the updated index because
> (1) it paid the cost of doing the refreshing already, and (2) if it
> can write into the index, it will help future operations in the
> repository.  But it does not report a failure to write the index
> exactly because it is merely doing an opportunistic write.
>
> And in the "we read from the split index, and we attempt to touch
> the underlying shared one to update its timestamp" case, it is OK
> not to report if we failed to touch.
> ...
> ... On the
> other hand, if you added new information, i.e. wrote the split index
> based on it, it is a good indication that the <split,shared> index
> pair has information that is more valuable.  We must warn in that
> case.

This reminds us of a third case. What should happen if we are doing the "opportunistic writing of the index" in "git status", managed to write the split index, but failed to touch the shared one?

In the ideal world, I think we should do the following sequence:

 - "status" tries to write cache to the file.

 - we try to write and on any error, we return error to the caller,
   who is already prepared to ignore it and stay silent.

    - as the first step of writing the index, we first try to touch
      the shared one.  If it fails, we return an error here without
      writing the split one out.

    - then we try to write the split one out.  If this fails, we
      also return an error.

    - otherwise, both touching of the shared one and writing of the
      split one are successful.

 - "status" finishes the opportunistic refreshing of the index, by
   either ignoring an error silently (if either touching of shared
   one or writing of split one fails) or writing the refreshed index
   out successfully.

It is OK to swap the order of touching the shared one and writing of the split one in the above sequence, as long as an error in either step signals a failure to the opportunistic caller.

I do not offhand know if the split-index code is structured in such a way to allow the above sequence easily, or it needs refactoring.

If such a restructuring is required, it might not be within the scope of the series and I am OK if you just left the NEEDSWORK comment that describes the above (i.e. what we should be doing) as an in-code comment so that we can pick it up later. The whole point of the step 14/21 on the other hand is to make sure that a shared index that is still in active use will not go stale, and from that point of view, such a "punting" may not be a good idea---it deliberately finishes the series knowing that it does not adequately do what it promises to do.

So, ... I dunno.