GNU bug report logs - #60722
30.0.50; [PATCH] Using Tramp to sudo in Eshell doesn't change prompt sigil

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 60722 in the body.
You can then email your comments to 60722 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: 30.0.50; [PATCH] Using Tramp to sudo in Eshell doesn't change prompt
 sigil
Date: Tue, 10 Jan 2023 15:50:03 -0800

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

X-Debbugs-Cc: michael.albinus <at> gmx.de

Starting with "sudo emacs -Q -f eshell", notice that the prompt looks like:

  /home/user #

Now start "emacs -Q -f eshell":

  ~ $ cd /sudo::
  /sudo:root <at> huginn:~ $

Notice that the prompt sigil is still "$" instead of "#". I think it 
would be better to indicate that you have a root shell by using "#" here.

Attached is a patch to do this. It adds a new function, 
'user-uid-for-file', which is aware of file name handlers. Then, Tramp 
adds the appropriate handler. Now, Eshell can use that function and we 
get the prompt sigil we expect.

[0001-Add-user-uid-for-file-to-get-the-effective-UID-for-r.patch (text/plain, attachment)]

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

From: Ruijie Yu <ruijie <at> netyu.xyz>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: michael.albinus <at> gmx.de, 60722 <at> debbugs.gnu.org
Subject: Re: bug#60722: 30.0.50; [PATCH] Using Tramp to sudo in Eshell
 doesn't change prompt sigil
Date: Tue, 10 Jan 2023 20:12:54 -0600

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

> X-Debbugs-Cc: michael.albinus <at> gmx.de
>
> Starting with "sudo emacs -Q -f eshell", notice that the prompt looks like:
>
>   /home/user #
>
> Now start "emacs -Q -f eshell":
>
>   ~ $ cd /sudo::
>   /sudo:root <at> huginn:~ $
>
> Notice that the prompt sigil is still "$" instead of "#". I think it would be
> better to indicate that you have a root shell by using "#" here.
>
> Attached is a patch to do this. It adds a new function, 'user-uid-for-file',
> which is aware of file name handlers. Then, Tramp adds the appropriate
> handler. Now, Eshell can use that function and we get the prompt sigil we
> expect.
>
> [2. text/plain; 0001-Add-user-uid-for-file-to-get-the-effective-UID-for-r.patch]...

Haven't looked into the patch, but I wonder if it addresses multi-hop
scenarios like the following:

1. /sudo:admin <at> localhost|sudo::
2. /sudo:|sudo:another_user <at> localhost::
3. /sshx:admin <at> remote|sudo::
4. /sudo:|sshx:admin <at> remote::
5. /doas::
6. /sshx:root <at> localhost:

Best,


RY

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 60722 <at> debbugs.gnu.org
Subject: Re: bug#60722: 30.0.50; [PATCH] Using Tramp to sudo in Eshell
 doesn't change prompt sigil
Date: Wed, 11 Jan 2023 10:33:41 +0100

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

Hi Jim,

> Attached is a patch to do this. It adds a new function,
> 'user-uid-for-file', which is aware of file name handlers. Then, Tramp
> adds the appropriate handler. Now, Eshell can use that function and we
> get the prompt sigil we expect.

I haven't tested, but in general it looks good. There are some minor
changes I'm missing, for example documentation of the new function in
the Lisp Reference Manual "(elisp) User Identification", and a new test
case in tramp-tests.el, but this can wait.

More serious are the following comments:

> +    ;; `user-uid-for-file' performed by default handler.

We shouldn't do this. user-uid-for-file would return the *local* uid,
which is wrong. So please, for all handlers apply

> +    (user-uid-for-file . tramp-handle-user-uid-for-file)

Except in tramp-archive.el, where ignore as handler seems to be appropriate.

> +(defun user-uid-for-file (filename)
> +  "Return the effective uid for FILENAME.
> +For local files, this is equivalent to `user-uid' (which see),
> +but for remote files, this returns the effective uid for that
> +remote connection."

Please add, that it returns -1 in case the remote uid could not be
determined.

Best regards, Michael.

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Ruijie Yu <ruijie <at> netyu.xyz>
Cc: Jim Porter <jporterbugs <at> gmail.com>, 60722 <at> debbugs.gnu.org
Subject: Re: bug#60722: 30.0.50; [PATCH] Using Tramp to sudo in Eshell
 doesn't change prompt sigil
Date: Wed, 11 Jan 2023 10:35:21 +0100

Ruijie Yu <ruijie <at> netyu.xyz> writes:

Hi,

>> Attached is a patch to do this. It adds a new function, 'user-uid-for-file',
>> which is aware of file name handlers. Then, Tramp adds the appropriate
>> handler. Now, Eshell can use that function and we get the prompt sigil we
>> expect.
>
> Haven't looked into the patch, but I wonder if it addresses multi-hop
> scenarios like the following:
>
> 1. /sudo:admin <at> localhost|sudo::
> 2. /sudo:|sudo:another_user <at> localhost::
> 3. /sshx:admin <at> remote|sudo::
> 4. /sudo:|sshx:admin <at> remote::
> 5. /doas::
> 6. /sshx:root <at> localhost:

Tramp handles these cases.

> Best,
>
> RY

Best regards, Michael.

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 60722 <at> debbugs.gnu.org
Subject: Re: bug#60722: 30.0.50; [PATCH] Using Tramp to sudo in Eshell
 doesn't change prompt sigil
Date: Wed, 11 Jan 2023 14:59:39 +0100

Michael Albinus <michael.albinus <at> gmx.de> writes:

Hi Jim,

> We shouldn't do this. user-uid-for-file would return the *local* uid,
> which is wrong. So please, for all handlers apply
>
>> +    (user-uid-for-file . tramp-handle-user-uid-for-file)
>
> Except in tramp-archive.el, where ignore as handler seems to be appropriate.

Thinking about, I believe we need an own function for
tramp-archive.el. Something like (untested)

--8<---------------cut here---------------start------------->8---
(defun tramp-archive-user-uid-for-file (filename)
  "Like `user-uid-for-file' for file archives."
  (user-uid-for-file (tramp-archive-gvfs-file-name filename)))
--8<---------------cut here---------------end--------------->8---

This should return the local uid for local archives, and the remote uid
for remote archives.

Best regards, Michael.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 60722 <at> debbugs.gnu.org
Subject: Re: bug#60722: 30.0.50; [PATCH] Using Tramp to sudo in Eshell doesn't
 change prompt sigil
Date: Sat, 14 Jan 2023 13:59:10 -0800

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

On 1/11/2023 5:59 AM, Michael Albinus wrote:
> Michael Albinus <michael.albinus <at> gmx.de> writes:
> 
> Hi Jim,
> 
>> We shouldn't do this. user-uid-for-file would return the *local* uid,
>> which is wrong. So please, for all handlers apply
>>
>>> +    (user-uid-for-file . tramp-handle-user-uid-for-file)
>>
>> Except in tramp-archive.el, where ignore as handler seems to be appropriate.
> 
> Thinking about, I believe we need an own function for
> tramp-archive.el. Something like (untested)
> 
> --8<---------------cut here---------------start------------->8---
> (defun tramp-archive-user-uid-for-file (filename)
>    "Like `user-uid-for-file' for file archives."
>    (user-uid-for-file (tramp-archive-gvfs-file-name filename)))
> --8<---------------cut here---------------end--------------->8---
> 
> This should return the local uid for local archives, and the remote uid
> for remote archives.

Thanks. See attached. I also updated the base implementation of 
'tramp-handle-user-uid-for-file' to return -1 even if 
'tramp-get-remote-uid' returns nil (some Tramp methods set the latter 
handler to be 'ignore').

I also added a new Eshell special variable, '$UID', which does about 
what you'd expect. This makes Eshell work like other shells, which also 
have a special '$UID' variable.

Finally, I added documentation to the manuals. I didn't add any Tramp 
regression tests though, since I wasn't sure of the right way to test this.

[0001-Add-user-uid-for-file-to-get-the-effective-UID-for-r.patch (text/plain, attachment)]

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 60722 <at> debbugs.gnu.org
Subject: Re: bug#60722: 30.0.50; [PATCH] Using Tramp to sudo in Eshell doesn't
 change prompt sigil
Date: Sat, 14 Jan 2023 14:10:27 -0800

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

On 1/14/2023 1:59 PM, Jim Porter wrote:
> Finally, I added documentation to the manuals. I didn't add any Tramp 
> regression tests though, since I wasn't sure of the right way to test this.

Oops. I missed an "@end defun" in the manual. Fixed.

[0001-Add-user-uid-for-file-to-get-the-effective-UID-for-r.patch (text/plain, attachment)]

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 60722 <at> debbugs.gnu.org
Subject: Re: bug#60722: 30.0.50; [PATCH] Using Tramp to sudo in Eshell
 doesn't change prompt sigil
Date: Sun, 15 Jan 2023 10:23:44 +0100

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

Hi Jim,

>> Finally, I added documentation to the manuals. I didn't add any
>> Tramp regression tests though, since I wasn't sure of the right way
>> to test this.
>
> Oops. I missed an "@end defun" in the manual. Fixed.

Thanks. LGTM. Don't care about the tests, I'll add them once your patch
has arrived Emacs master.

One question is left for me: do we really need FILENAME as argument? I
believe it would be sufficient to check default-directory; this would also
be consistent with the functions for remote hosts described in os.texi.

Btw, another idea is to simplify the implementation. Let Tramp set a
connection-local variable `tramp-user-uid' or alike, and your function
`remote-user-uid' or however you rename it will ask for this
connection-local variable, like we do it already in functions
`null-device' and `path-separator'. This would avoid the overhead of
running the file name handler mechanism.

WDYT?

Best regards, Michael.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 60722 <at> debbugs.gnu.org
Subject: Re: bug#60722: 30.0.50; [PATCH] Using Tramp to sudo in Eshell doesn't
 change prompt sigil
Date: Sun, 15 Jan 2023 21:38:41 -0800

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

On 1/15/2023 1:23 AM, Michael Albinus wrote:
> One question is left for me: do we really need FILENAME as argument? I
> believe it would be sufficient to check default-directory; this would also
> be consistent with the functions for remote hosts described in os.texi.

Good point. I removed the FILENAME argument.

> Btw, another idea is to simplify the implementation. Let Tramp set a
> connection-local variable `tramp-user-uid' or alike, and your function
> `remote-user-uid' or however you rename it will ask for this
> connection-local variable, like we do it already in functions
> `null-device' and `path-separator'. This would avoid the overhead of
> running the file name handler mechanism.

I didn't do this part though, since it wasn't so simple. Instead, I made 
the function (now named 'file-user-uid'[1]) work like the function 
'exec-path'. Since the regular 'user-uid' is also a function, I think 
this implementation is the most straightforward.

[1] I named it this way to mimic the relationship between 
'start-process' and 'start-file-process'. (But 'user-file-uid' didn't 
sound right, so 'file-user-uid' it is.)

[0001-Add-file-user-uid-to-get-the-connection-local-effect.patch (text/plain, attachment)]

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 60722 <at> debbugs.gnu.org
Subject: Re: bug#60722: 30.0.50; [PATCH] Using Tramp to sudo in Eshell
 doesn't change prompt sigil
Date: Mon, 16 Jan 2023 10:09:58 +0100

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

Hi Jim,

> I didn't do this part though, since it wasn't so simple. Instead, I
> made the function (now named 'file-user-uid'[1]) work like the
> function 'exec-path'. Since the regular 'user-uid' is also a function,
> I think this implementation is the most straightforward.
>
> [1] I named it this way to mimic the relationship between
> 'start-process' and 'start-file-process'. (But 'user-file-uid' didn't
> sound right, so 'file-user-uid' it is.)

Fine with me. I've roughly scanned the patch, it looks OK. So I guess
you shall install it; if there's something left to do we can still do it
on master.

After that, I will add the implementation for ange-ftp.el and some tests
in tramp-tests.el, and I'll check whether the implementation of
tramp-archive-handle-file-user-uid is OK.

> diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi
>
> +@defun file-user-uid
> +This function returns the connection-local for the user's effective

"connection-local value"

> diff --git a/etc/NEWS b/etc/NEWS
>
> +** New function 'file-user-uid'.
> +This function is like 'user-uid', but is aware of file name handlers,
> +so it will return the remote UID for remote files.

Perhaps you could also mention -1 as the "don't know" result.

Best regards, Michael.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 60722-done <at> debbugs.gnu.org
Subject: Re: bug#60722: 30.0.50; [PATCH] Using Tramp to sudo in Eshell doesn't
 change prompt sigil
Date: Mon, 16 Jan 2023 17:21:04 -0800

On 1/16/2023 1:09 AM, Michael Albinus wrote:
>> diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi
>>
>> +@defun file-user-uid
>> +This function returns the connection-local for the user's effective
> 
> "connection-local value"

Thanks, fixed.
> 
>> diff --git a/etc/NEWS b/etc/NEWS
>>
>> +** New function 'file-user-uid'.
>> +This function is like 'user-uid', but is aware of file name handlers,
>> +so it will return the remote UID for remote files.
> 
> Perhaps you could also mention -1 as the "don't know" result.

Also fixed.

I've now merged this to the master branch as 0bb8a011d5. Closing this 
bug, though we can obviously discuss further (here or elsewhere) if 
there's any fallout from the change.

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 60722 <at> debbugs.gnu.org
Subject: Re: bug#60722: 30.0.50; [PATCH] Using Tramp to sudo in Eshell
 doesn't change prompt sigil
Date: Tue, 17 Jan 2023 10:12:00 +0100

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

Hi Jim,

> I've now merged this to the master branch as 0bb8a011d5. Closing this
> bug, though we can obviously discuss further (here or elsewhere) if
> there's any fallout from the change.

Thanks. Pls note, that on emba two tests fail:

--8<---------------cut here---------------start------------->8---
lisp/eshell/esh-proc-tests.log:
  SKIPPED  esh-proc-test/kill-pipeline  ((skip-unless (not (getenv "EMACS_EMBA_CI"))) :form (not "1") :value nil)
lisp/eshell/em-prompt-tests.log:
   FAILED  em-prompt-test/field-properties  ((should (equal-including-properties last-prompt (propertize (format "%s $ " (directory-file-name default-directory)) 'read-only t 'field 'prompt 'font-lock-face 'eshell-prompt 'front-sticky '(read-only field font-lock-face) ...))) :form (equal-including-properties #("/checkout/test # " 0 17 (rear-nonsticky (read-only field font-lock-face) front-sticky (read-only field font-lock-face) font-lock-face eshell-prompt field prompt read-only t)) #("/checkout/test $ " 0 17 (read-only t field prompt font-lock-face eshell-prompt front-sticky (read-only field font-lock-face) rear-nonsticky (read-only field font-lock-face)))) :value nil :explanation (array-elt 15 (different-atoms (35 "#x23" "?#") (36 "#x24" "?$"))))
   FAILED  em-prompt-test/field-properties/no-highlight  ((should (equal-including-properties last-prompt (propertize (format "%s $ " (directory-file-name default-directory)) 'field 'prompt 'front-sticky '(field) 'rear-nonsticky '(field)))) :form (equal-including-properties #("/checkout/test # " 0 17 (rear-nonsticky (field) front-sticky (field) field prompt)) #("/checkout/test $ " 0 17 (field prompt front-sticky (field) rear-nonsticky (field)))) :value nil :explanation (array-elt 15 (different-atoms (35 "#x23" "?#") (36 "#x24" "?$"))))
--8<---------------cut here---------------end--------------->8---

Don't know whether this is related to your recent change, or something
else.

Best regards, Michael.

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 60722 <at> debbugs.gnu.org
Subject: Re: bug#60722: 30.0.50; [PATCH] Using Tramp to sudo in Eshell
 doesn't change prompt sigil
Date: Tue, 17 Jan 2023 16:03:52 +0100

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

Hi Jim,

> I've now merged this to the master branch as 0bb8a011d5. Closing this
> bug, though we can obviously discuss further (here or elsewhere) if
> there's any fallout from the change.

I've pushed my part as 013ab7e2a8.

Best regards, Michael.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 60722 <at> debbugs.gnu.org
Subject: Re: bug#60722: 30.0.50; [PATCH] Using Tramp to sudo in Eshell doesn't
 change prompt sigil
Date: Tue, 17 Jan 2023 17:04:50 -0800

On 1/17/2023 1:12 AM, Michael Albinus wrote:
> Jim Porter <jporterbugs <at> gmail.com> writes:
> 
> Hi Jim,
> 
>> I've now merged this to the master branch as 0bb8a011d5. Closing this
>> bug, though we can obviously discuss further (here or elsewhere) if
>> there's any fallout from the change.
> 
> Thanks. Pls note, that on emba two tests fail:
[snip]
> Don't know whether this is related to your recent change, or something
> else.

That's probably a bug from when those tests were introduced 
(558f04c39e). The tests didn't work when they were run as root. I 
hopefully fixed it with commit 281f48f19e.

Previous Next

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