Do not hardcode "/bin/sh" in compile. Bug#24338, Bug#29723

* doc/emacs/custom.texi (Connection Variables): New node.

* doc/emacs/emacs.texi (Top): Add entry for Connection Variables.

* doc/emacs/misc.texi (Single Shell): Mention default value for
remote buffers.

* doc/lispref/variables.texi (Connection Local Variables):
Describe `with-connection-local-variables' instead of
`with-connection-local-profiles'.

* doc/misc/tramp.texi (Remote processes): Refer to Emacs manual.
Mention default connection-local settings for `shell-file-name'
and `shell-command-switch'.

* etc/NEWS: Mention connection-local variables changes.

* lisp/files-x.el (hack-connection-local-variables):
Push connection-local variables to `file-local-variables-alist'.
(connection-local-criteria-for-default-directory): New defsubst.
(with-connection-local-variables): Rename from
`with-connection-local-profiles'.  Adapt implementation.

* lisp/files.el (hack-local-variables):
Call `hack-connection-local-variables'.

* lisp/shell.el (shell): Use `with-connection-local-variables'.

* lisp/subr.el (start-file-process-shell-command):
* lisp/progmodes/compile.el (compilation-start):
Use `with-connection-local-variables'.  Do not set "/bin/sh" for
remote buffers, trust settings of `shell-file-name'.  (Bug#24338),
(Bug#29723)

* lisp/net/ange-ftp.el (ange-ftp-compress, ange-ftp-uncompress):
Use `shell-command-switch'.

* lisp/net/tramp-adb.el (tramp-adb-connection-local-default-profile):
New defvar.  Add it to connection-local profiles after loading "shell".

* lisp/net/tramp-integration.el (tramp-compat): Require tramp-compat.
(tramp-compat-exec-path): Do not declare anymore.
(tramp-connection-local-safe-shell-file-names): New defvar.
(tramp-connection-local-default-profile): New defconst.  Activate
it after loading "shell".
(shell-file-name, shell-command-switch): Add safe-local-variable
property.

* lisp/net/tramp-sh.el (tramp-display-escape-sequence-regexp):
Add tramp-autoload cookie.

* test/lisp/files-x-tests.el (remote-shell-file-name):
Add safe-local-variable property to remote-* variables.
(tramp-connection-local-default-profile): Declare.
(files-x-test-with-connection-local-variables):
Rename from `files-x-test-with-connection-local-profiles'.  Adapt
implementation.

* test/lisp/net/tramp-tests.el
(tramp-test34-connection-local-variables): New test.
(tramp-test34-explicit-shell-file-name): Run it also for tramp-adb.
Bind connection-local-{profile,criteria}-alist.  Use tramp-adb
specific `shell-file-name'.  Add safe-local-variable property to
`explicit-shell-file-name' and `explicit-sh-args'.

2 files changed, 123 insertions, 43 deletions

diff --git a/test/lisp/files-x-tests.el b/test/lisp/files-x-tests.el
index d678be409d1..568a8984479 100644
--- a/test/lisp/files-x-tests.el
+++ b/test/lisp/files-x-tests.el
@@ -35,6 +35,11 @@
   '((remote-null-device . "/dev/null")))
 (defconst files-x-test--variables4
   '((remote-null-device . "null")))
+(put 'remote-shell-file-name 'safe-local-variable #'identity)
+(put 'remote-shell-command-switch 'safe-local-variable #'identity)
+(put 'remote-shell-interactive-switch 'safe-local-variable #'identity)
+(put 'remote-shell-login-switch 'safe-local-variable #'identity)
+(put 'remote-null-device 'safe-local-variable #'identity)
 
 (defconst files-x-test--application '(:application 'my-application))
 (defconst files-x-test--another-application
@@ -268,7 +273,9 @@
         (should-not (local-variable-p 'remote-shell-file-name))
         (should-not (boundp 'remote-shell-file-name))))))
 
-(ert-deftest files-x-test-with-connection-local-profiles ()
+(defvar tramp-connection-local-default-profile)
+
+(ert-deftest files-x-test-with-connection-local-variables ()
   "Test setting connection-local variables."
 
   (let (connection-local-profile-alist connection-local-criteria-alist)
@@ -303,46 +310,48 @@
          (string-equal (symbol-value 'remote-null-device) "/dev/null"))
 
         ;; A candidate connection-local variable is not bound yet.
-        (should-not (local-variable-p 'remote-shell-command-switch))
-
-        ;; Use the macro.
-        (with-connection-local-profiles '(remote-bash remote-ksh)
-          ;; All connection-local variables are set.  They apply in
-          ;; reverse order in `connection-local-variables-alist'.
-          ;; This variable keeps only the variables to be set inside
-          ;; the macro.
-          (should
-           (equal connection-local-variables-alist
-                  (nreverse (copy-tree files-x-test--variables1))))
-          ;; The variables exist also as local variables.
-          (should (local-variable-p 'remote-shell-file-name))
-          (should (local-variable-p 'remote-shell-command-switch))
-          ;; The proper variable values are set.  The settings from
-          ;; `remote-bash' overwrite the same variables as in
-          ;; `remote-ksh'.
-          (should
-           (string-equal (symbol-value 'remote-shell-file-name) "/bin/bash"))
-          (should
-           (string-equal (symbol-value 'remote-shell-command-switch) "-c")))
-
-        ;; Everything is rewound.  The old variable values are reset.
-        (should
-         (equal connection-local-variables-alist
-                (append
-                 (nreverse (copy-tree files-x-test--variables3))
-                 (nreverse (copy-tree files-x-test--variables2)))))
-        ;; The variables exist also as local variables.
-        (should (local-variable-p 'remote-shell-file-name))
-        (should (local-variable-p 'remote-null-device))
-        ;; The proper variable values are set.  The settings from
-        ;; `remote-ksh' are back.
-        (should
-         (string-equal (symbol-value 'remote-shell-file-name) "/bin/ksh"))
-        (should
-         (string-equal (symbol-value 'remote-null-device) "/dev/null"))
+        (should-not (local-variable-p 'remote-shell-command-switch))))
 
-        ;; The variable set temporarily is not unbound, again.
-        (should-not (local-variable-p 'remote-shell-command-switch))))))
+    (with-temp-buffer
+      ;; Use the macro.  We need a remote `default-directory'.
+      (let ((enable-connection-local-variables t)
+            (default-directory "/method:host:")
+            (remote-null-device "null"))
+        (should-not connection-local-variables-alist)
+        (should-not (local-variable-p 'remote-shell-file-name))
+        (should-not (local-variable-p 'remote-null-device))
+        (should-not (boundp 'remote-shell-file-name))
+        (should (string-equal (symbol-value 'remote-null-device) "null"))
+
+        (with-connection-local-variables
+         ;; All connection-local variables are set.  They apply in
+         ;; reverse order in `connection-local-variables-alist'.
+         ;; Since we ha a remote default directory, Tramp's settings
+         ;; are appended as well.
+         (should
+          (equal
+           connection-local-variables-alist
+           (append
+            (nreverse (copy-tree files-x-test--variables3))
+            (nreverse (copy-tree files-x-test--variables2))
+            (nreverse (copy-tree tramp-connection-local-default-profile)))))
+         ;; The variables exist also as local variables.
+         (should (local-variable-p 'remote-shell-file-name))
+         (should (local-variable-p 'remote-null-device))
+         ;; The proper variable values are set.
+         (should
+          (string-equal (symbol-value 'remote-shell-file-name) "/bin/ksh"))
+         (should
+          (string-equal (symbol-value 'remote-null-device) "/dev/null")))
+
+        ;; Everything is rewound.  The old variable values are reset.
+        (should-not connection-local-variables-alist)
+        ;; The variables don't exist as local variables.
+        (should-not (local-variable-p 'remote-shell-file-name))
+        (should-not (local-variable-p 'remote-null-device))
+        ;; The variable values are reset.
+        (should-not (boundp 'remote-shell-file-name))
+        (should (string-equal (symbol-value 'remote-null-device) "null"))))))
 
 (provide 'files-x-tests)
 ;;; files-x-tests.el ends here
diff --git a/test/lisp/net/tramp-tests.el b/test/lisp/net/tramp-tests.el
index 69d5ba8b7df..bf7cdfafabe 100644
--- a/test/lisp/net/tramp-tests.el
+++ b/test/lisp/net/tramp-tests.el
@@ -4274,12 +4274,78 @@ This tests also `make-symbolic-link', `file-truename' and `add-name-to-file'."
     (dolist (dir '("/mock:localhost#11111:" "/mock:localhost#22222:"))
       (tramp-cleanup-connection (tramp-dissect-file-name dir)))))
 
+;; Connection-local variables are enabled per default since Emacs 27.1.
+(ert-deftest tramp-test34-connection-local-variables ()
+  "Check that connection-local variables are enabled."
+  :tags '(:expensive-test)
+  (skip-unless (tramp--test-enabled))
+  ;; Since Emacs 27.1.
+  (skip-unless (fboundp 'with-connection-local-variables))
+
+  ;; `connection-local-set-profile-variables' and
+  ;; `connection-local-set-profiles' exist since Emacs 26.1.  We don't
+  ;; want to see compiler warnings for older Emacsen.
+  (let* ((default-directory tramp-test-temporary-file-directory)
+         (tmp-name1 (tramp--test-make-temp-name))
+         (tmp-name2 (expand-file-name "foo" tmp-name1))
+         (enable-local-variables :all)
+         (enable-remote-dir-locals t)
+         kill-buffer-query-functions
+         connection-local-profile-alist connection-local-criteria-alist)
+    (unwind-protect
+        (progn
+          (make-directory tmp-name1)
+          (should (file-directory-p tmp-name1))
+
+          ;; `local-variable' is buffer-local due to explicit setting.
+          (with-no-warnings
+            (defvar-local local-variable 'buffer))
+          (with-temp-buffer
+            (should (eq local-variable 'buffer)))
+
+          ;; `local-variable' is connection-local due to Tramp.
+          (write-region "foo" nil tmp-name2)
+          (should (file-exists-p tmp-name2))
+          (with-no-warnings
+            (connection-local-set-profile-variables
+             'local-variable-profile
+             '((local-variable . connect)))
+            (connection-local-set-profiles
+             `(:application tramp
+               :protocol ,(file-remote-p default-directory 'method)
+               :user ,(file-remote-p default-directory 'user)
+               :machine ,(file-remote-p default-directory 'host))
+             'local-variable-profile))
+          (with-current-buffer (find-file-noselect tmp-name2)
+            (should (eq local-variable 'connect))
+            (kill-buffer (current-buffer)))
+
+          ;; `local-variable' is dir-local due to existence of .dir-locals.el.
+          (write-region
+           "((nil . ((local-variable . dir))))" nil
+           (expand-file-name ".dir-locals.el" tmp-name1))
+          (should (file-exists-p (expand-file-name ".dir-locals.el" tmp-name1)))
+          (with-current-buffer (find-file-noselect tmp-name2)
+            (should (eq local-variable 'dir))
+            (kill-buffer (current-buffer)))
+
+          ;; `local-variable' is file-local due to specifying as file variable.
+          (write-region
+           "-*- mode: comint; local-variable: file; -*-" nil tmp-name2)
+          (should (file-exists-p tmp-name2))
+          (with-current-buffer (find-file-noselect tmp-name2)
+            (should (eq local-variable 'file))
+            (kill-buffer (current-buffer))))
+
+      ;; Cleanup.
+      (ignore-errors (delete-directory tmp-name1 'recursive)))))
+
 ;; 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)
   (skip-unless (tramp--test-enabled))
-  (skip-unless (tramp--test-sh-p))
+  (skip-unless (or (tramp--test-adb-p) (tramp--test-sh-p)))
   ;; Since Emacs 26.1.
   (skip-unless (and (fboundp 'connection-local-set-profile-variables)
                     (fboundp 'connection-local-set-profiles)))
@@ -4288,7 +4354,8 @@ This tests also `make-symbolic-link', `file-truename' and `add-name-to-file'."
   ;; `connection-local-set-profiles' exist since Emacs 26.1.  We don't
   ;; want to see compiler warnings for older Emacsen.
   (let ((default-directory tramp-test-temporary-file-directory)
-        explicit-shell-file-name kill-buffer-query-functions)
+        explicit-shell-file-name kill-buffer-query-functions
+        connection-local-profile-alist connection-local-criteria-alist)
     (unwind-protect
         (progn
           ;; `shell-mode' would ruin our test, because it deletes all
@@ -4298,7 +4365,8 @@ This tests also `make-symbolic-link', `file-truename' and `add-name-to-file'."
           (with-no-warnings
             (connection-local-set-profile-variables
              'remote-sh
-             '((explicit-shell-file-name . "/bin/sh")
+             `((explicit-shell-file-name
+                . ,(if (tramp--test-adb-p) "/system/bin/sh" "/bin/sh"))
                (explicit-sh-args . ("-i"))))
             (connection-local-set-profiles
              `(:application tramp
@@ -4306,6 +4374,8 @@ This tests also `make-symbolic-link', `file-truename' and `add-name-to-file'."
                :user ,(file-remote-p default-directory 'user)
                :machine ,(file-remote-p default-directory 'host))
              'remote-sh))
+          (put 'explicit-shell-file-name 'safe-local-variable #'identity)
+          (put 'explicit-sh-args 'safe-local-variable #'identity)
 
           ;; Run interactive shell.  Since the default directory is
           ;; remote, `explicit-shell-file-name' shall be set in order
@@ -4316,6 +4386,7 @@ This tests also `make-symbolic-link', `file-truename' and `add-name-to-file'."
             (call-interactively #'shell)
             (should explicit-shell-file-name)))
 
+      ;; Cleanup.
       (put 'explicit-shell-file-name 'permanent-local nil)
       (kill-buffer "*shell*"))))