Fix 'expand-file-name' for remote files

This reverts most of commit 14fb657ba82da346d36f05f88da26f1c5498b798
and its followup fixes, and instead fixes the original bugs in a
different manner that doesn't affect any unrelated use cases.  As
part of this, the code which caused 'expand-file-name' to enforce
a trailing slash on expanded directories is removed, as this kind
of semantic processing is outside of 'expand-file-name's scope.
* src/fileio.c (Fexpand_file_name): If expanding default_directory
yields a remote file name, call its handlers.  (Bug#26911)
(Bug#34834)

* doc/lispref/files.texi (File Name Expansion): Remove the
requirement that expanding a directory name yields a directory
name, i.e. that the expansion must end in a slash.

* etc/NEWS: Remove the announcement of the changed behavior of
'expand-file-name' wrt trailing slashes.

* test/src/fileio-tests.el (fileio-tests--HOME-trailing-slash)
(fileio-tests--expand-file-name-trailing-slash): Remove tests.
* test/lisp/net/tramp-tests.el (tramp-test05-expand-file-name): No
need to expect different results in Emacs 28 and later.

diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
index 090c54f8cd9861a870fe8d442471b61b55350495..92cbc2a1c9195905e7424b31e7b08f57787986f5 100644 (file)
--- a/doc/lispref/files.texi
+++ b/doc/lispref/files.texi
@@ -2438,26 +2438,14 @@ This is for the sake of filesystems that have the concept of a
 superroot above the root directory @file{/}.  On other filesystems,
 @file{/../} is interpreted exactly the same as @file{/}.
 
-If a filename must be that of a directory, its expansion must be too.
-For example, if a filename ends in @samp{/} or @samp{/.} or @samp{/..}
-then its expansion ends in @samp{/} so that it cannot be
-misinterpreted as the name of a symbolic link:
-
-@example
-@group
-(expand-file-name "/a///b//.")
-     @result{} "/a/b/"
-@end group
-@end example
-
 Expanding @file{.} or the empty string returns the default directory:
 
 @example
 @group
 (expand-file-name "." "/usr/spool/")
-     @result{} "/usr/spool/"
+     @result{} "/usr/spool"
 (expand-file-name "" "/usr/spool/")
-     @result{} "/usr/spool/"
+     @result{} "/usr/spool"
 @end group
 @end example
 

diff --git a/etc/NEWS b/etc/NEWS
index 1cb1b7ee4fd1135061e9a1f19e540b9a65c9c9d8..38cfeaee9b801e62e6e033d8e028f183024cee7a 100644 (file)
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1181,12 +1181,6 @@ region's (or buffer's) end.
 This function can be used by modes to add elements to the
 'choice' customization type of a variable.
 
-+++
-** 'expand-file-name' no longer omits a trailing slash if the omission
-changes the filename's meaning.  E.g., (expand-file-name "/a/b/.") now
-returns "/a/b/" not "/a/b", which might be misinterpreted as the name
-of a symbolic link rather than of the directory it points to.
-
 +++
 ** New function 'file-modes-number-to-symbolic' to convert a numeric
 file mode specification into symbolic form.

diff --git a/src/fileio.c b/src/fileio.c
index c91af36fdf661fa569095c50f6d00117d82873fa..1e4ca82e5f3f4b787c97b0cf82a14bc8eec9ac9d 100644 (file)
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -827,9 +827,9 @@ the root directory.  */)
   ptrdiff_t tlen;
 #ifdef DOS_NT
   int drive = 0;
+  bool collapse_newdir = true;
   bool is_escaped = 0;
 #endif /* DOS_NT */
-  bool collapse_newdir = true;
   ptrdiff_t length, nbytes;
   Lisp_Object handler, result, handled_name;
   bool multibyte;
@@ -947,6 +947,22 @@ the root directory.  */)
        )
       {
        default_directory = Fexpand_file_name (default_directory, Qnil);
+
+       /* The above expansion might have produced a remote file name,
+          so give the handlers one last chance to DTRT.  This can
+          happen when both NAME and DEFAULT-DIRECTORY arguments are
+          relative file names, and the buffer's default-directory is
+          remote.  */
+       handler = Ffind_file_name_handler (default_directory,
+                                          Qexpand_file_name);
+       if (!NILP (handler))
+         {
+           handled_name = call3 (handler, Qexpand_file_name,
+                                 name, default_directory);
+           if (STRINGP (handled_name))
+             return handled_name;
+           error ("Invalid handler in `file-name-handler-alist'");
+         }
       }
   }
   multibyte = STRING_MULTIBYTE (name);
@@ -1065,7 +1081,7 @@ the root directory.  */)
 #endif /* WINDOWSNT */
 #endif /* DOS_NT */
 
-  /* If nm is absolute, look for "/./" or "/../" or "//" sequences; if
+  /* If nm is absolute, look for `/./' or `/../' or `//''sequences; if
      none are found, we can probably return right away.  We will avoid
      allocating a new string if name is already fully expanded.  */
   if (
@@ -1183,7 +1199,9 @@ the root directory.  */)
              newdir = SSDATA (hdir);
              newdirlim = newdir + SBYTES (hdir);
            }
+#ifdef DOS_NT
          collapse_newdir = false;
+#endif
        }
       else                     /* ~user/filename */
        {
@@ -1203,7 +1221,9 @@ the root directory.  */)
 
              while (*++nm && !IS_DIRECTORY_SEP (*nm))
                continue;
+#ifdef DOS_NT
              collapse_newdir = false;
+#endif
            }
 
          /* If we don't find a user of that name, leave the name
@@ -1370,15 +1390,12 @@ the root directory.  */)
     }
 #endif /* DOS_NT */
 
-  length = newdirlim - newdir;
-
-#ifdef DOS_NT
   /* Ignore any slash at the end of newdir, unless newdir is
      just "/" or "//".  */
+  length = newdirlim - newdir;
   while (length > 1 && IS_DIRECTORY_SEP (newdir[length - 1])
         && ! (length == 2 && IS_DIRECTORY_SEP (newdir[0])))
     length--;
-#endif
 
   /* Now concatenate the directory and name to new space in the stack frame.  */
   tlen = length + file_name_as_directory_slop + (nmlim - nm) + 1;
@@ -1392,16 +1409,12 @@ the root directory.  */)
 #else  /* not DOS_NT */
   target = SAFE_ALLOCA (tlen);
 #endif /* not DOS_NT */
+  *target = 0;
   nbytes = 0;
 
   if (newdir)
     {
-#ifndef DOS_NT
-      bool treat_as_absolute = !collapse_newdir;
-#else
-      bool treat_as_absolute = !nm[0] || IS_DIRECTORY_SEP (nm[0]);
-#endif
-      if (treat_as_absolute)
+      if (nm[0] == 0 || IS_DIRECTORY_SEP (nm[0]))
        {
 #ifdef DOS_NT
          /* If newdir is effectively "C:/", then the drive letter will have
@@ -1413,23 +1426,13 @@ the root directory.  */)
                && newdir[1] == '\0'))
 #endif
            {
-             /* With ~ or ~user, leave NEWDIR as-is to avoid transforming
-                it from a symlink (or a regular file!) into a directory.  */
              memcpy (target, newdir, length);
+             target[length] = 0;
              nbytes = length;
            }
        }
       else
        nbytes = file_name_as_directory (target, newdir, length, multibyte);
-
-#ifndef DOS_NT
-      /* If TARGET ends in a directory separator, omit leading
-        directory separators from NM so that concatenating a TARGET "/"
-        to an NM "/foo" does not result in the incorrect "//foo".  */
-      if (nbytes && IS_DIRECTORY_SEP (target[nbytes - 1]))
-       while (IS_DIRECTORY_SEP (nm[0]))
-         nm++;
-#endif
     }
 
   memcpy (target + nbytes, nm, nmlim - nm + 1);
@@ -1446,20 +1449,6 @@ the root directory.  */)
          {
            *o++ = *p++;
          }
-#ifndef DOS_NT
-       else if (p[1] == '.' && IS_DIRECTORY_SEP (p[2]))
-         {
-           /* Replace "/./" with "/".  */
-           p += 2;
-         }
-       else if (p[1] == '.' && !p[2])
-         {
-           /* At the end of the file name, replace "/." with "/".
-              The trailing "/" is for symlinks.  */
-           *o++ = *p;
-           p += 2;
-         }
-#else
        else if (p[1] == '.'
                 && (IS_DIRECTORY_SEP (p[2])
                     || p[2] == 0))
@@ -1470,7 +1459,6 @@ the root directory.  */)
              *o++ = *p;
            p += 2;
          }
-#endif
        else if (p[1] == '.' && p[2] == '.'
                 /* `/../' is the "superroot" on certain file systems.
                    Turned off on DOS_NT systems because they have no
@@ -1484,35 +1472,21 @@ the root directory.  */)
 #endif
                 && (IS_DIRECTORY_SEP (p[3]) || p[3] == 0))
          {
-#ifndef DOS_NT
-           while (o != target)
-             {
-               o--;
-               if (IS_DIRECTORY_SEP (*o))
-                 {
-                   /* Keep "/" at the end of the name, for symlinks.  */
-                   o += p[3] == 0;
-
-                   break;
-                 }
-             }
-#else
-# ifdef WINDOWSNT
+#ifdef WINDOWSNT
            char *prev_o = o;
-# endif
+#endif
            while (o != target && (--o, !IS_DIRECTORY_SEP (*o)))
              continue;
-# ifdef WINDOWSNT
+#ifdef WINDOWSNT
            /* Don't go below server level in UNC filenames.  */
            if (o == target + 1 && IS_DIRECTORY_SEP (*o)
                && IS_DIRECTORY_SEP (*target))
              o = prev_o;
            else
-# endif
+#endif
            /* Keep initial / only if this is the whole name.  */
            if (o == target && IS_ANY_SEP (*o) && p[3] == 0)
              ++o;
-#endif
            p += 3;
          }
        else if (IS_DIRECTORY_SEP (p[1])

diff --git a/test/lisp/net/tramp-tests.el b/test/lisp/net/tramp-tests.el
index 71c6302e0ee3b81b9ffb5f3fa3222c5594ed16aa..c170d2c6bf0a75c89c79fad595530dce782e54fc 100644 (file)
--- a/test/lisp/net/tramp-tests.el
+++ b/test/lisp/net/tramp-tests.el
@@ -2139,19 +2139,16 @@ is greater than 10.
       (expand-file-name "/method:host:/path/../file") "/method:host:/file"))
     (should
      (string-equal
-      (expand-file-name "/method:host:/path/.")
-      (if (tramp--test-emacs28-p) "/method:host:/path/" "/method:host:/path")))
+      (expand-file-name "/method:host:/path/.") "/method:host:/path"))
     (should
      (string-equal
       (expand-file-name "/method:host:/path/..") "/method:host:/"))
     (should
      (string-equal
-      (expand-file-name "." "/method:host:/path/")
-      (if (tramp--test-emacs28-p) "/method:host:/path/" "/method:host:/path")))
+      (expand-file-name "." "/method:host:/path/") "/method:host:/path"))
     (should
      (string-equal
-      (expand-file-name "" "/method:host:/path/")
-      (if (tramp--test-emacs28-p) "/method:host:/path/" "/method:host:/path")))
+      (expand-file-name "" "/method:host:/path/") "/method:host:/path"))
     ;; Quoting local part.
     (should
      (string-equal

diff --git a/test/src/fileio-tests.el b/test/src/fileio-tests.el
index bedda83bbdb6df3128389e0673626a598aa9b426..ed381d151eeab8c3ac00de252fa6f71567d1a14f 100644 (file)
--- a/test/src/fileio-tests.el
+++ b/test/src/fileio-tests.el
@@ -107,43 +107,6 @@ Also check that an encoding error can appear in a symlink."
       (setenv "HOME" "x:foo")
       (should (equal (expand-file-name "~/bar") "x:/foo/bar")))))
 
-(ert-deftest fileio-tests--HOME-trailing-slash ()
-  "Test that expand-file-name of \"~\" respects trailing slash."
-  :expected-result (if (memq system-type '(windows-nt ms-dos))
-                       :failed
-                     :passed)
-  (let ((process-environment (copy-sequence process-environment)))
-    (dolist (home
-             (if (memq system-type '(windows-nt ms-dos))
-                 '("c:/a/b/c" "c:/a/b/c/")
-               '("/a/b/c" "/a/b/c/")))
-      (setenv "HOME" home)
-      (should (equal (expand-file-name "~") (expand-file-name home))))))
-
-(ert-deftest fileio-tests--expand-file-name-trailing-slash ()
-  (dolist (fooslashalias '("foo/" "foo//" "foo/." "foo//." "foo///././."
-                           "foo/a/.."))
-    (if (memq system-type '(windows-nt ms-dos))
-        (progn
-          (should (equal (expand-file-name fooslashalias "c:/") "c:/foo/"))
-          (should (equal (expand-file-name (concat "c:/" fooslashalias))
-                         "c:/foo/"))
-          (should (equal (expand-file-name "." "c:/usr/spool/")
-                         "c:/usr/spool/"))
-          (should (equal (expand-file-name "" "c:/usr/spool/")
-                         "c:/usr/spool/")))
-      (should (equal (expand-file-name fooslashalias "/") "/foo/"))
-      (should (equal (expand-file-name (concat "/" fooslashalias)) "/foo/"))
-      (should (equal (expand-file-name "." "/usr/spool/") "/usr/spool/"))
-      (should (equal (expand-file-name "" "/usr/spool/") "/usr/spool/"))))
-  ;; Trailing "B/C/.." means B must be a directory.
-  (if (memq system-type '(windows-nt ms-dos))
-      (progn
-        (should (equal (expand-file-name "c:/a/b/c/..") "c:/a/b/"))
-        (should (equal (expand-file-name "c:/a/b/c/../") "c:/a/b/")))
-    (should (equal (expand-file-name "/a/b/c/..") "/a/b/"))
-    (should (equal (expand-file-name "/a/b/c/../") "/a/b/"))))
-
 (ert-deftest fileio-tests--insert-file-interrupt ()
   (let ((text "-*- coding: binary -*-\n\xc3\xc3help")
         f)