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

From: Kévin Le Gouguec <[email-masked]>
Date: Wed, 12 Feb 2025 23:37:19 +0100

Kévin Le Gouguec <[email-masked]> writes:

>> My current working theories are:
>>
>> a. We fail to sync with some asynchronous processes spawned by vc before
>>    running these 'git checkout' commands; there is precedent for
>>    synchronization problems:
>>
[…]
>>
>> b. One of the test's git commands spawns a… background GC job or
>>    something, and the test's next git command trips over that.
>
> c. That vc-dir-kill-dir-status-process actually kills a Git process that
>    is "holding the lock", in which case…
>
>> I suppose either a or b would be solved by vc-git-test--run spinning for
>> a bit while (file-exists-p "…/index.lock"); ideally though I'd prefer to
>> catch the locking process red-handed… not sure how to achieve this
>> though.
>
> … this will not work.
>
> I'd like to believe that the present bug stems from that dodgy FIXME
> workaround, and addressing the latter will fix the former, but that
> FIXME was an admission of defeat already.  Haven't grown wiser since
> then unfortunately.

Bafflement continues.  With the attached patch that

* removes the call to vc-dir-kill-dir-status-process to eliminate
hypothesis c,
* replaces it with a (while (vc-dir-busy) …) loop to mitigate a,
* adds a (while (file-exists-p ".git/index.lock")) loop to mitigate b,

I still reproduce.  Here, after 700 runs in 6min:

      GEN      lisp/vc/vc-git-tests.log
    Running 8 tests (2025-02-12 23:16:08+0100, selector `(not (or (tag :unstable) (tag :nativecomp)))')
       passed  1/8  vc-git-test-annotate-time (0.002284 sec)
    Paused 1 time(s) waiting for vc-dir-busy
    err in /tmp/emacs-test-dhul2y-vc-git/: (error Failed (status 128): git --no-pager checkout -b feature/foo master .)
    (buffer-string:
    fatal: Unable to create '/tmp/emacs-test-dhul2y-vc-git/.git/index.lock': File exists.

    Another git process seems to be running in this repository, e.g.
    an editor opened by 'git commit'. Please make sure all processes
    are terminated then try again. If it still fails, a git process
    may have crashed in this repository earlier:
    remove the file manually to continue.

    )

I have a hard time making sense of the sequence of events.  Evidently:

1. vc-git-test--run did *not* pause for (file-exists-p
   ".git/index.lock"),
2. but Git *still* tripped over a preexisting .git/index.lock.

Logging default-directory in the condition-case error handler confirms
we are in the correct directory, so I do not understand why
file-exists-p did not see the lock that Git reports.  All I can think of
is a stray Git process ¿started at some point by someone? that grabs the
lock between steps 1 & 2.

Barring "turn off background processing" knobs for VC and/or Git that I
missed, not sure what to try next beside catching that "index.lock
exists" error and re-trying after a delay.

diff --git a/test/lisp/vc/vc-git-tests.el b/test/lisp/vc/vc-git-tests.el
index 4b5cb75df01..decfb2220d4 100644
--- a/test/lisp/vc/vc-git-tests.el
+++ b/test/lisp/vc/vc-git-tests.el
@@ -103,10 +103,32 @@ vc-git-test--with-repo
        (vc-create-repo 'Git)
        ,@body)))
 
+(defun vc-git-test--wait (what desc)
+  "TODO"
+  (let ((i 0)
+        (max 20))
+    (while (funcall what)
+      (when (> (cl-incf i) max)
+        (error "Timed out waiting for %s" desc))
+      (sleep-for .05))
+    (when (> i 0)
+      (message "Paused %d time(s) waiting for %s" i desc))))
+
 (defun vc-git-test--run (&rest args)
   "Run git ARGS…, check for non-zero status, and return output."
+  ;; bug#76187: Git sometimes reports an index.lock file and bails,
+  ;; suggesting another lingering Git process in the same repo.  Since
+  ;; this test runs in a temporarily-generated repo, that process is
+  ;; probably an asynchronous one initiated earlier by this test; for
+  ;; lack of better understanding, wait until the lock is released.
+  (vc-git-test--wait (lambda () (file-exists-p ".git/index.lock"))
+                     "index.lock")
   (with-temp-buffer
-    (apply 'vc-git-command t 0 nil args)
+    (condition-case err
+        (apply 'vc-git-command t 0 nil args)
+      (t (message "err in %s: %s" default-directory err)
+         (message "(buffer-string:\n%s\n)" (buffer-string))
+         (signal (car err) (cdr err))))
     (buffer-string)))
 
 (defun vc-git-test--start-branch ()
@@ -126,10 +148,10 @@ vc-git-test--dir-headers
 return a list of (NAME . VALUE) pairs, where VALUE is nil if the header
 is absent."
   ;; FIXME: to reproduce interactive sessions faithfully, we would need
-  ;; to wait for the dir-status-files process to terminate; have not
-  ;; found a reliable way to do this.  As a workaround, kill pending
-  ;; processes and revert the `vc-dir' buffer.
-  (vc-dir-kill-dir-status-process)
+  ;; to synchronize with the dir-status-files process; have not found a
+  ;; reliable way to do this.  As a workaround, wait until vc-dir stops
+  ;; being "busy" then revert the `vc-dir' buffer.
+  (vc-git-test--wait #'vc-dir-busy "vc-dir-busy")
   (revert-buffer)
   (mapcar
    (lambda (header)