master 5ee6583: Tramp: honor default file modes in make-directory

[Stephen Gildea]

branch: master
commit 5ee6583cb26801ba079e671071179482ccdd679a
Author: Stephen Gildea <stepheng+emacs@gildea.com>
Commit: Stephen Gildea <stepheng+emacs@gildea.com>

    Tramp: honor default file modes in make-directory
    
    * lisp/net/tramp-sh.el:
    * lisp/net/tramp-adb.el:
    * lisp/net/tramp-sudoedit.el:
    * lisp/net/tramp-gvfs.el: Add support for default file modes to
    relevant Tramp back ends for make-directory.  (Closes: Bug#50410)
    * test/lisp/net/tramp-tests.el 
(tramp-test13-make-directory-with-file-modes):
    New test.
    * etc/NEWS: Note this enhancement.
    
    Thanks to Michael Albinus for helping improve this patch.
---
 etc/NEWS                     |  5 +++-
 lisp/net/tramp-adb.el        |  4 ++-
 lisp/net/tramp-gvfs.el       | 11 +++++---
 lisp/net/tramp-sh.el         |  3 ++-
 lisp/net/tramp-sudoedit.el   |  1 +
 test/lisp/net/tramp-tests.el | 62 ++++++++++++++++++++++++++++++++------------
 6 files changed, 62 insertions(+), 24 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 9031ae5..ea7bb6b 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1955,7 +1955,7 @@ security key, like yubikey, solokey, or nitrokey.
 
 +++
 *** Trashed remote files are moved to the local trash directory.
-All remote files, which are trashed, are moved to the local trash
+All remote files that are trashed are moved to the local trash
 directory, except remote encrypted files, which are always deleted.
 
 +++
@@ -1991,6 +1991,9 @@ Writing auto-save, backup or lock files to the local 
temporary
 directory must be confirmed.  In order to suppress this confirmation,
 set user option 'tramp-allow-unsafe-temporary-files' to t.
 
++++
+*** 'make-directory' of a remote directory honors the default file modes.
+
 ** gdb-mi
 
 *** New user option 'gdb-registers-enable-filter'.
diff --git a/lisp/net/tramp-adb.el b/lisp/net/tramp-adb.el
index 70dbfdb..a35ac37 100644
--- a/lisp/net/tramp-adb.el
+++ b/lisp/net/tramp-adb.el
@@ -442,7 +442,9 @@ Emacs dired can't find files."
          (make-directory par parents))))
     (tramp-flush-directory-properties v localname)
     (unless (or (tramp-adb-send-command-and-check
-                v (format "mkdir %s" (tramp-shell-quote-argument localname)))
+                v (format "mkdir -m %#o %s"
+                          (default-file-modes)
+                          (tramp-shell-quote-argument localname)))
                (and parents (file-directory-p dir)))
       (tramp-error v 'file-error "Couldn't make directory %s" dir))))
 
diff --git a/lisp/net/tramp-gvfs.el b/lisp/net/tramp-gvfs.el
index e4f54cf..eb889bb 100644
--- a/lisp/net/tramp-gvfs.el
+++ b/lisp/net/tramp-gvfs.el
@@ -1574,10 +1574,13 @@ If FILE-SYSTEM is non-nil, return file system 
attributes."
        (when (and parents (not (file-directory-p ldir)))
          (make-directory ldir parents))
        ;; Just do it.
-       (unless (or (tramp-gvfs-send-command
-                    v "gvfs-mkdir" (tramp-gvfs-url-file-name dir))
-                   (and parents (file-directory-p dir)))
-         (tramp-error v 'file-error "Couldn't make directory %s" dir))))))
+       (or (let ((mkdir-succeeded
+                  (tramp-gvfs-send-command
+                   v "gvfs-mkdir" (tramp-gvfs-url-file-name dir))))
+             (if mkdir-succeeded (set-file-modes dir (default-file-modes)))
+             mkdir-succeeded)
+           (and parents (file-directory-p dir))
+           (tramp-error v 'file-error "Couldn't make directory %s" dir))))))
 
 (defun tramp-gvfs-handle-rename-file
   (filename newname &optional ok-if-already-exists)
diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
index a2bf0af..e57145e 100644
--- a/lisp/net/tramp-sh.el
+++ b/lisp/net/tramp-sh.el
@@ -2449,8 +2449,9 @@ The method used must be an out-of-band method."
     (tramp-flush-directory-properties
      v (if parents "/" (file-name-directory localname)))
     (tramp-barf-unless-okay
-     v (format "%s %s"
+     v (format "%s -m %#o %s"
               (if parents "mkdir -p" "mkdir")
+              (default-file-modes)
               (tramp-shell-quote-argument localname))
      "Couldn't make directory %s" dir)))
 
diff --git a/lisp/net/tramp-sudoedit.el b/lisp/net/tramp-sudoedit.el
index 5895f1d..051d145 100644
--- a/lisp/net/tramp-sudoedit.el
+++ b/lisp/net/tramp-sudoedit.el
@@ -597,6 +597,7 @@ the result will be a local, non-Tramp, file name."
      v (if parents "/" (file-name-directory localname)))
     (unless (tramp-sudoedit-send-command
             v (if parents '("mkdir" "-p") "mkdir")
+            "-m" (format "%#o" (default-file-modes))
             (tramp-compat-file-name-unquote localname))
       (tramp-error v 'file-error "Couldn't make directory %s" dir))))
 
diff --git a/test/lisp/net/tramp-tests.el b/test/lisp/net/tramp-tests.el
index 54a585c..393302d 100644
--- a/test/lisp/net/tramp-tests.el
+++ b/test/lisp/net/tramp-tests.el
@@ -2765,28 +2765,38 @@ This checks also `file-name-as-directory', 
`file-name-directory',
            (ignore-errors (delete-directory source 'recursive))
            (ignore-errors (delete-directory target 'recursive))))))))
 
-(ert-deftest tramp-test13-make-directory ()
-  "Check `make-directory'.
-This tests also `file-directory-p' and `file-accessible-directory-p'."
-  (skip-unless (tramp--test-enabled))
-
-  (dolist (quoted (if (tramp--test-expensive-test) '(nil t) '(nil)))
+(defun tramp-test-make-directory-helper (test-default-file-modes-p)
+  "Helper test used by tramp-test13-make-directory* tests."
+  (dolist (quoted (if (and (tramp--test-expensive-test)
+                           (not test-default-file-modes-p))
+                      '(nil t)
+                    '(nil)))
     (let* ((tmp-name1 (tramp--test-make-temp-name nil quoted))
-          (tmp-name2 (expand-file-name "foo/bar" tmp-name1)))
+          (tmp-name2 (expand-file-name "foo/bar" tmp-name1))
+          (unusual-file-mode-1 #o740)
+          (unusual-file-mode-2 #o710))
       (unwind-protect
          (progn
-           (make-directory tmp-name1)
+           (with-file-modes unusual-file-mode-1
+             (make-directory tmp-name1))
            (should-error
             (make-directory tmp-name1)
             :type 'file-already-exists)
            (should (file-directory-p tmp-name1))
            (should (file-accessible-directory-p tmp-name1))
+           (and test-default-file-modes-p
+                (should (equal (format "%#o" unusual-file-mode-1)
+                               (format "%#o" (file-modes tmp-name1)))))
            (should-error
             (make-directory tmp-name2)
             :type 'file-error)
-           (make-directory tmp-name2 'parents)
+           (with-file-modes unusual-file-mode-2
+             (make-directory tmp-name2 'parents))
            (should (file-directory-p tmp-name2))
            (should (file-accessible-directory-p tmp-name2))
+           (and test-default-file-modes-p
+                (should (equal (format "%#o" unusual-file-mode-2)
+                               (format "%#o" (file-modes tmp-name2)))))
            ;; If PARENTS is non-nil, `make-directory' shall not
            ;; signal an error when DIR exists already.
            (make-directory tmp-name2 'parents))
@@ -2794,6 +2804,20 @@ This tests also `file-directory-p' and 
`file-accessible-directory-p'."
        ;; Cleanup.
        (ignore-errors (delete-directory tmp-name1 'recursive))))))
 
+(ert-deftest tramp-test13-make-directory ()
+  "Check `make-directory'.
+This tests also `file-directory-p' and `file-accessible-directory-p'."
+  (skip-unless (tramp--test-enabled))
+  (tramp-test-make-directory-helper nil))
+
+(ert-deftest tramp-test13-make-directory-with-file-modes ()
+  "Check that `make-directory' honors `default-file-modes'.
+This is a separate test from `tramp-test13-make-directory' so
+it can be skipped for backends that do not support file modes."
+  (skip-unless (tramp--test-enabled))
+  (skip-unless (tramp--test-supports-file-modes-p))
+  (tramp-test-make-directory-helper t))
+
 (ert-deftest tramp-test14-delete-directory ()
   "Check `delete-directory'."
   (skip-unless (tramp--test-enabled))
@@ -3590,14 +3614,7 @@ They might differ only in time attributes or directory 
size."
   "Check `file-modes'.
 This tests also `file-executable-p', `file-writable-p' and `set-file-modes'."
   (skip-unless (tramp--test-enabled))
-  (skip-unless
-   (or (tramp--test-sh-p) (tramp--test-sshfs-p) (tramp--test-sudoedit-p)
-       ;; Not all tramp-gvfs.el methods support changing the file mode.
-       (and
-       (tramp--test-gvfs-p)
-       (string-match-p
-        "ftp" (file-remote-p tramp-test-temporary-file-directory 'method)))))
-
+  (skip-unless (tramp--test-supports-file-modes-p))
   (dolist (quoted (if (tramp--test-expensive-test) '(nil t) '(nil)))
     (let ((tmp-name1 (tramp--test-make-temp-name nil quoted))
          (tmp-name2 (tramp--test-make-temp-name nil quoted)))
@@ -6172,6 +6189,17 @@ This requires restrictions of file name syntax."
 This requires restrictions of file name syntax."
   (tramp-smb-file-name-p tramp-test-temporary-file-directory))
 
+(defun tramp--test-supports-file-modes-p ()
+  "Return whether the method under test supports file modes."
+  ;; "smb" does not unless the SMB server supports "posix" extensions.
+  ;; "adb" does not unless the Android device is rooted.
+  (or (tramp--test-sh-p) (tramp--test-sshfs-p) (tramp--test-sudoedit-p)
+      ;; Not all tramp-gvfs.el methods support changing the file mode.
+      (and
+       (tramp--test-gvfs-p)
+       (string-match-p
+       "ftp" (file-remote-p tramp-test-temporary-file-directory 'method)))))
+
 (defun tramp--test-check-files (&rest files)
   "Run a simple but comprehensive test over every file in FILES."
   ;; `filename-non-special' has been fixed in Emacs 27.1, see Bug#29579.