On Wed, Dec 7, 2016 at 1:08 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Stefan Beller <sbeller@xxxxxxxxxx> writes: > >> So my first question I had to answer was if we do the right thing here, >> i.e. if we could just fail instead. But we want to continue and just >> not write back the index, which is fine. >> >> So we do not have to guard refresh_cache, but just call >> update_index_if_able conditionally. > > An explanation with stepping back a little bit may help. > > You may be asked to visit a repository of a friend, to which you do > not have write access to but you can still read. You may want to do > "diff", "status" or "describe" there. > > In order to avoid getting fooled into thinking some paths are dirty > only because the cached stat information does not match, these need > to refresh the in-core index before doing their "comparison" to > report which paths are different (in "diff"), what are the modified > but not staged paths (in "status"), and if there is a need to add > the "-dirty" suffix (in "describe"). > > Since we are doing the expensive "bunch of lstat()" anyway, if we > could write it back to the index, it would help future operations in > the same repository--that is the reasoning behind the opportunistic > updates. It is perfectly OK if we do not have write access to the > repository and cannot write update the index. > Thanks for that explanation! So I agree that we should not call it _if_needed, but the _if_able part is still confusing, as we check for different parts here. The case of not being able to write (read only) sounds similar to the case of not being able to write (a concurrent lock), I'll think about it more. Thanks, Stefan