Recommend move eshell/su and eshell/sudo to em-tramp.el

Recommend move eshell/su and eshell/sudo to em-tramp.el

[John Wiegley]

Something I've noticed after moving to Emacs 24 is that using "sudo" in Eshell
has become an awful experience:

    - It's *much* slower than *sudo (by orders of magnitude)
    - It doesn't show any output until the command is done
    - It locks up Emacs until the command is done

I find this behavior distasteful, since sudo is such a common command for
users of Eshell to use.  It was added in 2009 by the Tramp author:

    2009-11-27  Michael Albinus  <michael.albinus@gmx.ed>

           * eshell/em-unix.el (eshell/su, eshell/sudo): New defuns,
           providing a Tramp related implementation of "su" and "sudo".
           (eshell-unix-initialize): Add "su" and "sudo".

I appreciate what these two functions are trying to accomplish, but they
should not have been made the default.  I recommend moving them out of
em-unix.el and into a new module, em-tramp.el, which is OFF by default.  Users
who prefer to use Tramp for everything can turn it on, and a note to that
effect should be added to the Tramp manual.

John

Re: Recommend move eshell/su and eshell/sudo to em-tramp.el

[Michael Albinus]

"John Wiegley" <johnw@newartisans.com> writes:

> Something I've noticed after moving to Emacs 24 is that using "sudo" in Eshell
> has become an awful experience:
>
>     - It's *much* slower than *sudo (by orders of magnitude)
>     - It doesn't show any output until the command is done
>     - It locks up Emacs until the command is done

There is much room for improvement, no question. If I have time, I'll
look on it next days.

> I find this behavior distasteful, since sudo is such a common command for
> users of Eshell to use.  It was added in 2009 by the Tramp author:
>
>     2009-11-27  Michael Albinus  <michael.albinus@gmx.ed>
>
>            * eshell/em-unix.el (eshell/su, eshell/sudo): New defuns,
>            providing a Tramp related implementation of "su" and "sudo".
>            (eshell-unix-initialize): Add "su" and "sudo".
>
> I appreciate what these two functions are trying to accomplish, but they
> should not have been made the default.

They aren't the default, if default-directory is local. And if the
default directory is remote, they do exactly what is intended: run the
command on that remote machine. It isn't only su or sudo, this is true
for all other commands as well.

> I recommend moving them out of em-unix.el and into a new module,
> em-tramp.el, which is OFF by default.  Users who prefer to use Tramp
> for everything can turn it on, and a note to that effect should be
> added to the Tramp manual.

If we do this (I'm not convinced yet), it shall be called em-remote.el
or so. Tramp is just an example for an implementation of process-file
and start-file-process; there could be other file name handlers which
will provide an own implementation.

> John

Best regards, Michael.

Re: Recommend move eshell/su and eshell/sudo to em-tramp.el

[Phil Hagelberg]

On Thu, Jun 28, 2012 at 8:32 PM, John Wiegley <johnw@newartisans.com> wrote:
> Something I've noticed after moving to Emacs 24 is that using "sudo" in Eshell
> has become an awful experience:
>
>    - It's *much* slower than *sudo (by orders of magnitude)
>    - It doesn't show any output until the command is done
>    - It locks up Emacs until the command is done

The fact that you cannot C-c out of an eshell/sudo command makes me
agree that it should be disabled by default.

-Phil

Recommend move eshell/su and eshell/sudo to em-tramp.el

[John Wiegley]

Something I've noticed after moving to Emacs 24 is that using "sudo" in Eshell
has become an awful experience:

    - It's *much* slower than *sudo (by orders of magnitude)
    - It doesn't show any output until the command is done
    - It locks up Emacs until the command is done

I find this behavior unfortunate, since sudo is such a common command for
users of Eshell to use.  It was added in 2009 by the Tramp author:

    2009-11-27  Michael Albinus  <michael.albinus@gmx.ed>

           * eshell/em-unix.el (eshell/su, eshell/sudo): New defuns,
           providing a Tramp related implementation of "su" and "sudo".
           (eshell-unix-initialize): Add "su" and "sudo".

I do not believe these should not have been made the default.  I recommend
moving them out of em-unix.el and into a new module, em-tramp.el, which is OFF
by default.  Users who prefer to use Tramp for everything can turn it on, and
a note to that effect should be added to the Tramp manual.

John

Re: Recommend move eshell/su and eshell/sudo to em-tramp.el

[Michael Albinus]

John Wiegley <johnw@newartisans.com> writes:

Hi John,

looks like it is the same message you have sent some days ago. Are there
changes I haven't seen? Anyway ...

> Something I've noticed after moving to Emacs 24 is that using "sudo" in Eshell
> has become an awful experience:
>
>     - It's *much* slower than *sudo (by orders of magnitude)

In a first reaction I've said this doesn't happen if default-directory
is local. This is wrong, I did oversee that I had defined an alias as
metioned in (info "(eshell) Built-ins")

alias sudo '*sudo $*'

Would such an alias serve you better?

>     - It doesn't show any output until the command is done

I'm working on this. Hopefully, I could change the behaviour when the
command is an external command. If the command is mapped to a lisp
function (like the command "ls" to "eshell/ls"), I have no clue how to
achieve this. The latter case happens also for Lisp code not using
Tramp, but there are shorter delays.

>     - It locks up Emacs until the command is done

If the command runs lisp code, you could interrupt with ^G as usual. For
external commands it is more complicate, because I don't know how to
propagate signals to such remote processes in Tramp. The only idea I
have is to interrupt the Tramp connection itself, which sounds like a
sledge-hammer approach.

> I do not believe these should not have been made the default.  I recommend
> moving them out of em-unix.el and into a new module, em-tramp.el, which is OFF
> by default.  Users who prefer to use Tramp for everything can turn it on, and
> a note to that effect should be added to the Tramp manual.

Would it be sufficient to propagate the above mentioned alias more
prominently? Or shall we make that alias the default?

> John

Best regards, Michael.

Re: Recommend move eshell/su and eshell/sudo to em-tramp.el

[John Wiegley]

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

> looks like it is the same message you have sent some days ago. Are there
> changes I haven't seen? Anyway ...

Ah, I never saw it come across on the emacs-devel Archives, so I believed it
hadn't been sent.

> In a first reaction I've said this doesn't happen if default-directory is
> local. This is wrong, I did oversee that I had defined an alias as metioned
> in (info "(eshell) Built-ins")

> alias sudo '*sudo $*'

> Would such an alias serve you better?

That would serve me personally, but I don't believe it should be the default.
It's not good to have a default behavior if we have to educate others on how
to then disable it.  It surprised me, so I'm sure it will surprise others.

>> - It doesn't show any output until the command is done

> I'm working on this. Hopefully, I could change the behaviour when the
> command is an external command. If the command is mapped to a lisp function
> (like the command "ls" to "eshell/ls"), I have no clue how to achieve
> this. The latter case happens also for Lisp code not using Tramp, but there
> are shorter delays.

Yes, any Lisp function blocks until all output is ready, that's just a
shortcoming of the way Eshell is using Emacs Lisp.

> Would it be sufficient to propagate the above mentioned alias more
> prominently? Or shall we make that alias the default?

This type of behavior, being that it is special, really deserves to be in its
own "opt-in" module.  It's better to educate people how to turn on behavior
that may have surprising consequences, rather than to make it the default and
then educate them how to disable it.

Also, eshell/sudo does not fit with the philosophy of the other functions in
em-unix.el.  Every other function in that file provides a *pure Lisp*
implementation of an equivalent Unix command.  As far as I can see, both *sudo
and eshell/sudo end up calling /usr/bin/sudo.  sehell/sudo is not pure Lisp
(i.e., it involves an external command).  So I'm not sure what benefit
eshell/sudo gains, since ultimately it does the same thing, only slower and
with blocking I/O?

Other than that, you know I'm a huge Tramp fan, and I use Tramp+eshell quite
often.  I'm very interested in seeing this work progress, albeit in its own
module.

Thanks,
  John

Re: Recommend move eshell/su and eshell/sudo to em-tramp.el

[Michael Albinus]

"John Wiegley" <johnw@newartisans.com> writes:

> This type of behavior, being that it is special, really deserves to be in its
> own "opt-in" module.  It's better to educate people how to turn on behavior
> that may have surprising consequences, rather than to make it the default and
> then educate them how to disable it.

Understood. Maybe we should change it such a way, that plain sudo is
called when your default directory is local. This is what people expect.

> Also, eshell/sudo does not fit with the philosophy of the other
> functions in em-unix.el.  Every other function in that file provides a
> *pure Lisp* implementation of an equivalent Unix command.  As far as I
> can see, both *sudo and eshell/sudo end up calling /usr/bin/sudo.
> sehell/sudo is not pure Lisp (i.e., it involves an external command).
> So I'm not sure what benefit eshell/sudo gains, since ultimately it
> does the same thing, only slower and with blocking I/O?

The idea behind eshell/sudo (and eshell/su) is

- Allow su(do) to run lisp commands. "sudo ls" would call the Lisp
  implementation of ls, not the UNIX ls .

- Allow su(do) on remote hosts. On the fly, tramp-default-proxies-alist
  is expanded, in order to apply sudo on that remote host, and not on
  the local host.

Two new features, which weren't in eshell before (IIRC). 

> Other than that, you know I'm a huge Tramp fan, and I use Tramp+eshell quite
> often.  I'm very interested in seeing this work progress, albeit in its own
> module.

I agree with you that we shall not surprise people, especially when
calling sudo for local machine purposes. OTOH,  I'd love to have a kind
of automagic to let it work on remote machines. Opt-in modules need time
and courage to discover.

> Thanks,
>   John

Best regards, Michael.

Re: Recommend move eshell/su and eshell/sudo to em-tramp.el

[Michael Albinus]

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

>>     - It doesn't show any output until the command is done
>
> I'm working on this. Hopefully, I could change the behaviour when the
> command is an external command.

I've committed a patch to esh-ext.el, which implements this. Finally, it
was much more simple than expected.

Best regards, Michael.

Re: Recommend move eshell/su and eshell/sudo to em-tramp.el

[John Wiegley]

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

> I've committed a patch to esh-ext.el, which implements this. Finally, it was
> much more simple than expected.

Can you use file-remote-p and only engage Tramp if a remote path is involved?
It would be nice if the non-remote case acted like it did before.

Thanks!
  John

Re: Recommend move eshell/su and eshell/sudo to em-tramp.el

[Michael Albinus]

"John Wiegley" <johnw@newartisans.com> writes:

> Can you use file-remote-p and only engage Tramp if a remote path is involved?
> It would be nice if the non-remote case acted like it did before.

Do you mean just for sudo? My patch I've mentioned the other mail was a
general one, that's why your question confuses me a little bit.

And what do you mean with "if a remote path is involved"? A remote
default directory?

> Thanks!
>   John

Best regards, Michael.

Re: Recommend move eshell/su and eshell/sudo to em-tramp.el

[John Wiegley]

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

> Do you mean just for sudo? My patch I've mentioned the other mail was a
> general one, that's why your question confuses me a little bit.

Sorry, I'll take a closer look before making any further comments.

John