Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora

From: Kévin Le Gouguec <[email-masked]>
Date: Thu, 13 Feb 2025 22:47:25 +0100

Eli Zaretskii <[email-masked]> writes:

>> From: Kévin Le Gouguec <[email-masked]>
>> Cc: [email-masked],  [email-masked]
>> Date: Thu, 13 Feb 2025 19:23:15 +0100
>> 
>> Eli Zaretskii <[email-masked]> writes:
>> 
>> > How about just ignoring the error in this case?  AFAIU, it's rare
>> > enough to not be important, right?  AFAIK, you cannot have more than
>> > one Git instance modifying the repository at any given time, so this
>> > problem cannot be fixed in principle if we are running tow or more Git
>> > sessions in parallel.  IOW, these tests must NOT be run via parallel
>> > make processing.
>> 
>> Apologies if my brain is too fried and I misunderstood: didn't I show
>> that this error shows up without parallelism?
>
> If that is true, it means _my_ brain is fried, but then how is that
> other Git process invoked, and by whom?  IOW, which process locked the
> repository exactly while we were running Git from the test?

No clue thus far.  You mention Git's automatic GC and that's one of my
scapegoats; another is vc-dir spawning more things than I realize.

FWIW, with that last patch I posted, the test has been passing on repeat
for 3h (47k iterations) and counting.  Maybe we have a winner?

Let me know if it looks acceptable, I'll add commentary & a changelog
entry.

>> (a) checking for index.lock presence before invoking vc-git-command does
>> not save us,
>> (b) replacing vc-dir-kill-dir-status-process with a busy loop on
>> vc-dir-busy does not save us either,
>> (a+b) together do not save us either)
>
> My suggestion was to ignore the error, not to do any of the above.
> IOW, treat this test as if it were skipped, if this error happens.

Right, I only mentioned the above to recap previous findings - since "I
can repro at -j1" did not get through, I figured I might have failed to
convey other things that bore repeating.  Was not suggesting that this
should be the solution.

(My main takeaway from the above being: "no, this is not about us
killing a VC process which in turn kills a Git process which leaves a
stray lock")

>> (If Git is responsible for that async process, then that change alone
>> will not help, although the scaffolding simplification would still be
>> nice TBH)
>
> The only way I can think of that Git itself causes this is if the
> automatic GC is invoked.  But AFAIR automatic GC doesn't lock the
> repository, does it?  If it does, perhaps disabling GC for the
> duration of the test will solve this?

Worth exploring, though my current conclusion from failing to reproduce
with my last patch (🤞) is that vc-dir is the likely cause of the
problem.

That too could be worth exploring, though I don't know how valuable the
findings would be - the testcase does not mean to test vc-dir, only the
logic computing extra-headers from branch metadata (mainly to check for
regressions re. bug#68183).

Doubtful users would experience the failure we are observing?  ISTM it
stems from running swaths of Git branching commands while playing with
vc-dir - if users invoke VC commands instead of the Git CLI, presumably
these commands synchronize with vc-dir correctly?

("Then why not rewrite the testcase to use VC commands too instead of
the Git CLI?"  That was my initial intention, but bug#68183 was about a
specific branch arrangement, and I remember giving up on re-creating
that arrangement purely with VC commands 🤷)