Should update-index --refresh force writing the index in case of racy timestamps?

Should update-index --refresh force writing the index in case of racy timestamps?

[Marc Strapetz]

For one of my Git-LFS test repositories, switching between branches 
quite often results in lots of racy index timestamps. Subsequent calls 
to "git update-index --refresh" or "git status" will invoke the "lfs" 
filter over and over again, just to figure out that all entries are 
still up-to-date. Hence, the index will never be rewritten and racy 
timestamps will remain.

To break out of this state, it seems favorable to write the index if any 
racy timestamp is detected. We will be able to provide a patch if this 
change sounds reasonable.

-Marc

Re: Should update-index --refresh force writing the index in case of racy timestamps?

[Marc Strapetz]

On 17/12/2021 11:44, Marc Strapetz wrote:
> Subsequent calls to "git update-index --refresh" or "git status" will invoke the "lfs" 
> filter over and over again, ...

Please forget about the note regarding "git status". cmd_status() will 
actually fix racy timestamps by calling repo_update_index_if_able(). So 
the problem is only about update-index.

-Marc

Re: Should update-index --refresh force writing the index in case of racy timestamps?

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 867 bytes --]

On 2021-12-17 at 10:44:32, Marc Strapetz wrote:
> For one of my Git-LFS test repositories, switching between branches quite
> often results in lots of racy index timestamps. Subsequent calls to "git
> update-index --refresh" or "git status" will invoke the "lfs" filter over
> and over again, just to figure out that all entries are still up-to-date.
> Hence, the index will never be rewritten and racy timestamps will remain.
> 
> To break out of this state, it seems favorable to write the index if any
> racy timestamp is detected. We will be able to provide a patch if this
> change sounds reasonable.

Sure, this sounds reasonable, especially if, as you mentioned, git
status already does this.  We might as well make the plumbing commands
as functional as the porcelain commands.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: Should update-index --refresh force writing the index in case of racy timestamps?

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> On 2021-12-17 at 10:44:32, Marc Strapetz wrote:
>> For one of my Git-LFS test repositories, switching between branches quite
>> often results in lots of racy index timestamps. Subsequent calls to "git
>> update-index --refresh" or "git status" will invoke the "lfs" filter over
>> and over again, just to figure out that all entries are still up-to-date.
>> Hence, the index will never be rewritten and racy timestamps will remain.
>> 
>> To break out of this state, it seems favorable to write the index if any
>> racy timestamp is detected. We will be able to provide a patch if this
>> change sounds reasonable.
>
> Sure, this sounds reasonable, especially if, as you mentioned, git
> status already does this.  We might as well make the plumbing commands
> as functional as the porcelain commands.

Given that "update-index --refresh" is a way to say "we know
something changed to make cached stat information dirty even for
otherwise clean paths and we want our 'diff-files' and other
plumbing command to start relying on the cached stat information
again, so please do as much I/O as you need", I agree that it should
do as thourough job as necessary.

Re: Should update-index --refresh force writing the index in case of racy timestamps?

[Marc Strapetz]

On 18/12/2021 01:08, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
>> On 2021-12-17 at 10:44:32, Marc Strapetz wrote:
>>> For one of my Git-LFS test repositories, switching between branches quite
>>> often results in lots of racy index timestamps. Subsequent calls to "git
>>> update-index --refresh" or "git status" will invoke the "lfs" filter over
>>> and over again, just to figure out that all entries are still up-to-date.
>>> Hence, the index will never be rewritten and racy timestamps will remain.
>>>
>>> To break out of this state, it seems favorable to write the index if any
>>> racy timestamp is detected. We will be able to provide a patch if this
>>> change sounds reasonable.
>>
>> Sure, this sounds reasonable, especially if, as you mentioned, git
>> status already does this.  We might as well make the plumbing commands
>> as functional as the porcelain commands.
> 
> Given that "update-index --refresh" is a way to say "we know
> something changed to make cached stat information dirty even for
> otherwise clean paths and we want our 'diff-files' and other
> plumbing command to start relying on the cached stat information
> again, so please do as much I/O as you need", I agree that it should
> do as thourough job as necessary.

Thanks! I have now submitted a pull request via gitgitgadget.

I had some problems to understand what the correct place to set 
active_cache_changed is and have decided to keep it as close as possible 
to refresh_cache(): unresolve_callback() and reupdate_callback() may 
reset active_cache_changed in case of errors. On the other hand, 
active_cache_changed may be set again later for a preferred_index_format 
change and for read_from_stdin (as part of update_one()), so in the end 
the callbacks may be overruled and the index may finally be written. I'm 
not sure whether these combinations can actually occur and whether this 
might cause any troubles. I just wanted to point out.

-Marc