From 450e057a3b2305d1a709fd48ca4defca0bb85bbc Mon Sep 17 00:00:00 2001 From: Jim Porter 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..a58fa6f07d9 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