bug#59064: 29.0.50; build problem git worktree linked to main worktree (repo)

[Philip Kaludercic]

Matt Armstrong <matt@rfc20.org> writes:

> X-Debbugs-CC: Philip Kaludercic <philipk@posteo.net>
>
> Hi Philip, this bug manifests for Gregor as Emacs build error in a
> particular kind of git repository, but it is really a problem with
> `vc-git-mode-line-string' caused by a recent commit of yours to
> `vc-working-revision'.  Could you take a look?

Could this be a duplicate of bug#58709 or bug#59011?  I have argued in
both threads that the commit ought to be reversed.

This particular issue could be resolved by making the code more robust
and removing the hard-coded assumption about how git revisions have to
look like.

> Gregor Zattler <telegraph@gmx.net> writes:
>
>> I will try to bisect.  But since I have no clue regarding the build
>> system I only hope that this will not hit other build failures in
>> between.  This will take days...
>
> Gregor, no need to bisect.  Your most recent instructions helped.  This
> is not a mysterious problem with the build system, but a simpler one
> with a recent regression in Emacs' vc-mode.
>
> The steps to reproduce are simple:
>
> 1) In the root directory of an existing Emacs git repository, run this:
>
>       git switch master
>       git worktree add -d ../b59064
>
>    Note that the -d is important.  This creates a "detached" work tree
>    attached to no branch at all, but just happens to be at the same rev
>    as "master".
>
> 2) cd ../b59064
>
> 3) git status
>
>    Confirm this prints "Not currently on any branch."
>
> 3) emacs -Q (using a recent Emacs built on master, not one built in
>              this new repository)
>
> 3) M-: (setq debug-on-error t)
>
> Edit any file in this repository, I did C-x C-f "INSTALL".  You may then
> need to run M-x vc-mode.
>
> You get the following problem in `vc-git-mode-line-string':
>
> Debugger entered--Lisp error: (args-out-of-range "master" 0 7)
>   
> vc-git-mode-line-string("/home/matt/git/e/b59064/admin/grammars/srecode-tem...")
>   apply(vc-git-mode-line-string 
> "/home/matt/git/e/b59064/admin/grammars/srecode-tem...")
>   vc-call-backend(Git mode-line-string 
> "/home/matt/git/e/b59064/admin/grammars/srecode-tem...")
>   vc-mode-line("/home/matt/git/e/b59064/admin/grammars/srecode-tem..." Git)
>   vc-refresh-state()
>   run-hooks(find-file-hook)
>   after-find-file(nil t)
>   find-file-noselect-1(#<buffer srecode-template.wy> 
> "~/git/e/b59064/admin/grammars/srecode-template.wy" nil nil 
> "~/git/e/b59064/admin/grammars/srecode-template.wy" (67952095 27))
>   find-file-noselect("/home/matt/git/e/b59064/admin/grammars/srecode-tem..." 
> nil nil nil)
>   find-file("/home/matt/git/e/b59064/admin/grammars/srecode-tem...")
>   dired--find-file(find-file 
> "/home/matt/git/e/b59064/admin/grammars/srecode-tem...")
>   
> dired--find-possibly-alternative-file("/home/matt/git/e/b59064/admin/grammars/srecode-tem...")
>   dired-find-file()
>   funcall-interactively(dired-find-file)
>   call-interactively(dired-find-file nil nil)
>   command-execute(dired-find-file)
>
> The `vc-git-mode-line-string' code assumes that if
> `vc-git--symbolic-ref` returns nil then `vc-working-revision' must
> necessarily return a full hex git rev ID, so it unconditionally performs
> the following on that value:
>
>   (substring rev 0 7)
>
> However, Philip Kaludercic's recent commit 307ad210040 changed
> `vc-working-revision' to resolve a hex ID to a symbolic revision if
> possible, so in this case the function returns "master", causing
> `substring' to signal as above.
>
> This is not currently a problem in "normal" git trees because the
> following command works in them:
>
>   git symbolic-ref HEAD
>
> ...and this is what vc-git-mode-line-string normally uses to construct
> the displayed revision used in that line.
>
> In detached worktrees that command fails:
>
>     $ git symbolic-ref HEAD
>     fatal: ref HEAD is not a symbolic ref
>
> ...yet the following works:
>
>     $ git rev-parse HEAD
>     6e5ec085510ccf52ac6cb07c3a1a2778324a1d89
>
> ...and from that we can get to a symbolic name (the new code Philip
> added to `vc-working-revision'):
>
>     $ git name-rev --no-undefined --name-only 
> 6e5ec085510ccf52ac6cb07c3a1a2778324a1d89
>     master
>
> Arguably, `vc-git-mode-line-string' should no longer call
> `vc-working-revision' but instead a lower level variation that must
> return the hex rev id.  Perhaps?