GNU bug report logs - #26911
25.2; eshell "cd .." doesn't work correctly with TRAMP

Previous Next

Full log

Message #80 received at 26911 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 26911 <at> debbugs.gnu.org, mattiase <at> acm.org, michael.albinus <at> gmx.de,
 yegortimoshenko <at> gmail.com
Subject: Re: bug#26911: 25.2; eshell "cd .." doesn't work correctly with TRAMP
Date: Sun, 30 Aug 2020 17:09:55 +0300

> Cc: 26911 <at> debbugs.gnu.org, mattiase <at> acm.org, michael.albinus <at> gmx.de,
>  yegortimoshenko <at> gmail.com
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Sat, 29 Aug 2020 13:42:23 -0700
> 
> It surely would be better to fix the bug on MS-Windows. A good way to start 
> doing that is to refactor the code a bit to avoid the tricky #ifdefs it 
> currently uses, as these #ifdefs make bugs like this painful to fix. I can draft 
> a patch along those lines if you like. I realize you're dubious about 
> refactoring and so wouldn't install the patch without checking with you.

It isn't right for us to make significant changes in expand-file-name,
let alone refactor it.  Its code is complex and full of subtle dark
corners, many of which are not well covered by our test suite.
Refactoring it into something more elegant is a large job -- for a
very small gain, since the function does its job 99.99% of the time.
We would be wasting our sparse resources if we do any serious changes
there, and will almost certainly introduce quite a few bugs on the
way.

It isn't worth it, definitely not in order to fix the couple of rare
bugs we are facing.

Instead, we should fix these particular bugs by localized changes
whose effect is limited to the specific situations we need to fix.

So with that in mind, let's please go back to the problems we have and
see how they can be fixed without unnecessary refactoring.

The original problem in this bug report was identified by Michael:

> In fileio.c, lines 1393-1394, the following loop
> 
> --8<---------------cut here---------------start------------->8---
> 	    while (o != target && (--o, !IS_DIRECTORY_SEP (*o)))
> 	      continue;
> --8<---------------cut here---------------end--------------->8---
> 
> replaces "/ssh:host:/bin/.." by "/ssh:host:". But it should be
> "/ssh:host:/".

Actually, 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.

In your log message you alluded to another use case, unrelated to
remote file names, but didn't provide any details.  Can you please
provide them now?  Is that other use case really similar to the one
which started this bug report (if not, no need to fix them both
together)?

The related bug#34834 is again about remote file names, but it's a
different situation.  AFAICT, it should also be easy to fix in a
localized manner.

Making such localized changes will allow us to be certain we don't
break any use cases we already support successfully.

Finally, I see no reason to require expand-file-name to preserve the
trailing slash -- we never required this until now, and AFAIK had no
problems with that.  If some specialized use case does need this, I'd
prefer to fix that only where and when it really matters.  For
example, if some Eshell command needs it, let's first consider fixing
that in Eshell.

In any case, the trailing slash issue is only tangentially related to
the bugs discussed here, so let's not mix these separate issues.

Thanks.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.