bug#28451: 26.0.50; copy-directory no longer creates parent directories of target if they do not exist

[Andrew Christianson]

Ah.  I didn’t fully specify my test case, sorry about that.  In reality I did 
mkdir ~/test; touch ~/test/{a,b,c,d} .

Looks like that’s the key though.  If the first file in the directory to be 
copied is a normal file, not a directory, the call fails. So

mkdir ~/test
touch ~/test/{a,b,c,d}
path/to/emacs -Q -batch -eval '(copy-directory "~/test" "~/a/new/directory/" t 
t t)’

will fail, but

mkdir -p ~/test2/a
touch ~/test2/{b,c,d}
touch ~/test2/a/e
path/to/emacs -Q -batch -eval '(copy-directory "~/test2" 
"~/another/new/directory/" t t t)’

will succeed.

I've replicated the former behavior on Fedora 26, on a clean build of the most 
recent master (bc511a64f6). M-x report-emacs-bug template for that system is 
attached, along with the strace output.  There are no references to mkdir, 
unfortunately.  It seems like the issue is that copy-directory never actually 
calls make-directory in this case.  If neither condition in the middle cond 
block (list/files.el#5543) applies (which seems to be why this only happens 
with a NEWNAME with a trailing slash, and non-nil COPY-CONTENTS) then 
copy-directory just proceeds to the dolist, and if the first item is a file, it 
goes straight to copy-file, which then fails, as the target directory doesn’t 
exist.

Looks like the issue may have arose in commit 
e22794867d878d53675fcc91d2ef1ad2494a2ff2, trading file-directory-p for 
directory-name-p in the first first condition in that cond block.

Would adding a condition like:

1 file changed, 3 insertions(+), 1 deletion(-)
lisp/files.el | 4 +++-

modified   lisp/files.el
@@ -5541,31 +5541,33 @@ into NEWNAME instead."
            newname (expand-file-name newname))
 
       (cond ((not (directory-name-p newname))
             ;; If NEWNAME is not a directory name, create it;
             ;; that is where we will copy the files of DIRECTORY.
             (make-directory newname parents))
            ;; If NEWNAME is a directory name and COPY-CONTENTS
            ;; is nil, copy into NEWNAME/[DIRECTORY-BASENAME].
            ((not copy-contents)
             (setq newname (concat newname
                            (file-name-nondirectory directory)))
             (and (file-exists-p newname)
                  (not (file-directory-p newname))
                  (error "Cannot overwrite non-directory %s with a directory"
                         newname))
-            (make-directory newname t)))
+            (make-directory newname t))
+            ((and copy-contents (not (file-directory-p newname)))
+             (make-directory newname parents)))
 
       ;; Copy recursively.

Impact the current security concern?

Andrew

*message*-20170913-204729
Description: Binary data

cd-test-trace
Description: Binary data

> On Sep 13, 2017, at 6:22 PM, Paul Eggert <eggert@cs.ucla.edu> wrote:
> 
> Thanks for the bug report and test case. I cannot reproduce the problem on 
> Fedora 26 x86-64.
> 
> What are the permissions on the files and directories involved? E.g., 'cd; ls 
> -lR a test'.
> 
> I have a sneaking suspicion that the problem lies in the recent changes I 
> made to make-directory (commit cf9891e14e48a93bca2065fdd7998f5f677786dc). Can 
> you please try something like this:
> 
> cd
> rm -fr a test
> mkdir -p a test/{a,b,c,d}
> strace -o tr path/to/emacs -Q -batch -eval '(copy-directory "~/test" 
> "~/a/new/directory/" t t t)'
> grep mkdir tr
> 
> Here's what I observe on Fedora:
> 
> mkdir("/home/eggert/a/new/directory/a", 0777) = -1 ENOENT (No such file or 
> directory)
> mkdir("/home/eggert/a/new/directory", 0777) = -1 ENOENT (No such file or 
> directory)
> mkdir("/home/eggert/a/new", 0777)       = 0
> mkdir("/home/eggert/a/new/directory", 0777) = 0
> mkdir("/home/eggert/a/new/directory/a", 0777) = 0
> mkdir("/home/eggert/a/new/directory/b", 0777) = 0
> mkdir("/home/eggert/a/new/directory/c", 0777) = 0
> mkdir("/home/eggert/a/new/directory/d", 0777) = 0
> 
> which has the desired behavior. If Darwin doesn't have strace, please use the 
> equivalent there to trace system calls
> 
> If you don't have an strace equivalent, please try make-directory and see 
> whether it has a similar problem:
> 
> cd
> rm -fr a
> mkdir a
> path/to/emacs -Q -batch -eval '(make-directory "a/new/directory/a" t)'
> ls -al a/new/directory/a
>