Subject: Re: bug#68074: eshell sudo/doas does not work for aliases

From: Jim Porter <[email-masked]>
Date: Fri, 26 Jan 2024 16:27:44 -0800

On 1/26/2024 3:53 PM, Alfonso Sanchez-Beato via Bug reports for GNU 
Emacs, the Swiss army knife of text editors wrote:
> Unfortunately that does not seem to be the case, I see an error saying "Invalid function" after applying on top of current master:

Oops, I messed up the quoting. How about this?

From c82f0d3dda8ab666438ec7192b0b1ee1dbba4613 Mon Sep 17 00:00:00 2001
From: Jim Porter <[email-masked]>
Date: Wed, 24 Jan 2024 18:32:00 -0800
Subject: [PATCH] Fix command replacement with the Eshell builtin versions of
 "sudo" and "doas"

This is particularly important when the inner command to execute is an
alias.  Aliases throw 'eshell-replace-command' too, so we want to do
this in two phases: first, replace the "sudo"/"doas" with a
let-binding of 'default-directory', and then later, let the alias code
do its own replacement (bug#68074).

* lisp/eshell/em-tramp.el (eshell/sudo, eshell/doas): Use
'eshell-replace-command' to wrap the inner command.
* test/lisp/eshell/em-tramp-tests.el (mock-eshell-named-command):
Remove.
(em-tramp-test/sudo-basic, em-tramp-test/sudo-user)
(em-tramp-test/doas-basic, em-tramp-test/doas-user): Catch
'eshell-replace-command'.
---
 lisp/eshell/em-tramp.el            | 22 ++++----
 test/lisp/eshell/em-tramp-tests.el | 89 ++++++++++++++----------------
 2 files changed, 50 insertions(+), 61 deletions(-)

diff --git a/lisp/eshell/em-tramp.el b/lisp/eshell/em-tramp.el
index 90f9c6cf78d..9812c9e762a 100644
--- a/lisp/eshell/em-tramp.el
+++ b/lisp/eshell/em-tramp.el
@@ -121,12 +121,11 @@ eshell/sudo
      :usage "[(-u | --user) USER] (-s | --shell) | COMMAND
 Execute a COMMAND as the superuser or another USER.")
    (let ((dir (eshell--method-wrap-directory default-directory "sudo" user)))
-     (if shell
-         (throw 'eshell-replace-command
-                (eshell-parse-command "cd" (list dir)))
-       (throw 'eshell-external
-              (let ((default-directory dir))
-                (eshell-named-command (car args) (cdr args))))))))
+     (throw 'eshell-replace-command
+            (if shell
+                (eshell-parse-command "cd" (list dir))
+              `(let ((default-directory ,dir))
+                 (eshell-named-command ',(car args) ',(cdr args))))))))
 
 (put 'eshell/sudo 'eshell-no-numeric-conversions t)
 
@@ -144,12 +143,11 @@ eshell/doas
      :usage "[(-u | --user) USER] (-s | --shell) | COMMAND
 Execute a COMMAND as the superuser or another USER.")
    (let ((dir (eshell--method-wrap-directory default-directory "doas" user)))
-     (if shell
-         (throw 'eshell-replace-command
-                (eshell-parse-command "cd" (list dir)))
-       (throw 'eshell-external
-              (let ((default-directory dir))
-                (eshell-named-command (car args) (cdr args))))))))
+     (throw 'eshell-replace-command
+            (if shell
+                (eshell-parse-command "cd" (list dir))
+              `(let ((default-directory ,dir))
+                 (eshell-named-command ,(car args) ,(cdr args))))))))
 
 (put 'eshell/doas 'eshell-no-numeric-conversions t)
 
diff --git a/test/lisp/eshell/em-tramp-tests.el b/test/lisp/eshell/em-tramp-tests.el
index d33f6a2b46a..deae6579401 100644
--- a/test/lisp/eshell/em-tramp-tests.el
+++ b/test/lisp/eshell/em-tramp-tests.el
@@ -59,35 +59,31 @@ em-tramp-test/su-login
         "cd"
         (list ,(format "/su:root@%s:~/" tramp-default-host))))))
 
-(defun mock-eshell-named-command (&rest args)
-  "Dummy function to test Eshell `sudo' command rewriting."
-  (list default-directory args))
-
 (ert-deftest em-tramp-test/sudo-basic ()
   "Test Eshell `sudo' command with default user."
-  (cl-letf (((symbol-function 'eshell-named-command)
-             #'mock-eshell-named-command))
-    (should (equal
-             (catch 'eshell-external (eshell/sudo "echo" "hi"))
-             `(,(format "/sudo:root@%s:%s" tramp-default-host default-directory)
-               ("echo" ("hi")))))
-    (should (equal
-             (catch 'eshell-external (eshell/sudo "echo" "-u" "hi"))
-             `(,(format "/sudo:root@%s:%s" tramp-default-host default-directory)
-               ("echo" ("-u" "hi")))))))
+  (let ((sudo-directory (format "/sudo:root@%s:%s"
+                                tramp-default-host default-directory)))
+    (should (equal (catch 'eshell-replace-command
+                     (eshell/sudo "echo" "hi"))
+                   `(let ((default-directory ,sudo-directory))
+                      (eshell-named-command "echo" ("hi")))))
+    (should (equal (catch 'eshell-replace-command
+                     (eshell/sudo "echo" "-u" "hi"))
+                   `(let ((default-directory ,sudo-directory))
+                      (eshell-named-command "echo" ("-u" "hi")))))))
 
 (ert-deftest em-tramp-test/sudo-user ()
   "Test Eshell `sudo' command with specified user."
-  (cl-letf (((symbol-function 'eshell-named-command)
-             #'mock-eshell-named-command))
-    (should (equal
-             (catch 'eshell-external (eshell/sudo "-u" "USER" "echo" "hi"))
-             `(,(format "/sudo:USER@%s:%s" tramp-default-host default-directory)
-               ("echo" ("hi")))))
-    (should (equal
-             (catch 'eshell-external (eshell/sudo "-u" "USER" "echo" "-u" "hi"))
-             `(,(format "/sudo:USER@%s:%s" tramp-default-host default-directory)
-               ("echo" ("-u" "hi")))))))
+  (let ((sudo-directory (format "/sudo:USER@%s:%s"
+                                tramp-default-host default-directory)))
+    (should (equal (catch 'eshell-replace-command
+                     (eshell/sudo "-u" "USER" "echo" "hi"))
+                   `(let ((default-directory ,sudo-directory))
+                      (eshell-named-command "echo" ("hi")))))
+    (should (equal (catch 'eshell-replace-command
+                     (eshell/sudo "-u" "USER" "echo" "-u" "hi"))
+                   `(let ((default-directory ,sudo-directory))
+                      (eshell-named-command "echo" ("-u" "hi")))))))
 
 (ert-deftest em-tramp-test/sudo-shell ()
   "Test Eshell `sudo' command with -s/--shell option."
@@ -109,34 +105,29 @@ em-tramp-test/sudo-user-shell
 
 (ert-deftest em-tramp-test/doas-basic ()
   "Test Eshell `doas' command with default user."
-  (cl-letf (((symbol-function 'eshell-named-command)
-             #'mock-eshell-named-command))
-    (should (equal
-             (catch 'eshell-external (eshell/doas "echo" "hi"))
-             `(,(format "/doas:root@%s:%s"
-                        tramp-default-host default-directory)
-               ("echo" ("hi")))))
-    (should (equal
-             (catch 'eshell-external (eshell/doas "echo" "-u" "hi"))
-             `(,(format "/doas:root@%s:%s"
-                        tramp-default-host default-directory)
-               ("echo" ("-u" "hi")))))))
+  (let ((doas-directory (format "/doas:root@%s:%s"
+                                tramp-default-host default-directory)))
+    (should (equal (catch 'eshell-replace-command
+                     (eshell/doas "echo" "hi"))
+                   `(let ((default-directory ,doas-directory))
+                      (eshell-named-command "echo" ("hi")))))
+    (should (equal (catch 'eshell-replace-command
+                     (eshell/doas "echo" "-u" "hi"))
+                   `(let ((default-directory ,doas-directory))
+                      (eshell-named-command "echo" ("-u" "hi")))))))
 
 (ert-deftest em-tramp-test/doas-user ()
   "Test Eshell `doas' command with specified user."
-  (cl-letf (((symbol-function 'eshell-named-command)
-             #'mock-eshell-named-command))
-    (should (equal
-             (catch 'eshell-external (eshell/doas "-u" "USER" "echo" "hi"))
-             `(,(format "/doas:USER@%s:%s"
-                        tramp-default-host default-directory)
-               ("echo" ("hi")))))
-    (should (equal
-             (catch 'eshell-external
-               (eshell/doas "-u" "USER" "echo" "-u" "hi"))
-             `(,(format "/doas:USER@%s:%s"
-                        tramp-default-host default-directory)
-               ("echo" ("-u" "hi")))))))
+  (let ((doas-directory (format "/doas:USER@%s:%s"
+                                tramp-default-host default-directory)))
+    (should (equal (catch 'eshell-replace-command
+                     (eshell/doas "-u" "USER" "echo" "hi"))
+                   `(let ((default-directory ,doas-directory))
+                      (eshell-named-command "echo" ("hi")))))
+    (should (equal (catch 'eshell-replace-command
+                     (eshell/doas "-u" "USER" "echo" "-u" "hi"))
+                   `(let ((default-directory ,doas-directory))
+                      (eshell-named-command "echo" ("-u" "hi")))))))
 
 (ert-deftest em-tramp-test/doas-shell ()
   "Test Eshell `doas' command with -s/--shell option."
-- 
2.25.1