Fix Bug#34943

* lisp/files-x.el (cl-lib): Require when compiling.
(hack-connection-local-variables): Regard connection-local
variables as safe.  (Bug#34943)

* lisp/shell.el (shell): Use `with-connection-local-variables' for
the whole code block.  (Bug#34943)

* lisp/net/tramp-adb.el (tramp-connection-local-safe-shell-file-names):
Do not set values.

* lisp/net/tramp-integration.el
(tramp-connection-local-safe-shell-file-names): Remove.
(shell-file-name, shell-command-switch): Do not add safe-local-variable
property.

* lisp/net/tramp.el (tramp-handle-shell-command): Use proper
buffer name.

* test/lisp/net/tramp-tests.el (tramp-test34-explicit-shell-file-name):
Tag it :unstable.

6 files changed, 40 insertions, 39 deletions

diff --git a/lisp/files-x.el b/lisp/files-x.el
index eedf630b71f..5353e54158e 100644
--- a/lisp/files-x.el
+++ b/lisp/files-x.el
@@ -31,6 +31,7 @@
 ;;; Code:
 
 (eval-when-compile (require 'subr-x)) ; for string-trim-right
+(eval-when-compile (require 'cl-lib)) ; for cl-letf
 
 
 ;;; Commands to add/delete file-local/directory-local variables.
@@ -686,8 +687,11 @@ This does nothing if `enable-connection-local-variables' is nil."
       (dolist (variable (connection-local-get-profile-variables profile))
         (unless (assq (car variable) connection-local-variables-alist)
           (push variable connection-local-variables-alist))))
-    ;; Push them to `file-local-variables-alist'.
-    (hack-local-variables-filter connection-local-variables-alist nil)))
+    ;; Push them to `file-local-variables-alist'.  Connection-local
+    ;; variables do not appear from external files.  So we can regard
+    ;; them as safe.
+    (cl-letf (((symbol-function 'safe-local-variable-p) (lambda (_sym _val) t)))
+      (hack-local-variables-filter connection-local-variables-alist nil))))
 
 ;;;###autoload
 (defun hack-connection-local-variables-apply (criteria)
diff --git a/lisp/net/tramp-adb.el b/lisp/net/tramp-adb.el
index 8eeaa8be415..68960426b68 100644
--- a/lisp/net/tramp-adb.el
+++ b/lisp/net/tramp-adb.el
@@ -1299,7 +1299,6 @@ connection if a previous connection has died for some reason."
   '((shell-file-name . "/system/bin/sh")
     (shell-command-switch . "-c"))
   "Default connection-local variables for remote adb connections.")
-(add-to-list 'tramp-connection-local-safe-shell-file-names "/system/bin/sh")
 
 ;; `connection-local-set-profile-variables' and
 ;; `connection-local-set-profiles' exists since Emacs 26.1.
diff --git a/lisp/net/tramp-integration.el b/lisp/net/tramp-integration.el
index 946d7f8cbab..da168adce77 100644
--- a/lisp/net/tramp-integration.el
+++ b/lisp/net/tramp-integration.el
@@ -173,21 +173,10 @@ NAME must be equal to `tramp-current-connection'."
 
 ;;; Default connection-local variables for Tramp:
 
-;;;###tramp-autoload
-(defvar tramp-connection-local-safe-shell-file-names nil
-  "List of safe `shell-file-name' values for remote hosts.")
-(add-to-list 'tramp-connection-local-safe-shell-file-names "/bin/sh")
-
 (defconst tramp-connection-local-default-profile
   '((shell-file-name . "/bin/sh")
     (shell-command-switch . "-c"))
   "Default connection-local variables for remote connections.")
-(put 'shell-file-name 'safe-local-variable
-     (lambda (item)
-       (and (stringp item)
-            (member item tramp-connection-local-safe-shell-file-names))))
-(put 'shell-command-switch 'safe-local-variable
-     (lambda (item) (and (stringp item) (string-equal item "-c"))))
 
 ;; `connection-local-set-profile-variables' and
 ;; `connection-local-set-profiles' exists since Emacs 26.1.
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 48c363a873a..1d7242ba8f9 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -3641,7 +3641,7 @@ support symbolic links."
         (prog1
             ;; Run the process.
             (setq p (start-file-process-shell-command
-                     "*Async Shell*" buffer command))
+                     (buffer-name output-buffer) buffer command))
           ;; Display output.
           (with-current-buffer output-buffer
             (display-buffer output-buffer '(nil (allow-no-window . t)))
@@ -4892,6 +4892,12 @@ Only works for Bourne-like shells."
 ;;   and friends, for most of the handlers this is the major
 ;;   difference between the different backends.  Other handlers but
 ;;   *-process-file would profit from this as well.
+;;
+;; * Get rid of `shell-command'.  In its primary implementation, it
+;;   uses `process-file-shell-command' and
+;;   `start-file-process-shell-command', which is sufficient due to
+;;   connection-local `shell-file-name'.
+
 
 ;;; tramp.el ends here
 
diff --git a/lisp/shell.el b/lisp/shell.el
index 264981bd435..53570272111 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -721,9 +721,9 @@ Otherwise, one argument `-i' is passed to the shell.
                  (current-buffer)))
 
   (with-current-buffer buffer
-    (when (file-remote-p default-directory)
-      ;; On remote hosts, the local `shell-file-name' might be useless.
-      (with-connection-local-variables
+    (with-connection-local-variables
+     ;; On remote hosts, the local `shell-file-name' might be useless.
+     (when (file-remote-p default-directory)
        (if (and (called-interactively-p 'any)
                 (null explicit-shell-file-name)
                 (null (getenv "ESHELL")))
@@ -732,26 +732,26 @@ Otherwise, one argument `-i' is passed to the shell.
                  (expand-file-name
                   (read-file-name
                    "Remote shell path: " default-directory shell-file-name
-                   t shell-file-name))))))))
-
-  ;; The buffer's window must be correctly set when we call comint
-  ;; (so that comint sets the COLUMNS env var properly).
-  (pop-to-buffer buffer)
-  ;; Rain or shine, BUFFER must be current by now.
-  (unless (comint-check-proc buffer)
-    (let* ((prog (or explicit-shell-file-name
-                     (getenv "ESHELL") shell-file-name))
-           (name (file-name-nondirectory prog))
-           (startfile (concat "~/.emacs_" name))
-           (xargs-name (intern-soft (concat "explicit-" name "-args"))))
-      (unless (file-exists-p startfile)
-        (setq startfile (concat user-emacs-directory "init_" name ".sh")))
-      (apply #'make-comint-in-buffer "shell" buffer prog
-             (if (file-exists-p startfile) startfile)
-             (if (and xargs-name (boundp xargs-name))
-                 (symbol-value xargs-name)
-               '("-i")))
-      (shell-mode)))
+                   t shell-file-name))))))
+
+     ;; The buffer's window must be correctly set when we call comint
+     ;; (so that comint sets the COLUMNS env var properly).
+     (pop-to-buffer buffer)
+     ;; Rain or shine, BUFFER must be current by now.
+     (unless (comint-check-proc buffer)
+       (let* ((prog (or explicit-shell-file-name
+                        (getenv "ESHELL") shell-file-name))
+              (name (file-name-nondirectory prog))
+              (startfile (concat "~/.emacs_" name))
+              (xargs-name (intern-soft (concat "explicit-" name "-args"))))
+         (unless (file-exists-p startfile)
+           (setq startfile (concat user-emacs-directory "init_" name ".sh")))
+         (apply #'make-comint-in-buffer "shell" buffer prog
+                (if (file-exists-p startfile) startfile)
+                (if (and xargs-name (boundp xargs-name))
+                    (symbol-value xargs-name)
+                  '("-i")))
+         (shell-mode)))))
   buffer)
 
 ;;; Directory tracking
diff --git a/test/lisp/net/tramp-tests.el b/test/lisp/net/tramp-tests.el
index 34b676ee324..7290eb23925 100644
--- a/test/lisp/net/tramp-tests.el
+++ b/test/lisp/net/tramp-tests.el
@@ -4350,7 +4350,9 @@ This tests also `make-symbolic-link', `file-truename' and `add-name-to-file'."
 ;; The functions were introduced in Emacs 26.1.
 (ert-deftest tramp-test34-explicit-shell-file-name ()
   "Check that connection-local `explicit-shell-file-name' is set."
-  :tags '(:expensive-test)
+  ;; The handling of connection-local variables has changed.  Test
+  ;; must be reworked.
+  :tags '(:expensive-test :unstable)
   (skip-unless (tramp--test-enabled))
   (skip-unless (or (tramp--test-adb-p) (tramp--test-sh-p)))
   ;; Since Emacs 26.1.
@@ -5715,6 +5717,7 @@ Since it unloads Tramp, it shall be the last test to run."
 ;; * Fix `tramp-test29-start-file-process',
 ;;   `tramp-test30-make-process' and `tramp-test32-shell-command' for
 ;;   `adb' (see comment in `tramp-adb-send-command').
+;; * Rework `tramp-test34-explicit-shell-file-name'.
 ;; * Fix Bug#16928 in `tramp-test43-asynchronous-requests'.
 
 (provide 'tramp-tests)