unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Paul Eggert <eggert@cs.ucla.edu>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 28483@debbugs.gnu.org, Adam Plaice <plaice.adam+lists@gmail.com>,
	28520-done@debbugs.gnu.org, aaronjensen@gmail.com,
	Tino Calancha <tino.calancha@gmail.com>
Subject: bug#28520: bug#28483: 26.0.50; copy-directory does not create directories
Date: Wed, 20 Sep 2017 11:59:37 -0700	[thread overview]
Message-ID: <be3fa356-6f9a-e470-2bb8-c9a2a0bc0095@cs.ucla.edu> (raw)
In-Reply-To: <83mv5pfw5b.fsf@gnu.org>

[-- Attachment #1: Type: text/plain, Size: 1007 bytes --]

On 09/20/2017 04:26 AM, Eli Zaretskii wrote:
> I think the problem is in this line:
>
> 	  (cond
> 	   ((eq filetype t)       ; Directory but not a symlink.
> 	    (copy-directory file newname keep-time parents)) <<<<<<<<<<<
>
> Instead of 'newname', there should be (file-name-as-directory newname)
> there.  I think.

Thanks for diagnosing the problem and for the fix. That fix should work, 
and I think we can improve on it slightly by using (copy-directory file 
target keep-time parents t), as this makes the cond branch more parallel 
with the other alternatives and avoids a call to file-name-as-directory. 
So I installed the attached patch into the emacs-26 branch to do that, 
and to add a test case for this bug. This patch should also fix 
Bug#28520 "Dired recursive copy of directory fails", so I'll CC: that 
bug report and boldly close it.

PS. Sorry, Adam, for misspelling your first name in the commit message. 
I'll try to remember to fix that when it spills out into the ChangeLog file.


[-- Attachment #2: 0001-Fix-new-copy-directory-bug-with-empty-dirs.patch --]
[-- Type: text/x-patch, Size: 2116 bytes --]

From b4531a78ded7efb0c133763b5efe1f4dab1aa4de Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 20 Sep 2017 11:49:12 -0700
Subject: [PATCH] Fix new copy-directory bug with empty dirs

Problem reported by Afdam Plaice (Bug#28520) and by Eli Zaretskii
(Bug#28483#34).  This is another bug that I introduced in my
recent copy-directory changes.
* lisp/files.el (copy-directory): Work with empty subdirectories, too.
* test/lisp/files-tests.el (files-tests--copy-directory):
Test for this bug.
---
 lisp/files.el            | 2 +-
 test/lisp/files-tests.el | 7 ++++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/lisp/files.el b/lisp/files.el
index 0c30d40c13..f0a1f2380d 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -5564,7 +5564,7 @@ copy-directory
 	      (filetype (car (file-attributes file))))
 	  (cond
 	   ((eq filetype t)       ; Directory but not a symlink.
-	    (copy-directory file newname keep-time parents))
+	    (copy-directory file target keep-time parents t))
 	   ((stringp filetype)    ; Symbolic link
 	    (make-symbolic-link filetype target t))
 	   ((copy-file file target t keep-time)))))
diff --git a/test/lisp/files-tests.el b/test/lisp/files-tests.el
index f2a9a32180..285a884b69 100644
--- a/test/lisp/files-tests.el
+++ b/test/lisp/files-tests.el
@@ -399,11 +399,16 @@ files-tests--with-temp-file
 	 (dirname (file-name-as-directory dir))
 	 (source (concat dirname "source"))
 	 (dest (concat dirname "dest/new/directory/"))
-	 (file (concat (file-name-as-directory source) "file")))
+	 (file (concat (file-name-as-directory source) "file"))
+	 (source2 (concat dirname "source2"))
+	 (dest2 (concat dirname "dest/new2")))
     (make-directory source)
     (write-region "" nil file)
     (copy-directory source dest t t t)
     (should (file-exists-p (concat dest "file")))
+    (make-directory (concat (file-name-as-directory source2) "a") t)
+    (copy-directory source2 dest2)
+    (should (file-directory-p (concat (file-name-as-directory dest2) "a")))
     (delete-directory dir 'recursive)))
 
 (provide 'files-tests)
-- 
2.13.5


  reply	other threads:[~2017-09-20 18:59 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-17 19:10 bug#28483: 26.0.50; copy-directory does not create directories Aaron Jensen
2017-09-17 19:15 ` bug#28483: [PATCH] Fix copy-directory creating missing directory Aaron Jensen
2017-09-17 19:48   ` Philipp Stephani
2017-09-17 20:19     ` Aaron Jensen
2017-09-18 17:40       ` Philipp Stephani
2017-09-17 19:20 ` bug#28483: 26.0.50; copy-directory does not create directories Aaron Jensen
2017-09-17 20:40   ` Paul Eggert
2017-09-17 23:12     ` Aaron Jensen
2017-09-17 23:52       ` Paul Eggert
2017-09-20 11:26     ` Eli Zaretskii
2017-09-20 18:59       ` Paul Eggert [this message]
2017-09-20 19:54         ` bug#28520: " Eli Zaretskii
2017-09-20 23:04           ` Adam Plaice

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=be3fa356-6f9a-e470-2bb8-c9a2a0bc0095@cs.ucla.edu \
    --to=eggert@cs.ucla.edu \
    --cc=28483@debbugs.gnu.org \
    --cc=28520-done@debbugs.gnu.org \
    --cc=aaronjensen@gmail.com \
    --cc=eliz@gnu.org \
    --cc=plaice.adam+lists@gmail.com \
    --cc=tino.calancha@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).