bug#26911: 25.2; eshell "cd .." doesn't work correctly with TRAMP

[Eli Zaretskii]

> Cc: 26911@debbugs.gnu.org, mattiase@acm.org, michael.albinus@gmx.de,
>  yegortimoshenko@gmail.com
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sun, 30 Aug 2020 14:39:28 -0700
> 
> This bug occurred because file-symlink-p calls expand-file-name which 
> incorrectly stripped trailing "/." from the file name before checking the 
> file's 
> status.

It is not expand-file-name's job to know about these subtleties.
expand-file-name deals only with the syntax of file names.  It doesn't
know anything about the semantics of "." and ".." except that they can
be removed to bring the file name to a standard form.  It doesn't know
whether a file is a directory or a symlink or something else; it
doesn't even care if the file exists.  It isn't supposed to hit the
disk for its job.

It is therefore perfectly valid for it to remove the trailing "/."
without appending a slash.  If file-symlink-p needs to handle such
file names specially, it should do it in its own code.

So this job should not be imposed on expand-file-name, and we should
remove the code added for that purpose.

> > I see no reason to require expand-file-name to preserve the
> > trailing slash
> 
> It's required because trailing slash affects how file names are interpreted 
> on 
> GNU and other POSIXish platforms.

It is not the job of expand-file-name to interpret file names.  Lisp
programs that need a directory's name to end in a slash should call
file-name-as-directory, this is why we have that function.  If we
insist on appending the slash in all cases, then some code will
benefit, but other code will break (and will need to call
directory-file-name to avoid the breakage).  There's no net win here,
so we should not do this, either.

expand-file-name is simply the wrong place for this kind of
functionality, even before we consider its complexity.

> > IMO the problem is immediately following the above snippet:
> > 
> >         /* Keep initial / only if this is the whole name.  */
> >         if (o == target && IS_ANY_SEP (*o) && p[3] == 0)
> >           ++o;
> > 
> > This is very easy to fix without affecting any other uses of the
> > function: we should consider one other case in addition to "only if /
> > is the whole name" -- the case where this fails to DTRT with remote
> > directories.
> 
> Such a fix should be no problem for the GNU/POSIXish side, as that snippet is 
> in 
> the DOS_NT code and any fixes there should affect only MS-Windows and DOS. I 
> don't know what a "remote directory" is in that context, though, so I can't 
> give 
> specific advice.

You are talking about the code after your changes, whereas I (and
Michael at the time he wrote that) were talking about the code before
your changes: then the above snippet affected all platforms.

> Right now the code is needlessly hard to understand, and that makes
> it hard to fix - something I encountered while trying to fix some of
> the abovementioned bugs.

It isn't hard for me to understand the current code, although it is
indeed complex (because the job it does is not trivial).  But the
problem is not the complexity, the problem starts when we make changes
there which are either not strictly necessary, or affect more use
cases than the few we need to fix.

That function works, and works well.  Let's not make it less
dependable than it is today.

Anyway, I think I understand all the issues now, so I will work on
fixing them soon in a way that will avoid unnecessary fallout.

Thanks.