* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP
@ 2021-10-30 1:24 dima
2021-10-30 12:48 ` Lars Ingebrigtsen
0 siblings, 1 reply; 44+ messages in thread
From: dima @ 2021-10-30 1:24 UTC (permalink / raw)
To: 51497; +Cc: Wolfgang Scherer
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@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.
^ permalink raw reply [flat|nested] 44+ messages in thread* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-10-30 1:24 bug#51497: 29.0.50; (vc-print-log) broken over TRAMP dima @ 2021-10-30 12:48 ` Lars Ingebrigtsen 2021-10-30 13:21 ` Dmitry Gutov 2021-10-30 19:01 ` Dima Kogan 0 siblings, 2 replies; 44+ messages in thread From: Lars Ingebrigtsen @ 2021-10-30 12:48 UTC (permalink / raw) To: dima; +Cc: 51497, Wolfgang Scherer dima@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 ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-10-30 12:48 ` Lars Ingebrigtsen @ 2021-10-30 13:21 ` Dmitry Gutov 2021-10-30 19:01 ` Dima Kogan 1 sibling, 0 replies; 44+ messages in thread From: Dmitry Gutov @ 2021-10-30 13:21 UTC (permalink / raw) To: Lars Ingebrigtsen, dima; +Cc: 51497, Wolfgang Scherer 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. ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-10-30 12:48 ` Lars Ingebrigtsen 2021-10-30 13:21 ` Dmitry Gutov @ 2021-10-30 19:01 ` Dima Kogan 2021-10-31 0:56 ` Dmitry Gutov 1 sibling, 1 reply; 44+ messages in thread From: Dima Kogan @ 2021-10-30 19:01 UTC (permalink / raw) To: Lars Ingebrigtsen, Dmitry Gutov; +Cc: 51497, Wolfgang Scherer Hi. Thanks for replying. Notes inline Lars Ingebrigtsen <larsi@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@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@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. ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-10-30 19:01 ` Dima Kogan @ 2021-10-31 0:56 ` Dmitry Gutov 2021-10-31 6:58 ` Dima Kogan 0 siblings, 1 reply; 44+ messages in thread From: Dmitry Gutov @ 2021-10-31 0:56 UTC (permalink / raw) To: Dima Kogan, Lars Ingebrigtsen; +Cc: 51497, Wolfgang Scherer 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. ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-10-31 0:56 ` Dmitry Gutov @ 2021-10-31 6:58 ` Dima Kogan 2021-10-31 8:16 ` Michael Albinus 2021-11-03 2:03 ` Dmitry Gutov 0 siblings, 2 replies; 44+ messages in thread From: Dima Kogan @ 2021-10-31 6:58 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 51497, Lars Ingebrigtsen, Wolfgang Scherer 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@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 ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-10-31 6:58 ` Dima Kogan @ 2021-10-31 8:16 ` Michael Albinus 2021-10-31 12:26 ` Dmitry Gutov 2021-11-03 2:03 ` Dmitry Gutov 1 sibling, 1 reply; 44+ messages in thread From: Michael Albinus @ 2021-10-31 8:16 UTC (permalink / raw) To: Dima Kogan; +Cc: 51497, Lars Ingebrigtsen, Dmitry Gutov, Wolfgang Scherer Dima Kogan <dima@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@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. ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-10-31 8:16 ` Michael Albinus @ 2021-10-31 12:26 ` Dmitry Gutov 2021-10-31 16:05 ` Michael Albinus 0 siblings, 1 reply; 44+ messages in thread From: Dmitry Gutov @ 2021-10-31 12:26 UTC (permalink / raw) To: Michael Albinus, Dima Kogan; +Cc: 51497, Lars Ingebrigtsen, Wolfgang Scherer 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? ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-10-31 12:26 ` Dmitry Gutov @ 2021-10-31 16:05 ` Michael Albinus 2021-11-03 2:00 ` Dmitry Gutov 0 siblings, 1 reply; 44+ messages in thread From: Michael Albinus @ 2021-10-31 16:05 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 51497, Lars Ingebrigtsen, Dima Kogan, Wolfgang Scherer Dmitry Gutov <dgutov@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. ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-10-31 16:05 ` Michael Albinus @ 2021-11-03 2:00 ` Dmitry Gutov 2021-11-03 17:09 ` Michael Albinus 0 siblings, 1 reply; 44+ messages in thread From: Dmitry Gutov @ 2021-11-03 2:00 UTC (permalink / raw) To: Michael Albinus; +Cc: 51497, Lars Ingebrigtsen, Dima Kogan, Wolfgang Scherer 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. ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-03 2:00 ` Dmitry Gutov @ 2021-11-03 17:09 ` Michael Albinus 2021-11-05 2:00 ` Dmitry Gutov 0 siblings, 1 reply; 44+ messages in thread From: Michael Albinus @ 2021-11-03 17:09 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 51497, Lars Ingebrigtsen, Dima Kogan, Wolfgang Scherer Dmitry Gutov <dgutov@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. ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-03 17:09 ` Michael Albinus @ 2021-11-05 2:00 ` Dmitry Gutov 0 siblings, 0 replies; 44+ messages in thread From: Dmitry Gutov @ 2021-11-05 2:00 UTC (permalink / raw) To: Michael Albinus; +Cc: 51497, Lars Ingebrigtsen, Dima Kogan, Wolfgang Scherer 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. ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-10-31 6:58 ` Dima Kogan 2021-10-31 8:16 ` Michael Albinus @ 2021-11-03 2:03 ` Dmitry Gutov 2021-11-03 3:03 ` Dima Kogan 1 sibling, 1 reply; 44+ messages in thread From: Dmitry Gutov @ 2021-11-03 2:03 UTC (permalink / raw) To: Dima Kogan; +Cc: 51497, Lars Ingebrigtsen, Wolfgang Scherer [-- Attachment #1: Type: text/plain, Size: 1068 bytes --] 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@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. [-- Attachment #2: no-literal-pathspecs-on-centos.diff --] [-- Type: text/x-patch, Size: 2651 bytes --] diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el index 3f89fad235..80455b2010 100644 --- a/lisp/vc/vc-git.el +++ b/lisp/vc/vc-git.el @@ -249,7 +249,9 @@ vc-git--literal-pathspec ;; Expand abbreviated file names. (when (file-name-absolute-p file) (setq file (expand-file-name file))) - (concat ":(literal)" (file-local-name file)))) + (if (version<= "2.1.4" (vc-git--program-version)) + (concat ":(literal)" (file-local-name file)) + (file-local-name file)))) (defun vc-git--literal-pathspecs (files) "Prepend :(literal) path magic to FILES." @@ -293,18 +295,32 @@ vc-git--state-code (defvar vc-git--program-version nil) (defun vc-git--program-version () - (or vc-git--program-version - (let ((version-string - (vc-git--run-command-string nil "version"))) - (setq vc-git--program-version - (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"))))) + (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)))) (defun vc-git--git-status-to-vc-state (code-list) "Convert CODE-LIST to a VC status. ^ permalink raw reply related [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-03 2:03 ` Dmitry Gutov @ 2021-11-03 3:03 ` Dima Kogan 2021-11-03 12:06 ` Dmitry Gutov 0 siblings, 1 reply; 44+ messages in thread From: Dima Kogan @ 2021-11-03 3:03 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 51497, Lars Ingebrigtsen, Wolfgang Scherer Dmitry Gutov <dgutov@yandex.ru> writes: > On 31.10.2021 09:58, Dima Kogan wrote: >> kogan@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. ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-03 3:03 ` Dima Kogan @ 2021-11-03 12:06 ` Dmitry Gutov 2021-11-06 13:22 ` Dmitry Gutov 0 siblings, 1 reply; 44+ messages in thread From: Dmitry Gutov @ 2021-11-03 12:06 UTC (permalink / raw) To: Dima Kogan; +Cc: 51497, Lars Ingebrigtsen, Wolfgang Scherer On 03.11.2021 06:03, Dima Kogan wrote: > Dmitry Gutov <dgutov@yandex.ru> writes: > >> On 31.10.2021 09:58, Dima Kogan wrote: >>> kogan@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? ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-03 12:06 ` Dmitry Gutov @ 2021-11-06 13:22 ` Dmitry Gutov 2021-11-06 15:51 ` Eli Zaretskii 0 siblings, 1 reply; 44+ messages in thread From: Dmitry Gutov @ 2021-11-06 13:22 UTC (permalink / raw) To: Lars Ingebrigtsen, Eli Zaretskii; +Cc: Dima Kogan, 51497, Wolfgang Scherer On 03.11.2021 15:06, Dmitry Gutov wrote: > Lars, Eli, can we put it in Emacs 28? Ping. ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-06 13:22 ` Dmitry Gutov @ 2021-11-06 15:51 ` Eli Zaretskii 2021-11-06 19:44 ` Dmitry Gutov 0 siblings, 1 reply; 44+ messages in thread From: Eli Zaretskii @ 2021-11-06 15:51 UTC (permalink / raw) To: Dmitry Gutov; +Cc: lists, 51497, larsi, wolfgang.scherer > From: Dmitry Gutov <dgutov@yandex.ru> > Cc: 51497@debbugs.gnu.org, Wolfgang Scherer <wolfgang.scherer@gmx.de>, > Dima Kogan <lists@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? ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-06 15:51 ` Eli Zaretskii @ 2021-11-06 19:44 ` Dmitry Gutov 2021-11-06 19:52 ` Eli Zaretskii 0 siblings, 1 reply; 44+ messages in thread From: Dmitry Gutov @ 2021-11-06 19:44 UTC (permalink / raw) To: Eli Zaretskii; +Cc: lists, 51497, larsi, wolfgang.scherer On 06.11.2021 18:51, Eli Zaretskii wrote: >> From: Dmitry Gutov <dgutov@yandex.ru> >> Cc: 51497@debbugs.gnu.org, Wolfgang Scherer <wolfgang.scherer@gmx.de>, >> Dima Kogan <lists@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. ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-06 19:44 ` Dmitry Gutov @ 2021-11-06 19:52 ` Eli Zaretskii 2021-11-06 22:11 ` Andy Moreton 2021-11-06 22:19 ` Dmitry Gutov 0 siblings, 2 replies; 44+ messages in thread From: Eli Zaretskii @ 2021-11-06 19:52 UTC (permalink / raw) To: Dmitry Gutov; +Cc: lists, 51497, larsi, wolfgang.scherer > Cc: lists@dima.secretsauce.net, 51497@debbugs.gnu.org, larsi@gnus.org, > wolfgang.scherer@gmx.de > From: Dmitry Gutov <dgutov@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. ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-06 19:52 ` Eli Zaretskii @ 2021-11-06 22:11 ` Andy Moreton 2021-11-06 22:21 ` Dmitry Gutov 2021-11-07 6:31 ` Eli Zaretskii 2021-11-06 22:19 ` Dmitry Gutov 1 sibling, 2 replies; 44+ messages in thread From: Andy Moreton @ 2021-11-06 22:11 UTC (permalink / raw) To: 51497 On Sat 06 Nov 2021, Eli Zaretskii wrote: >> Cc: lists@dima.secretsauce.net, 51497@debbugs.gnu.org, larsi@gnus.org, >> wolfgang.scherer@gmx.de >> From: Dmitry Gutov <dgutov@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 ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-06 22:11 ` Andy Moreton @ 2021-11-06 22:21 ` Dmitry Gutov 2021-11-06 23:03 ` Andy Moreton 2021-11-07 6:31 ` Eli Zaretskii 1 sibling, 1 reply; 44+ messages in thread From: Dmitry Gutov @ 2021-11-06 22:21 UTC (permalink / raw) To: Andy Moreton, 51497 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. ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-06 22:21 ` Dmitry Gutov @ 2021-11-06 23:03 ` Andy Moreton 2021-11-07 0:11 ` Dmitry Gutov 2021-11-07 6:43 ` Eli Zaretskii 0 siblings, 2 replies; 44+ messages in thread From: Andy Moreton @ 2021-11-06 23:03 UTC (permalink / raw) To: 51497 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 ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-06 23:03 ` Andy Moreton @ 2021-11-07 0:11 ` Dmitry Gutov 2021-11-07 6:47 ` Eli Zaretskii 2021-11-07 6:43 ` Eli Zaretskii 1 sibling, 1 reply; 44+ messages in thread From: Dmitry Gutov @ 2021-11-07 0:11 UTC (permalink / raw) To: Andy Moreton, 51497 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. ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-07 0:11 ` Dmitry Gutov @ 2021-11-07 6:47 ` Eli Zaretskii 2021-11-07 10:43 ` Andy Moreton 2021-11-07 22:36 ` Dmitry Gutov 0 siblings, 2 replies; 44+ messages in thread From: Eli Zaretskii @ 2021-11-07 6:47 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 51497, andrewjmoreton > From: Dmitry Gutov <dgutov@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. ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-07 6:47 ` Eli Zaretskii @ 2021-11-07 10:43 ` Andy Moreton 2021-11-07 22:36 ` Dmitry Gutov 1 sibling, 0 replies; 44+ messages in thread From: Andy Moreton @ 2021-11-07 10:43 UTC (permalink / raw) To: 51497 On Sun 07 Nov 2021, Eli Zaretskii wrote: >> From: Dmitry Gutov <dgutov@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 ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-07 6:47 ` Eli Zaretskii 2021-11-07 10:43 ` Andy Moreton @ 2021-11-07 22:36 ` Dmitry Gutov 2021-11-08 12:49 ` Eli Zaretskii 1 sibling, 1 reply; 44+ messages in thread From: Dmitry Gutov @ 2021-11-07 22:36 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 51497, andrewjmoreton On 07.11.2021 09:47, Eli Zaretskii wrote: >> From: Dmitry Gutov <dgutov@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. ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-07 22:36 ` Dmitry Gutov @ 2021-11-08 12:49 ` Eli Zaretskii 2021-11-08 17:30 ` Dmitry Gutov 0 siblings, 1 reply; 44+ messages in thread From: Eli Zaretskii @ 2021-11-08 12:49 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 51497, andrewjmoreton > Cc: 51497@debbugs.gnu.org, andrewjmoreton@gmail.com > From: Dmitry Gutov <dgutov@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? ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-08 12:49 ` Eli Zaretskii @ 2021-11-08 17:30 ` Dmitry Gutov 2021-11-08 18:18 ` Eli Zaretskii 0 siblings, 1 reply; 44+ messages in thread From: Dmitry Gutov @ 2021-11-08 17:30 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 51497, andrewjmoreton On 08.11.2021 15:49, Eli Zaretskii wrote: >> Cc: 51497@debbugs.gnu.org, andrewjmoreton@gmail.com >> From: Dmitry Gutov <dgutov@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. ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-08 17:30 ` Dmitry Gutov @ 2021-11-08 18:18 ` Eli Zaretskii 2021-12-23 10:28 ` Lars Ingebrigtsen 2021-12-27 1:36 ` Dmitry Gutov 0 siblings, 2 replies; 44+ messages in thread From: Eli Zaretskii @ 2021-11-08 18:18 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 51497, andrewjmoreton > Cc: 51497@debbugs.gnu.org, andrewjmoreton@gmail.com > From: Dmitry Gutov <dgutov@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? ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-08 18:18 ` Eli Zaretskii @ 2021-12-23 10:28 ` Lars Ingebrigtsen 2021-12-26 0:53 ` Dmitry Gutov 2021-12-27 1:36 ` Dmitry Gutov 1 sibling, 1 reply; 44+ messages in thread From: Lars Ingebrigtsen @ 2021-12-23 10:28 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 51497, andrewjmoreton, Dmitry Gutov Eli Zaretskii <eliz@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 ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-12-23 10:28 ` Lars Ingebrigtsen @ 2021-12-26 0:53 ` Dmitry Gutov 0 siblings, 0 replies; 44+ messages in thread From: Dmitry Gutov @ 2021-12-26 0:53 UTC (permalink / raw) To: Lars Ingebrigtsen, Eli Zaretskii; +Cc: 51497, andrewjmoreton On 23.12.2021 13:28, Lars Ingebrigtsen wrote: > Eli Zaretskii<eliz@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. ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-08 18:18 ` Eli Zaretskii 2021-12-23 10:28 ` Lars Ingebrigtsen @ 2021-12-27 1:36 ` Dmitry Gutov 2022-01-03 3:59 ` Dmitry Gutov 1 sibling, 1 reply; 44+ messages in thread From: Dmitry Gutov @ 2021-12-27 1:36 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 51497, andrewjmoreton [-- Attachment #1: Type: text/plain, Size: 886 bytes --] 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. [-- Attachment #2: redo-literal-pathspecs.diff --] [-- Type: text/x-patch, Size: 11333 bytes --] diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el index 3b634471ac..4c4eb915ed 100644 --- a/lisp/progmodes/project.el +++ b/lisp/progmodes/project.el @@ -502,10 +502,12 @@ project-files (declare-function vc-hg-command "vc-hg") (defun project--vc-list-files (dir backend extra-ignores) + (defvar vc-git-use-literal-pathspecs) (pcase backend (`Git (let ((default-directory (expand-file-name (file-name-as-directory dir))) (args '("-z")) + (vc-git-use-literal-pathspecs nil) files) ;; Include unregistered. (setq args (append args '("-c" "-o" "--exclude-standard"))) diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el index 5c6a39aec9..19264c9d3c 100644 --- a/lisp/vc/vc-git.el +++ b/lisp/vc/vc-git.el @@ -223,6 +223,12 @@ vc-git-revision-complete-only-branches ;; History of Git commands. (defvar vc-git-history nil) +;; Default to t because commands which don't support literal pathspecs +;; ignore the environment variable silently. +(defvar vc-git-use-literal-pathspecs t + "Non-nil to treat pathspecs in commands literally. +Good example of file name that needs this: \"test[56].xx\".") + ;; Clear up the cache to force vc-call to check again and discover ;; new functions when we reload this file. (put 'Git 'vc-functions nil) @@ -242,20 +248,6 @@ vc-git-update-on-retrieve-tag ;;;###autoload (load "vc-git" nil t) ;;;###autoload (vc-git-registered file)))) -;; Good example of file name that needs this: "test[56].xx". -(defun vc-git--literal-pathspec (file) - "Prepend :(literal) path magic to FILE." - (when file - ;; Expand abbreviated file names. - (when (file-name-absolute-p file) - (setq file (expand-file-name file))) - (concat ":(literal)" (file-local-name file)))) - -(defun vc-git--literal-pathspecs (files) - "Prepend :(literal) path magic to FILES." - (unless (vc-git--file-list-is-rootdir files) - (mapcar #'vc-git--literal-pathspec files))) - (defun vc-git-registered (file) "Check whether FILE is registered with git." (let ((dir (vc-git-root file))) @@ -269,12 +261,12 @@ vc-git-registered (name (file-relative-name file dir)) (str (with-demoted-errors "Error: %S" (cd dir) - (vc-git--out-ok "ls-files" "-c" "-z" "--" (vc-git--literal-pathspec name)) + (vc-git--out-ok "ls-files" "-c" "-z" "--" name) ;; If result is empty, use ls-tree to check for deleted ;; file. (when (eq (point-min) (point-max)) (vc-git--out-ok "ls-tree" "--name-only" "-z" "HEAD" - "--" (vc-git--literal-pathspec name))) + "--" name)) (buffer-string)))) (and str (> (length str) (length name)) @@ -358,7 +350,7 @@ vc-git-state ,@(when (version<= "1.7.6.3" (vc-git--program-version)) '("--ignored")) "--")) - (status (apply #'vc-git--run-command-string (vc-git--literal-pathspec file) args))) + (status (apply #'vc-git--run-command-string file args))) (if (null status) ;; If status is nil, there was an error calling git, likely because ;; the file is not in a git repo. @@ -636,28 +628,28 @@ vc-git-dir-status-goto-stage (pcase (vc-git-dir-status-state->stage git-state) ('update-index (if files - (vc-git-command (current-buffer) 'async (vc-git--literal-pathspecs files) "add" "--refresh" "--") + (vc-git-command (current-buffer) 'async files "add" "--refresh" "--") (vc-git-command (current-buffer) 'async nil "update-index" "--refresh"))) ('ls-files-added - (vc-git-command (current-buffer) 'async (vc-git--literal-pathspecs files) + (vc-git-command (current-buffer) 'async files "ls-files" "-z" "-c" "-s" "--")) ('ls-files-up-to-date - (vc-git-command (current-buffer) 'async (vc-git--literal-pathspecs files) + (vc-git-command (current-buffer) 'async files "ls-files" "-z" "-c" "-s" "--")) ('ls-files-conflict - (vc-git-command (current-buffer) 'async (vc-git--literal-pathspecs files) + (vc-git-command (current-buffer) 'async files "ls-files" "-z" "-u" "--")) ('ls-files-unknown - (vc-git-command (current-buffer) 'async (vc-git--literal-pathspecs files) + (vc-git-command (current-buffer) 'async files "ls-files" "-z" "-o" "--exclude-standard" "--")) ('ls-files-ignored - (vc-git-command (current-buffer) 'async (vc-git--literal-pathspecs files) + (vc-git-command (current-buffer) 'async files "ls-files" "-z" "-o" "-i" "--directory" "--no-empty-directory" "--exclude-standard" "--")) ;; --relative added in Git 1.5.5. ('diff-index - (vc-git-command (current-buffer) 'async (vc-git--literal-pathspecs files) + (vc-git-command (current-buffer) 'async files "diff-index" "--relative" "-z" "-M" "HEAD" "--"))) (vc-run-delayed (vc-git-after-dir-status-stage git-state)))) @@ -885,12 +877,12 @@ vc-git-register (when flist (vc-git-command nil 0 flist "update-index" "--add" "--")) (when dlist - (vc-git-command nil 0 (vc-git--literal-pathspecs dlist) "add")))) + (vc-git-command nil 0 dlist "add")))) (defalias 'vc-git-responsible-p #'vc-git-root) (defun vc-git-unregister (file) - (vc-git-command nil 0 (vc-git--literal-pathspec file) "rm" "-f" "--cached" "--")) + (vc-git-command nil 0 file "rm" "-f" "--cached" "--")) (declare-function log-edit-mode "log-edit" ()) (declare-function log-edit-toggle-header "log-edit" (header value)) @@ -956,7 +948,7 @@ vc-git-checkin (lambda (value) (when (equal value "yes") (list argument))))) ;; When operating on the whole tree, better pass "-a" than ".", since "." ;; fails when we're committing a merge. - (apply #'vc-git-command nil 0 (if only (vc-git--literal-pathspecs files)) + (apply #'vc-git-command nil 0 (if only files) (nconc (if msg-file (list "commit" "-F" (file-local-name msg-file)) (list "commit" "-m")) @@ -983,7 +975,7 @@ vc-git-find-revision (coding-system-for-write 'binary) (fullname (let ((fn (vc-git--run-command-string - (vc-git--literal-pathspec file) "ls-files" "-z" "--full-name" "--"))) + file "ls-files" "-z" "--full-name" "--"))) ;; ls-files does not return anything when looking for a ;; revision of a file that has been renamed or removed. (if (string= fn "") @@ -1000,14 +992,14 @@ vc-git-find-ignore-file (vc-git-root file))) (defun vc-git-checkout (file &optional rev) - (vc-git-command nil 0 (vc-git--literal-pathspec file) "checkout" (or rev "HEAD"))) + (vc-git-command nil 0 file "checkout" (or rev "HEAD"))) (defun vc-git-revert (file &optional contents-done) "Revert FILE to the version stored in the git repository." (if contents-done (vc-git-command nil 0 file "update-index" "--") - (vc-git-command nil 0 (vc-git--literal-pathspec file) "reset" "-q" "--") - (vc-git-command nil nil (vc-git--literal-pathspec file) "checkout" "-q" "--"))) + (vc-git-command nil 0 file "reset" "-q" "--") + (vc-git-command nil nil file "checkout" "-q" "--"))) (defvar vc-git-error-regexp-alist '(("^ \\(.+\\)\\> *|" 1 nil nil 0)) @@ -1091,7 +1083,7 @@ vc-git-merge-branch (defun vc-git-conflicted-files (directory) "Return the list of files with conflicts in DIRECTORY." (let* ((status - (vc-git--run-command-string (vc-git--literal-pathspec directory) "status" "--porcelain" "--")) + (vc-git--run-command-string directory "status" "--porcelain" "--")) (lines (when status (split-string status "\n" 'omit-nulls))) files) (dolist (line lines files) @@ -1180,7 +1172,7 @@ vc-git-print-log (let ((inhibit-read-only t)) (with-current-buffer buffer (apply #'vc-git-command buffer - 'async (vc-git--literal-pathspecs files) + 'async files (append '("log" "--no-color") (when (and vc-git-print-log-follow @@ -1434,7 +1426,7 @@ vc-git-diff (if vc-git-diff-switches (apply #'vc-git-command (or buffer "*vc-diff*") 1 ; bug#21969 - (vc-git--literal-pathspecs files) + files command "--exit-code" (append (vc-switches 'git 'diff) @@ -1519,7 +1511,7 @@ vc-git-previous-revision (let* ((fname (file-relative-name file)) (prev-rev (with-temp-buffer (and - (vc-git--out-ok "rev-list" "-2" rev "--" (vc-git--literal-pathspec fname)) + (vc-git--out-ok "rev-list" "-2" rev "--" fname) (goto-char (point-max)) (bolp) (zerop (forward-line -1)) @@ -1547,7 +1539,7 @@ vc-git-next-revision (current-rev (with-temp-buffer (and - (vc-git--out-ok "rev-list" "-1" rev "--" (vc-git--literal-pathspec file)) + (vc-git--out-ok "rev-list" "-1" rev "--" file) (goto-char (point-max)) (bolp) (zerop (forward-line -1)) @@ -1559,7 +1551,7 @@ vc-git-next-revision (and current-rev (with-temp-buffer (and - (vc-git--out-ok "rev-list" "HEAD" "--" (vc-git--literal-pathspec file)) + (vc-git--out-ok "rev-list" "HEAD" "--" file) (goto-char (point-min)) (search-forward current-rev nil t) (zerop (forward-line -1)) @@ -1569,13 +1561,13 @@ vc-git-next-revision (or (vc-git-symbolic-commit next-rev) next-rev))) (defun vc-git-delete-file (file) - (vc-git-command nil 0 (vc-git--literal-pathspec file) "rm" "-f" "--")) + (vc-git-command nil 0 file "rm" "-f" "--")) (defun vc-git-rename-file (old new) (vc-git-command nil 0 (list old new) "mv" "-f" "--")) (defun vc-git-mark-resolved (files) - (vc-git-command nil 0 (vc-git--literal-pathspecs files) "add")) + (vc-git-command nil 0 files "add")) (defvar vc-git-extra-menu-map (let ((map (make-sparse-keymap))) @@ -1797,6 +1789,8 @@ vc-git-command (process-environment (append `("GIT_DIR" + ,@(when vc-git-use-literal-pathspecs + "GIT_LITERAL_PATHSPECS=1") ;; Avoid repository locking during background operations ;; (bug#21559). ,@(when revert-buffer-in-progress-p @@ -1834,6 +1828,8 @@ vc-git--call (process-environment (append `("GIT_DIR" + ,@(when vc-git-use-literal-pathspecs + "GIT_LITERAL_PATHSPECS=1") ;; Avoid repository locking during background operations ;; (bug#21559). ,@(when revert-buffer-in-progress-p ^ permalink raw reply related [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-12-27 1:36 ` Dmitry Gutov @ 2022-01-03 3:59 ` Dmitry Gutov 2022-01-03 21:15 ` Dima Kogan 2022-01-03 21:16 ` Andy Moreton 0 siblings, 2 replies; 44+ messages in thread From: Dmitry Gutov @ 2022-01-03 3:59 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 51497, andrewjmoreton, Dima Kogan [-- Attachment #1: Type: text/plain, Size: 319 bytes --] 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. [-- Attachment #2: redo-literal-pathspecs.diff --] [-- Type: text/x-patch, Size: 10688 bytes --] diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el index 2d35061b26..4c873b7264 100644 --- a/lisp/vc/vc-git.el +++ b/lisp/vc/vc-git.el @@ -223,6 +223,12 @@ vc-git-revision-complete-only-branches ;; History of Git commands. (defvar vc-git-history nil) +;; Default to t because commands which don't support literal pathspecs +;; ignore the environment variable silently. +(defvar vc-git-use-literal-pathspecs t + "Non-nil to treat pathspecs in commands literally. +Good example of file name that needs this: \"test[56].xx\".") + ;; Clear up the cache to force vc-call to check again and discover ;; new functions when we reload this file. (put 'Git 'vc-functions nil) @@ -242,20 +248,6 @@ vc-git-update-on-retrieve-tag ;;;###autoload (load "vc-git" nil t) ;;;###autoload (vc-git-registered file)))) -;; Good example of file name that needs this: "test[56].xx". -(defun vc-git--literal-pathspec (file) - "Prepend :(literal) path magic to FILE." - (when file - ;; Expand abbreviated file names. - (when (file-name-absolute-p file) - (setq file (expand-file-name file))) - (concat ":(literal)" (file-local-name file)))) - -(defun vc-git--literal-pathspecs (files) - "Prepend :(literal) path magic to FILES." - (unless (vc-git--file-list-is-rootdir files) - (mapcar #'vc-git--literal-pathspec files))) - (defun vc-git-registered (file) "Check whether FILE is registered with git." (let ((dir (vc-git-root file))) @@ -269,12 +261,12 @@ vc-git-registered (name (file-relative-name file dir)) (str (with-demoted-errors "Error: %S" (cd dir) - (vc-git--out-ok "ls-files" "-c" "-z" "--" (vc-git--literal-pathspec name)) + (vc-git--out-ok "ls-files" "-c" "-z" "--" name) ;; If result is empty, use ls-tree to check for deleted ;; file. (when (eq (point-min) (point-max)) (vc-git--out-ok "ls-tree" "--name-only" "-z" "HEAD" - "--" (vc-git--literal-pathspec name))) + "--" name)) (buffer-string)))) (and str (> (length str) (length name)) @@ -356,7 +348,7 @@ vc-git-state ,@(when (version<= "1.7.6.3" (vc-git--program-version)) '("--ignored")) "--")) - (status (apply #'vc-git--run-command-string (vc-git--literal-pathspec file) args))) + (status (apply #'vc-git--run-command-string file args))) (if (null status) ;; If status is nil, there was an error calling git, likely because ;; the file is not in a git repo. @@ -634,28 +626,28 @@ vc-git-dir-status-goto-stage (pcase (vc-git-dir-status-state->stage git-state) ('update-index (if files - (vc-git-command (current-buffer) 'async (vc-git--literal-pathspecs files) "add" "--refresh" "--") + (vc-git-command (current-buffer) 'async files "add" "--refresh" "--") (vc-git-command (current-buffer) 'async nil "update-index" "--refresh"))) ('ls-files-added - (vc-git-command (current-buffer) 'async (vc-git--literal-pathspecs files) + (vc-git-command (current-buffer) 'async files "ls-files" "-z" "-c" "-s" "--")) ('ls-files-up-to-date - (vc-git-command (current-buffer) 'async (vc-git--literal-pathspecs files) + (vc-git-command (current-buffer) 'async files "ls-files" "-z" "-c" "-s" "--")) ('ls-files-conflict - (vc-git-command (current-buffer) 'async (vc-git--literal-pathspecs files) + (vc-git-command (current-buffer) 'async files "ls-files" "-z" "-u" "--")) ('ls-files-unknown - (vc-git-command (current-buffer) 'async (vc-git--literal-pathspecs files) + (vc-git-command (current-buffer) 'async files "ls-files" "-z" "-o" "--exclude-standard" "--")) ('ls-files-ignored - (vc-git-command (current-buffer) 'async (vc-git--literal-pathspecs files) + (vc-git-command (current-buffer) 'async files "ls-files" "-z" "-o" "-i" "--directory" "--no-empty-directory" "--exclude-standard" "--")) ;; --relative added in Git 1.5.5. ('diff-index - (vc-git-command (current-buffer) 'async (vc-git--literal-pathspecs files) + (vc-git-command (current-buffer) 'async files "diff-index" "--relative" "-z" "-M" "HEAD" "--"))) (vc-run-delayed (vc-git-after-dir-status-stage git-state)))) @@ -883,12 +875,12 @@ vc-git-register (when flist (vc-git-command nil 0 flist "update-index" "--add" "--")) (when dlist - (vc-git-command nil 0 (vc-git--literal-pathspecs dlist) "add")))) + (vc-git-command nil 0 dlist "add")))) (defalias 'vc-git-responsible-p #'vc-git-root) (defun vc-git-unregister (file) - (vc-git-command nil 0 (vc-git--literal-pathspec file) "rm" "-f" "--cached" "--")) + (vc-git-command nil 0 file "rm" "-f" "--cached" "--")) (declare-function log-edit-mode "log-edit" ()) (declare-function log-edit-toggle-header "log-edit" (header value)) @@ -954,7 +946,7 @@ vc-git-checkin (lambda (value) (when (equal value "yes") (list argument))))) ;; When operating on the whole tree, better pass "-a" than ".", since "." ;; fails when we're committing a merge. - (apply #'vc-git-command nil 0 (if only (vc-git--literal-pathspecs files)) + (apply #'vc-git-command nil 0 (if only files) (nconc (if msg-file (list "commit" "-F" (file-local-name msg-file)) (list "commit" "-m")) @@ -981,7 +973,7 @@ vc-git-find-revision (coding-system-for-write 'binary) (fullname (let ((fn (vc-git--run-command-string - (vc-git--literal-pathspec file) "ls-files" "-z" "--full-name" "--"))) + file "ls-files" "-z" "--full-name" "--"))) ;; ls-files does not return anything when looking for a ;; revision of a file that has been renamed or removed. (if (string= fn "") @@ -998,14 +990,14 @@ vc-git-find-ignore-file (vc-git-root file))) (defun vc-git-checkout (file &optional rev) - (vc-git-command nil 0 (vc-git--literal-pathspec file) "checkout" (or rev "HEAD"))) + (vc-git-command nil 0 file "checkout" (or rev "HEAD"))) (defun vc-git-revert (file &optional contents-done) "Revert FILE to the version stored in the git repository." (if contents-done (vc-git-command nil 0 file "update-index" "--") - (vc-git-command nil 0 (vc-git--literal-pathspec file) "reset" "-q" "--") - (vc-git-command nil nil (vc-git--literal-pathspec file) "checkout" "-q" "--"))) + (vc-git-command nil 0 file "reset" "-q" "--") + (vc-git-command nil nil file "checkout" "-q" "--"))) (defvar vc-git-error-regexp-alist '(("^ \\(.+\\)\\> *|" 1 nil nil 0)) @@ -1089,7 +1081,7 @@ vc-git-merge-branch (defun vc-git-conflicted-files (directory) "Return the list of files with conflicts in DIRECTORY." (let* ((status - (vc-git--run-command-string (vc-git--literal-pathspec directory) "status" "--porcelain" "--")) + (vc-git--run-command-string directory "status" "--porcelain" "--")) (lines (when status (split-string status "\n" 'omit-nulls))) files) (dolist (line lines files) @@ -1178,7 +1170,7 @@ vc-git-print-log (let ((inhibit-read-only t)) (with-current-buffer buffer (apply #'vc-git-command buffer - 'async (vc-git--literal-pathspecs files) + 'async files (append '("log" "--no-color") (when (and vc-git-print-log-follow @@ -1432,7 +1424,7 @@ vc-git-diff (if vc-git-diff-switches (apply #'vc-git-command (or buffer "*vc-diff*") 1 ; bug#21969 - (vc-git--literal-pathspecs files) + files command "--exit-code" (append (vc-switches 'git 'diff) @@ -1517,7 +1509,7 @@ vc-git-previous-revision (let* ((fname (file-relative-name file)) (prev-rev (with-temp-buffer (and - (vc-git--out-ok "rev-list" "-2" rev "--" (vc-git--literal-pathspec fname)) + (vc-git--out-ok "rev-list" "-2" rev "--" fname) (goto-char (point-max)) (bolp) (zerop (forward-line -1)) @@ -1545,7 +1537,7 @@ vc-git-next-revision (current-rev (with-temp-buffer (and - (vc-git--out-ok "rev-list" "-1" rev "--" (vc-git--literal-pathspec file)) + (vc-git--out-ok "rev-list" "-1" rev "--" file) (goto-char (point-max)) (bolp) (zerop (forward-line -1)) @@ -1557,7 +1549,7 @@ vc-git-next-revision (and current-rev (with-temp-buffer (and - (vc-git--out-ok "rev-list" "HEAD" "--" (vc-git--literal-pathspec file)) + (vc-git--out-ok "rev-list" "HEAD" "--" file) (goto-char (point-min)) (search-forward current-rev nil t) (zerop (forward-line -1)) @@ -1567,13 +1559,13 @@ vc-git-next-revision (or (vc-git-symbolic-commit next-rev) next-rev))) (defun vc-git-delete-file (file) - (vc-git-command nil 0 (vc-git--literal-pathspec file) "rm" "-f" "--")) + (vc-git-command nil 0 file "rm" "-f" "--")) (defun vc-git-rename-file (old new) (vc-git-command nil 0 (list old new) "mv" "-f" "--")) (defun vc-git-mark-resolved (files) - (vc-git-command nil 0 (vc-git--literal-pathspecs files) "add")) + (vc-git-command nil 0 files "add")) (defvar vc-git-extra-menu-map (let ((map (make-sparse-keymap))) @@ -1796,6 +1788,8 @@ vc-git-command (process-environment (append `("GIT_DIR" + ,@(when vc-git-use-literal-pathspecs + '("GIT_LITERAL_PATHSPECS=1")) ;; Avoid repository locking during background operations ;; (bug#21559). ,@(when revert-buffer-in-progress-p @@ -1833,6 +1827,8 @@ vc-git--call (process-environment (append `("GIT_DIR" + ,@(when vc-git-use-literal-pathspecs + '("GIT_LITERAL_PATHSPECS=1")) ;; Avoid repository locking during background operations ;; (bug#21559). ,@(when revert-buffer-in-progress-p ^ permalink raw reply related [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2022-01-03 3:59 ` Dmitry Gutov @ 2022-01-03 21:15 ` Dima Kogan 2022-01-03 22:51 ` Dmitry Gutov 2022-01-03 21:16 ` Andy Moreton 1 sibling, 1 reply; 44+ messages in thread From: Dima Kogan @ 2022-01-03 21:15 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 51497 Dmitry Gutov <dgutov@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! ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2022-01-03 21:15 ` Dima Kogan @ 2022-01-03 22:51 ` Dmitry Gutov 2022-01-04 3:28 ` Eli Zaretskii 0 siblings, 1 reply; 44+ messages in thread From: Dmitry Gutov @ 2022-01-03 22:51 UTC (permalink / raw) To: Dima Kogan, Andy Moreton, Eli Zaretskii; +Cc: 51497 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? ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2022-01-03 22:51 ` Dmitry Gutov @ 2022-01-04 3:28 ` Eli Zaretskii 2022-01-05 2:09 ` Dmitry Gutov 0 siblings, 1 reply; 44+ messages in thread From: Eli Zaretskii @ 2022-01-04 3:28 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 51497, andrewjmoreton, dima > Cc: 51497@debbugs.gnu.org > From: Dmitry Gutov <dgutov@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. ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2022-01-04 3:28 ` Eli Zaretskii @ 2022-01-05 2:09 ` Dmitry Gutov 2022-01-21 13:50 ` Lars Ingebrigtsen 0 siblings, 1 reply; 44+ messages in thread From: Dmitry Gutov @ 2022-01-05 2:09 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 51497, andrewjmoreton, dima On 04.01.2022 06:28, Eli Zaretskii wrote: >> Cc: 51497@debbugs.gnu.org >> From: Dmitry Gutov <dgutov@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. ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2022-01-05 2:09 ` Dmitry Gutov @ 2022-01-21 13:50 ` Lars Ingebrigtsen 2022-01-21 14:11 ` Dmitry Gutov 0 siblings, 1 reply; 44+ messages in thread From: Lars Ingebrigtsen @ 2022-01-21 13:50 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 51497, andrewjmoreton, dima Dmitry Gutov <dgutov@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 ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2022-01-21 13:50 ` Lars Ingebrigtsen @ 2022-01-21 14:11 ` Dmitry Gutov 0 siblings, 0 replies; 44+ messages in thread From: Dmitry Gutov @ 2022-01-21 14:11 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: 51497, andrewjmoreton, dima On 21.01.2022 15:50, Lars Ingebrigtsen wrote: > Dmitry Gutov <dgutov@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. ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2022-01-03 3:59 ` Dmitry Gutov 2022-01-03 21:15 ` Dima Kogan @ 2022-01-03 21:16 ` Andy Moreton 1 sibling, 0 replies; 44+ messages in thread From: Andy Moreton @ 2022-01-03 21:16 UTC (permalink / raw) To: 51497 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 ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-06 23:03 ` Andy Moreton 2021-11-07 0:11 ` Dmitry Gutov @ 2021-11-07 6:43 ` Eli Zaretskii 2021-11-07 10:45 ` Andy Moreton 1 sibling, 1 reply; 44+ messages in thread From: Eli Zaretskii @ 2021-11-07 6:43 UTC (permalink / raw) To: Andy Moreton; +Cc: 51497 > From: Andy Moreton <andrewjmoreton@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. ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-07 6:43 ` Eli Zaretskii @ 2021-11-07 10:45 ` Andy Moreton 0 siblings, 0 replies; 44+ messages in thread From: Andy Moreton @ 2021-11-07 10:45 UTC (permalink / raw) To: 51497 On Sun 07 Nov 2021, Eli Zaretskii wrote: >> From: Andy Moreton <andrewjmoreton@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 ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-06 22:11 ` Andy Moreton 2021-11-06 22:21 ` Dmitry Gutov @ 2021-11-07 6:31 ` Eli Zaretskii 1 sibling, 0 replies; 44+ messages in thread From: Eli Zaretskii @ 2021-11-07 6:31 UTC (permalink / raw) To: Andy Moreton; +Cc: 51497 > From: Andy Moreton <andrewjmoreton@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? ^ permalink raw reply [flat|nested] 44+ messages in thread
* bug#51497: 29.0.50; (vc-print-log) broken over TRAMP 2021-11-06 19:52 ` Eli Zaretskii 2021-11-06 22:11 ` Andy Moreton @ 2021-11-06 22:19 ` Dmitry Gutov 1 sibling, 0 replies; 44+ messages in thread From: Dmitry Gutov @ 2021-11-06 22:19 UTC (permalink / raw) To: Eli Zaretskii; +Cc: lists, 51497, larsi, wolfgang.scherer On 06.11.2021 22:52, Eli Zaretskii wrote: >> Cc: lists@dima.secretsauce.net, 51497@debbugs.gnu.org, larsi@gnus.org, >> wolfgang.scherer@gmx.de >> From: Dmitry Gutov <dgutov@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. ^ permalink raw reply [flat|nested] 44+ messages in thread
end of thread, other threads:[~2022-01-21 14:11 UTC | newest] Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-30 1:24 bug#51497: 29.0.50; (vc-print-log) broken over TRAMP dima 2021-10-30 12:48 ` Lars Ingebrigtsen 2021-10-30 13:21 ` Dmitry Gutov 2021-10-30 19:01 ` Dima Kogan 2021-10-31 0:56 ` Dmitry Gutov 2021-10-31 6:58 ` Dima Kogan 2021-10-31 8:16 ` Michael Albinus 2021-10-31 12:26 ` Dmitry Gutov 2021-10-31 16:05 ` Michael Albinus 2021-11-03 2:00 ` Dmitry Gutov 2021-11-03 17:09 ` Michael Albinus 2021-11-05 2:00 ` Dmitry Gutov 2021-11-03 2:03 ` Dmitry Gutov 2021-11-03 3:03 ` Dima Kogan 2021-11-03 12:06 ` Dmitry Gutov 2021-11-06 13:22 ` Dmitry Gutov 2021-11-06 15:51 ` Eli Zaretskii 2021-11-06 19:44 ` Dmitry Gutov 2021-11-06 19:52 ` Eli Zaretskii 2021-11-06 22:11 ` Andy Moreton 2021-11-06 22:21 ` Dmitry Gutov 2021-11-06 23:03 ` Andy Moreton 2021-11-07 0:11 ` Dmitry Gutov 2021-11-07 6:47 ` Eli Zaretskii 2021-11-07 10:43 ` Andy Moreton 2021-11-07 22:36 ` Dmitry Gutov 2021-11-08 12:49 ` Eli Zaretskii 2021-11-08 17:30 ` Dmitry Gutov 2021-11-08 18:18 ` Eli Zaretskii 2021-12-23 10:28 ` Lars Ingebrigtsen 2021-12-26 0:53 ` Dmitry Gutov 2021-12-27 1:36 ` Dmitry Gutov 2022-01-03 3:59 ` Dmitry Gutov 2022-01-03 21:15 ` Dima Kogan 2022-01-03 22:51 ` Dmitry Gutov 2022-01-04 3:28 ` Eli Zaretskii 2022-01-05 2:09 ` Dmitry Gutov 2022-01-21 13:50 ` Lars Ingebrigtsen 2022-01-21 14:11 ` Dmitry Gutov 2022-01-03 21:16 ` Andy Moreton 2021-11-07 6:43 ` Eli Zaretskii 2021-11-07 10:45 ` Andy Moreton 2021-11-07 6:31 ` Eli Zaretskii 2021-11-06 22:19 ` Dmitry Gutov
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).