GNU bug report logs - #76187
vc-git-test-dir-branch-headers failure on Fedora

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 76187 in the body.
You can then email your comments to 76187 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Emacs bug reports <bug-gnu-emacs <at> gnu.org>
Subject: vc-git-test-dir-branch-headers failure on Fedora
Date: Mon, 10 Feb 2025 14:58:31 -0800

On Fedora 41 x86-64 with the current Emacs master (commit 
4936a8d5acbfee2dee6d903400eba48cb2e3a6a7) I occasionally see failures of 
the vc-git-test-dir-branch-headers test. Usually it works. Here are the 
two failures I've seen.

The first was with a "make -j5 check" so it may have been a collision 
with some other test:

>   GEN      lisp/vc/ediff-diff-tests.log
>   GEN      lisp/vc/ediff-ptch-tests.log
>   GEN      lisp/vc/log-edit-tests.log
>   GEN      lisp/vc/smerge-mode-tests.log
>   GEN      lisp/vc/vc-bzr-tests.log
>   GEN      lisp/vc/vc-cvs-tests.log
>   GEN      lisp/vc/vc-git-tests.log
>   GEN      lisp/vc/vc-hg-tests.log
>   GEN      lisp/vc/vc-tests.log
>   GEN      lisp/version-tests.log
>   GEN      lisp/wdired-tests.log
>   GEN      lisp/which-key-tests.log
>   GEN      lisp/whitespace-tests.log
>   ELC+ELN  lisp/wid-edit-tests.elc
> Running 8 tests (2025-02-10 14:40:44-0800, selector `(not (or (tag :expensive-test) (tag :unstable)))')
>    passed  1/8  vc-git-test-annotate-time (0.006682 sec)
> Test vc-git-test-dir-branch-headers backtrace:
>   signal(error ("Failed (status 128): git --no-pager checkout -b featu
>   error("Failed (%s): %s" "status 128" "git --no-pager checkout -b fea
>   vc-do-command(t 0 "git" nil "--no-pager" "checkout" "-b" "feature/fo
>   apply(vc-do-command t 0 "git" nil "--no-pager" ("checkout" "-b" "fea
>   vc-git-command(t 0 nil "checkout" "-b" "feature/foo" "master")
>   apply(vc-git-command t 0 nil ("checkout" "-b" "feature/foo" "master"
>   vc-git-test--run("checkout" "-b" "feature/foo" "master")
>   #f(compiled-function () #<bytecode 0xca76da6649814e1>)()
>   #f(compiled-function () #<bytecode 0xf56a36e7feeaf22>)()
>   handler-bind-1(#f(compiled-function () #<bytecode 0xf56a36e7feeaf22>
>   ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
>   ert-run-test(#s(ert-test :name vc-git-test-dir-branch-headers :docum
>   ert-run-or-rerun-test(#s(ert--stats :selector ... :tests ... :test-m
>   ert-run-tests((not (or (tag :expensive-test) (tag :unstable))) #f(co
>   ert-run-tests-batch((not (or (tag :expensive-test) (tag :unstable)))
>   ert-run-tests-batch-and-exit((not (or (tag :expensive-test) (tag :un
>   eval((ert-run-tests-batch-and-exit '(not (or (tag :expensive-test) (
>   command-line-1(("-L" ":." "-l" "ert" "--eval" "(setq treesit-extra-l
>   command-line()
>   normal-top-level()
> Test vc-git-test-dir-branch-headers condition:
>     (error
>      "Failed (status 128): git --no-pager checkout -b feature/foo master .")
>    FAILED  2/8  vc-git-test-dir-branch-headers (0.349513 sec) at lisp/vc/vc-git-tests.el:146
>    passed  3/8  vc-git-test-program-version-apple (0.000109 sec)
>    passed  4/8  vc-git-test-program-version-general (0.000061 sec)
>    passed  5/8  vc-git-test-program-version-invalid-leading-dot (0.000059 sec)
>    passed  6/8  vc-git-test-program-version-invalid-leading-string (0.000052 sec)
>    passed  7/8  vc-git-test-program-version-other (0.000088 sec)
>    passed  8/8  vc-git-test-program-version-windows (0.000060 sec)
> 
> Ran 8 tests, 7 results as expected, 1 unexpected (2025-02-10 14:40:45-0800, 0.544882 sec)
> 
> 1 unexpected results:
>    FAILED  vc-git-test-dir-branch-headers
> 
> make[3]: *** [Makefile:185: lisp/vc/vc-git-tests.log] Error 1
>   GEN      lisp/x-dnd-tests.log
>   GEN      lisp/xdg-tests.log


The second failure was with "make lisp/vc/vc-git-tests" in the test 
directory. I repeatedly ran that test, and it failed on the 20th 
execution as follows:

> make[1]: Entering directory '/home/eggert/src/gnu/emacs/static-checking/test'
>   GEN      lisp/vc/vc-git-tests.log
> Running 8 tests (2025-02-10 14:50:43-0800, selector `(not (tag :unstable))')
>    passed  1/8  vc-git-test-annotate-time (0.006192 sec)
> Test vc-git-test-dir-branch-headers backtrace:
>   signal(error ("Failed (status 128): git --no-pager checkout -b featu
>   error("Failed (%s): %s" "status 128" "git --no-pager checkout -b fea
>   vc-do-command(t 0 "git" nil "--no-pager" "checkout" "-b" "feature/ba
>   apply(vc-do-command t 0 "git" nil "--no-pager" ("checkout" "-b" "fea
>   vc-git-command(t 0 nil "checkout" "-b" "feature/bar" "--track" "mast
>   apply(vc-git-command t 0 nil ("checkout" "-b" "feature/bar" "--track
>   (progn (apply 'vc-git-command t 0 nil args) (buffer-string))
>   (unwind-protect (progn (apply 'vc-git-command t 0 nil args) (buffer-
>   (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn
>   (let ((temp-buffer (generate-new-buffer " *temp*" t))) (save-current
>   vc-git-test--run("checkout" "-b" "feature/bar" "--track" "master")
>   (progn (vc-git-test--run "clone" origin-repo clone-repo) (vc-dir clo
>   (unwind-protect (progn (vc-git-test--run "clone" origin-repo clone-r
>   (let* ((coding-system-for-write nil) (temp-file (file-name-as-direct
>   (let ((main-branch (vc-git-test--start-branch))) (let* ((coding-syst
>   (let ((default-directory origin-repo) (process-environment (append '
>   (progn (let ((default-directory origin-repo) (process-environment (a
>   (unwind-protect (progn (let ((default-directory origin-repo) (proces
>   (let* ((coding-system-for-write nil) (temp-file (file-name-as-direct
>   #f(lambda () [t] (let* ((fn-25 #'executable-find) (args-26 (conditio
>   #f(compiled-function () #<bytecode 0x97dde0d98f0ae9a>)()
>   handler-bind-1(#f(compiled-function () #<bytecode 0x97dde0d98f0ae9a>
>   ert--run-test-internal(#s(ert--test-execution-info :test ... :result
>   ert-run-test(#s(ert-test :name vc-git-test-dir-branch-headers :docum
>   ert-run-or-rerun-test(#s(ert--stats :selector ... :tests ... :test-m
>   ert-run-tests((not (tag :unstable)) #f(compiled-function (event-type
>   ert-run-tests-batch((not (tag :unstable)))
>   ert-run-tests-batch-and-exit((not (tag :unstable)))
>   eval((ert-run-tests-batch-and-exit '(not (tag :unstable))) t)
>   command-line-1(("-L" ":." "-l" "ert" "--eval" "(setq treesit-extra-l
>   command-line()
>   normal-top-level()
> Test vc-git-test-dir-branch-headers condition:
>     (error
>      "Failed (status 128): git --no-pager checkout -b feature/bar --track master .")
>    FAILED  2/8  vc-git-test-dir-branch-headers (0.316582 sec) at lisp/vc/vc-git-tests.el:146
>    passed  3/8  vc-git-test-program-version-apple (0.000139 sec)
>    passed  4/8  vc-git-test-program-version-general (0.000085 sec)
>    passed  5/8  vc-git-test-program-version-invalid-leading-dot (0.000079 sec)
>    passed  6/8  vc-git-test-program-version-invalid-leading-string (0.000114 sec)
>    passed  7/8  vc-git-test-program-version-other (0.000076 sec)
>    passed  8/8  vc-git-test-program-version-windows (0.000075 sec)
> 
> Ran 8 tests, 7 results as expected, 1 unexpected (2025-02-10 14:50:44-0800, 0.508602 sec)
> 
> 1 unexpected results:
>    FAILED  vc-git-test-dir-branch-headers
> 
> make[1]: *** [Makefile:185: lisp/vc/vc-git-tests.log] Error 1
> make[1]: Leaving directory '/home/eggert/src/gnu/emacs/static-checking/test'
> make: *** [Makefile:251: lisp/vc/vc-git-tests] Error 2

Message #8 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 76187 <at> debbugs.gnu.org
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Tue, 11 Feb 2025 19:41:17 +0100

Paul Eggert <eggert <at> cs.ucla.edu> writes:

> The second failure was with "make lisp/vc/vc-git-tests" in the test directory. I repeatedly ran that test, and it failed on the 20th execution as follows:

> Test vc-git-test-dir-branch-headers condition:
>     (error
>      "Failed (status 128): git --no-pager checkout -b feature/bar --track master .")
>    FAILED  2/8  vc-git-test-dir-branch-headers (0.316582 sec) at lisp/vc/vc-git-tests.el:146

Interesting.  I reproduced the 'checkout -b feature/foo master' failure
after 300 executions.

"status 128" confounds me a bit; tho there is a surprising (to me)
number of hits for "git exit 128" on the net.  Wondering if Git outputs
anything useful…

Anyhoo, no clue yet - just wanted to ACK (openSUSE Tumbleweed FWIW),
what with having authored that test.

Message #11 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 76187 <at> debbugs.gnu.org
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Tue, 11 Feb 2025 21:43:50 +0100

Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:

> Paul Eggert <eggert <at> cs.ucla.edu> writes:
>
>> The second failure was with "make lisp/vc/vc-git-tests" in the test directory. I repeatedly ran that test, and it failed on the 20th execution as follows:
>
>> Test vc-git-test-dir-branch-headers condition:
>>     (error
>>      "Failed (status 128): git --no-pager checkout -b feature/bar --track master .")
>>    FAILED  2/8  vc-git-test-dir-branch-headers (0.316582 sec) at lisp/vc/vc-git-tests.el:146
>
> Interesting.  I reproduced the 'checkout -b feature/foo master' failure
> after 300 executions.
>
> "status 128" confounds me a bit; tho there is a surprising (to me)
> number of hits for "git exit 128" on the net.  Wondering if Git outputs
> anything useful…
>
> Anyhoo, no clue yet - just wanted to ACK (openSUSE Tumbleweed FWIW),
> what with having authored that test.

Ah-ha.

diff --git a/test/lisp/vc/vc-git-tests.el b/test/lisp/vc/vc-git-tests.el
index 4b5cb75df01..b9d815e9a5d 100644
--- a/test/lisp/vc/vc-git-tests.el
+++ b/test/lisp/vc/vc-git-tests.el
@@ -106,7 +106,11 @@ vc-git-test--with-repo
 (defun vc-git-test--run (&rest args)
   "Run git ARGS…, check for non-zero status, and return output."
   (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: %s" err)
+         (message "buffer-string: %s" (buffer-string))
+         (signal (car err) (cdr err))))
     (buffer-string)))
 
 (defun vc-git-test--start-branch ()

Yields:

> err: (error Failed (status 128): git --no-pager checkout -b feature/foo master .)
> buffer-string: fatal: Unable to create '/tmp/emacs-test-uJtGe2-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.

Pondering.

Message #14 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: 76187 <at> debbugs.gnu.org
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Tue, 11 Feb 2025 13:12:44 -0800

On 2/11/25 12:43, Kévin Le Gouguec wrote:
> Another git process seems to be running in this repository,

I guess the test won't work with "make -j check", since some other test 
might also be running git in that repository. If my guess is right, to 
fix this you can arrange for the test to run in a clone instead. 
Similarly for any other test that might want to alter that repository or 
its working files.

Message #17 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 76187 <at> debbugs.gnu.org
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Tue, 11 Feb 2025 22:36:43 +0100

Paul Eggert <eggert <at> cs.ucla.edu> writes:

> On 2/11/25 12:43, Kévin Le Gouguec wrote:
>> Another git process seems to be running in this repository,
>
> I guess the test won't work with "make -j check", since some other test might also be running git in that repository. If my guess is right, to fix this you can arrange for the test to run in a clone instead. Similarly for any other test that might want to alter that repository or its working files.

The test does arrange to run in isolated directories, using
ert-with-temp-directory:

(defmacro vc-git-test--with-repo (name &rest body)
  ;; …
  `(ert-with-temp-directory ,name
     (let ((default-directory ,name)
           ;; …
           )
       (vc-create-repo 'Git)
       ,@body)))

(ert-deftest vc-git-test-dir-branch-headers ()
  ;; …
  ;; Create a repository that will serve as the "remote".
  (vc-git-test--with-repo origin-repo                                   
    (let ((main-branch (vc-git-test--start-branch)))
      ;; 'git clone' this repository and test things in this clone.
      (ert-with-temp-directory clone-repo
        ;; …
        ))))

So unless I am missing something, 'make -j' ought not be a factor.

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:

     (defun vc-git-test--dir-headers (headers)
       "…"
       ;; 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)
       (revert-buffer)
       ;; …
       )

   FWIW, sticking one more speculative (vc-dir-kill-dir-status-process)
   after (revert-buffer) lowers the failure rate (took 10 minutes to
   reproduce it once; no second occurrence after >5k runs over 30min).

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.

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.

Message #20 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 76187 <at> debbugs.gnu.org
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Tue, 11 Feb 2025 22:39:44 +0100

Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:

> Paul Eggert <eggert <at> cs.ucla.edu> writes:
>
>> On 2/11/25 12:43, Kévin Le Gouguec wrote:
>>> Another git process seems to be running in this repository,
>>
>> I guess the test won't work with "make -j check", since some other test might also be running git in that repository. If my guess is right, to fix this you can arrange for the test to run in a clone instead. Similarly for any other test that might want to alter that repository or its working files.
>
> The test does arrange to run in isolated directories, using
> ert-with-temp-directory:

(As a matter of fact, to reproduce this issue, I have been running

  while make -C test lisp/vc/vc-git-tests ; do continue ; done

so AFAIU 'make -j' cannot be a factor?)

Message #23 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 76187 <at> debbugs.gnu.org
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Tue, 11 Feb 2025 23:09:45 +0100

Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:

> Paul Eggert <eggert <at> cs.ucla.edu> writes:
>
>> On 2/11/25 12:43, Kévin Le Gouguec wrote:
>>> Another git process seems to be running in this repository,
>>
>> I guess the test won't work with "make -j check", since some other test might also be running git in that repository. If my guess is right, to fix this you can arrange for the test to run in a clone instead. Similarly for any other test that might want to alter that repository or its working files.
>
> The test does arrange to run in isolated directories, using
> ert-with-temp-directory:
>
> (defmacro vc-git-test--with-repo (name &rest body)
>   ;; …
>   `(ert-with-temp-directory ,name
>      (let ((default-directory ,name)
>            ;; …
>            )
>        (vc-create-repo 'Git)
>        ,@body)))
>
> (ert-deftest vc-git-test-dir-branch-headers ()
>   ;; …
>   ;; Create a repository that will serve as the "remote".
>   (vc-git-test--with-repo origin-repo                                   
>     (let ((main-branch (vc-git-test--start-branch)))
>       ;; 'git clone' this repository and test things in this clone.
>       (ert-with-temp-directory clone-repo
>         ;; …
>         ))))
>
> So unless I am missing something, 'make -j' ought not be a factor.
>
> 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:
>
>      (defun vc-git-test--dir-headers (headers)
>        "…"
>        ;; 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)
>        (revert-buffer)
>        ;; …
>        )
>
>    FWIW, sticking one more speculative (vc-dir-kill-dir-status-process)
>    after (revert-buffer) lowers the failure rate (took 10 minutes to
>    reproduce it once; no second occurrence after >5k runs over 30min).
>
> 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.

(No luck reproducing any of this under strace FWIW)

Message #26 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: 76187 <at> debbugs.gnu.org
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Tue, 11 Feb 2025 16:35:51 -0800

On 2/11/25 13:36, Kévin Le Gouguec wrote:
>         ;; 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.

One way would be set up those processes with a pipe connecting them to 
you, and then read from the pipe.

Admittedly I'm just spitballing at this point.

Message #29 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 76187 <at> debbugs.gnu.org, kevin.legouguec <at> gmail.com
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Wed, 12 Feb 2025 15:03:57 +0200

> Cc: 76187 <at> debbugs.gnu.org
> Date: Tue, 11 Feb 2025 13:12:44 -0800
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> 
> On 2/11/25 12:43, Kévin Le Gouguec wrote:
> > Another git process seems to be running in this repository,
> 
> I guess the test won't work with "make -j check", since some other test 
> might also be running git in that repository. If my guess is right, to 
> fix this you can arrange for the test to run in a clone instead. 
> Similarly for any other test that might want to alter that repository or 
> its working files.

Or just arrange for this test to wait for a while until the lock is
freed.

Message #32 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: 76187 <at> debbugs.gnu.org, eggert <at> cs.ucla.edu
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Wed, 12 Feb 2025 15:21:34 +0200

> Cc: 76187 <at> debbugs.gnu.org
> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
> Date: Tue, 11 Feb 2025 23:09:45 +0100
> 
> Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:
> 
> >    FWIW, sticking one more speculative (vc-dir-kill-dir-status-process)
> >    after (revert-buffer) lowers the failure rate (took 10 minutes to
> >    reproduce it once; no second occurrence after >5k runs over 30min).
> >
> > 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…

When we kill a Git process, we should also remove the lock file, I
think.

Message #35 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 76187 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Wed, 12 Feb 2025 23:37:19 +0100

[Message part 1 (text/plain, inline)]

Kévin Le Gouguec <kevin.legouguec <at> gmail.com> 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.

[debug.patch (text/x-patch, attachment)]

Message #38 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: 76187 <at> debbugs.gnu.org, eggert <at> cs.ucla.edu
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Thu, 13 Feb 2025 09:06:27 +0200

> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
> Cc: 76187 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
> Date: Wed, 12 Feb 2025 23:37:19 +0100
> 
> 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.

Yes, something like that.  A.k.a. "a race condition".

> 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.

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.

Message #41 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76187 <at> debbugs.gnu.org, eggert <at> cs.ucla.edu
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Thu, 13 Feb 2025 19:23:15 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

>> 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.
>
> Yes, something like that.  A.k.a. "a race condition".
>
>> 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.
>
> 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?

  $ while make -j1 -C test lisp/vc/vc-git-tests ; do continue ; done
  [… 800 iterations later …]
  Running 8 tests (2025-02-13 19:12:17+0100, selector `(not (or (tag :unstable) (tag :nativecomp)))')
     passed  1/8  vc-git-test-annotate-time (0.002258 sec)
  Test vc-git-test-dir-branch-headers backtrace:
    […]
    vc-git-command(t 0 nil "checkout" "-b" "feature/foo" "master")
    […]
    vc-git-test--run("checkout" "-b" "feature/foo" "master")
    […]
  Test vc-git-test-dir-branch-headers condition:
      (error
       "Failed (status 128): git --no-pager checkout -b feature/foo master .")

(That was run on a fresh master checkout, to dispel interference from my
debugging changes.  See previous messages for more logs, in particular
proof that
(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)


At any rate, to hopefully contribute something to the discussion if I
misunderstood your comment: given the goals of this test, wondering if
it would be acceptable to have it invoke vc-git-dir-extra-headers
directly, instead of "pretending to be a user" and invoking vc-dir.  If
vc-dir is indeed responsible for whatever async process is thwarting
that test, cutting that middle man out would work around that (and let
us remove some scaffolding from the test to boot).

(If Git is responsible for that async process, then that change alone
will not help, although the scaffolding simplification would still be
nice TBH)

Message #44 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76187 <at> debbugs.gnu.org, eggert <at> cs.ucla.edu
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Thu, 13 Feb 2025 19:56:26 +0100

[Message part 1 (text/plain, inline)]

Kévin Le Gouguec <kevin.legouguec <at> gmail.com> 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?
>
>   $ while make -j1 -C test lisp/vc/vc-git-tests ; do continue ; done
[… snip demo …]
>
>
> At any rate, to hopefully contribute something to the discussion if I
> misunderstood your comment: given the goals of this test, wondering if
> it would be acceptable to have it invoke vc-git-dir-extra-headers
> directly, instead of "pretending to be a user" and invoking vc-dir.  If
> vc-dir is indeed responsible for whatever async process is thwarting
> that test, cutting that middle man out would work around that (and let
> us remove some scaffolding from the test to boot).

Attaching what I have in mind (including an --ignore-all-space variant).
Mainly FTR and to clarify my yammering ; ATM it is not clear that it
solves the problem.  (≈3k iterations in 12min and counting; 🤞 but I had
to wait an actual hour once or twice to reproduce the error…)

> (If Git is responsible for that async process, then that change alone
> will not help, although the scaffolding simplification would still be
> nice TBH)

[test-extra-headers-directly.patch (text/x-patch, attachment)]

[test-extra-headers-directly-w.patch (text/x-patch, attachment)]

Message #47 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: 76187 <at> debbugs.gnu.org, eggert <at> cs.ucla.edu
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Thu, 13 Feb 2025 22:44:32 +0200

> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
> Cc: eggert <at> cs.ucla.edu,  76187 <at> debbugs.gnu.org
> Date: Thu, 13 Feb 2025 19:23:15 +0100
> 
> Eli Zaretskii <eliz <at> gnu.org> 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?

> (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.

> (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?

Message #50 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76187 <at> debbugs.gnu.org, eggert <at> cs.ucla.edu
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Thu, 13 Feb 2025 22:47:25 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
>> Cc: eggert <at> cs.ucla.edu,  76187 <at> debbugs.gnu.org
>> Date: Thu, 13 Feb 2025 19:23:15 +0100
>> 
>> Eli Zaretskii <eliz <at> gnu.org> 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 🤷)

Message #53 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76187 <at> debbugs.gnu.org, eggert <at> cs.ucla.edu
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Thu, 13 Feb 2025 23:58:21 +0100

[Message part 1 (text/plain, inline)]

Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:

> 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?

4h10min, 66k iterations, no failures.  Stopping the count because this
brain needs zzz's.

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

Done; attached for your consideration.

[0001-Test-vc-git-dir-extra-headers-directly-bug-76187.patch (text/x-patch, attachment)]

Message #56 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76187 <at> debbugs.gnu.org, eggert <at> cs.ucla.edu
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Fri, 14 Feb 2025 08:51:39 +0100

[Message part 1 (text/plain, inline)]

Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:

>>> Eli Zaretskii <eliz <at> gnu.org> 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.

With the attached debugging patch (same as [1], plus binding
vc-command-messages to t), I get these traces when the failure happens:

    Running 8 tests (2025-02-14 08:17:56+0100, selector `(not (or (tag :unstable) (tag :nativecomp)))')
       passed  1/8  vc-git-test-annotate-time (0.002302 sec)
    Running in foreground: git --no-pager clone /tmp/emacs-test-go8R94-vc-git/ /tmp/emacs-test-NVSvcg-vc-git/ .
    Done (status=0): git --no-pager clone /tmp/emacs-test-go8R94-vc-git/ /tmp/emacs-test-NVSvcg-vc-git/ .
    Running in foreground: git --no-pager config --get remote.origin.url
    Done (status=0): git --no-pager config --get remote.origin.url
    Running in background: git --no-pager update-index --refresh .
    Done in background: git --no-pager update-index --refresh .
    Running in background: git --no-pager diff-index --relative -z -M HEAD -- .
    Done in background: git --no-pager diff-index --relative -z -M HEAD -- .
    Running in background: git --no-pager ls-files -z -u -- .
    Done in background: git --no-pager ls-files -z -u -- .
    Running in background: git --no-pager ls-files -z -o --exclude-standard -- .
    Done in background: git --no-pager ls-files -z -o --exclude-standard -- .
    Paused 1 time(s) waiting for vc-dir-busy
    Running in foreground: git --no-pager config --get remote.origin.url
    Done (status=0): git --no-pager config --get remote.origin.url
    Running in background: git --no-pager update-index --refresh .
    Running in foreground: git --no-pager checkout -b feature/foo master .
    err in /tmp/emacs-test-NVSvcg-vc-git/: (error Failed (status 128): git --no-pager checkout -b feature/foo master .)
    (buffer-string:
    fatal: Unable to create '/tmp/emacs-test-NVSvcg-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.

    )

Looking at vc-dir-refresh (invoked from vc-dir-revert-buffer-function,
invoked by the testcase reverting the vc-dir buffer), I'd conclude that
the sequence of events is (lighting a candle to the Muse of Manual
Whitespace Adjustment; citing evidence with "> TRACES")

* vc-git-test--dir-headers
  * revert-buffer
    * vc-dir-refresh
      * vc-dir-headers
        * vc-git-dir-extra-headers (👈 the code under test)
          > Running in foreground: git --no-pager config --get remote.origin.url
          > Done (status=0): git --no-pager config --get remote.origin.url
      * "asynchronous-looking things"
        > Running in background: git --no-pager update-index --refresh .
        (⚠️ no proof of termination) 
* vc-git-test--run "checkout" "-b" "feature/foo" main-branch
  * vc-git-test--wait (lambda () (file-exists-p ".git/index.lock"))
    > 🦗 [no log, ergo no lock file detected]
  * vc-git-command 
    > Running in foreground: git --no-pager checkout -b feature/foo master .
    (⚠️ presumably the concurrent locking happens around here)
    > fatal: Unable to create '/tmp/emacs-test-NVSvcg-vc-git/.git/index.lock': File exists.

I think 'git update-index' is a good suspect: cursory glance at
builtin/update-index.c:cmd_update_index confirms that it unconditionally
does:

	newfd = repo_hold_locked_index(the_repository, &lock_file, 0);

Bottomline:

* maybe we could amend the testcase to sync with this background
command,
* unclear whether we should; barring fault in my logic, I stand by my
opinion in [2] and would prefer to install the patch in that message, on
the grounds that contending with vc-dir's asynchrony is outside the
scope of this test (to wit: testing the extra-headers logic).


[1] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=76187#35
[2] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=76187#53

[more-debug.patch (text/x-patch, attachment)]

Message #59 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>,
 Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 76187 <at> debbugs.gnu.org, eggert <at> cs.ucla.edu
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Fri, 14 Feb 2025 10:07:19 +0200

> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
> Cc: 76187 <at> debbugs.gnu.org,  eggert <at> cs.ucla.edu
> Date: Thu, 13 Feb 2025 23:58:21 +0100
> 
> Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:
> 
> > 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?
> 
> 4h10min, 66k iterations, no failures.  Stopping the count because this
> brain needs zzz's.
> 
> > Let me know if it looks acceptable, I'll add commentary & a changelog
> > entry.
> 
> Done; attached for your consideration.

I'm okay with this if Dmitry agrees.

Message #62 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76187 <at> debbugs.gnu.org, Dmitry Gutov <dmitry <at> gutov.dev>, eggert <at> cs.ucla.edu
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Sat, 15 Feb 2025 11:42:33 +0100

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
>> Cc: 76187 <at> debbugs.gnu.org,  eggert <at> cs.ucla.edu
>> Date: Thu, 13 Feb 2025 23:58:21 +0100
>> 
>> Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:
>> 
>> > 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?
>> 
>> 4h10min, 66k iterations, no failures.  Stopping the count because this
>> brain needs zzz's.
>> 
>> > Let me know if it looks acceptable, I'll add commentary & a changelog
>> > entry.
>> 
>> Done; attached for your consideration.
>
> I'm okay with this if Dmitry agrees.

For the sake of intellectual honesty (and to prove, if need be, that I
am a clown), while I do still prefer getting vc-dir out of the equation
for the purpose of testing vc-git-dir-extra-headers (and so, applying
the patch to which you reply),

I should note that with the alternative attached patch, the test has
been passing 7k times for 1h, with reassuring traces such as:

    Running in foreground: git --no-pager clone /tmp/emacs-test-Aoc9gk-vc-git/ /tmp/emacs-test-qRZ6nt-vc-git/ .
    Done (status=0): git --no-pager clone /tmp/emacs-test-Aoc9gk-vc-git/ /tmp/emacs-test-qRZ6nt-vc-git/ .
    Running in foreground: git --no-pager config --get remote.origin.url
    Done (status=0): git --no-pager config --get remote.origin.url
    Running in background: git --no-pager update-index --refresh .
    Done in background: git --no-pager update-index --refresh .
    Running in background: git --no-pager diff-index --relative -z -M HEAD -- .
    Done in background: git --no-pager diff-index --relative -z -M HEAD -- .
    Running in background: git --no-pager ls-files -z -u -- .
    Done in background: git --no-pager ls-files -z -u -- .
    Running in background: git --no-pager ls-files -z -o --exclude-standard -- .
    Done in background: git --no-pager ls-files -z -o --exclude-standard -- .
    Paused 50 ms waiting for vc-dir-busy
    Running in foreground: git --no-pager checkout -b feature/foo master .

IOW waiting for vc-dir-busy *after* invoking vc-dir seems to work fine,
which makes complete sense, and I am not sure why I never thought about
this in the first place?

Let me know if we favor this approach, in which case I'll add the
finishing touches on that patch (docstring, changelog).

[wait-AFTER-dir-🤦.patch (text/x-patch, attachment)]

Message #65 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>,
 Eli Zaretskii <eliz <at> gnu.org>
Cc: 76187 <at> debbugs.gnu.org, Dmitry Gutov <dmitry <at> gutov.dev>
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Sat, 15 Feb 2025 10:25:29 -0800

On 2025-02-15 02:42, Kévin Le Gouguec wrote:
> waiting for vc-dir-busy*after* invoking vc-dir seems to work fine,
> which makes complete sense

Makes sense to me too. I didn't look at the patch in detail, but did 
notice a "TODO" comment.

Message #68 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 76187 <at> debbugs.gnu.org, Dmitry Gutov <dmitry <at> gutov.dev>,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Sat, 15 Feb 2025 23:17:45 +0100

[Message part 1 (text/plain, inline)]

Paul Eggert <eggert <at> cs.ucla.edu> writes:

> On 2025-02-15 02:42, Kévin Le Gouguec wrote:
>> waiting for vc-dir-busy*after* invoking vc-dir seems to work fine,
>> which makes complete sense
>
> Makes sense to me too. I didn't look at the patch in detail, but did notice a "TODO" comment.

One of these "finishing touches" I mentioned, yup.

Attaching for your consideration:

1. a patch that fixes the vc-dir synchronization,
2. a patch that ditches vc-dir entirely.

Either fixes the issue, as far as I can validate.  I still have a slight
preference for the second one¹.  FWIW, here the test takes ≈330ms with
patch 1, and ≈68ms with patch 2².

Happy to install whichever.


¹ tl;dr vc-dir is a middleman with no added value for the purposes of
this test.

² Which is only worth consideration if you happen to need run that test
repeatedly to reproduce a weird race condition 🥲

[0001-Fix-race-condition-in-vc-git-tests.el-bug-76187.patch (text/x-patch, attachment)]

[0001-Test-vc-git-dir-extra-headers-directly-bug-76187.patch (text/x-patch, attachment)]

Message #71 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: 76187 <at> debbugs.gnu.org, Dmitry Gutov <dmitry <at> gutov.dev>,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Sat, 15 Feb 2025 15:07:36 -0800

On 2025-02-15 14:17, Kévin Le Gouguec wrote:
> Either fixes the issue, as far as I can validate.  I still have a slight
> preference for the second one

... and the second one tests faster, so that's a good argument for going 
with it.

Message #74 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 76187 <at> debbugs.gnu.org, Dmitry Gutov <dmitry <at> gutov.dev>,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Sun, 16 Feb 2025 11:18:58 +0100

Paul Eggert <eggert <at> cs.ucla.edu> writes:

> On 2025-02-15 14:17, Kévin Le Gouguec wrote:
>> Either fixes the issue, as far as I can validate.  I still have a slight
>> preference for the second one
>
> ... and the second one tests faster, so that's a good argument for going with it.

Right.

Waiting on Dmitry's thoughts before installing; OT1H I have no qualms
about this testcase invoking backend functions directly (just noticed
that there is a precedent, e.g. vc-git-test-annotate-time), OTOH maybe
this is not a practice we want to encourage.

Message #77 received at 76187 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>,
 Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 76187 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Mon, 17 Feb 2025 05:07:36 +0200

Hi!

On 16/02/2025 12:18, Kévin Le Gouguec wrote:
> Paul Eggert<eggert <at> cs.ucla.edu> writes:
> 
>> On 2025-02-15 14:17, Kévin Le Gouguec wrote:
>>> Either fixes the issue, as far as I can validate.  I still have a slight
>>> preference for the second one
>> ... and the second one tests faster, so that's a good argument for going with it.
> Right.
> 
> Waiting on Dmitry's thoughts before installing; OT1H I have no qualms
> about this testcase invoking backend functions directly (just noticed
> that there is a precedent, e.g. vc-git-test-annotate-time), OTOH maybe
> this is not a practice we want to encourage.

Thanks for the investigation, and I agree that the patch that "ditches 
vc-dir entirely" is TRT.

The vc-<backend>-tests.el files are supposed (or at least are allowed) 
to test backend-specific functionality.

We currently call vc-dir from vc-bzr-tests as well (helps with code 
coverage) - which BTW has this workaround:

      (while (vc-dir-busy)
        (sit-for 0.1))

If/when we decide to make the bzr tests similarly more isolated, we 
should try to create 1-2 cases in vc-tests.el that exercise the general 
code path. But that's a future problem.

Message #82 received at 76187-done <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 76187-done <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Paul Eggert <eggert <at> cs.ucla.edu>
Subject: Re: bug#76187: vc-git-test-dir-branch-headers failure on Fedora
Date: Mon, 17 Feb 2025 22:11:02 +0100

Dmitry Gutov <dmitry <at> gutov.dev> writes:

>> Paul Eggert<eggert <at> cs.ucla.edu> writes:
>> 
>>> On 2025-02-15 14:17, Kévin Le Gouguec wrote:
>>>> Either fixes the issue, as far as I can validate.  I still have a slight
>>>> preference for the second one
>>> ... and the second one tests faster, so that's a good argument for going with it.
>> Right.
>> Waiting on Dmitry's thoughts before installing; OT1H I have no qualms
>> about this testcase invoking backend functions directly (just noticed
>> that there is a precedent, e.g. vc-git-test-annotate-time), OTOH maybe
>> this is not a practice we want to encourage.
>
> Thanks for the investigation, and I agree that the patch that "ditches vc-dir entirely" is TRT.
>
> The vc-<backend>-tests.el files are supposed (or at least are allowed) to test backend-specific functionality.

Thanks for weighing in on this!  I therefore installed the patch that
invokes vc-git-dir-extra-headers directly.  Boldly closing this now.

> We currently call vc-dir from vc-bzr-tests as well (helps with code coverage) - which BTW has this workaround:
>
>       (while (vc-dir-busy)
>         (sit-for 0.1))

(🤦 Would that I had thought of grepping the tests for vc-dir(-busy),
instead of reinventing the square wheel and wasting everyone's time…)

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.