bug#24441: 24.5; rename directory in dired to change case

[Ken Brown <kbrown@cornell.edu>]

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

On 11/11/2016 3:27 AM, Eli Zaretskii wrote:
> Thanks, please see a few comments below.
>
>> +			  (if (and (file-name-case-insensitive-p
>> +				    (expand-file-name (car fn-list)))
>
> You shouldn't need to expand-file-name here, as all primitives do that
> internally.  (Yours didn't, but that's a mistake, see below.)

Fixed.

> What about filesystems mounted on Posix hosts, like Samba, NFS-mounted
> Windows volumes, etc. -- do those behave as case-sensitive or not?  If
> the latter, can that be detected?  Just asking, this shouldn't hold
> the patch, unless incorporating that is easy (or you feel like it even
> if it isn't ;-).

I don't have an immediate idea, but I added a "FIXME" comment and will 
think about it.

>> +DEFUN ("file-name-case-insensitive-p", Ffile_name_case_insensitive_p,
>> +       Sfile_name_case_insensitive_p, 1, 1, 0,
>> +       doc: /* Return t if file FILENAME is on a case-insensitive filesystem.
>> +The arg must be a string.  */)
>> +  (Lisp_Object filename)
>> +{
>> +  CHECK_STRING (filename);
>> +  return file_name_case_insensitive_p (SSDATA (filename)) ? Qt : Qnil;
>> +}
>
> This "needs work"™.  First, it should call expand-file-name, as all
> file-related primitives do.  Second, it should see if there's a file
> handler for this operation, and if so, call it instead (Michael,
> please take note ;-).  Finally, it should run the expanded file name
> through ENCODE_FILE before it calls file_name_case_insensitive_p, and
> pass to the latter the result of the encoding, otherwise the call to
> getattrlist will most certainly fail in some cases.

Fixed.

>>  DEFUN ("rename-file", Frename_file, Srename_file, 2, 3,
>>         "fRename file: \nGRename %s to file: \np",
>>         doc: /* Rename FILE as NEWNAME.  Both args must be strings.
>> @@ -2250,12 +2301,11 @@ This is what happens in interactive use with M-x.  */)
>>    file = Fexpand_file_name (file, Qnil);
>>
>>    if ((!NILP (Ffile_directory_p (newname)))
>> -#ifdef DOS_NT
>> -      /* If the file names are identical but for the case,
>> -	 don't attempt to move directory to itself. */
>> -      && (NILP (Fstring_equal (Fdowncase (file), Fdowncase (newname))))
>> -#endif
>> -      )
>> +      /* If the filesystem is case-insensitive and the file names are
>> +	 identical but for the case, don't attempt to move directory
>> +	 to itself.  */
>> +      && (NILP (Ffile_name_case_insensitive_p (file))
>> +	  || NILP (Fstring_equal (Fdowncase (file), Fdowncase (newname)))))
>
> This should call file_name_case_insensitive_p, and pass it the encoded
> file names.  That's because the file handlers for rename-file were
> already called, so we are now sure the file doesn't have any handlers.

Actually, the handlers haven't been called yet at this point in the 
code, so I left this one alone.

>> @@ -2276,14 +2326,12 @@ This is what happens in interactive use with M-x.  */)
>>    encoded_file = ENCODE_FILE (file);
>>    encoded_newname = ENCODE_FILE (newname);
>>
>> -#ifdef DOS_NT
>> -  /* If the file names are identical but for the case, don't ask for
>> -     confirmation: they simply want to change the letter-case of the
>> -     file name.  */
>> -  if (NILP (Fstring_equal (Fdowncase (file), Fdowncase (newname))))
>> -#endif
>> -  if (NILP (ok_if_already_exists)
>> -      || INTEGERP (ok_if_already_exists))
>> +  /* If the filesystem is case-insensitive and the file names are
>> +     identical but for the case, don't ask for confirmation: they
>> +     simply want to change the letter-case of the file name.  */
>> +  if ((NILP (Ffile_name_case_insensitive_p (file))
>
> Likewise here.

Fixed.

> Finally, please provide a NEWS entry for the new primitive and its
> documentation in the ELisp manual.

Done, but I'm not sure I found the best place for it in the manual.

Thanks for the review.  Revised patch attached.

Ken


[-- Attachment #2: 0001-Check-case-sensitivity-when-renaming-files.patch --]
[-- Type: text/plain, Size: 9023 bytes --]

From 209e47df634bcbc7e4239b9c7f7b62f17419edbf Mon Sep 17 00:00:00 2001
From: Ken Brown <kbrown@cornell.edu>
Date: Sat, 22 Oct 2016 19:10:18 -0400
Subject: [PATCH] Check case-sensitivity when renaming files

* src/fileio.c (file_name_case_insensitive_p)
(Ffile_name_case_insensitive_p):  New functions.
(Frename_file): Allow renames that simply change case when the
FILE argument is on a case-insensitive filesystem.  (Bug#24441)

* lisp/dired-aux.el (dired-do-create-files): Use
'file-name-case-insensitive-p' instead of 'system-type' to check
for case-insensitivity.  (Bug#24441)

* doc/lispref/files.texi (File Attributes): Document
'file-name-case-insensitive-p'.
---
 doc/lispref/files.texi | 14 ++++++++
 etc/NEWS               |  4 +++
 lisp/dired-aux.el      | 13 +++----
 src/fileio.c           | 97 ++++++++++++++++++++++++++++++++++++++++++--------
 4 files changed, 108 insertions(+), 20 deletions(-)

diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
index 9af5ce9..037f84f 100644
--- a/doc/lispref/files.texi
+++ b/doc/lispref/files.texi
@@ -1369,6 +1369,20 @@ File Attributes
 @end example
 @end defun
 
+@defun file-name-case-insensitive-p filename
+This function returns @code{t} if file @var{filename} is on a
+case-insensitive filesystem.  This is always the case on MS-DOS and
+MS-Windows.  On Cygwin and Mac OS X, filesystems may or may not be
+case-insensitive, and the function tries to determine case-sensitivity
+by a runtime test.  If the test is inconclusive, the function returns
+@code{t} on Cygwin and @code{nil} on Mac OS X.
+
+This function always returns @code{nil} on platforms other than
+MS-DOS, MS-Windows, Cygwin, and Mac OS X.  It does not currently
+detect case-insensitivity of mounted filesystems, such as Samba shares
+or NFS-mounted Windows volumes.
+@end defun
+
 @node Extended Attributes
 @subsection Extended File Attributes
 @cindex extended file attributes
diff --git a/etc/NEWS b/etc/NEWS
index 14450a6..4aea604 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -61,6 +61,10 @@ affected by this, as SGI stopped supporting IRIX in December 2013.
 \f
 * Changes in Emacs 26.1
 
++++
+** The new function 'file-name-case-insensitive-p' tests whether a
+given file is on a case-insensitive filesystem.
+
 ---
 The group 'wp', whose label was "text", is now deprecated.
 Use the new group 'text', which inherits from 'wp', instead.
diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index d25352e..64ae4a9 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -1799,13 +1799,14 @@ dired-do-create-files
 		     (concat (if dired-one-file op1 operation) " %s to: ")
 		     target-dir op-symbol arg rfn-list default))))
 	 (into-dir (cond ((null how-to)
-			  ;; Allow DOS/Windows users to change the letter
-			  ;; case of a directory.  If we don't test these
-			  ;; conditions up front, file-directory-p below
-			  ;; will return t because the filesystem is
-			  ;; case-insensitive, and Emacs will try to move
+			  ;; Allow users to change the letter case of
+			  ;; a directory on a case-insensitive
+			  ;; filesystem.  If we don't test these
+			  ;; conditions up front, file-directory-p
+			  ;; below will return t on a case-insensitive
+			  ;; filesystem, and Emacs will try to move
 			  ;; foo -> foo/foo, which fails.
-			  (if (and (memq system-type '(ms-dos windows-nt cygwin))
+			  (if (and (file-name-case-insensitive-p (car fn-list))
 				   (eq op-symbol 'move)
 				   dired-one-file
 				   (string= (downcase
diff --git a/src/fileio.c b/src/fileio.c
index 6026d8e..c26be31 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -25,6 +25,10 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include <sys/stat.h>
 #include <unistd.h>
 
+#ifdef DARWIN_OS
+#include <sys/attr.h>
+#endif
+
 #ifdef HAVE_PWD_H
 #include <pwd.h>
 #endif
@@ -2231,6 +2235,72 @@ internal_delete_file (Lisp_Object filename)
   return NILP (tem);
 }
 \f
+/* Filesystems are case-sensitive on all supported systems except
+   MS-Windows, MS-DOS, Cygwin, and Mac OS X.  They are always
+   case-insensitive on the first two, but they may or may not be
+   case-insensitive on Cygwin and OS X.  The following function
+   attempts to provide a runtime test on those two systems.  If the
+   test is not conclusive, we assume case-insensitivity on Cygwin and
+   case-sensitivity on Mac OS X.
+
+   FIXME: Mounted filesystems on Posix hosts, like Samba shares or
+   NFS-mounted Windows volumes, might be case-insensitive.  Can we
+   detect this?  */
+
+static bool
+file_name_case_insensitive_p (const char *filename)
+{
+#ifdef DOS_NT
+  return 1;
+#elif defined CYGWIN
+/* As of Cygwin-2.6.1, pathconf supports _PC_CASE_INSENSITIVE.  */
+# ifdef _PC_CASE_INSENSITIVE
+  int res = pathconf (filename, _PC_CASE_INSENSITIVE);
+  if (res < 0)
+    return 1;
+  return res > 0;
+# else
+  return 1;
+# endif
+#elif defined DARWIN_OS
+  /* The following is based on
+     http://lists.apple.com/archives/darwin-dev/2007/Apr/msg00010.html.  */
+  struct attrlist alist;
+  unsigned char buffer[sizeof (vol_capabilities_attr_t) + sizeof (size_t)];
+
+  memset (&alist, 0, sizeof (alist));
+  alist.volattr = ATTR_VOL_CAPABILITIES;
+  if (getattrlist (filename, &alist, buffer, sizeof (buffer), 0)
+      || !(alist.volattr & ATTR_VOL_CAPABILITIES))
+    return 0;
+  vol_capabilities_attr_t *vcaps = buffer;
+  return !(vcaps->capabilities[0] & VOL_CAP_FMT_CASE_SENSITIVE);
+#else
+  return 0;
+#endif
+}
+
+DEFUN ("file-name-case-insensitive-p", Ffile_name_case_insensitive_p,
+       Sfile_name_case_insensitive_p, 1, 1, 0,
+       doc: /* Return t if file FILENAME is on a case-insensitive filesystem.
+The arg must be a string.  */)
+  (Lisp_Object filename)
+{
+  Lisp_Object handler;
+
+  CHECK_STRING (filename);
+  filename = Fexpand_file_name (filename, Qnil);
+
+  /* If the file name has special constructs in it,
+     call the corresponding file handler.  */
+  handler = Ffind_file_name_handler (filename, Qfile_name_case_insensitive_p);
+  if (!NILP (handler))
+    return call2 (handler, Qfile_name_case_insensitive_p, filename);
+
+  filename = ENCODE_FILE (filename);
+  return file_name_case_insensitive_p (SSDATA (filename)) ? Qt : Qnil;
+}
+
 DEFUN ("rename-file", Frename_file, Srename_file, 2, 3,
        "fRename file: \nGRename %s to file: \np",
        doc: /* Rename FILE as NEWNAME.  Both args must be strings.
@@ -2250,12 +2320,11 @@ This is what happens in interactive use with M-x.  */)
   file = Fexpand_file_name (file, Qnil);
 
   if ((!NILP (Ffile_directory_p (newname)))
-#ifdef DOS_NT
-      /* If the file names are identical but for the case,
-	 don't attempt to move directory to itself. */
-      && (NILP (Fstring_equal (Fdowncase (file), Fdowncase (newname))))
-#endif
-      )
+      /* If the filesystem is case-insensitive and the file names are
+	 identical but for the case, don't attempt to move directory
+	 to itself.  */
+      && (NILP (Ffile_name_case_insensitive_p (file))
+	  || NILP (Fstring_equal (Fdowncase (file), Fdowncase (newname)))))
     {
       Lisp_Object fname = (NILP (Ffile_directory_p (file))
 			   ? file : Fdirectory_file_name (file));
@@ -2276,14 +2345,12 @@ This is what happens in interactive use with M-x.  */)
   encoded_file = ENCODE_FILE (file);
   encoded_newname = ENCODE_FILE (newname);
 
-#ifdef DOS_NT
-  /* If the file names are identical but for the case, don't ask for
-     confirmation: they simply want to change the letter-case of the
-     file name.  */
-  if (NILP (Fstring_equal (Fdowncase (file), Fdowncase (newname))))
-#endif
-  if (NILP (ok_if_already_exists)
-      || INTEGERP (ok_if_already_exists))
+  /* If the filesystem is case-insensitive and the file names are
+     identical but for the case, don't ask for confirmation: they
+     simply want to change the letter-case of the file name.  */
+  if ((!(file_name_case_insensitive_p (SSDATA (encoded_file)))
+       || NILP (Fstring_equal (Fdowncase (file), Fdowncase (newname))))
+      && ((NILP (ok_if_already_exists) || INTEGERP (ok_if_already_exists))))
     barf_or_query_if_file_exists (newname, false, "rename to it",
 				  INTEGERP (ok_if_already_exists), false);
   if (rename (SSDATA (encoded_file), SSDATA (encoded_newname)) < 0)
@@ -5836,6 +5903,7 @@ syms_of_fileio (void)
   DEFSYM (Qmake_directory_internal, "make-directory-internal");
   DEFSYM (Qmake_directory, "make-directory");
   DEFSYM (Qdelete_file, "delete-file");
+  DEFSYM (Qfile_name_case_insensitive_p, "file-name-case-insensitive-p");
   DEFSYM (Qrename_file, "rename-file");
   DEFSYM (Qadd_name_to_file, "add-name-to-file");
   DEFSYM (Qmake_symbolic_link, "make-symbolic-link");
@@ -6093,6 +6161,7 @@ This includes interactive calls to `delete-file' and
   defsubr (&Smake_directory_internal);
   defsubr (&Sdelete_directory_internal);
   defsubr (&Sdelete_file);
+  defsubr (&Sfile_name_case_insensitive_p);
   defsubr (&Srename_file);
   defsubr (&Sadd_name_to_file);
   defsubr (&Smake_symbolic_link);
-- 
2.8.3