GNU bug report logs - #51497
29.0.50; (vc-print-log) broken over TRAMP

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 51497 in the body.
You can then email your comments to 51497 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: dima <at> secretsauce.net
To: bug-gnu-emacs <at> gnu.org
Cc: Wolfgang Scherer <wolfgang.scherer <at> gmx.de>
Subject: 29.0.50; (vc-print-log) broken over TRAMP
Date: Fri, 29 Oct 2021 18:24:55 -0700

Hi. I'm using emacs built from git. I'm observing that it's no longer
possible to "C-x v l" when looking at version-controlled files over
TRAMP. This is a regression. "git bisect" tells me that the breaking
commit is this:

  3572613550f5d1d0b3392dbc809b32f3989e2981 is the first bad commit
  commit 3572613550f5d1d0b3392dbc809b32f3989e2981
  Author: Wolfgang Scherer <wolfgang.scherer <at> gmx.de>
  Date:   Sun Aug 15 04:02:23 2021 +0300

      Fix vc-git-state for filenames with wildcards

      * lisp/vc/vc-git.el: (vc-git--literal-pathspec-inner),
      (vc-git--literal-pathspec), (vc-git--literal-pathspecs) new functions
      to add ":(literal)" pathspec magic (bug#39452).

      (vc-git-registered), (vc-git-state), (vc-git-dir-status-goto-stage),
      (vc-git-register), (vc-git-unregister), (vc-git-checkin),
      (vc-git-find-revision), (vc-git-checkout), (vc-git-revert),
      (vc-git-conflicted-files), (vc-git-print-log), (vc-git-diff),
      (vc-git-previous-revision), (vc-git-next-revision),
      (vc-git-delete-file), (vc-git-rename-file) functions
      vc-git--literal-pathspec, vc-git--literal-pathspecs applied.

   lisp/vc/vc-git.el | 63 +++++++++++++++++++++++++++++++------------------------
   1 file changed, 36 insertions(+), 27 deletions(-)

Recipe to reproduce:

1. emacs -Q /ssh:some_server:some_file
   where the remote file is in a git repo

2. C-x v l
   I should see the git log, but instead I get this in the *Messages*:

   vc-deduce-fileset-1: File is not under version control

"C-x v L" still works

Thanks.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: dima <at> secretsauce.net
Cc: 51497 <at> debbugs.gnu.org, Wolfgang Scherer <wolfgang.scherer <at> gmx.de>
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Sat, 30 Oct 2021 14:48:37 +0200

dima <at> secretsauce.net writes:

> Hi. I'm using emacs built from git. I'm observing that it's no longer
> possible to "C-x v l" when looking at version-controlled files over
> TRAMP. This is a regression. "git bisect" tells me that the breaking
> commit is this:

[...]

> 1. emacs -Q /ssh:some_server:some_file
>    where the remote file is in a git repo
>
> 2. C-x v l
>    I should see the git log, but instead I get this in the *Messages*:
>
>    vc-deduce-fileset-1: File is not under version control

I'm unable to reproduce this on the current trunk.

I tried:

C-x C-f /ssh:stories:/home/larsi/src/emacs/trunk/etc/NEWS RET
C-x v l

and I got the vc-change-log buffer.

I did see something similar a few weeks ago, but it went away after I
said "make bootstrap" (so it's either a problem that shows up only
sometimes or it's a genuine build thing).  Can you try "make bootstrap"
to see whether that has any effect?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, dima <at> secretsauce.net
Cc: 51497 <at> debbugs.gnu.org, Wolfgang Scherer <wolfgang.scherer <at> gmx.de>
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Sat, 30 Oct 2021 16:21:58 +0300

On 30.10.2021 15:48, Lars Ingebrigtsen wrote:
> I did see something similar a few weeks ago, but it went away after I
> said "make bootstrap" (so it's either a problem that shows up only
> sometimes or it's a genuine build thing).

I think it was bug#51112, which we fixed.

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

From: Dima Kogan <dima <at> secretsauce.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 51497 <at> debbugs.gnu.org, Wolfgang Scherer <wolfgang.scherer <at> gmx.de>
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Sat, 30 Oct 2021 12:01:59 -0700

Hi. Thanks for replying. Notes inline


Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> I'm unable to reproduce this on the current trunk.
> I did see something similar a few weeks ago, but it went away after I
> said "make bootstrap" (so it's either a problem that shows up only
> sometimes or it's a genuine build thing).  Can you try "make bootstrap"
> to see whether that has any effect?

This is still a problem for me here:

  commit c3499b8ddc357544a58917bfd3846f88caf5d97c
  Author: Eli Zaretskii <eliz <at> gnu.org>
  Date:   Fri Oct 29 22:07:27 2021 +0300

I've been building from scratch, which is the same, as "make bootstrap"
I imagine. This was my test in "git bisect"

  git clean -idx &&
  git reset --hard &&
  ./autogen.sh &&
  ./configure --with-gnutls=ifavailable &&
  make -j19 &&
  src/emacs -nw -Q /ssh:SERVER:FILE

I haven't done any debugging other than the bisection. Would you like me
to dig into it in some way?


Dmitry Gutov <dgutov <at> yandex.ru> writes:

> On 30.10.2021 15:48, Lars Ingebrigtsen wrote:
>> I did see something similar a few weeks ago, but it went away after I
>> said "make bootstrap" (so it's either a problem that shows up only
>> sometimes or it's a genuine build thing).
>
> I think it was bug#51112, which we fixed.

I'm at a later revision than any notes in that bug, so I would guess
this is different.

Thanks.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Dima Kogan <dima <at> secretsauce.net>, Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 51497 <at> debbugs.gnu.org, Wolfgang Scherer <wolfgang.scherer <at> gmx.de>
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Sun, 31 Oct 2021 03:56:51 +0300

On 30.10.2021 22:01, Dima Kogan wrote:
> I haven't done any debugging other than the bisection. Would you like me
> to dig into it in some way?

If you (setq vc-command-messages t), you should be able to see all VC 
commands Emacs tries to run.

If you can catch the exact command (or several) which were constructed 
incorrectly, that would help a lot.

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

From: Dima Kogan <dima <at> secretsauce.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 51497 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Wolfgang Scherer <wolfgang.scherer <at> gmx.de>
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Sat, 30 Oct 2021 23:58:23 -0700

Hi. I figured this out.

(setq vc-command-messages t) doesn't do anything: emacs doesn't
recognize this as a valid file in git, so it only runs git commands at
file open time, which apparently aren't reported with
vc-command-messages.

Instead I ran with (setq tramp-verbose 6) and the key line is this:

  23:46:27.247358 tramp-send-command (6) # ( cd DIR && unset GIT_DIR && git --no-pager ls-files -c -z -- \:\(literal\)FILE.c </dev/null 2>/dev/null; echo tramp_exit_status $? )
  23:46:27.258502 tramp-wait-for-regexp (6) # 
  tramp_exit_status 128

Note the :(literal) stuff. This is what was added in the commit that the
bisection found to be the cause of the issue. I can try to run this
command directly (outside of emacs) on the target box:

  $ git --no-pager ls-files -c -z -- ':(literal)FILE.c' </dev/null

  fatal: Invalid pathspec magic 'literal' in ':(literal)FILE.c'

The issue is that this target box has a version of git too old to know
about :(literal):

  kogan <at> aargh:~/stereo-server$ git --version
  git version 1.8.3.1

Yeah, it's ancient, but I don't control this particular machine, and I
don't think basic stuff like "C-x v l" should be non-functional here.
Can we add some logic to emacs to not hard-depend on this stuff?

Thanks

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Dima Kogan <dima <at> secretsauce.net>
Cc: 51497 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Wolfgang Scherer <wolfgang.scherer <at> gmx.de>, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Sun, 31 Oct 2021 09:16:03 +0100

Dima Kogan <dima <at> secretsauce.net> writes:

> Hi. I figured this out.

Hi,

> I can try to run this
> command directly (outside of emacs) on the target box:
>
>   $ git --no-pager ls-files -c -z -- ':(literal)FILE.c' </dev/null
>
>   fatal: Invalid pathspec magic 'literal' in ':(literal)FILE.c'
>
> The issue is that this target box has a version of git too old to know
> about :(literal):
>
>   kogan <at> aargh:~/stereo-server$ git --version
>   git version 1.8.3.1
>
> Yeah, it's ancient, but I don't control this particular machine, and I
> don't think basic stuff like "C-x v l" should be non-functional here.
> Can we add some logic to emacs to not hard-depend on this stuff?

vc-git.el could declare a variable which determines, whether the git
command supports :(literal) (perhaps it does already). A user could
change this variable via connection-local settings.

> Thanks

Best regards, Michael.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Michael Albinus <michael.albinus <at> gmx.de>, Dima Kogan <dima <at> secretsauce.net>
Cc: 51497 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Wolfgang Scherer <wolfgang.scherer <at> gmx.de>
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Sun, 31 Oct 2021 15:26:55 +0300

On 31.10.2021 11:16, Michael Albinus wrote:
> vc-git.el could declare a variable which determines, whether the git
> command supports :(literal) (perhaps it does already). A user could
> change this variable via connection-local settings.

We normally use 'vc-git--program-version' for this.

Can we define the variable 'vc-git--program-version' as connection-local 
or something?

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 51497 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Dima Kogan <dima <at> secretsauce.net>, Wolfgang Scherer <wolfgang.scherer <at> gmx.de>
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Sun, 31 Oct 2021 17:05:18 +0100

Dmitry Gutov <dgutov <at> yandex.ru> writes:

Hi Dmitry,

>> vc-git.el could declare a variable which determines, whether the git
>> command supports :(literal) (perhaps it does already). A user could
>> change this variable via connection-local settings.
>
> We normally use 'vc-git--program-version' for this.
>
> Can we define the variable 'vc-git--program-version' as
> connection-local or something?

--8<---------------cut here---------------start------------->8---
(defun vc-git--program-version ()
  (let ((current
	 (if (file-remote-p default-directory)
	     (with-connection-local-variables
	      (and (local-variable-p 'vc-git--program-version)
		   vc-git--program-version))
	   vc-git--program-version)))
    (or current
	(let ((version-string
               (vc-git--run-command-string nil "version")))
          (setq version-string
		(if (and version-string
			 ;; Git for Windows appends ".windows.N" to the
			 ;; numerical version reported by Git.
			 (string-match
                          "git version \\([0-9.]+\\)\\(\\.windows\\.[0-9]+\\)?$"
                          version-string))
                    (match-string 1 version-string)
                  "0"))
	  (if (file-remote-p default-directory)
	      (let ((profile (gensym "connection-local-profile-")))
		(connection-local-set-profile-variables
		 profile `((vc-git--program-version . ,version-string))) ;
		(connection-local-set-profiles
		 (connection-local-criteria-for-default-directory) profile))
	    (setq vc-git--program-version version-string))
	  version-string))))
--8<---------------cut here---------------end--------------->8---

Best regards, Michael.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 51497 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Dima Kogan <dima <at> secretsauce.net>, Wolfgang Scherer <wolfgang.scherer <at> gmx.de>
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Wed, 3 Nov 2021 05:00:49 +0300

Hi Michael,

On 31.10.2021 19:05, Michael Albinus wrote:
> (defun vc-git--program-version ()
>    (let ((current
> 	 (if (file-remote-p default-directory)
> 	     (with-connection-local-variables
> 	      (and (local-variable-p 'vc-git--program-version)
> 		   vc-git--program-version))
> 	   vc-git--program-version)))
>      (or current
> 	(let ((version-string
>                 (vc-git--run-command-string nil "version")))
>            (setq version-string
> 		(if (and version-string
> 			 ;; Git for Windows appends ".windows.N" to the
> 			 ;; numerical version reported by Git.
> 			 (string-match
>                            "git version \\([0-9.]+\\)\\(\\.windows\\.[0-9]+\\)?$"
>                            version-string))
>                      (match-string 1 version-string)
>                    "0"))
> 	  (if (file-remote-p default-directory)
> 	      (let ((profile (gensym "connection-local-profile-")))
> 		(connection-local-set-profile-variables
> 		 profile `((vc-git--program-version . ,version-string))) ;
> 		(connection-local-set-profiles
> 		 (connection-local-criteria-for-default-directory) profile))
> 	    (setq vc-git--program-version version-string))
> 	  version-string))))

I wonder if we could have some helper that is more succinct. One that 
didn't require the client code to auto-generate the profile name, for 
instance.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Dima Kogan <dima <at> secretsauce.net>
Cc: 51497 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Wolfgang Scherer <wolfgang.scherer <at> gmx.de>
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Wed, 3 Nov 2021 05:03:56 +0300

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

On 31.10.2021 09:58, Dima Kogan wrote:
> Note the :(literal) stuff. This is what was added in the commit that the
> bisection found to be the cause of the issue. I can try to run this
> command directly (outside of emacs) on the target box:
> 
>    $ git --no-pager ls-files -c -z -- ':(literal)FILE.c' </dev/null
> 
>    fatal: Invalid pathspec magic 'literal' in ':(literal)FILE.c'
> 
> The issue is that this target box has a version of git too old to know
> about :(literal):
> 
>    kogan <at> aargh:~/stereo-server$ git --version
>    git version 1.8.3.1

Sounds like CentOS 7. Released 7 years ago, but updated last year, even.

> Yeah, it's ancient, but I don't control this particular machine, and I
> don't think basic stuff like "C-x v l" should be non-functional here.
> Can we add some logic to emacs to not hard-depend on this stuff?

Any idea which version of Git introduced literal pathspecs?

The docs on the official website only go back to 2.1.4 (also from 2014), 
which had it already.

Anyway, with Michael's code, see the attached patch you can try.

[no-literal-pathspecs-on-centos.diff (text/x-patch, attachment)]

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

From: Dima Kogan <dima <at> secretsauce.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 51497 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Wolfgang Scherer <wolfgang.scherer <at> gmx.de>
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Tue, 02 Nov 2021 20:03:43 -0700

Dmitry Gutov <dgutov <at> yandex.ru> writes:

> On 31.10.2021 09:58, Dima Kogan wrote:
>>    kogan <at> aargh:~/stereo-server$ git --version
>>    git version 1.8.3.1
>
> Sounds like CentOS 7. Released 7 years ago, but updated last year,
> even.

Yessir. I wasn't a fan of this even 7 years ago, but it's not a battle I
want to fight.


>> Yeah, it's ancient, but I don't control this particular machine, and I
>> don't think basic stuff like "C-x v l" should be non-functional here.
>> Can we add some logic to emacs to not hard-depend on this stuff?
>
> Any idea which version of Git introduced literal pathspecs?

I think 1.8.5:

  https://github.com/git/git/commit/5c6933d201fab183a9779dca0fe43bf2f1eca098


> Anyway, with Michael's code, see the attached patch you can try.

It fixes the problem. Thank you very much.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Dima Kogan <dima <at> secretsauce.net>
Cc: 51497 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Wolfgang Scherer <wolfgang.scherer <at> gmx.de>
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Wed, 3 Nov 2021 15:06:41 +0300

On 03.11.2021 06:03, Dima Kogan wrote:
> Dmitry Gutov <dgutov <at> yandex.ru> writes:
> 
>> On 31.10.2021 09:58, Dima Kogan wrote:
>>>     kogan <at> aargh:~/stereo-server$ git --version
>>>     git version 1.8.3.1
>>
>> Sounds like CentOS 7. Released 7 years ago, but updated last year,
>> even.
> 
> Yessir. I wasn't a fan of this even 7 years ago, but it's not a battle I
> want to fight.
> 
> 
>>> Yeah, it's ancient, but I don't control this particular machine, and I
>>> don't think basic stuff like "C-x v l" should be non-functional here.
>>> Can we add some logic to emacs to not hard-depend on this stuff?
>>
>> Any idea which version of Git introduced literal pathspecs?
> 
> I think 1.8.5:
> 
>    https://github.com/git/git/commit/5c6933d201fab183a9779dca0fe43bf2f1eca098

Looks like it. Thanks!

>> Anyway, with Michael's code, see the attached patch you can try.
> 
> It fixes the problem. Thank you very much.

All right.

Lars, Eli, can we put it in Emacs 28?

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 51497 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Dima Kogan <dima <at> secretsauce.net>, Wolfgang Scherer <wolfgang.scherer <at> gmx.de>
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Wed, 03 Nov 2021 18:09:04 +0100

Dmitry Gutov <dgutov <at> yandex.ru> writes:

> Hi Michael,

Hi Dmitry,

> I wonder if we could have some helper that is more succinct. One that
> didn't require the client code to auto-generate the profile name, for
> instance.

Perhaps. I have some health problems these days, so I will check next
week or so. However, a helper function (in files-x.el) would be good for
Emacs 29 only.

Best regards, Michael.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 51497 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Dima Kogan <dima <at> secretsauce.net>, Wolfgang Scherer <wolfgang.scherer <at> gmx.de>
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Fri, 5 Nov 2021 05:00:16 +0300

Hi Michael,

On 03.11.2021 20:09, Michael Albinus wrote:
>> I wonder if we could have some helper that is more succinct. One that
>> didn't require the client code to auto-generate the profile name, for
>> instance.
> 
> Perhaps. I have some health problems these days, so I will check next
> week or so. However, a helper function (in files-x.el) would be good for
> Emacs 29 only.

Of course.

Take care,
Dmitry.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>
Cc: Dima Kogan <lists <at> dima.secretsauce.net>, 51497 <at> debbugs.gnu.org,
 Wolfgang Scherer <wolfgang.scherer <at> gmx.de>
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Sat, 6 Nov 2021 16:22:56 +0300

On 03.11.2021 15:06, Dmitry Gutov wrote:
> Lars, Eli, can we put it in Emacs 28?

Ping.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: lists <at> dima.secretsauce.net, 51497 <at> debbugs.gnu.org, larsi <at> gnus.org,
 wolfgang.scherer <at> gmx.de
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Sat, 06 Nov 2021 17:51:28 +0200

> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Cc: 51497 <at> debbugs.gnu.org, Wolfgang Scherer <wolfgang.scherer <at> gmx.de>,
>  Dima Kogan <lists <at> dima.secretsauce.net>
> Date: Sat, 6 Nov 2021 16:22:56 +0300
> 
> On 03.11.2021 15:06, Dmitry Gutov wrote:
> > Lars, Eli, can we put it in Emacs 28?
> 
> Ping.

Sorry for missing the original question.

I'm a bit worried by the function relying on the fact that
default-directory is the directory of the repository.  Wouldn't it be
better to explicitly let-bind it inside the function?

A (perhaps safer) alternative for emacs-28 would be not to use
:(literal) for remote repositories.  What are the disadvantages of
that?

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: lists <at> dima.secretsauce.net, 51497 <at> debbugs.gnu.org, larsi <at> gnus.org,
 wolfgang.scherer <at> gmx.de
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Sat, 6 Nov 2021 22:44:55 +0300

On 06.11.2021 18:51, Eli Zaretskii wrote:
>> From: Dmitry Gutov <dgutov <at> yandex.ru>
>> Cc: 51497 <at> debbugs.gnu.org, Wolfgang Scherer <wolfgang.scherer <at> gmx.de>,
>>   Dima Kogan <lists <at> dima.secretsauce.net>
>> Date: Sat, 6 Nov 2021 16:22:56 +0300
>>
>> On 03.11.2021 15:06, Dmitry Gutov wrote:
>>> Lars, Eli, can we put it in Emacs 28?
>>
>> Ping.
> 
> Sorry for missing the original question.
> 
> I'm a bit worried by the function relying on the fact that
> default-directory is the directory of the repository.  Wouldn't it be
> better to explicitly let-bind it inside the function?

We could, but notice how most of vc-git-* functions don't bind 
default-directory, thus relying on its implicit value. It just how VC 
works: expecting default-directory to have the right value around the calls.

The only current caller of vc-git--program-version (vc-git-state) does 
not either. The backend methods that do, seem to do that with some 
additional purpose (like having default-directory point to the 
repository root, rather than be a random directory inside it).

> A (perhaps safer) alternative for emacs-28 would be not to use
> :(literal) for remote repositories.  What are the disadvantages of
> that?

That would mean leaving bug#39452 unfixed on remote hosts. Seems like a 
significant disadvantage to me (inconsistent behavior leads to more 
difficult reproduction and reporting of bugs, in particular for those 
who will notice this problem remotely but would not be able to reproduce 
locally). Given that the code complexity added by fixing this bug would 
remain with us, seems more like worst-of-both-worlds kind of situation.

But it would make VC work on remote CentOS 7 hosts again, there's that.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: lists <at> dima.secretsauce.net, 51497 <at> debbugs.gnu.org, larsi <at> gnus.org,
 wolfgang.scherer <at> gmx.de
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Sat, 06 Nov 2021 21:52:53 +0200

> Cc: lists <at> dima.secretsauce.net, 51497 <at> debbugs.gnu.org, larsi <at> gnus.org,
>  wolfgang.scherer <at> gmx.de
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Sat, 6 Nov 2021 22:44:55 +0300
> 
> > I'm a bit worried by the function relying on the fact that
> > default-directory is the directory of the repository.  Wouldn't it be
> > better to explicitly let-bind it inside the function?
> 
> We could, but notice how most of vc-git-* functions don't bind 
> default-directory, thus relying on its implicit value. It just how VC 
> works: expecting default-directory to have the right value around the calls.

How certain are you that default-directory has the right value?
Because if it doesn't, AFAIU all the connection-specific stuff will
fall apart.

> > A (perhaps safer) alternative for emacs-28 would be not to use
> > :(literal) for remote repositories.  What are the disadvantages of
> > that?
> 
> That would mean leaving bug#39452 unfixed on remote hosts.

Only for files with wildcard characters in their names.  How
frequently does that happen?  Also, it will be only unsolved in Emacs
28.

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

From: Andy Moreton <andrewjmoreton <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Sat, 06 Nov 2021 22:11:03 +0000

On Sat 06 Nov 2021, Eli Zaretskii wrote:

>> Cc: lists <at> dima.secretsauce.net, 51497 <at> debbugs.gnu.org, larsi <at> gnus.org,
>>  wolfgang.scherer <at> gmx.de
>> From: Dmitry Gutov <dgutov <at> yandex.ru>
>> Date: Sat, 6 Nov 2021 22:44:55 +0300
>> 
>> > I'm a bit worried by the function relying on the fact that
>> > default-directory is the directory of the repository.  Wouldn't it be
>> > better to explicitly let-bind it inside the function?
>> 
>> We could, but notice how most of vc-git-* functions don't bind 
>> default-directory, thus relying on its implicit value. It just how VC 
>> works: expecting default-directory to have the right value around the calls.
>
> How certain are you that default-directory has the right value?
> Because if it doesn't, AFAIU all the connection-specific stuff will
> fall apart.
>
>> > A (perhaps safer) alternative for emacs-28 would be not to use
>> > :(literal) for remote repositories.  What are the disadvantages of
>> > that?
>> 
>> That would mean leaving bug#39452 unfixed on remote hosts.
>
> Only for files with wildcard characters in their names.  How
> frequently does that happen?  Also, it will be only unsolved in Emacs
> 28.

I missed the discussion in bug#39452 at the time, but while the solution
was being worked on, I used advice to stub out the literal pathspec
functions:
  (advice-add 'vc-git--literal-pathspec  :override #'identity)
  (advice-add 'vc-git--literal-pathspecs :override #'identity)

That workaround is still needed for me. Without that, nothing in vc-git
works in my environment (64bit minw64 build on win10, using cygwin bash
and git together with cygwin-mount.el from emacs wiki).

While I realise my setup is somewhat non-standard, other users may also
find that the literal pathspec code also misbehaves.

    AndyM

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: lists <at> dima.secretsauce.net, 51497 <at> debbugs.gnu.org, larsi <at> gnus.org,
 wolfgang.scherer <at> gmx.de
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Sun, 7 Nov 2021 01:19:26 +0300

On 06.11.2021 22:52, Eli Zaretskii wrote:
>> Cc: lists <at> dima.secretsauce.net, 51497 <at> debbugs.gnu.org, larsi <at> gnus.org,
>>   wolfgang.scherer <at> gmx.de
>> From: Dmitry Gutov <dgutov <at> yandex.ru>
>> Date: Sat, 6 Nov 2021 22:44:55 +0300
>>
>>> I'm a bit worried by the function relying on the fact that
>>> default-directory is the directory of the repository.  Wouldn't it be
>>> better to explicitly let-bind it inside the function?
>>
>> We could, but notice how most of vc-git-* functions don't bind
>> default-directory, thus relying on its implicit value. It just how VC
>> works: expecting default-directory to have the right value around the calls.
> 
> How certain are you that default-directory has the right value?
> Because if it doesn't, AFAIU all the connection-specific stuff will
> fall apart.

Reasonably, but not 100%. Especially with third-party code which calls 
into VC (it could adapt independently from Emacs releases, though).

We could try to bind default-directory inside vc-git--literal-pathspec, 
but this approach is not 100% reliable either: for all I know, sometimes 
FILE will be a relative name (we even have a file-name-absolute-p check 
inside).

But what's the worst thing that can happen because of this? Suppose some 
caller will leave default-directory at a wrong value. Then 
vc-git--program-version will return the version from a wrong host. And 
some particular command (probably a less popular one) will remain broken 
on remote CentOS 7 machines. That's still an improvement compared to the 
current sutuation.

So I suggest we push the proposed change to emacs-28 and then maybe back 
it out (or modify as necessary) if problems arise.

>>> A (perhaps safer) alternative for emacs-28 would be not to use
>>> :(literal) for remote repositories.  What are the disadvantages of
>>> that?
>>
>> That would mean leaving bug#39452 unfixed on remote hosts.
> 
> Only for files with wildcard characters in their names.  How
> frequently does that happen?  Also, it will be only unsolved in Emacs
> 28.

I've never seen this in practice, but some other categories of users 
might encounter it more often (e.g. science people, who tend to use more 
exotic files names). And when one does see it, it must be very annoying 
to debug.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Andy Moreton <andrewjmoreton <at> gmail.com>, 51497 <at> debbugs.gnu.org
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Sun, 7 Nov 2021 01:21:26 +0300

On 07.11.2021 01:11, Andy Moreton wrote:
> That workaround is still needed for me. Without that, nothing in vc-git
> works in my environment (64bit minw64 build on win10, using cygwin bash
> and git together with cygwin-mount.el from emacs wiki).

Does that environment also have an old version of Git? Or is there 
another reason why it's broken?

> While I realise my setup is somewhat non-standard, other users may also
> find that the literal pathspec code also misbehaves.

I would like to fix it for all users, but debugging would fall on your 
shoulders, I'm afraid.

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

From: Andy Moreton <andrewjmoreton <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Sat, 06 Nov 2021 23:03:24 +0000

On Sun 07 Nov 2021, Dmitry Gutov wrote:

> On 07.11.2021 01:11, Andy Moreton wrote:
>> That workaround is still needed for me. Without that, nothing in vc-git
>> works in my environment (64bit minw64 build on win10, using cygwin bash
>> and git together with cygwin-mount.el from emacs wiki).
>
> Does that environment also have an old version of Git? Or is there another
> reason why it's broken?

I have git 2.33 in cygwin and MSYS2, so git is not old. I'll look at
this again now that the changes have stablised.

>> While I realise my setup is somewhat non-standard, other users may also
>> find that the literal pathspec code also misbehaves.
>
> I would like to fix it for all users, but debugging would fall on your
> shoulders, I'm afraid.

My note was more to warn that adding this to emacs-28 may bring
problems.

Looking at this again, Trying "C-x v l" for INSTALL in the repo master
branch gives (rewrapped for clarity):

  fatal: :(literal)c:/emacs/git/emacs/master/nt/INSTALL:
  'c:/emacs/git/emacs/master/nt/INSTALL' is outside repository at
  '/c/emacs/git/emacs/master'

This appears to be due to the translation between win32 and cygwin
(posix) filenames.

    AndyM

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Andy Moreton <andrewjmoreton <at> gmail.com>, 51497 <at> debbugs.gnu.org
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Sun, 7 Nov 2021 03:11:43 +0300

On 07.11.2021 02:03, Andy Moreton wrote:
> On Sun 07 Nov 2021, Dmitry Gutov wrote:
> 
>> On 07.11.2021 01:11, Andy Moreton wrote:
>>> That workaround is still needed for me. Without that, nothing in vc-git
>>> works in my environment (64bit minw64 build on win10, using cygwin bash
>>> and git together with cygwin-mount.el from emacs wiki).
>>
>> Does that environment also have an old version of Git? Or is there another
>> reason why it's broken?
> 
> I have git 2.33 in cygwin and MSYS2, so git is not old. I'll look at
> this again now that the changes have stablised.

It would have been nice to receive this feedback before the emacs-28 
branch was cut, when we had more freedom to alter the implementation.

>>> While I realise my setup is somewhat non-standard, other users may also
>>> find that the literal pathspec code also misbehaves.
>>
>> I would like to fix it for all users, but debugging would fall on your
>> shoulders, I'm afraid.
> 
> My note was more to warn that adding this to emacs-28 may bring
> problems.

Adding what? The literal pathspec stuff is already there.

> Looking at this again, Trying "C-x v l" for INSTALL in the repo master
> branch gives (rewrapped for clarity):
> 
>    fatal: :(literal)c:/emacs/git/emacs/master/nt/INSTALL:
>    'c:/emacs/git/emacs/master/nt/INSTALL' is outside repository at
>    '/c/emacs/git/emacs/master'
> 
> This appears to be due to the translation between win32 and cygwin
> (posix) filenames.

It might be fixable inside vc-git--literal-pathspec. Is there some other 
more general path conversion function we should use instead of 
'file-local-name' there? A tested patch would help a lot.

Failing that, I think we'll need to change the "literal pathspecs" 
implementation to yet another approach (adding --literl-pathspecs flag 
instead of manipulating file names). It comes with the same general 
drawbacks as the env var (which is used under the hood), but the 
explicit approach of specifying it in every command would avoid the 
problem of my original fix for that bug.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Andy Moreton <andrewjmoreton <at> gmail.com>
Cc: 51497 <at> debbugs.gnu.org
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Sun, 07 Nov 2021 08:31:14 +0200

> From: Andy Moreton <andrewjmoreton <at> gmail.com>
> Date: Sat, 06 Nov 2021 22:11:03 +0000
> 
> I missed the discussion in bug#39452 at the time, but while the solution
> was being worked on, I used advice to stub out the literal pathspec
> functions:
>   (advice-add 'vc-git--literal-pathspec  :override #'identity)
>   (advice-add 'vc-git--literal-pathspecs :override #'identity)
> 
> That workaround is still needed for me. Without that, nothing in vc-git
> works in my environment (64bit minw64 build on win10, using cygwin bash
> and git together with cygwin-mount.el from emacs wiki).

We need to understand why the original code doesn't work for you,
otherwise we cannot decide what to do about that issue.  FWIW, Git
works for me on Windows from Emacs without any changes.  But I don't
use the Cygwin Bash and cygwin-mount.el, which I always warned people
about.  If you want to use Cygwin, why not also use Cygwin Emacs and
Git, and save yourself those troubles?

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Andy Moreton <andrewjmoreton <at> gmail.com>
Cc: 51497 <at> debbugs.gnu.org
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Sun, 07 Nov 2021 08:43:17 +0200

> From: Andy Moreton <andrewjmoreton <at> gmail.com>
> Date: Sat, 06 Nov 2021 23:03:24 +0000
> 
> Looking at this again, Trying "C-x v l" for INSTALL in the repo master
> branch gives (rewrapped for clarity):
> 
>   fatal: :(literal)c:/emacs/git/emacs/master/nt/INSTALL:
>   'c:/emacs/git/emacs/master/nt/INSTALL' is outside repository at
>   '/c/emacs/git/emacs/master'
> 
> This appears to be due to the translation between win32 and cygwin
> (posix) filenames.

Right, and so the immediate suspect is cygwin-mount.el.  That command
works flawlessly for me, FWIW.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 51497 <at> debbugs.gnu.org, andrewjmoreton <at> gmail.com
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Sun, 07 Nov 2021 08:47:47 +0200

> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Sun, 7 Nov 2021 03:11:43 +0300
> 
> Failing that, I think we'll need to change the "literal pathspecs" 
> implementation to yet another approach (adding --literl-pathspecs flag 
> instead of manipulating file names). It comes with the same general 
> drawbacks as the env var (which is used under the hood), but the 
> explicit approach of specifying it in every command would avoid the 
> problem of my original fix for that bug.

Why wasn't --literal-pathspecs used in the first place? what are the
downsides?  IME, using magic file names is always worse, because it
can run afoul of various shells that consider some characters special.

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

From: Andy Moreton <andrewjmoreton <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Sun, 07 Nov 2021 10:43:31 +0000

On Sun 07 Nov 2021, Eli Zaretskii wrote:

>> From: Dmitry Gutov <dgutov <at> yandex.ru>
>> Date: Sun, 7 Nov 2021 03:11:43 +0300
>> 
>> Failing that, I think we'll need to change the "literal pathspecs" 
>> implementation to yet another approach (adding --literl-pathspecs flag 
>> instead of manipulating file names). It comes with the same general 
>> drawbacks as the env var (which is used under the hood), but the 
>> explicit approach of specifying it in every command would avoid the 
>> problem of my original fix for that bug.
>
> Why wasn't --literal-pathspecs used in the first place? what are the
> downsides?  IME, using magic file names is always worse, because it
> can run afoul of various shells that consider some characters special.

Indeed. I agree it is much simpler to use the flag.

    AndyM

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

From: Andy Moreton <andrewjmoreton <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Sun, 07 Nov 2021 10:45:50 +0000

On Sun 07 Nov 2021, Eli Zaretskii wrote:

>> From: Andy Moreton <andrewjmoreton <at> gmail.com>
>> Date: Sat, 06 Nov 2021 23:03:24 +0000
>> 
>> Looking at this again, Trying "C-x v l" for INSTALL in the repo master
>> branch gives (rewrapped for clarity):
>> 
>>   fatal: :(literal)c:/emacs/git/emacs/master/nt/INSTALL:
>>   'c:/emacs/git/emacs/master/nt/INSTALL' is outside repository at
>>   '/c/emacs/git/emacs/master'
>> 
>> This appears to be due to the translation between win32 and cygwin
>> (posix) filenames.
>
> Right, and so the immediate suspect is cygwin-mount.el.  That command
> works flawlessly for me, FWIW.

Yes. It is probably due to the ":(literal)" prefix names not matching
the patterns that cygwin-mount.el adds to file-name-handler-alist.

    AndyM

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 51497 <at> debbugs.gnu.org, andrewjmoreton <at> gmail.com
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Mon, 8 Nov 2021 01:36:18 +0300

On 07.11.2021 09:47, Eli Zaretskii wrote:
>> From: Dmitry Gutov <dgutov <at> yandex.ru>
>> Date: Sun, 7 Nov 2021 03:11:43 +0300
>>
>> Failing that, I think we'll need to change the "literal pathspecs"
>> implementation to yet another approach (adding --literl-pathspecs flag
>> instead of manipulating file names). It comes with the same general
>> drawbacks as the env var (which is used under the hood), but the
>> explicit approach of specifying it in every command would avoid the
>> problem of my original fix for that bug.
> 
> Why wasn't --literal-pathspecs used in the first place? what are the
> downsides?  IME, using magic file names is always worse, because it
> can run afoul of various shells that consider some characters special.

It wasn't among the proposed solutions.

I went with the env var solution initially (because it required less 
code and brought fewer -- none -- Git version compatibility problems), 
but it didn't yield itself as easily to the per-action opt-in as the 
other proposal (currently installed).

But now that I think about it, it would be possible to do this without a 
new macro, just adding a new variable that default to nil, and set it to 
t in every backend method that needs it.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 51497 <at> debbugs.gnu.org, andrewjmoreton <at> gmail.com
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Mon, 08 Nov 2021 14:49:47 +0200

> Cc: 51497 <at> debbugs.gnu.org, andrewjmoreton <at> gmail.com
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Mon, 8 Nov 2021 01:36:18 +0300
> 
> > Why wasn't --literal-pathspecs used in the first place? what are the
> > downsides?  IME, using magic file names is always worse, because it
> > can run afoul of various shells that consider some characters special.
> 
> It wasn't among the proposed solutions.
> 
> I went with the env var solution initially (because it required less 
> code and brought fewer -- none -- Git version compatibility problems), 
> but it didn't yield itself as easily to the per-action opt-in as the 
> other proposal (currently installed).
> 
> But now that I think about it, it would be possible to do this without a 
> new macro, just adding a new variable that default to nil, and set it to 
> t in every backend method that needs it.

But would that solve our problems for which :(literal) was introduced?
AFAIU, the difference between that and --literal-pathspecs is that the
latter is global: it affects all the file names of the Git command,
while the former can be applied only to some file names.  Do we have
valid use cases where only some of the file names need to be treated
as literal?

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 51497 <at> debbugs.gnu.org, andrewjmoreton <at> gmail.com
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Mon, 8 Nov 2021 20:30:55 +0300

On 08.11.2021 15:49, Eli Zaretskii wrote:
>> Cc: 51497 <at> debbugs.gnu.org, andrewjmoreton <at> gmail.com
>> From: Dmitry Gutov <dgutov <at> yandex.ru>
>> Date: Mon, 8 Nov 2021 01:36:18 +0300
>>
>>> Why wasn't --literal-pathspecs used in the first place? what are the
>>> downsides?  IME, using magic file names is always worse, because it
>>> can run afoul of various shells that consider some characters special.
>>
>> It wasn't among the proposed solutions.
>>
>> I went with the env var solution initially (because it required less
>> code and brought fewer -- none -- Git version compatibility problems),
>> but it didn't yield itself as easily to the per-action opt-in as the
>> other proposal (currently installed).
>>
>> But now that I think about it, it would be possible to do this without a
>> new macro, just adding a new variable that default to nil, and set it to
>> t in every backend method that needs it.
> 
> But would that solve our problems for which :(literal) was introduced?
> AFAIU, the difference between that and --literal-pathspecs is that the
> latter is global: it affects all the file names of the Git command,
> while the former can be applied only to some file names.

Both can be used per-command, but indeed it's true: the :(literal) 
syntax can also be used to apply to individual specs only.

>Do we have
> valid use cases where only some of the file names need to be treated
> as literal?

Even though it's plausible, I haven't encountered this particular use 
case so far. Perhaps when we do, we could mix-and-match :(literal) and 
--literal-pathspecs.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 51497 <at> debbugs.gnu.org, andrewjmoreton <at> gmail.com
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Mon, 08 Nov 2021 20:18:12 +0200

> Cc: 51497 <at> debbugs.gnu.org, andrewjmoreton <at> gmail.com
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Mon, 8 Nov 2021 20:30:55 +0300
> 
> >> But now that I think about it, it would be possible to do this without a
> >> new macro, just adding a new variable that default to nil, and set it to
> >> t in every backend method that needs it.
> > 
> > But would that solve our problems for which :(literal) was introduced?
> > AFAIU, the difference between that and --literal-pathspecs is that the
> > latter is global: it affects all the file names of the Git command,
> > while the former can be applied only to some file names.
> 
> Both can be used per-command, but indeed it's true: the :(literal) 
> syntax can also be used to apply to individual specs only.
> 
> >Do we have
> > valid use cases where only some of the file names need to be treated
> > as literal?
> 
> Even though it's plausible, I haven't encountered this particular use 
> case so far. Perhaps when we do, we could mix-and-match :(literal) and 
> --literal-pathspecs.

So what would you suggest as the way forward, for both emacs-28 and
the master branches (the 2 solutions could be different)?  Do you
still prefer to go with your original patch for emacs-28?

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 51497 <at> debbugs.gnu.org, andrewjmoreton <at> gmail.com,
 Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Thu, 23 Dec 2021 11:28:40 +0100

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

>> Even though it's plausible, I haven't encountered this particular use 
>> case so far. Perhaps when we do, we could mix-and-match :(literal) and 
>> --literal-pathspecs.
>
> So what would you suggest as the way forward, for both emacs-28 and
> the master branches (the 2 solutions could be different)?  Do you
> still prefer to go with your original patch for emacs-28?

This was the final message in this thread, and it was six weeks ago.  I
haven't paid attention to the patches in this area -- is this still an
issue, or has it been fixed?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 51497 <at> debbugs.gnu.org, andrewjmoreton <at> gmail.com
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Sun, 26 Dec 2021 02:53:42 +0200

On 23.12.2021 13:28, Lars Ingebrigtsen wrote:
> Eli Zaretskii<eliz <at> gnu.org>  writes:
> 
>>> Even though it's plausible, I haven't encountered this particular use
>>> case so far. Perhaps when we do, we could mix-and-match :(literal) and
>>> --literal-pathspecs.
>> So what would you suggest as the way forward, for both emacs-28 and
>> the master branches (the 2 solutions could be different)?  Do you
>> still prefer to go with your original patch for emacs-28?
> This was the final message in this thread, and it was six weeks ago.  I
> haven't paid attention to the patches in this area -- is this still an
> issue, or has it been fixed?

Not fixed, no.

I'll send a patch which reverts to my original approach with an escape 
hatch, tomorrow.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 51497 <at> debbugs.gnu.org, andrewjmoreton <at> gmail.com
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Mon, 27 Dec 2021 03:36:33 +0200

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

On 08.11.2021 21:18, Eli Zaretskii wrote:
> So what would you suggest as the way forward, for both emacs-28 and
> the master branches (the 2 solutions could be different)?  Do you
> still prefer to go with your original patch for emacs-28?

I'm still of two minds a little bit: conceptually, the current approach 
is a little cleaner because it forces the opt-in approach, and thus 
won't affect any command (or use of functions like 
vc-git--run-command-string outside of vc-git.el) that didn't opt into 
using literal pathspecs.

But my original approach is simpler and shorter, and together with an 
opt-out var seems to solve every problem so far. It doesn't need version 
detection either (a patch to have it work on remote hosts was discussed 
previously here).

So here's the patch (my current preferred solution for both emacs-28 and 
master).

Waiting for feedback from AndyM.

[redo-literal-pathspecs.diff (text/x-patch, attachment)]

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 51497 <at> debbugs.gnu.org, andrewjmoreton <at> gmail.com,
 Dima Kogan <dima <at> secretsauce.net>
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Mon, 3 Jan 2022 05:59:47 +0200

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

On 27.12.2021 04:36, Dmitry Gutov wrote:
> So here's the patch (my current preferred solution for both emacs-28 and 
> master).
> 
> Waiting for feedback from AndyM.

Sorry, here's a version of the patch that doesn't break vc-checkin.

Waiting for Andy's feedback, I wouldn't mind some testing from Dima as 
well, BTW.

[redo-literal-pathspecs.diff (text/x-patch, attachment)]

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

From: Andy Moreton <andrewjmoreton <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Mon, 03 Jan 2022 21:16:56 +0000

On Mon 03 Jan 2022, Dmitry Gutov wrote:

> On 27.12.2021 04:36, Dmitry Gutov wrote:
>> So here's the patch (my current preferred solution for both emacs-28 and
>> master).
>> Waiting for feedback from AndyM.
>
> Sorry, here's a version of the patch that doesn't break vc-checkin.
>
> Waiting for Andy's feedback, I wouldn't mind some testing from Dima as well,
> BTW.

I've not tested checkin, but some light testing for looking at repo
history with this patch applied works nicely for me on both emacs-28 and
master.

Thanks,

    AndyM

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

From: Dima Kogan <dima <at> secretsauce.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 51497 <at> debbugs.gnu.org
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Mon, 03 Jan 2022 13:15:39 -0800

Dmitry Gutov <dgutov <at> yandex.ru> writes:

> I wouldn't mind some testing from Dima as well, BTW.

I just tested it: it works. I started up a very recent emacs, opened a
remote file on an old box, and saw that vc-git doesn't work. Then I
applied the patch, refreshed, and vc-git works again.

Thanks!

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Dima Kogan <dima <at> secretsauce.net>, Andy Moreton
 <andrewjmoreton <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 51497 <at> debbugs.gnu.org
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Tue, 4 Jan 2022 00:51:18 +0200

On 04.01.2022 00:15, Dima Kogan wrote:
> I just tested it: it works. I started up a very recent emacs, opened a
> remote file on an old box, and saw that vc-git doesn't work. Then I
> applied the patch, refreshed, and vc-git works again.

On 04.01.2022 00:16, Andy Moreton wrote:
> I've not tested checkin, but some light testing for looking at repo
> history with this patch applied works nicely for me on both emacs-28 and
> master.

Thanks for checking, fellas.

Eli, good for emacs-28?

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 51497 <at> debbugs.gnu.org, andrewjmoreton <at> gmail.com, dima <at> secretsauce.net
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Tue, 04 Jan 2022 05:28:37 +0200

> Cc: 51497 <at> debbugs.gnu.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Tue, 4 Jan 2022 00:51:18 +0200
> 
> On 04.01.2022 00:15, Dima Kogan wrote:
> > I just tested it: it works. I started up a very recent emacs, opened a
> > remote file on an old box, and saw that vc-git doesn't work. Then I
> > applied the patch, refreshed, and vc-git works again.
> 
> On 04.01.2022 00:16, Andy Moreton wrote:
>  > I've not tested checkin, but some light testing for looking at repo
>  > history with this patch applied works nicely for me on both emacs-28 and
>  > master.
> 
> Thanks for checking, fellas.
> 
> Eli, good for emacs-28?

Yes, thanks.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 51497 <at> debbugs.gnu.org, andrewjmoreton <at> gmail.com, dima <at> secretsauce.net
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Wed, 5 Jan 2022 04:09:49 +0200

On 04.01.2022 06:28, Eli Zaretskii wrote:
>> Cc: 51497 <at> debbugs.gnu.org
>> From: Dmitry Gutov <dgutov <at> yandex.ru>
>> Date: Tue, 4 Jan 2022 00:51:18 +0200
>>
>> On 04.01.2022 00:15, Dima Kogan wrote:
>>> I just tested it: it works. I started up a very recent emacs, opened a
>>> remote file on an old box, and saw that vc-git doesn't work. Then I
>>> applied the patch, refreshed, and vc-git works again.
>>
>> On 04.01.2022 00:16, Andy Moreton wrote:
>>   > I've not tested checkin, but some light testing for looking at repo
>>   > history with this patch applied works nicely for me on both emacs-28 and
>>   > master.
>>
>> Thanks for checking, fellas.
>>
>> Eli, good for emacs-28?
> 
> Yes, thanks.

Done!

Thanks all, I'm closing this bug.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 51497 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 andrewjmoreton <at> gmail.com, dima <at> secretsauce.net
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Fri, 21 Jan 2022 14:50:16 +0100

Dmitry Gutov <dgutov <at> yandex.ru> writes:

> Thanks all, I'm closing this bug.

Looks like the bug was still open (perhaps due to the debbugs server
problems some weeks back?), so I'm closing it now.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 51497 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 andrewjmoreton <at> gmail.com, dima <at> secretsauce.net
Subject: Re: bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
Date: Fri, 21 Jan 2022 16:11:28 +0200

On 21.01.2022 15:50, Lars Ingebrigtsen wrote:
> Dmitry Gutov <dgutov <at> yandex.ru> writes:
> 
>> Thanks all, I'm closing this bug.
> 
> Looks like the bug was still open (perhaps due to the debbugs server
> problems some weeks back?), so I'm closing it now.

Sorry. My current version of Thunderbird occasionally undoes edits in 
the participants' email addresses.

Previous Next

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