GNU bug report logs - #49229
27.2; `M-x shell' fails over TRAMP from local MS Windows

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 49229 in the body.
You can then email your comments to 49229 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: Jim Porter <jporterbugs <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.2; `M-x shell' fails over TRAMP from local MS Windows
Date: Fri, 25 Jun 2021 16:05:57 -0700

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

(Note: I primarily tested on 27.2, but this doesn't look to be any
different on 28.0.50.) When invoking `M-x shell' over TRAMP from a
local MS Windows system, the default remote shell path is corrupted:

  emacs -Q
  C-x C-f /sshx:server:~/some/file.txt
  M-x shell
  ;; See the default prompt value:
  ;;   /sshx:server:/path/to/some//bin/sh
  ;; ("/path/to/some/" is greyed out)
  RET

The result is: "env: ‘c:/bin/sh’: No such file or directory". You can
also see this with the code used in `M-x shell' to get the remote
shell path:

  (read-file-name
   "Remote shell path: " default-directory shell-file-name
   t shell-file-name)

Eval'ing that from a TRAMP buffer and hitting RET returns "/bin/sh"
(i.e. `shell-file-name'); that is, we lost the TRAMP prefix, even
though the prompt made it look like we'd keep it. If you edit the path
to, say, "/sshx:server:/path/to/some//usr/bin/zsh" and hit RET, the
result is "/sshx:server:/usr/bin/zsh", which is good. The result of
this call is then passed to `expand-file-name', which on MS Windows,
turns "/bin/sh" into "c:/bin/sh". Finally, that gets called on the
remote (running GNU/Linux), and things break.

I've attached a WIP patch that resolves this, but I don't think it's
quite right (hence, I didn't use `git format-patch'). This seems to be
more of an issue with `read-file-name' not being smart enough; even if
we set the `default-filename' argument to nil, the default return
value is still a local (non-TRAMP) path, which isn't right. Since
`read-file-name' is better able to tell whether the user wanted the
default value or they specifically wanted a local shell, it might be
better to fix the issue there. However, that's a pretty widely-used
function, and I'm hesitant to change the behavior in
potentially-breaking ways.

If the current WIP patch does look good though, I can clean it up (add
a comment and a commit message) for it to be merged. Or I can try to
fix `read-file-name' if there's agreement about how it should work in
this case.

[shell-tramp.patch (application/octet-stream, attachment)]

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 49229 <at> debbugs.gnu.org
Subject: Re: bug#49229: 27.2; `M-x shell' fails over TRAMP from local MS
 Windows
Date: Sat, 26 Jun 2021 16:33:23 +0200

Jim Porter <jporterbugs <at> gmail.com> writes:

Hi Jim,

> Eval'ing that from a TRAMP buffer and hitting RET returns "/bin/sh"
> (i.e. `shell-file-name'); that is, we lost the TRAMP prefix, even
> though the prompt made it look like we'd keep it. If you edit the path
> to, say, "/sshx:server:/path/to/some//usr/bin/zsh" and hit RET, the
> result is "/sshx:server:/usr/bin/zsh", which is good. The result of
> this call is then passed to `expand-file-name', which on MS Windows,
> turns "/bin/sh" into "c:/bin/sh". Finally, that gets called on the
> remote (running GNU/Linux), and things break.

Thanks for this report.

Occasionally, I've seen this problem on MS Windows already. Since I
don't run anything on MS Windows unless for bug hunting, I couldn't
locate it yet. With your recipe, it's reproducible now. It's not related
to "M-x shell" only, but more general.

> I've attached a WIP patch that resolves this, but I don't think it's
> quite right (hence, I didn't use `git format-patch'). This seems to be
> more of an issue with `read-file-name' not being smart enough; even if
> we set the `default-filename' argument to nil, the default return
> value is still a local (non-TRAMP) path, which isn't right. Since
> `read-file-name' is better able to tell whether the user wanted the
> default value or they specifically wanted a local shell, it might be
> better to fix the issue there. However, that's a pretty widely-used
> function, and I'm hesitant to change the behavior in
> potentially-breaking ways.

I've pushed a fix to the master branches of Emacs and Tramp. Could you,
pls, check?

Best regards, Michael.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 49229 <at> debbugs.gnu.org
Subject: Re: bug#49229: 27.2;
 `M-x shell' fails over TRAMP from local MS Windows
Date: Sat, 26 Jun 2021 11:01:41 -0700

On Sat, Jun 26, 2021 at 7:33 AM Michael Albinus <michael.albinus <at> gmx.de> wrote:
>
> Occasionally, I've seen this problem on MS Windows already. Since I
> don't run anything on MS Windows unless for bug hunting, I couldn't
> locate it yet. With your recipe, it's reproducible now. It's not related
> to "M-x shell" only, but more general.

Looking at your patch, it makes sense to me. It's not where I
originally expected the fix to go, but I remember some previous corner
cases that were fixed with `tramp-drop-volume-letter', so this should
be good.

> I've pushed a fix to the master branches of Emacs and Tramp. Could you,
> pls, check?

I tested on MS Windows and it works correctly for me. (Note that I
just copied and eval'ed the new version of `tramp-file-name-handler',
since I don't have a proper build environment on MS Windows.)

While testing it, I discovered one other oddity though. This doesn't
necessarily need a fix, but it's a bit surprising, and I'll mention it
here in case someone thinks it's a problem worth fixing.

If I erase the default text and instead enter
"C:/Windows/System32/cmd.exe" into the `M-x shell' prompt, it treats
*that* as a remote path too. Now, the prompt does say to enter a
*remote* shell path, so if I enter a local path, I made a mistake.
However, the default shell path for `M-x shell' from a remote
directory is a TRAMP path ("/sshx:server:/path/to/some//bin/sh"), so
it's surprising that when I delete the TRAMP host prefix, I still end
up running a shell on the remote server.

Perhaps it would be nicer if, when `M-x shell' prompted for the remote
shell path, it didn't include the TRAMP prefix by default (e.g. the
default value would just be "/bin/sh"). That might not interact well
with `read-file-name' completion though; is it possible to use
file-name completion on a remote path without the TRAMP prefix?

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 49229 <at> debbugs.gnu.org
Subject: Re: bug#49229: 27.2; `M-x shell' fails over TRAMP from local MS
 Windows
Date: Sun, 27 Jun 2021 10:41:52 +0200

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

Jim Porter <jporterbugs <at> gmail.com> writes:

Hi Jim,

>> I've pushed a fix to the master branches of Emacs and Tramp. Could you,
>> pls, check?
>
> I tested on MS Windows and it works correctly for me. (Note that I
> just copied and eval'ed the new version of `tramp-file-name-handler',
> since I don't have a proper build environment on MS Windows.)

Thanks for confirmation.

> If I erase the default text and instead enter
> "C:/Windows/System32/cmd.exe" into the `M-x shell' prompt, it treats
> *that* as a remote path too. Now, the prompt does say to enter a
> *remote* shell path, so if I enter a local path, I made a mistake.
> However, the default shell path for `M-x shell' from a remote
> directory is a TRAMP path ("/sshx:server:/path/to/some//bin/sh"), so
> it's surprising that when I delete the TRAMP host prefix, I still end
> up running a shell on the remote server.

`read-file-name' as used in `shell' just reads a file name, no matter
whether a local or remote one.

> Perhaps it would be nicer if, when `M-x shell' prompted for the remote
> shell path, it didn't include the TRAMP prefix by default (e.g. the
> default value would just be "/bin/sh"). That might not interact well
> with `read-file-name' completion though; is it possible to use
> file-name completion on a remote path without the TRAMP prefix?

No, that doesn't work. File name completion and alike wouldn't work any
more.

But we could teach `read-file-name' to accept only remote file
names. What about this patch:

[Message part 2 (text/x-patch, inline)]

diff --git a/lisp/shell.el b/lisp/shell.el
index 62de5be817..4339e8c0a3 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -759,7 +759,8 @@ shell
                  (file-local-name
                   (expand-file-name
                    (read-file-name "Remote shell path: " default-directory
-                                   shell-file-name t shell-file-name)))))
+                                   shell-file-name t shell-file-name
+                                   #'file-remote-p)))))

    ;; Rain or shine, BUFFER must be current by now.
    (unless (comint-check-proc buffer)

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

It is not perfect, one can still enter "/sudo::/bin/sh" when
`default-directory' is "/ssh::". But I wouldn't count this as mistake,
it would be rather a "user error by intention" :-)

Best regards, Michael.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 49229 <at> debbugs.gnu.org
Subject: Re: bug#49229: 27.2;
 `M-x shell' fails over TRAMP from local MS Windows
Date: Sun, 27 Jun 2021 17:47:49 -0700

On Sun, Jun 27, 2021 at 1:41 AM Michael Albinus <michael.albinus <at> gmx.de> wrote:
>
> But we could teach `read-file-name' to accept only remote file
> names. What about this patch:
>
>
> It is not perfect, one can still enter "/sudo::/bin/sh" when
> `default-directory' is "/ssh::". But I wouldn't count this as mistake,
> it would be rather a "user error by intention" :-)

This seems like a reasonable compromise to me. Like you say, it's not
perfect, but it should at least prevent users from mistakenly entering
local paths here.

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 49229-done <at> debbugs.gnu.org
Subject: Re: bug#49229: 27.2; `M-x shell' fails over TRAMP from local MS
 Windows
Date: Mon, 28 Jun 2021 08:18:24 +0200

Version: 28.1

Jim Porter <jporterbugs <at> gmail.com> writes:

Hi Jim,

> This seems like a reasonable compromise to me. Like you say, it's not
> perfect, but it should at least prevent users from mistakenly entering
> local paths here.

I've pushed the patch to master, closing the bug. The Tramp part of the
patch will appear with Tramp 2.5.1 on GNU ELPA.

Best regards, Michael.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 49229 <at> debbugs.gnu.org
Subject: Re: bug#49229: 27.2;
 `M-x shell' fails over TRAMP from local MS Windows
Date: Thu, 1 Jul 2021 10:07:53 -0700

(Hopefully I don't need to do anything special to comment on a closed bug...)

On Sun, Jun 27, 2021 at 11:18 PM Michael Albinus <michael.albinus <at> gmx.de> wrote:
>
> I've pushed the patch to master, closing the bug. The Tramp part of the
> patch will appear with Tramp 2.5.1 on GNU ELPA.

I discovered a minor issue with the Tramp part of the patch. On MS
Windows, if I'm editing a remote file and then open a local file, the
newly-opened local file's default directory doesn't have a volume
letter. This still works overall, but it does break the "~" file
alias.

Here are steps to reproduce:

  C-x C-f /sshx:server:/path/to/file.txt
  ;; "/sshx:server:/path/to/" is pre-filled here:
  C-x C-f /sshx:server:/path/to//~/local.txt
  ;; local.txt opens/saves correctly, but...
  C-x C-f
  ;; Initial value in minibuffer is "/Users/Jim/Documents/".
  ;; Before Tramp 2.5.1, it was "~/".

This might cause some problems in other places, but so far the only
issue I've found is the file alias for "~" no longer working. However,
the fact that `default-directory' for local.txt isn't an absolute path
could cause issues if we use that `default-directory' elsewhere. Since
"/Users/Jim/Documents/" is relative to the current drive, it can mean
different things depending on what Emacs thinks the current drive is
at the time.

Maybe it would be best to revert the Tramp part of this patch and do
something similar to the patch in my original message. That would
ensure that nothing unexpected happens as a result of
`default-directory' being relative to the current drive. If that makes
sense, I can put together a proper patch for the `M-x shell' part.

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 49229 <at> debbugs.gnu.org
Subject: Re: bug#49229: 27.2; `M-x shell' fails over TRAMP from local MS
 Windows
Date: Fri, 02 Jul 2021 14:58:46 +0200

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

Jim Porter <jporterbugs <at> gmail.com> writes:

Hi Jim,

> (Hopefully I don't need to do anything special to comment on a closed bug...)

No problem,

> Maybe it would be best to revert the Tramp part of this patch and do
> something similar to the patch in my original message. That would
> ensure that nothing unexpected happens as a result of
> `default-directory' being relative to the current drive. If that makes
> sense, I can put together a proper patch for the `M-x shell' part.

As I've said in my reply, I believe the problem is more general, and not
restricted to just the `shell' function.

So I have pushed a patch to master, which reverts my Tramp change, and
which adds the following change to `read-file-name-default' of minibuffer.el:

[Message part 2 (text/x-patch, inline)]

*** /tmp/ediffZeoKgh	2021-07-02 14:52:22.462303385 +0200
--- /home/albinus/src/emacs/lisp/minibuffer.el	2021-07-02 14:50:37.716338781 +0200
***************
*** 3161,3166 ****
--- 3161,3167 ----
          (unless val (error "No file name specified"))

          (if (and default-filename
+ 		 (not (file-remote-p dir))
                   (string-equal val (if (consp insdef) (car insdef) insdef)))
              (setq val default-filename))
          (setq val (substitute-in-file-name val))

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

Could you, pls, test?

Best regards, Michael.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 49229 <at> debbugs.gnu.org
Subject: Re: bug#49229: 27.2;
 `M-x shell' fails over TRAMP from local MS Windows
Date: Fri, 2 Jul 2021 14:30:59 -0700

On Fri, Jul 2, 2021 at 5:58 AM Michael Albinus <michael.albinus <at> gmx.de> wrote:
>
> As I've said in my reply, I believe the problem is more general, and not
> restricted to just the `shell' function.
>
> So I have pushed a patch to master, which reverts my Tramp change, and
> which adds the following change to `read-file-name-default' of minibuffer.el:
>
>
> Could you, pls, test?

Thanks for the fix. I tested this both by running `M-x shell' from a
remote buffer and by opening a local file while in a remote buffer and
both of these work as expected. Note that I just eval'ed the updated
functions in my Emacs 27.2 instance, so it's not a *perfect* test;
still, hopefully it's enough to verify that this works correctly.

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 49229 <at> debbugs.gnu.org
Subject: Re: bug#49229: 27.2; `M-x shell' fails over TRAMP from local MS
 Windows
Date: Sat, 03 Jul 2021 08:42:41 +0200

Jim Porter <jporterbugs <at> gmail.com> writes:

Hi Jim,

> Thanks for the fix. I tested this both by running `M-x shell' from a
> remote buffer and by opening a local file while in a remote buffer and
> both of these work as expected. Note that I just eval'ed the updated
> functions in my Emacs 27.2 instance, so it's not a *perfect* test;
> still, hopefully it's enough to verify that this works correctly.

Thanks for confirmation. The Tramp part of the patch will appear with
the next Tramp ELPA release, likely 2.5.1.1.

Best regards, Michael.

Previous Next

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