Eshell and Sudo

Eshell and Sudo

[Ian D]

Eshell Developers,

       I've been trying out Eshell, using the trunk from the Emacs
       repository.  However, everytime I try to do anything except print
       out command options with sudo, I get the following:

       Recursive load:
 "EMACS_BUILD_DIR/share/emacs/24.3.50/lisp/net/tramp.elc",
 "EMACS_BUILD_DIR/share/emacs/24.3.50/lisp/net/tramp.elc",
 "EMACS_BUILD_DIR/share/emacs/24.3.50/lisp/net/tramp.elc",
 "EMACS_BUILD_DIR/share/emacs/24.3.50/lisp/net/tramp.elc",
 "EMACS_BUILD_DIR/share/emacs/24.3.50/lisp/net/tramp.elc"

        EMACS_BUILD_DIR is the directory I installed the repo Emacs
        into, i.e. ./configure --prefix=EMACS_BUILD_DIR.  I tried
        running Emacs without a configuration file, using -q, and got
        the same thing.  Does anyone know why it's doing this?

Ian D

Re: Eshell and Sudo

[Michael Albinus]

Ian D <dunni@gnu.org> writes:

> Eshell Developers,

Hi Ian,

>         I tried running Emacs without a configuration file, using -q,
>         and got the same thing.  Does anyone know why it's doing this?

I guess you mean -Q. Anyway, same result here.

Even stranger, if I preload tramp by "emacs -Q -l tramp", I get random
results:

--8<---------------cut here---------------start------------->8---
~/src/emacs $ sudo id
uid=0(root) gid=0(root) groups=0(root)
~/src/emacs $ sudo id
Wrong type argument: processp, nil
~/src/emacs $ sudo id
Wrong type argument: processp, nil
~/src/emacs $ sudo id
Wrong type argument: processp, nil
~/src/emacs $ sudo id
uid=0(root) gid=0(root) groups=0(root)
~/src/emacs $ 
--8<---------------cut here---------------end--------------->8---

I'll check tomorrow.

> Ian D

Best regards, Michael.

Re: Eshell and Sudo

[Aidan Gauland]

Michael Albinus <michael.albinus@gmx.de> writes:
> Ian D <dunni@gnu.org> writes:
>
>> Eshell Developers,
>
> Hi Ian,
>
>>         I tried running Emacs without a configuration file, using -q,
>>         and got the same thing.  Does anyone know why it's doing this?
>
> I guess you mean -Q. Anyway, same result here.
>
> Even stranger, if I preload tramp by "emacs -Q -l tramp", I get random
> results:

No one, not even John Wiegley, seems to know why sudo/su works for some
people and not others.  Debugging Eshell sudo/su is on my TODO list.  I
don't even know whether this is an Eshell problem or a TRAMP problem.  I
suspect tramp, since Eshell is only calling TRAMP (which should *not* be
blowing up like it does for some people).

Regards,
Aidan Gauland

Re: Eshell and Sudo

[Aidan Gauland]

Ian D <dunni@gnu.org> writes:
> Aidan Gauland <aidalgol@no8wireless.co.nz> writes:
>> No one, not even John Wiegley, seems to know why sudo/su works for some
>> people and not others.  Debugging Eshell sudo/su is on my TODO list.  I
>> don't even know whether this is an Eshell problem or a TRAMP problem.  I
>> suspect tramp, since Eshell is only calling TRAMP (which should *not* be
>> blowing up like it does for some people).
>
>   I looked into this myself a little, and I got it working by adding
>   (require 'tramp) to my .emacs file.  I checked in the eshell files,
>   and found this in esh-util.el, but within an 'eval-when-compile'
>   block, and I'm not sure what this would do to the other files.  Since
>   eshell/sudo is within em-unix.el, is it possible that it's not loading
>   correctly?

That could be.  The Eshell code is overdue for some housekeeping.  I'll
take a look at those files and post back with my findings (if any).

Regards,
Aidan Gauland

Re: Eshell and Sudo

[John Wiegley]

>>>>> Aidan Gauland <aidalgol@no8wireless.co.nz> writes:

> No one, not even John Wiegley, seems to know why sudo/su works for some
> people and not others.  Debugging Eshell sudo/su is on my TODO list.  I
> don't even know whether this is an Eshell problem or a TRAMP problem.  I
> suspect tramp, since Eshell is only calling TRAMP (which should *not* be
> blowing up like it does for some people).

As a note, I do not think tramp should be involved by default in eshell/sudo.
This should be an optional module, em-tramp, which one can enable to bring in
such behavior.  It makes using sudo on localhost many, many times slower, with
no functional benefit.

Thanks,
  John

Re: Eshell and Sudo

[Aidan Gauland]

"John Wiegley" <johnw@newartisans.com> writes:
> As a note, I do not think tramp should be involved by default in eshell/sudo.
> This should be an optional module, em-tramp, which one can enable to bring in
> such behavior.  It makes using sudo on localhost many, many times slower, with
> no functional benefit.

I fully agree; it's confusing just about every new user.  On this note,
may I please have commit access now?

Thanks,
Aidan Gauland

Re: Eshell and Sudo

[Michael Albinus]

Aidan Gauland <aidalgol@no8wireless.co.nz> writes:

> "John Wiegley" <johnw@newartisans.com> writes:
>> As a note, I do not think tramp should be involved by default in eshell/sudo.
>> This should be an optional module, em-tramp, which one can enable to bring in
>> such behavior.  It makes using sudo on localhost many, many times slower, with
>> no functional benefit.
>
> I fully agree; it's confusing just about every new user.

This was discussed already. In short

* There is still "*sudo", which calls the external command. Like for all
  other built-ins.

* If we remove eshell/sudo, some functionality might be lost, like
  running eshell internal commands as arguments of sudo, and like
  Tramp's password caching.

I understand that it might cause confusion the way it runs now. But I
wanted to recall the reasons I have added eshell/sudo 3 years ago.

> Thanks,
> Aidan Gauland

Best regards, Michael.

Re: Eshell and Sudo

[Michael Albinus]

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

>> Eshell Developers,

Hi Ian & Aidan,

Finally, we have two problems.

>>        Recursive load:
>>  "EMACS_BUILD_DIR/share/emacs/24.3.50/lisp/net/tramp.elc",
>>  "EMACS_BUILD_DIR/share/emacs/24.3.50/lisp/net/tramp.elc",
>>  "EMACS_BUILD_DIR/share/emacs/24.3.50/lisp/net/tramp.elc",
>>  "EMACS_BUILD_DIR/share/emacs/24.3.50/lisp/net/tramp.elc",
>>  "EMACS_BUILD_DIR/share/emacs/24.3.50/lisp/net/tramp.elc"
>>         I tried running Emacs without a configuration file, using -q,
>>         and got the same thing.  Does anyone know why it's doing this?
>
> I guess you mean -Q. Anyway, same result here.

I've debugged it in Tramp. According to the backtrace, in
`tramp-file-name-handler' there is a call of `file-truename', which
causes the recursion. But I cannot see such a call :-(

However, the following patch cures the problem:

--8<---------------cut here---------------start------------->8---
=== modified file 'lisp/eshell/em-unix.el'
--- lisp/eshell/em-unix.el	2013-01-02 16:13:04 +0000
+++ lisp/eshell/em-unix.el	2013-02-17 16:26:24 +0000
@@ -1041,6 +1041,7 @@
 (defun eshell/su (&rest args)
   "Alias \"su\" to call Tramp."
   (setq args (eshell-stringify-list (eshell-flatten-list args)))
+  (require 'tramp)
   (let ((orig-args (copy-tree args)))
     (eshell-eval-using-options
      "su" args
@@ -1077,6 +1078,7 @@

 (defun eshell/sudo (&rest args)
   "Alias \"sudo\" to call Tramp."
+  (require 'tramp)
   (setq args (eshell-stringify-list (eshell-flatten-list args)))
   (let ((orig-args (copy-tree args)))
     (eshell-eval-using-options
@@ -1111,8 +1113,6 @@
 			  (format "%s|sudo:%s@%s:%s"
 				  (substring prefix 0 -1) user host dir)
 			(format "/sudo:%s@%s:%s" user host dir))))
-		;; Ensure, that Tramp has connected to that construct already.
-		(ignore (file-exists-p default-directory))
 		(eshell-named-command (car orig-args) (cdr orig-args))))))))

 (put 'eshell/sudo 'eshell-no-numeric-conversions t)
--8<---------------cut here---------------end--------------->8---

I've added the require also for `eshell/su', just in case.

> Even stranger, if I preload tramp by "emacs -Q -l tramp", I get random
> results:
>
> ~/src/emacs $ sudo id
> uid=0(root) gid=0(root) groups=0(root)
> ~/src/emacs $ sudo id
> Wrong type argument: processp, nil

This is a Tramp error. As defense mechanism, Tramp detects how often a
new connection is opened. If it happens too often in a short time frame,
it refuses to work. This shall avoid yo-yo effects on instable
connections.

If you call an external command several times, this mechanism applies. A
false alarm, obviously. The following patch solves this:

--8<---------------cut here---------------start------------->8---
[albinus@arthur emacs]$ bzr diff lisp/net/tramp-sh.el
=== modified file 'lisp/net/tramp-sh.el'
--- lisp/net/tramp-sh.el	2013-02-16 17:44:00 +0000
+++ lisp/net/tramp-sh.el	2013-02-17 16:52:48 +0000
@@ -2784,7 +2784,11 @@
 	   (or (null program) tramp-process-connection-type))
 	  (bmp (and (buffer-live-p buffer) (buffer-modified-p buffer)))
 	  (name1 name)
-	  (i 0))
+	  (i 0)
+	  ;; We do not want to raise an error when
+	  ;; `start-file-process' has been started several time in
+	  ;; `eshell' and friends.
+	  (tramp-current-connection nil))

       (unless buffer
 	;; BUFFER can be nil.  We use a temporary buffer.
--8<---------------cut here---------------end--------------->8---

Both patches are pretty simple. I would like to install them in the
emacs-24 branch. Objections?

>> Ian D

Best regards, Michael.

Re: Eshell and Sudo

[Aidan Gauland]

Michael Albinus <michael.albinus@gmx.de> writes:
> Both patches are pretty simple. I would like to install them in the
> emacs-24 branch. Objections?

Thanks for coming to the rescue.  The changes look fine, except for the
`require' in a defun.  Is that a good idea?  (Not a rhetorical question;
I don't know, I just haven't seen this before.)

Regards,
Aidan Gauland

Re: Eshell and Sudo

[Michael Albinus]

Aidan Gauland <aidalgol@no8wireless.co.nz> writes:

> Thanks for coming to the rescue.  The changes look fine, except for the
> `require' in a defun.  Is that a good idea?  (Not a rhetorical question;
> I don't know, I just haven't seen this before.)

Look at eshell :-) There are other defuns which apply the same trick.

I would like to avoid this, but I run out of ideas why Tramp loads
itself recursively in this special situation.

> Regards,
> Aidan Gauland

Best regards, Michael.

Re: Eshell and Sudo

[Aidan Gauland]

Michael Albinus <michael.albinus@gmx.de> writes:
> Aidan Gauland <aidalgol@no8wireless.co.nz> writes:
>> Thanks for coming to the rescue.  The changes look fine, except for the
>> `require' in a defun.  Is that a good idea?  (Not a rhetorical question;
>> I don't know, I just haven't seen this before.)
>
> Look at eshell :-) There are other defuns which apply the same trick.
>
> I would like to avoid this, but I run out of ideas why Tramp loads
> itself recursively in this special situation.

OK, then I'd say go ahead and commit the changes.  I hope this clears up
the recursive load problem.  Any idea why this appears erratically (for
some people and not others)?  Have you been able to reproduce it
consistently?

Regards,
Aidan Gauland

Re: Eshell and Sudo

[Michael Albinus]

Aidan Gauland <aidalgol@no8wireless.co.nz> writes:

Hi Aidan,

> OK, then I'd say go ahead and commit the changes.

I will, for sure. The question is rather whether to commit in the
emacs-24 branch, or in trunk. I prefer the emacs-24 branch, because the
changes are such simple. Glenn? Stefan?

> I hope this clears up the recursive load problem.  Any idea why this
> appears erratically (for some people and not others)?  Have you been
> able to reproduce it consistently?

Maybe there is a misunderstanding. The recursive load problem happens
consistently with the recipe given by Ian. This is fixed by the (require
'tramp) forms in the patch.

I was rather surprised to see that a "sudo id" sometimes refused to
run. But this is covered by my second patch, which handles the defense
mechanism of Tramp.

> Regards,
> Aidan Gauland

Best regards, Michael.

Re: Eshell and Sudo

[Stefan Monnier]

>> OK, then I'd say go ahead and commit the changes.
> I will, for sure. The question is rather whether to commit in the
> emacs-24 branch, or in trunk. I prefer the emacs-24 branch, because the
> changes are such simple. Glenn? Stefan?

They don't look "obviously safe" to me, so it'd only be OK on emacs-24
if it's a regression.  IOW: "how old is this problem"?


        Stefan

Re: Eshell and Sudo

[Michael Albinus]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> They don't look "obviously safe" to me, so it'd only be OK on emacs-24
> if it's a regression.  IOW: "how old is this problem"?

Regression wrt Emacs 24.2. I've started "emacs -Q" of

GNU Emacs 24.2.1 (i686-redhat-linux-gnu, GTK+ Version 3.6.4)
 of 2013-02-02 on buildvm-19.phx2.fedoraproject.org

No problem there.

>         Stefan

Best regards, Michael.

Re: Eshell and Sudo

[Stefan Monnier]

>> They don't look "obviously safe" to me, so it'd only be OK on emacs-24
>> if it's a regression.  IOW: "how old is this problem"?
> Regression wrt Emacs 24.2. I've started "emacs -Q" of

OK for emacs-24, then,


        Stefan

Re: Eshell and Sudo

[Michael Albinus]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> They don't look "obviously safe" to me, so it'd only be OK on emacs-24
>>> if it's a regression.  IOW: "how old is this problem"?
>> Regression wrt Emacs 24.2. I've started "emacs -Q" of
>
> OK for emacs-24, then,

Done.

>         Stefan

Best regards, Michael.