GNU bug report logs - #50410
Tramp does not honor default file modes in make-directory

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 50410 in the body.
You can then email your comments to 50410 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stephen Gildea <stepheng+emacs <at> gildea.com>
To: submit <at> debbugs.gnu.org
Subject: Tramp does not honor default file modes in make-directory
Date: Sun, 05 Sep 2021 11:17:15 -0700

[Message part 1 (text/plain, inline)]

Package: emacs
Version: 28.0.50
Tags: patch
X-Debbugs-CC: michael.albinus <at> gmx.de

[tramp-make-directory-honor-default-file-modes.patch (text/x-diff, inline)]

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.
* test/lisp/net/tramp-tests.el (tramp-test13-make-directory-with-file-modes):
New test.

diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
index a2bf0afbf5..e57145e8e7 100644
--- a/lisp/net/tramp-sh.el
+++ b/lisp/net/tramp-sh.el
@@ -2449,8 +2449,9 @@ tramp-sh-handle-make-directory
     (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-adb.el b/lisp/net/tramp-adb.el
index 70dbfdb947..a35ac37a20 100644
--- a/lisp/net/tramp-adb.el
+++ b/lisp/net/tramp-adb.el
@@ -442,7 +442,9 @@ tramp-adb-handle-make-directory
 	  (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-sudoedit.el b/lisp/net/tramp-sudoedit.el
index 5895f1d25b..051d145c2a 100644
--- a/lisp/net/tramp-sudoedit.el
+++ b/lisp/net/tramp-sudoedit.el
@@ -597,6 +597,7 @@ tramp-sudoedit-handle-make-directory
      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/lisp/net/tramp-gvfs.el b/lisp/net/tramp-gvfs.el
index e4f54cf4c4..eb889bb4f2 100644
--- a/lisp/net/tramp-gvfs.el
+++ b/lisp/net/tramp-gvfs.el
@@ -1574,10 +1574,13 @@ tramp-gvfs-handle-make-directory
 	(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/test/lisp/net/tramp-tests.el b/test/lisp/net/tramp-tests.el
index 9a9684dd73..249ee9e94d 100644
--- a/test/lisp/net/tramp-tests.el
+++ b/test/lisp/net/tramp-tests.el
@@ -2765,28 +2765,38 @@ tramp-test12-rename-file
 	    (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,21 @@ tramp-test13-make-directory
 	;; 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' because
+some backends cannot pass this test.  The \"smb\" backend fails
+unless the SMB server supports \"posix\" extensions.
+The \"adb\" backend fails on the /sdcard filesystem."
+  (skip-unless (tramp--test-enabled))
+  (tramp-test-make-directory-helper t))
+
 (ert-deftest tramp-test14-delete-directory ()
   "Check `delete-directory'."
   (skip-unless (tramp--test-enabled))

Message #8 received at 50410 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Stephen Gildea <stepheng+emacs <at> gildea.com>
Cc: 50410 <at> debbugs.gnu.org
Subject: Re: bug#50410: Tramp does not honor default file modes in
 make-directory
Date: Mon, 06 Sep 2021 15:39:27 +0200

Stephen Gildea <stepheng+emacs <at> gildea.com> writes:

Hi Stephen,

> Tramp: honor default file modes in make-directory

Thanks for this. As discussed privately, I believe it is a good
extension to Tramp.

I gave it a short try, and it works as expected for the "mock" test
method. Please push to the master branch; perhaps you could also add a
short NEWS entry.

Next days I let run the test suite for the other Tramp methods.

Best regards, Michael.

Message #11 received at 50410 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Stephen Gildea <stepheng+emacs <at> gildea.com>
Cc: 50410 <at> debbugs.gnu.org
Subject: Re: bug#50410: Tramp does not honor default file modes in
 make-directory
Date: Wed, 08 Sep 2021 18:59:02 +0200

Michael Albinus <michael.albinus <at> gmx.de> writes:

Hi Stephen,
>
>> Tramp: honor default file modes in make-directory
>
> Thanks for this. As discussed privately, I believe it is a good
> extension to Tramp.
>
> I gave it a short try, and it works as expected for the "mock" test
> method. Please push to the master branch; perhaps you could also add a
> short NEWS entry.
>
> Next days I let run the test suite for the other Tramp methods.

The results are good. In tramp-test13-make-directory-with-file-modes
I have applied the same rules like in tramp-test20-file-modes, it looks now

--8<---------------cut here---------------start------------->8---
(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' because
some backends cannot pass this test."
  (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)))))
  (tramp-test-make-directory-helper t))
--8<---------------cut here---------------end--------------->8---

In tramp-adb.el I believe it would work as well, if the Android device
is rooted. But this we don't support.

From my POV, you could push it to master.

Best regards, Michael.

Message #16 received at 50410-done <at> debbugs.gnu.org (full text, mbox):

From: Stephen Gildea <stepheng+emacs <at> gildea.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 50410-done <at> debbugs.gnu.org
Subject: Re: bug#50410: Tramp does not honor default file modes in
 make-directory
Date: Fri, 10 Sep 2021 06:51:01 -0700

Version: 28.1

>   Michael Albinus <michael.albinus <at> gmx.de> writes:
>   
>   From my POV, you could push it to master.

Done, commit 5ee6583cb2

Message #19 received at 50410 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Stephen Gildea <stepheng+emacs <at> gildea.com>
Cc: 50410 <at> debbugs.gnu.org
Subject: Re: bug#50410: Tramp does not honor default file modes in
 make-directory
Date: Fri, 10 Sep 2021 18:55:36 +0200

Stephen Gildea <stepheng+emacs <at> gildea.com> writes:

Hi Stephen,

>>   From my POV, you could push it to master.
>
> Done, commit 5ee6583cb2

Thanks!

Now, that we have `tramp--test-supports-file-modes-p', we can go back to
just one test `tramp-test13-make-directory' I believe. I'll play a
little bit with this, and let run further Tramp tests. We'll see.

Best regards, Michael.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.