GNU bug report logs - #68183
28.3; vc-dir fails when I have a certain branch checked out

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 68183 in the body.
You can then email your comments to 68183 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: Tom Tromey <tom <at> tromey.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.3; vc-dir fails when I have a certain branch checked out
Date: Sun, 31 Dec 2023 11:59:11 -0700

I use vc-dir frequently.

When I check out one particular branch of my gdb repository, vc-dir
fails with this error:

vc-do-command: Failed (status 2): git --no-pager remote get-url . .

The only thing that might be peculiar about this branch is that it uses
another local branch as its tracking branch.

If I check out other branches in this repository -- ones that track
remote branches -- vc-dir works again.



In GNU Emacs 28.3 (build 1, x86_64-redhat-linux-gnu, GTK+ Version 3.24.38, cairo version 1.17.8)
 of 2023-09-23 built on fd97b702fbea4aa3b70f5e90b3f3f165
Windowing system distributor 'The X.Org Foundation', version 11.0.12201009
System Description: Fedora Linux 38 (Workstation Edition)

Configured using:
 'configure --build=x86_64-redhat-linux-gnu
 --host=x86_64-redhat-linux-gnu --program-prefix=
 --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr
 --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc
 --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64
 --libexecdir=/usr/libexec --localstatedir=/var --runstatedir=/run
 --sharedstatedir=/var/lib --mandir=/usr/share/man
 --infodir=/usr/share/info --with-dbus --with-gif --with-jpeg --with-png
 --with-rsvg --with-tiff --with-xpm --with-x-toolkit=gtk3 --with-gpm=no
 --with-xwidgets --with-modules --with-harfbuzz --with-cairo --with-json
 --with-native-compilation build_alias=x86_64-redhat-linux-gnu
 host_alias=x86_64-redhat-linux-gnu CC=gcc 'CFLAGS=-DMAIL_USE_LOCKF -O2
 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches
 -pipe -Wall -Werror=format-security
 -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS
 -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong
 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic
 -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection
 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer '
 LDFLAGS=-Wl,-z,relro
 PKG_CONFIG_PATH=:/usr/lib64/pkgconfig:/usr/share/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NATIVE_COMP NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS
X11 XDBE XIM XPM XWIDGETS GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Compilation

Minor modes in effect:
  shell-dirtrack-mode: t
  which-function-mode: t
  erc-services-mode: t
  erc-networks-mode: t
  savehist-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  column-number-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t

Load-path shadows:
/home/tromey/.emacs.d/elpa/bubbles-0.5/bubbles hides /usr/share/emacs/28.3/lisp/play/bubbles
/home/tromey/.emacs.d/elpa/dictionary-1.10/dictionary hides /usr/share/emacs/28.3/lisp/net/dictionary

Features:
(shadow emacsbug markdown-mode tcl m4-mode gud novice pcmpl-unix
pcmpl-gnu two-column url-http url-gw url-auth sh-script smie executable
dired-aux rng-xsd xsd-regexp rng-cmpct rng-nxml rng-valid rng-loc
rng-uri rng-parse nxml-parse rng-match rng-dt rng-util rng-pttrn nxml-ns
nxml-mode nxml-outln nxml-rap sgml-mode facemenu nxml-util nxml-enc
xmltok autoconf autoconf-mode gnus-html help-fns radix-tree url-cache
org-bullets org-element avl-tree ol-eww eww xdg url-queue mm-url
ol-rmail ol-mhe ol-irc ol-info ol-gnus nnselect gnus-search eieio-opt
speedbar ezimage dframe ol-docview doc-view image-mode exif ol-bibtex
ol-bbdb ol-w3m ol-doi org-link-doi org ob ob-tangle ob-ref ob-lob
ob-table ob-exp org-macro org-footnote org-src ob-comint org-pcomplete
org-list org-faces org-entities noutline outline org-version
ob-emacs-lisp ob-core ob-eval org-table oc-basic bibtex ol org-keys oc
org-compat org-macs org-loaddefs find-func flow-fill python tramp-sh
term/xterm xterm face-remap goto-addr log-edit vc-annotate mule-util
jka-compr find-dired texinfo texinfo-loaddefs find-file make-mode
log-view pcvs-util vc-mtn vc-hg vc-bzr vc-src vc-sccs vc-svn vc-cvs
vc-rcs copyright pulse ffap scheme mailalias dabbrev supercite regi
mail-hist ggtags hippie-exp etags fileloop generator xref project
bug-reference vc-git cc-mode cc-fonts cc-guess cc-menus cc-cmds
smerge-mode diff diff-mode shr-color mm-archive sort smiley gnus-cite
mail-extr qp gnus-async gnus-bcklg gnus-ml disp-table misearch
multi-isearch gnus-topic nndraft nnmh nnfolder utf-7 gnutls
network-stream nsm gnus-agent gnus-srvr gnus-score score-mode nnvirtual
gnus-msg gnus-art mm-uu mml2015 mm-view mml-smime smime dig nntp
gnus-cache gnus-sum shr kinsoku svg dom gnus-group gnus-undo smtpmail
sendmail gnus-start gnus-dbus dbus gnus-cloud nnimap nnmail mail-source
utf7 netrc nnoo gnus-spec gnus-int gnus-range message rmc puny dired
dired-loaddefs rfc822 mml mml-sec epa derived epg rfc6068 epg-config
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader gnus-win gnus nnheader gnus-util rmail rmail-loaddefs rfc2047
rfc2045 ietf-drums mail-utils mm-util mail-prsvr add-log flyspell ispell
diminish projectile ibuf-macs pcase edmacro kmacro grep compile
text-property-search ibuf-ext ibuffer ibuffer-loaddefs dash appt
diary-lib diary-loaddefs cal-menu calendar cal-loaddefs tramp
tramp-loaddefs trampver tramp-integration files-x tramp-compat shell
pcomplete parse-time ls-lisp which-func imenu minimap autorevert
filenotify cus-load erc-track erc-match erc-services erc-networks
erc-hl-nicks easy-mmode color erc-button erc-fill erc-stamp wid-edit
erc-goodies erc erc-backend iso8601 time-date thingatpt pp format-spec
erc-loaddefs comp comp-cstr rx cl-extra help-mode warnings advice vc-dir
ewoc vc vc-dispatcher cc-styles cc-align cc-engine cc-vars cc-defs
ange-ftp comint ansi-color ring server savehist clang-rename
clang-include-fixer let-alist clang-format xml finder-inf
gdb-shell-autoloads lisppaste-autoloads pydoc-info-autoloads info-look
info cl weblogger-autoloads package browse-url url url-proxy url-privacy
url-expand url-methods url-history url-cookie url-domsuf url-util
mailcap url-handlers url-parse auth-source cl-seq eieio eieio-core
cl-macs eieio-loaddefs password-cache json subr-x map url-vars seq
byte-opt gv bytecomp byte-compile cconv cl-loaddefs cl-lib iso-transl
tooltip eldoc paren electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode mwheel term/x-win x-win term/common-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer cl-generic
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads
xwidget-internal dbusbind inotify dynamic-setting system-font-setting
font-render-setting cairo move-toolbar gtk x-toolkit x multi-tty
make-network-process native-compile emacs)

Memory information:
((conses 16 2511390 280777)
 (symbols 48 68379 41)
 (strings 32 200925 47137)
 (string-bytes 1 9410331)
 (vectors 16 109364)
 (vector-slots 8 3054321 332806)
 (floats 8 532 632)
 (intervals 56 469509 4701)
 (buffers 992 1022))

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Tom Tromey <tom <at> tromey.com>
Cc: 68183 <at> debbugs.gnu.org
Subject: Re: bug#68183: 28.3;
 vc-dir fails when I have a certain branch checked out
Date: Sun, 31 Dec 2023 21:34:27 +0200

> From: Tom Tromey <tom <at> tromey.com>
> Date: Sun, 31 Dec 2023 11:59:11 -0700
> 
> 
> I use vc-dir frequently.
> 
> When I check out one particular branch of my gdb repository, vc-dir
> fails with this error:
> 
> vc-do-command: Failed (status 2): git --no-pager remote get-url . .
> 
> The only thing that might be peculiar about this branch is that it uses
> another local branch as its tracking branch.
> 
> If I check out other branches in this repository -- ones that track
> remote branches -- vc-dir works again.

Thanks, but I think we'd appreciate a reproducible recipe for this:
how can one create a Git repository which can be used to reproduce
this issue?

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

From: Tom Tromey <tom <at> tromey.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Tom Tromey <tom <at> tromey.com>, 68183 <at> debbugs.gnu.org
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Sun, 31 Dec 2023 13:14:42 -0700

Eli> Thanks, but I think we'd appreciate a reproducible recipe for this:
Eli> how can one create a Git repository which can be used to reproduce
Eli> this issue?

This worked for me:

$ cd ~/Emacs/trunk
# This is my Emacs git repository
$ git checkout --track -b vc-dir-bug master
branch 'vc-dir-bug' set up to track 'master'.
Switched to a new branch 'vc-dir-bug'


Now invoke vc-dir on that directory.

Tom

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

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Tom Tromey <tom <at> tromey.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68183 <at> debbugs.gnu.org
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Wed, 03 Jan 2024 10:46:39 +0100

Tom Tromey <tom <at> tromey.com> writes:

> Eli> Thanks, but I think we'd appreciate a reproducible recipe for this:
> Eli> how can one create a Git repository which can be used to reproduce
> Eli> this issue?
>
> This worked for me:
>
> $ cd ~/Emacs/trunk
> # This is my Emacs git repository
> $ git checkout --track -b vc-dir-bug master
> branch 'vc-dir-bug' set up to track 'master'.
> Switched to a new branch 'vc-dir-bug'
>
>
> Now invoke vc-dir on that directory.

I can reproduce; IIUC the salient point is setting start-point to a
local revision when calling git checkout, by opposition to
e.g. origin/master.  Continuing off of your recipe:

$ git branch --set-upstream-to origin/master

Then M-x vc-dir works again.

IIUC, to display

  Remote     : https://git.savannah.gnu.org/git/emacs.git

vc-git-dir-extra-headers runs

1. git config branch.vc-dir-bug.remote ⇒ "."
2. (vc-git-repository-url "[… EMACS DIR …]" ".")
    1. git config remote...url ⇒ error

git-config(1) says that branch.<name>.remote is "." when <name> is
tracking a local branch, whereas branch.<name>.merge points to the local
branch 'git pull' will resync with.  Wonder what TRT would be for the
purposes of vc-dir?

(1) Drop the "Remote" header: the current branch is not sync'd with a
remote branch, after all.

(2) Print "Remote: https://git.savannah.gnu.org/git/emacs.git" by making
vc-git-repository-url fall back to remote.origin.url when remote-name is
".".

(3) Print "Remote: master" by making vc-git-dir-extra-headers fall back
to branch.<name>.merge when .remote is ".".

(4) Make vc-git-dir-extra-headers fall back to
branch.<branch.<name>.merge>.remote.  In our example, that would yield
"Remote: https://git.savannah.gnu.org/git/emacs.git", but in general
that seems unreliable, since that remote could be "." as well, and
nothing prevents cycles AFAIU.

IMO (3) would be the most robust, though maybe confusing (calling a
local branch "remote"); (2) makes sense as well since
vc-git-repository-url already falls back to remote.origin.url when
remote-name is nil.

(1) sounds trivially "robust" and "not too incorrect", but maybe not the
most helpful.  (4) is under-specified and I'm not convinced it is
possible to make it generally useful.


Hope I've not mis-diagnosed the problem; apologies for the noise if so.

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

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Tom Tromey <tom <at> tromey.com>
Cc: Dmitry Gutov <dmitry <at> gutov.dev>, Eli Zaretskii <eliz <at> gnu.org>,
 68183 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Mon, 12 Feb 2024 09:08:50 +0100

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

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

> Tom Tromey <tom <at> tromey.com> writes:
>
>> Eli> Thanks, but I think we'd appreciate a reproducible recipe for this:
>> Eli> how can one create a Git repository which can be used to reproduce
>> Eli> this issue?
>>
>> This worked for me:
>>
>> $ cd ~/Emacs/trunk
>> # This is my Emacs git repository
>> $ git checkout --track -b vc-dir-bug master
>> branch 'vc-dir-bug' set up to track 'master'.
>> Switched to a new branch 'vc-dir-bug'
>>
>>
>> Now invoke vc-dir on that directory.
>

[…]

> git-config(1) says that branch.<name>.remote is "." when <name> is
> tracking a local branch, whereas branch.<name>.merge points to the local
> branch 'git pull' will resync with.  Wonder what TRT would be for the
> purposes of vc-dir?

Here's a patch.  tl;dr

(1) When branch.<name>.remote is ".", display…

    > Remote    : none (tracking local branch)

    … instead of raising an error.

(2) When branch.<name>.merge is set to BRANCH, display…

    > Tracking  : BRANCH

(3) Add tests, because why not.

CC'ing Dmitry and Juri, whom my brain associates with vc-git maintenance
(rightly or wrongly; apologies for the noise if the latter).

I might be slow to respond for the next couple of weeks, so please feel
free to adapt or dismiss the patch as convenient.  FWIW, off the top of
my head,

* the test should probably have a (skip-unless (have-git-or-something)),
* maybe "none (tracking local branch)" is not informative and we should
  ditch it,
  * maybe we should fall back to "origin", like vc-git-repository-url
    does,
* rushed the ChangeLog entry; vc-git-test--run should also be declared
  as a "new helper" (and maybe I should spell out that I used it to not
  have to depend on vc-git-- internal functions),
* maybe the new header deserves a NEWS entry.

I can tackle any of the above (and any other feedback), though again, if
someone has a vision and I don't answer in a timely manner, don't wait
on me.

[0001-Fix-vc-dir-when-remote-Git-branch-is-local.patch (text/x-patch, attachment)]

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

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Tom Tromey <tom <at> tromey.com>
Cc: Dmitry Gutov <dmitry <at> gutov.dev>, Eli Zaretskii <eliz <at> gnu.org>,
 68183 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Wed, 14 Feb 2024 20:56:28 +0100

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

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

> Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:
>
>> Tom Tromey <tom <at> tromey.com> writes:
>>
>>> Eli> Thanks, but I think we'd appreciate a reproducible recipe for this:
>>> Eli> how can one create a Git repository which can be used to reproduce
>>> Eli> this issue?
>>>
>>> This worked for me:
>>>
>>> $ cd ~/Emacs/trunk
>>> # This is my Emacs git repository
>>> $ git checkout --track -b vc-dir-bug master
>>> branch 'vc-dir-bug' set up to track 'master'.
>>> Switched to a new branch 'vc-dir-bug'
>>>
>>>
>>> Now invoke vc-dir on that directory.
>>

[…]

> Here's a patch.  […]

And here's another revision, addressing most of the points below.
WDY'allT?

> * the test should probably have a (skip-unless (have-git-or-something)),

Done.

> * maybe "none (tracking local branch)" is not informative and we should
>   ditch it,
>   * maybe we should fall back to "origin", like vc-git-repository-url
>     does,

FWIW, the current patch will show

    Branch     : vc-dir-tracking-branch
    Tracking   : origin/master
    Remote     : https://git.savannah.gnu.org/git/emacs.git

for my checkout of this work-in-progress patch, and

    Branch     : vc-dir-bug
    Tracking   : master
    Remote     : none (tracking local branch)

for a checkout made following Tom's recipe, and

    Branch     : trunk

for a fresh 'git init' with just a default branch.

OT1H "none (tracking local branch)" is redundant with "Tracking" not
being prefixed with "origin"; OTOH

* stripping "Remote" altogether might confuse users - at least "tracking
  local branch" hints at what's going on,

* Falling back to origin's URL might also cause confusion: users might
  then expect 'vc-pull' to fetch changes from that URL, which is not the
  case.

So all in all I think the above is reasonably useful.

> * rushed the ChangeLog entry; vc-git-test--run should also be declared
>   as a "new helper" (and maybe I should spell out that I used it to not
>   have to depend on vc-git-- internal functions),

Done.

> * maybe the new header deserves a NEWS entry.

Maybe?

[0001-Fix-vc-dir-when-remote-Git-branch-is-local.patch (text/x-diff, attachment)]

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

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Tom Tromey <tom <at> tromey.com>
Cc: Dmitry Gutov <dmitry <at> gutov.dev>, Eli Zaretskii <eliz <at> gnu.org>,
 68183 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Wed, 13 Mar 2024 21:03:51 +0100

Hello folks,

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

> And here's another revision, addressing most of the points below.
> WDY'allT?
>

[…]

>> * maybe "none (tracking local branch)" is not informative and we should
>>   ditch it,
>>   * maybe we should fall back to "origin", like vc-git-repository-url
>>     does,
>
> FWIW, the current patch will show
>
>     Branch     : vc-dir-tracking-branch
>     Tracking   : origin/master
>     Remote     : https://git.savannah.gnu.org/git/emacs.git
>
> for my checkout of this work-in-progress patch, and
>
>     Branch     : vc-dir-bug
>     Tracking   : master
>     Remote     : none (tracking local branch)
>
> for a checkout made following Tom's recipe, and
>
>     Branch     : trunk
>
> for a fresh 'git init' with just a default branch.

Wondering if there's anything I can do to help with review, or if I
should install this.  OT1H I don't want to be pushy, OTOH I should be in
the area to answer for any collateral damage.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>,
 Tom Tromey <tom <at> tromey.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68183 <at> debbugs.gnu.org,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Fri, 15 Mar 2024 04:57:08 +0200

Hi!

Sorry about the late reply.

It seems like you've done a fair amount of testing, both manual and 
automated - thanks, more tests are welcome.

On 14/02/2024 21:56, Kévin Le Gouguec wrote:
> And here's another revision, addressing most of the points below.
> WDY'allT?
> 
>> * the test should probably have a (skip-unless (have-git-or-something)),
> Done.
> 
>> * maybe "none (tracking local branch)" is not informative and we should
>>    ditch it,
>>    * maybe we should fall back to "origin", like vc-git-repository-url
>>      does,
> FWIW, the current patch will show
> 
>      Branch     : vc-dir-tracking-branch
>      Tracking   : origin/master
>      Remote     :https://git.savannah.gnu.org/git/emacs.git
> 
> for my checkout of this work-in-progress patch, and
> 
>      Branch     : vc-dir-bug
>      Tracking   : master
>      Remote     : none (tracking local branch)
> 
> for a checkout made following Tom's recipe, and
> 
>      Branch     : trunk
> 
> for a fresh 'git init' with just a default branch.

IIUC you're adding the new "Tracking" header to the output? That seems 
like it should be helpful.

Is there a way that we could/should optimize the display? I.e., I guess 
the most common case will be something like:

      Branch     : foo-bar
      Tracking   : origin/foo-bar

which is not bad, but might be less useful than indicating that the 
current branch does not track anything (and so the next 'git push' 
should come with '-u'), or tracks a differently named branch. It might 
be more ergonomic to emphasize "irregular" scenarios and maybe even save 
on the extra line in the "common" one.

Just a thought. Not something that needs to be addressed right now. And 
I might as well be off the mark here.

> OT1H "none (tracking local branch)" is redundant with "Tracking" not
> being prefixed with "origin"; OTOH
> 
> * stripping "Remote" altogether might confuse users - at least "tracking
>    local branch" hints at what's going on,
> 
> * Falling back to origin's URL might also cause confusion: users might
>    then expect 'vc-pull' to fetch changes from that URL, which is not the
>    case.

That seems fine.

> So all in all I think the above is reasonably useful.
> 
>> * rushed the ChangeLog entry; vc-git-test--run should also be declared
>>    as a "new helper" (and maybe I should spell out that I used it to not
>>    have to depend on vc-git-- internal functions),
> Done.
> 
>> * maybe the new header deserves a NEWS entry.
> Maybe?

It wouldn't hurt. Up to you.

Anyway, I think the patch is good to go. Please feel free to install it; 
whatever cosmetic changes we might like to add could be done later.

Message #29 received at 68183 <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: Eli Zaretskii <eliz <at> gnu.org>, Tom Tromey <tom <at> tromey.com>,
 68183 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Sat, 16 Mar 2024 18:56:32 +0100

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

> Sorry about the late reply.

(Not at all, thanks for taking a look 🙏)

>> FWIW, the current patch will show
>>      Branch     : vc-dir-tracking-branch
>>      Tracking   : origin/master
>>      Remote     :https://git.savannah.gnu.org/git/emacs.git
>> for my checkout of this work-in-progress patch, and
>>      Branch     : vc-dir-bug
>>      Tracking   : master
>>      Remote     : none (tracking local branch)
>> for a checkout made following Tom's recipe, and
>>      Branch     : trunk
>> for a fresh 'git init' with just a default branch.
>
> IIUC you're adding the new "Tracking" header to the output? That seems like it should be helpful.
>
> Is there a way that we could/should optimize the display? I.e., I guess the most common case will be something like:
>
>       Branch     : foo-bar
>       Tracking   : origin/foo-bar

Right, the current patch indeed shows this for a common-case clone of
the Emacs repo:

    VC backend : Git
    Working dir: ~/src/emacs/master/
    Branch     : master
    Tracking   : origin/master
    Remote     : https://git.savannah.gnu.org/git/emacs.git

> which is not bad, but might be less useful than indicating that the current branch does not track anything (and so the next 'git push' should come with '-u'), or tracks a differently named branch. It might be more ergonomic to emphasize "irregular" scenarios and maybe even save on the extra line in the "common" one.

Good food for thought.

Re. optimizing the display (which I interpret as reducing redundant
information): as someone who often works with multiple remotes, seeing
"Branch: FOO ; Tracking: origin/FOO" is actually useful, so I wouldn't
want to remove the "tracking" bit unconditionally.  OTOH it could surely
be condensed to a single line, say

    Branch     : master (tracking: origin/master)

Likewise, in the local-tracking-branch case, we could go from

    Branch:    : vc-dir-bug
    Tracking   : master
    Remote     : none (tracking local branch)

to just

    Branch:    : vc-dir-bug (tracking: local master)

Re. emphasizing irregular scenarios, specifically those where 'git push'
will require '-u': I like the idea, but I am wary of us getting lost in
the weeds second-guessing Git's intentions.  E.g. even when
branch.foo.merge and branch.foo.remote are unset, 'git push' can still
be called without '-u' if push.default is 'current' and
remote.pushDefault is set (whereas 'git pull' will fail).

> Just a thought. Not something that needs to be addressed right now. And I might as well be off the mark here.

I agree it's worth thinking about.  The Right Solution™ would probably
come with user options to let users fine-tune which details they care
about?  It would be interesting to survey Git UIs to see how they
approach this (FWIW I am partial to Magit's presentation, but I have not
studied how it handles all the corner cases we considered).

>>> * maybe the new header deserves a NEWS entry.
>> Maybe?
>
> It wouldn't hurt. Up to you.
>
> Anyway, I think the patch is good to go. Please feel free to install it; whatever cosmetic changes we might like to add could be done later.

ACK.  I might go a head and install then (after polishing the changelog,
i.e. re-filling and scrubbing Unicode ellipses) in the spirit of getting
the original issue fixed; perhaps worth holding off on the NEWS entry
until we decide how exactly things should look.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Tom Tromey <tom <at> tromey.com>,
 68183 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Sun, 17 Mar 2024 03:06:36 +0200

On 16/03/2024 19:56, Kévin Le Gouguec wrote:

>> IIUC you're adding the new "Tracking" header to the output? That seems like it should be helpful.
>>
>> Is there a way that we could/should optimize the display? I.e., I guess the most common case will be something like:
>>
>>        Branch     : foo-bar
>>        Tracking   : origin/foo-bar
> 
> Right, the current patch indeed shows this for a common-case clone of
> the Emacs repo:
> 
>      VC backend : Git
>      Working dir: ~/src/emacs/master/
>      Branch     : master
>      Tracking   : origin/master
>      Remote     : https://git.savannah.gnu.org/git/emacs.git
> 
>> which is not bad, but might be less useful than indicating that the current branch does not track anything (and so the next 'git push' should come with '-u'), or tracks a differently named branch. It might be more ergonomic to emphasize "irregular" scenarios and maybe even save on the extra line in the "common" one.
> 
> Good food for thought.
> 
> Re. optimizing the display (which I interpret as reducing redundant
> information): as someone who often works with multiple remotes, seeing
> "Branch: FOO ; Tracking: origin/FOO" is actually useful, so I wouldn't
> want to remove the "tracking" bit unconditionally.  OTOH it could surely
> be condensed to a single line, say
> 
>      Branch     : master (tracking: origin/master)

Yeah, something like this could work. I was imagining pseudo-graphical

     Branch     : master -> origin/master

, but it's good to have words. Maybe drop the last colon...

> Likewise, in the local-tracking-branch case, we could go from
> 
>      Branch:    : vc-dir-bug
>      Tracking   : master
>      Remote     : none (tracking local branch)
> 
> to just
> 
>      Branch:    : vc-dir-bug (tracking: local master)

I would say that the local-tracking scenario is a minority, and so it's 
not as important to optimize, but the above makes sense. But the word 
"local" is very close to "master", while the latter is a special string 
which should probably somehow stand out. So the "unoptimized" version 
might still have its advantages:

      Branch:    : vc-dir-bug (tracking master)
      Remote     : none (local branch)

or

      Branch:    : vc-dir-bug (-> master)
      Remote     : none (local branch)

or

      Branch:    : vc-dir-bug (tracking -> master)
      Remote     : none (local branch)

> Re. emphasizing irregular scenarios, specifically those where 'git push'
> will require '-u': I like the idea, but I am wary of us getting lost in
> the weeds second-guessing Git's intentions.  E.g. even when
> branch.foo.merge and branch.foo.remote are unset, 'git push' can still
> be called without '-u' if push.default is 'current' and
> remote.pushDefault is set (whereas 'git pull' will fail).

Okay, if you're sure. A (no upstream) note might make sense in the above 
scenario too, but I don't have a strong opinion. We could also make a 
user option later, if the new behavior makes sense for most usage habits 
but not all.

>> Just a thought. Not something that needs to be addressed right now. And I might as well be off the mark here.
> 
> I agree it's worth thinking about.  The Right Solution™ would probably
> come with user options to let users fine-tune which details they care
> about?  It would be interesting to survey Git UIs to see how they
> approach this (FWIW I am partial to Magit's presentation, but I have not
> studied how it handles all the corner cases we considered).

Magit, meaning just one line for Head: and another for Merge:?

>>>> * maybe the new header deserves a NEWS entry.
>>> Maybe?
>>
>> It wouldn't hurt. Up to you.
>>
>> Anyway, I think the patch is good to go. Please feel free to install it; whatever cosmetic changes we might like to add could be done later.
> 
> ACK.  I might go a head and install then (after polishing the changelog,
> i.e. re-filling and scrubbing Unicode ellipses) in the spirit of getting
> the original issue fixed; perhaps worth holding off on the NEWS entry
> until we decide how exactly things should look.

I'm okay with that.

Message #35 received at 68183 <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: Eli Zaretskii <eliz <at> gnu.org>, Tom Tromey <tom <at> tromey.com>,
 68183 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Sun, 17 Mar 2024 19:09:42 +0100

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

>> Re. optimizing the display (which I interpret as reducing redundant
>> information): as someone who often works with multiple remotes, seeing
>> "Branch: FOO ; Tracking: origin/FOO" is actually useful, so I wouldn't
>> want to remove the "tracking" bit unconditionally.  OTOH it could surely
>> be condensed to a single line, say
>>      Branch     : master (tracking: origin/master)
>
> Yeah, something like this could work. I was imagining pseudo-graphical
>
>      Branch     : master -> origin/master
>
> , but it's good to have words. Maybe drop the last colon...

Right, feels like we have some horizontal space to work with, so we can
spend the extra word if it conveys more meaning than an arrow (and yeah,
OTOH that last colon does not bring much).

>> Likewise, in the local-tracking-branch case, we could go from
>>      Branch:    : vc-dir-bug
>>      Tracking   : master
>>      Remote     : none (tracking local branch)
>> to just
>>      Branch:    : vc-dir-bug (tracking: local master)
>
> I would say that the local-tracking scenario is a minority, and so it's not as important to optimize, but the above makes sense. But the word "local" is very close to "master", while the latter is a special string which should probably somehow stand out. So the "unoptimized" version might still have its advantages:
>
>       Branch:    : vc-dir-bug (tracking master)
>       Remote     : none (local branch)

That looks sensible.  If I were to argue in favor of keeping "local"
juxtaposed to the tracking branch, I'd say that in the _absence_ of an
explicit qualifier in "Branch: vc-dir-bug (tracking master)", my brain
might "default" to assuming we are tracking the remote "master"; a
solution to let branch names stand out might then be faces:
e.g. vc-git-log-view-mode paints them with change-log-list…

But I don't know if it's a very powerful argument.

>>> Just a thought. Not something that needs to be addressed right now. And I might as well be off the mark here.
>> I agree it's worth thinking about.  The Right Solution™ would probably
>> come with user options to let users fine-tune which details they care
>> about?  It would be interesting to survey Git UIs to see how they
>> approach this (FWIW I am partial to Magit's presentation, but I have not
>> studied how it handles all the corner cases we considered).
>
> Magit, meaning just one line for Head: and another for Merge:?

So, given

𝒷   ≔ current branch
𝓂   ≔ branch.𝒷.merge
𝓇   ≔ branch.𝒷.remote
𝓅   ≔ branch.𝒷.pushRemote or remote.pushDefault

By default, magit-status shows:

"Head:"                 𝒷, or the current commit when detached
"Merge:" (or "Rebase:") 𝓇/𝓂, or just 𝓂 if 𝓇 = "."
"Push:"                 𝓅/𝒷, appending "does not exist" if applicable

(And another header related to tags I'm going to ignore for now)

Of note, Magit offers an option (magit-status-headers-hook) to let users
control which of these (and more) to display.

One of the available headers that is disabled by default shows

"Remote: ℛ <remote.ℛ.url>"

where ℛ is 𝓇, or "origin" if this branch's remote is unset or ".", or
"the first remote in alphabetic order" if "origin" does not exist.

(
  Also of note, IMO, is the branch-variable-configuration menu (the
  magit-branch-configure transient), which looks like this:

      Configure vc-dir-bug
       d branch.vc-dir-bug.description unset
       u branch.vc-dir-bug.merge refs/heads/master
         branch.vc-dir-bug.remote .
       r branch.vc-dir-bug.rebase [true|false|default:false]
       p branch.vc-dir-bug.pushRemote [hirondell|origin|savannah-rw|vps|remote.pushDefault:vps]

      Configure repository defaults
       R pull.rebase [true|false|default:false]
       P remote.pushDefault [hirondell|origin|savannah-rw|vps]
       b Update default branch

      Configure branch creation
       a m branch.autoSetupMerge [always|true|false|default:true]
       a r branch.autoSetupRebase [always|local|remote|never|default:never]
      __
  (Not pictured: the currently active value in [foo|bar|baz] is
  highlighted with a face; the keybindings on the left cycle between
  alternatives)

  No idea how/where that would fit into VC, but golly is it a neat UI to
  work with, so I thought it deserved a shout-out.
)

>>>>> * maybe the new header deserves a NEWS entry.
>>>> Maybe?
>>>
>>> It wouldn't hurt. Up to you.
>>>
>>> Anyway, I think the patch is good to go. Please feel free to install it; whatever cosmetic changes we might like to add could be done later.
>> ACK.  I might go a head and install then (after polishing the changelog,
>> i.e. re-filling and scrubbing Unicode ellipses) in the spirit of getting
>> the original issue fixed; perhaps worth holding off on the NEWS entry
>> until we decide how exactly things should look.
>
> I'm okay with that.

(Pushed 📨)

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Tom Tromey <tom <at> tromey.com>,
 68183 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Mon, 18 Mar 2024 17:26:11 +0200

On 17/03/2024 20:09, Kévin Le Gouguec wrote:

>> I would say that the local-tracking scenario is a minority, and so it's not as important to optimize, but the above makes sense. But the word "local" is very close to "master", while the latter is a special string which should probably somehow stand out. So the "unoptimized" version might still have its advantages:
>>
>>        Branch:    : vc-dir-bug (tracking master)
>>        Remote     : none (local branch)
> 
> That looks sensible.  If I were to argue in favor of keeping "local"
> juxtaposed to the tracking branch, I'd say that in the _absence_ of an
> explicit qualifier in "Branch: vc-dir-bug (tracking master)", my brain
> might "default" to assuming we are tracking the remote "master"; a
> solution to let branch names stand out might then be faces:
> e.g. vc-git-log-view-mode paints them with change-log-list…
> 
> But I don't know if it's a very powerful argument.

I figured that when "local" is on the next line already, it's easy 
enough to notice. Anyway, it's up to you, I think.

>>>> Just a thought. Not something that needs to be addressed right now. And I might as well be off the mark here.
>>> I agree it's worth thinking about.  The Right Solution™ would probably
>>> come with user options to let users fine-tune which details they care
>>> about?  It would be interesting to survey Git UIs to see how they
>>> approach this (FWIW I am partial to Magit's presentation, but I have not
>>> studied how it handles all the corner cases we considered).
>>
>> Magit, meaning just one line for Head: and another for Merge:?
> 
> So, given
> 
> 𝒷   ≔ current branch
> 𝓂   ≔ branch.𝒷.merge
> 𝓇   ≔ branch.𝒷.remote
> 𝓅   ≔ branch.𝒷.pushRemote or remote.pushDefault
> 
> By default, magit-status shows:
> 
> "Head:"                 𝒷, or the current commit when detached
> "Merge:" (or "Rebase:") 𝓇/𝓂, or just 𝓂 if 𝓇 = "."
> "Push:"                 𝓅/𝒷, appending "does not exist" if applicable

So Magit also prints just "master" is it's a local tracking branch and 
something like "origin/master" when it belongs to a remote? Going with 
that approach, we'd seem justified to print

      Branch:    : vc-dir-bug (tracking master)

in the former case and

      Branch:    : vc-dir-bug (tracking origin/master)

in the latter.

> (And another header related to tags I'm going to ignore for now)
> 
> Of note, Magit offers an option (magit-status-headers-hook) to let users
> control which of these (and more) to display.
> 
> One of the available headers that is disabled by default shows
> 
> "Remote: ℛ <remote.ℛ.url>"
> 
> where ℛ is 𝓇, or "origin" if this branch's remote is unset or ".", or
> "the first remote in alphabetic order" if "origin" does not exist.

Meaning it prepends the remote's url with remote's name, if that line is 
configured to be shown. We could do that too.

> (
>    Also of note, IMO, is the branch-variable-configuration menu (the
>    magit-branch-configure transient), which looks like this:
> 
>        Configure vc-dir-bug
>         d branch.vc-dir-bug.description unset
>         u branch.vc-dir-bug.merge refs/heads/master
>           branch.vc-dir-bug.remote .
>         r branch.vc-dir-bug.rebase [true|false|default:false]
>         p branch.vc-dir-bug.pushRemote [hirondell|origin|savannah-rw|vps|remote.pushDefault:vps]
> 
>        Configure repository defaults
>         R pull.rebase [true|false|default:false]
>         P remote.pushDefault [hirondell|origin|savannah-rw|vps]
>         b Update default branch
> 
>        Configure branch creation
>         a m branch.autoSetupMerge [always|true|false|default:true]
>         a r branch.autoSetupRebase [always|local|remote|never|default:never]
>        __
>    (Not pictured: the currently active value in [foo|bar|baz] is
>    highlighted with a face; the keybindings on the left cycle between
>    alternatives)
> 
>    No idea how/where that would fit into VC, but golly is it a neat UI to
>    work with, so I thought it deserved a shout-out.
> )

Magit is definitely a great UI to learn Git's capabilities.

>>>>>> * maybe the new header deserves a NEWS entry.
>>>>> Maybe?
>>>>
>>>> It wouldn't hurt. Up to you.
>>>>
>>>> Anyway, I think the patch is good to go. Please feel free to install it; whatever cosmetic changes we might like to add could be done later.
>>> ACK.  I might go a head and install then (after polishing the changelog,
>>> i.e. re-filling and scrubbing Unicode ellipses) in the spirit of getting
>>> the original issue fixed; perhaps worth holding off on the NEWS entry
>>> until we decide how exactly things should look.
>>
>> I'm okay with that.
> 
> (Pushed 📨)

Thanks!

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

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: 68183 <at> debbugs.gnu.org
Cc: Dmitry Gutov <dmitry <at> gutov.dev>, Eli Zaretskii <eliz <at> gnu.org>,
 Tom Tromey <tom <at> tromey.com>, Sean Whitton <spwhitton <at> spwhitton.name>,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Wed, 07 Aug 2024 16:25:31 +0200

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

Heya,

Have spent cycles on this on-and-off these past few months; finally have
something worth discussing, I think 🤞

To recap where we stand, AFAIU: the reported vc-dir bug has been fixed
(in time for the emacs-30 branch), but the changes could feel intrusive,
since a new vc-dir header was added ("Tracking") that some users may not
care for.

I've now drafted a user option to give users more control over this new
header; see patch #3 in the attached series.

The first two patches are yak-shaving: patch #1 adds regression tests,
patch #2 splits vc-git-dir-extra-headers into more manageable chunks
(pure refactoring, no functional change intended).

Also attaching a 'squashed.patch' if that helps review.

About patch #2, CC'ing Sean Whitton for perspective on
vc-git--cmds-in-progress: I was puzzled by the function supporting many
commands (rebase, am, merge, bisect), whereas AFAICT its sole user only
heeds 'bisect & 'rebase.  Wondering if I've missed other in-tree uses,
or if we should add headers for 'am and 'merge, "while in there".

Curious what y'all think.  OT1H not sure an alist is the best UX, OTOH
struggled to keep option names concise otherwise.

[0001-Test-more-vc-dir-scenarios-with-Git-bug-68183.patch (text/x-diff, attachment)]

[0002-Split-vc-git-dir-extra-headers-into-more-manageable-.patch (text/x-diff, attachment)]

[0003-Let-users-choose-when-and-how-to-display-Git-trackin.patch (text/x-diff, attachment)]

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

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: Dmitry Gutov <dmitry <at> gutov.dev>, Eli Zaretskii <eliz <at> gnu.org>,
 68183 <at> debbugs.gnu.org, Tom Tromey <tom <at> tromey.com>,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Thu, 08 Aug 2024 08:32:26 +0800

Hello,

On Wed 07 Aug 2024 at 04:25pm +02, Kévin Le Gouguec wrote:

> About patch #2, CC'ing Sean Whitton for perspective on
> vc-git--cmds-in-progress: I was puzzled by the function supporting many
> commands (rebase, am, merge, bisect), whereas AFAICT its sole user only
> heeds 'bisect & 'rebase.  Wondering if I've missed other in-tree uses,
> or if we should add headers for 'am and 'merge, "while in there".

I have some WIP which uses this function for some other purposes.
It's a pretty cleanly identifiable piece of functionality so I would
like to keep it separate.

I would be happy if you were to add detecting some other operations
there, for sure, if you are confident in your detection methods.

-- 
Sean Whitton

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

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: Dmitry Gutov <dmitry <at> gutov.dev>, Eli Zaretskii <eliz <at> gnu.org>,
 68183 <at> debbugs.gnu.org, Tom Tromey <tom <at> tromey.com>,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Thu, 08 Aug 2024 09:07:43 +0200

Sean Whitton <spwhitton <at> spwhitton.name> writes:

>> About patch #2, CC'ing Sean Whitton for perspective on
>> vc-git--cmds-in-progress: I was puzzled by the function supporting many
>> commands (rebase, am, merge, bisect), whereas AFAICT its sole user only
>> heeds 'bisect & 'rebase.  Wondering if I've missed other in-tree uses,
>> or if we should add headers for 'am and 'merge, "while in there".
>
> I have some WIP which uses this function for some other purposes.
> It's a pretty cleanly identifiable piece of functionality so I would
> like to keep it separate.

Gotcha, thanks for weighing in!  Then I'll work on another revision of
the series to keep that function intact.

> I would be happy if you were to add detecting some other operations
> there, for sure, if you are confident in your detection methods.

Do you mean that the detection methods for 'am and 'merge currently in
place…

    (when (file-exists-p
	   (expand-file-name "rebase-apply/applying" gitdir))
      (push 'am cmds))
    (when (file-exists-p (expand-file-name "MERGE_HEAD" gitdir))
      (push 'merge cmds))

… are unreliable somehow?  I wasn't planning on detecting more
operations, merely adding vc-dir headers for these two, since despite
vc-git--cmds-in-progress picking these operations up,
vc-git-dir-extra-headers ignores them 😶

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: Dmitry Gutov <dmitry <at> gutov.dev>, Eli Zaretskii <eliz <at> gnu.org>,
 68183 <at> debbugs.gnu.org, Tom Tromey <tom <at> tromey.com>,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Thu, 08 Aug 2024 20:02:00 +0800

Hello,

On Thu 08 Aug 2024 at 09:07am +02, Kévin Le Gouguec wrote:

> Do you mean that the detection methods for 'am and 'merge currently in
> place…
>
>     (when (file-exists-p
> 	   (expand-file-name "rebase-apply/applying" gitdir))
>       (push 'am cmds))
>     (when (file-exists-p (expand-file-name "MERGE_HEAD" gitdir))
>       (push 'merge cmds))
>
> … are unreliable somehow?  I wasn't planning on detecting more
> operations, merely adding vc-dir headers for these two, since despite
> vc-git--cmds-in-progress picking these operations up,
> vc-git-dir-extra-headers ignores them 😶

Oh sorry, I misread.  Those tests are reliable.  Yes -- if you think it
would be helpful to have them displayed in vc-dir, it makes sense to me
to add them.

-- 
Sean Whitton

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>,
 68183 <at> debbugs.gnu.org
Cc: Eli Zaretskii <eliz <at> gnu.org>, Tom Tromey <tom <at> tromey.com>,
 Sean Whitton <spwhitton <at> spwhitton.name>, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Tue, 13 Aug 2024 04:32:40 +0300

Hi Kevin,

On 07/08/2024 17:25, Kévin Le Gouguec wrote:
> Heya,
> 
> Have spent cycles on this on-and-off these past few months; finally have
> something worth discussing, I think 🤞
> 
> To recap where we stand, AFAIU: the reported vc-dir bug has been fixed
> (in time for the emacs-30 branch), but the changes could feel intrusive,
> since a new vc-dir header was added ("Tracking") that some users may not
> care for.

I haven't looked at the patch in detail, but FWIW the new header feels 
like a minor enough detail to not be annoying (or even noticeable) when 
you don't want it, but it's good to have when you do need that info.

This is my impression after having that feature around for a few months 
since it's been introduced anyway.

Just a data point. Not to imply that the code couldn't be improved, or 
that the new option can't be useful.

Message #56 received at 68183 <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: Eli Zaretskii <eliz <at> gnu.org>, 68183 <at> debbugs.gnu.org,
 Tom Tromey <tom <at> tromey.com>, Sean Whitton <spwhitton <at> spwhitton.name>,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Tue, 20 Aug 2024 08:15:37 +0200

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

> Hi Kevin,
>
> On 07/08/2024 17:25, Kévin Le Gouguec wrote:
>> Heya,
>> Have spent cycles on this on-and-off these past few months; finally have
>> something worth discussing, I think 🤞
>> To recap where we stand, AFAIU: the reported vc-dir bug has been fixed
>> (in time for the emacs-30 branch), but the changes could feel intrusive,
>> since a new vc-dir header was added ("Tracking") that some users may not
>> care for.
>
> I haven't looked at the patch in detail, but FWIW the new header feels like a minor enough detail to not be annoying (or even noticeable) when you don't want it, but it's good to have when you do need that info.
>
> This is my impression after having that feature around for a few months since it's been introduced anyway.
>
> Just a data point. Not to imply that the code couldn't be improved, or that the new option can't be useful.

Thanks for weighing in.  Since I am not a frequent vc-dir user, my
disposition would then be to avoid committing to sophisticated user
options until there is demand for it.

So, at this stage, ISTM:

* We could apply [patch 1] (new tests) on emacs-30.

* I should draft a NEWS entry for the new header, for emacs-30.

* We could apply [patch 2] (refactoring; modulo the considerations
  re. vc-git--cmds-in-progress discussed with Sean) on master, if we
  feel like it makes vc-git-dir-extra-headers easier to work with.


[patch 1]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68183;filename=0001-Test-more-vc-dir-scenarios-with-Git-bug-68183.patch;msg=41;att=1

[patch 2]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68183;filename=0002-Split-vc-git-dir-extra-headers-into-more-manageable-.patch;msg=41;att=2

Message #59 received at 68183 <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: dmitry <at> gutov.dev, 68183 <at> debbugs.gnu.org, tom <at> tromey.com,
 spwhitton <at> spwhitton.name, juri <at> linkov.net
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Tue, 20 Aug 2024 15:15:49 +0300

> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
> Cc: 68183 <at> debbugs.gnu.org,  Eli Zaretskii <eliz <at> gnu.org>,  Tom Tromey
>  <tom <at> tromey.com>,  Juri Linkov <juri <at> linkov.net>,  Sean Whitton
>  <spwhitton <at> spwhitton.name>
> Date: Tue, 20 Aug 2024 08:15:37 +0200
> 
> Dmitry Gutov <dmitry <at> gutov.dev> writes:
> 
> > Hi Kevin,
> >
> > On 07/08/2024 17:25, Kévin Le Gouguec wrote:
> >> Heya,
> >> Have spent cycles on this on-and-off these past few months; finally have
> >> something worth discussing, I think 🤞
> >> To recap where we stand, AFAIU: the reported vc-dir bug has been fixed
> >> (in time for the emacs-30 branch), but the changes could feel intrusive,
> >> since a new vc-dir header was added ("Tracking") that some users may not
> >> care for.
> >
> > I haven't looked at the patch in detail, but FWIW the new header feels like a minor enough detail to not be annoying (or even noticeable) when you don't want it, but it's good to have when you do need that info.
> >
> > This is my impression after having that feature around for a few months since it's been introduced anyway.
> >
> > Just a data point. Not to imply that the code couldn't be improved, or that the new option can't be useful.
> 
> Thanks for weighing in.  Since I am not a frequent vc-dir user, my
> disposition would then be to avoid committing to sophisticated user
> options until there is demand for it.
> 
> So, at this stage, ISTM:
> 
> * We could apply [patch 1] (new tests) on emacs-30.
> 
> * I should draft a NEWS entry for the new header, for emacs-30.
> 
> * We could apply [patch 2] (refactoring; modulo the considerations
>   re. vc-git--cmds-in-progress discussed with Sean) on master, if we
>   feel like it makes vc-git-dir-extra-headers easier to work with.
> 
> 
> [patch 1]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68183;filename=0001-Test-more-vc-dir-scenarios-with-Git-bug-68183.patch;msg=41;att=1
> 
> [patch 2]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68183;filename=0002-Split-vc-git-dir-extra-headers-into-more-manageable-.patch;msg=41;att=2

A NEWS entry is probably OK, though I'd like to see it first.

As for adding tests, I think they should go to master.  There's no
purpose in adding them to the release branch, which is basically
frozen to code changes except bugfixes.  Since we merge code to master
routinely, any regression that happens on the branch will soon enough
land on master and will be detected there.

Thanks.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68183 <at> debbugs.gnu.org,
 Tom Tromey <tom <at> tromey.com>, Sean Whitton <spwhitton <at> spwhitton.name>,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Wed, 21 Aug 2024 03:42:13 +0300

On 20/08/2024 09:15, Kévin Le Gouguec wrote:
> Thanks for weighing in.  Since I am not a frequent vc-dir user, my
> disposition would then be to avoid committing to sophisticated user
> options until there is demand for it.

We're in agreement then.

> So, at this stage, ISTM:
> 
> * We could apply [patch 1] (new tests) on emacs-30.
> 
> * I should draft a NEWS entry for the new header, for emacs-30.
> 
> * We could apply [patch 2] (refactoring; modulo the considerations
>    re. vc-git--cmds-in-progress discussed with Sean) on master, if we
>    feel like it makes vc-git-dir-extra-headers easier to work with.

Thanks!

NEWS for emacs-30 sounds good, and I've pushed the other two changes 
(tests and refactoring) to the master branch.

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Juri Linkov <juri <at> linkov.net>, Eli Zaretskii <eliz <at> gnu.org>,
 68183 <at> debbugs.gnu.org, Tom Tromey <tom <at> tromey.com>,
 Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Wed, 21 Aug 2024 09:40:52 +0800

Hello,

On Wed 21 Aug 2024 at 03:42am +03, Dmitry Gutov wrote:

> NEWS for emacs-30 sounds good, and I've pushed the other two changes (tests
> and refactoring) to the master branch.

Hang on, I think Kevin hadn't incorporated my requested changes into
that version :)

Kevin, could you provide a follow-up patch, please?  Thanks.

-- 
Sean Whitton

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

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: Dmitry Gutov <dmitry <at> gutov.dev>, Eli Zaretskii <eliz <at> gnu.org>,
 68183 <at> debbugs.gnu.org, Tom Tromey <tom <at> tromey.com>,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Wed, 21 Aug 2024 09:05:50 +0200

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

Sean Whitton <spwhitton <at> spwhitton.name> writes:

> Hello,
>
> On Wed 21 Aug 2024 at 03:42am +03, Dmitry Gutov wrote:
>
>> NEWS for emacs-30 sounds good, 

(ACK'ing Eli's request to submit here for review before pushing 👌)

>>                                and I've pushed the other two changes (tests
>> and refactoring) to the master branch.

(Thanks!)

> Hang on, I think Kevin hadn't incorporated my requested changes into
> that version :)

(Right, I've been less reactive than ideal these past few days,
apologies for that)

> Kevin, could you provide a follow-up patch, please?  Thanks.

Something like the attached, right?  Tested manually with a rebase.

[0001-Restore-vc-git-helper-function-bug-68183.patch (text/x-diff, attachment)]

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: Dmitry Gutov <dmitry <at> gutov.dev>, Eli Zaretskii <eliz <at> gnu.org>,
 68183 <at> debbugs.gnu.org, Tom Tromey <tom <at> tromey.com>,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Wed, 21 Aug 2024 15:59:44 +0800

Hello,

On Wed 21 Aug 2024 at 09:05am +02, Kévin Le Gouguec wrote:

> (Right, I've been less reactive than ideal these past few days,
> apologies for that)

No, I think it was that Dmitry misunderstood you and thought that was
the final version of the patch.

>> Kevin, could you provide a follow-up patch, please?  Thanks.
>
> Something like the attached, right?  Tested manually with a rebase.

Thanks -- pushed.

(I did s/find some use/find use/ in your log message because it seemed
more(?) grammatical.  Now I read it again, I'm not sure that is true.
I hope this is okay with you.  It's a very small change.)

-- 
Sean Whitton

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Sean Whitton <spwhitton <at> spwhitton.name>,
 Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68183 <at> debbugs.gnu.org,
 Tom Tromey <tom <at> tromey.com>, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Wed, 21 Aug 2024 15:29:58 +0300

On 21/08/2024 10:59, Sean Whitton wrote:
> Hello,
> 
> On Wed 21 Aug 2024 at 09:05am +02, Kévin Le Gouguec wrote:
> 
>> (Right, I've been less reactive than ideal these past few days,
>> apologies for that)
> No, I think it was that Dmitry misunderstood you and thought that was
> the final version of the patch.
> 
>>> Kevin, could you provide a follow-up patch, please?  Thanks.
>> Something like the attached, right?  Tested manually with a rebase.
> Thanks -- pushed.
> 
> (I did s/find some use/find use/ in your log message because it seemed
> more(?) grammatical.  Now I read it again, I'm not sure that is true.
> I hope this is okay with you.  It's a very small change.)

Thank you both.

Message #77 received at 68183 <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: dmitry <at> gutov.dev, juri <at> linkov.net, 68183 <at> debbugs.gnu.org, tom <at> tromey.com,
 spwhitton <at> spwhitton.name
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Thu, 22 Aug 2024 09:15:06 +0200

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

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

> A NEWS entry is probably OK, though I'd like to see it first.

Please find a draft attached.  Lifted the "upstream branch" definition
from the horse's mouth, i.e. gitglossary(7), adjusting for VC relevance,
i.e. "where vc-pull fetches".

(Agonized a bit over how to typeset "VC-dir buffers"; went with that
spelling because a bunch of vc-dir.el deffaces use "VC-dir".

*5 seconds later*

But I now realize etc/NEWSes also use "VC Dired", "VC-dired", "vc-dir
for <backend>", "*vc-dir*", "VC directory mode", vc-dir-mode, and "VC
directory buffer", so maybe I oughtn't worry so much?

*5 seconds later*

💡 Ah, well, guess I could follow The Fine Manual's nomenclature?
Changed to "VC Directory buffers", per '(emacs) VC Directory Mode'.
Kept the shorthand for the ChangeLog message for brevity)


In another subthread, Sean Whitton <spwhitton <at> spwhitton.name> writes:

> (I did s/find some use/find use/ in your log message because it seemed
> more(?) grammatical.  Now I read it again, I'm not sure that is true.
> I hope this is okay with you.  It's a very small change.)

(Can't attest to grammaticality, but AFAICT you ditched a weasel word
and reduced noise, and that's quite OK, thanks!)

[0001-etc-NEWS-Announce-VC-dir-Tracking-header.-bug-68183.patch (text/x-patch, attachment)]

Message #80 received at 68183 <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: dmitry <at> gutov.dev, juri <at> linkov.net, 68183 <at> debbugs.gnu.org, tom <at> tromey.com,
 spwhitton <at> spwhitton.name
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Thu, 22 Aug 2024 15:46:56 +0300

> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
> Cc: dmitry <at> gutov.dev,  68183 <at> debbugs.gnu.org,  tom <at> tromey.com,
>   spwhitton <at> spwhitton.name,  juri <at> linkov.net
> Date: Thu, 22 Aug 2024 09:15:06 +0200
> 
> +---
> +*** VC Directory buffers now display the upstream branch in Git repositories.
> +The "upstream branch" is the branch 'vc-pull' fetches changes from by
> +default.  In Git terms, the upstream branch of branch B is determined by
> +configuration variables branch.B.remote and branch.B.merge.

That's okay, with the following nit: I find phrases like "the branch
'vc-pull' fetches changes from by default" hard to grasp.  I prefer
rephrasing to make the grammar simpler:

  The "upstream branch" is the branch from which 'vc-pull' fetches
  changes by default.

Message #83 received at 68183 <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: dmitry <at> gutov.dev, 68183 <at> debbugs.gnu.org, tom <at> tromey.com,
 spwhitton <at> spwhitton.name, juri <at> linkov.net
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Thu, 29 Aug 2024 16:36:28 +0100

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

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

>> +---
>> +*** VC Directory buffers now display the upstream branch in Git repositories.
>> +The "upstream branch" is the branch 'vc-pull' fetches changes from by
>> +default.  In Git terms, the upstream branch of branch B is determined by
>> +configuration variables branch.B.remote and branch.B.merge.
>
> That's okay, with the following nit: I find phrases like "the branch
> 'vc-pull' fetches changes from by default" hard to grasp.  I prefer
> rephrasing to make the grammar simpler:
>
>   The "upstream branch" is the branch from which 'vc-pull' fetches
>   changes by default.

ACK; got the attached at the tip of my emacs-30 branch, and my finger
over the 'git push' button.  Let me know if it's time to 'C-x v P'.
(And mark this bug "done", since I believe this was the last loose end?)

(Prefer asking since I let a week fly by, on the off chance that the
release branch might have reached a "hands off ✋" stage)

[0001-etc-NEWS-Announce-VC-dir-Tracking-header.-bug-68183.patch (text/x-diff, attachment)]

Message #86 received at 68183 <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: dmitry <at> gutov.dev, 68183 <at> debbugs.gnu.org, tom <at> tromey.com,
 spwhitton <at> spwhitton.name, juri <at> linkov.net
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Thu, 29 Aug 2024 18:46:28 +0300

> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
> Cc: dmitry <at> gutov.dev,  juri <at> linkov.net,  68183 <at> debbugs.gnu.org,
>   tom <at> tromey.com,  spwhitton <at> spwhitton.name
> Date: Thu, 29 Aug 2024 16:36:28 +0100
> 
> ACK; got the attached at the tip of my emacs-30 branch, and my finger
> over the 'git push' button.  Let me know if it's time to 'C-x v P'.
> (And mark this bug "done", since I believe this was the last loose end?)

OK, thanks.

Message #91 received at 68183-done <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: dmitry <at> gutov.dev, juri <at> linkov.net, tom <at> tromey.com,
 68183-done <at> debbugs.gnu.org, spwhitton <at> spwhitton.name
Subject: Re: bug#68183: 28.3; vc-dir fails when I have a certain branch
 checked out
Date: Thu, 29 Aug 2024 17:41:25 +0100

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

>> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
>> Cc: dmitry <at> gutov.dev,  juri <at> linkov.net,  68183 <at> debbugs.gnu.org,
>>   tom <at> tromey.com,  spwhitton <at> spwhitton.name
>> Date: Thu, 29 Aug 2024 16:36:28 +0100
>> 
>> ACK; got the attached at the tip of my emacs-30 branch, and my finger
>> over the 'git push' button.  Let me know if it's time to 'C-x v P'.
>> (And mark this bug "done", since I believe this was the last loose end?)
>
> OK, thanks.

Thank y'all; pushed, and closing with this message 🙇

Previous Next

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