bug#57572: 29.0.50; Tramp error with tramp-file-name-unify

[Michael Albinus]

Thierry Volpiatto <thievol@posteo.net> writes:

> Hello,

Hi Thierry,

(sorry for the delayed reply, but I was too busy with changing Tramp to
the rx machinery)

> with helm-find-files connecting to "/sudo::" I have this error:
>
>     (file-error "File `' must be absolute")
>
> This because helm connect to tramp as soon the last ":" is entered.

Indeed.

> Debugger entered--Lisp error: (file-error "File `' must be absolute")
>   signal(file-error ("File `' must be absolute"))
>   tramp-error((tramp-file-name "sudo" "root" nil "IPad-S340" nil "" nil) 
> file-error "File `%s' must be absolute" "")
>   tramp-file-name-unify((tramp-file-name "sudo" "root" nil "IPad-S340" nil "" 
> nil) "")
>   tramp-get-file-property((tramp-file-name "sudo" "root" nil "IPad-S340" nil 
> "" nil) "" "file-readable-p" undef)
>   tramp-sh-handle-file-readable-p("/sudo:root@IPad-S340:")

[...]

Well, the reason for the change in Tramp is, that the file name cache
must work over absolute file names. Otherwise, it wouldn't be obvious,
for with file the cached values of "foo.txt" belong to. This file name
could be used in different directories.

> The error can be fixed with this patch modifying
> `tramp-file-name-unify`:
>
> diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
> index bb6eeaa7417..171d67b42af 100644
> --- a/lisp/net/tramp.el
> +++ b/lisp/net/tramp.el
> @@ -1495,7 +1495,7 @@ same connection.  Make a copy in order to avoid side 
> effects."
>         (and (stringp localname)
>              ;; FIXME: This is a sanity check.  When this error
>              ;; doesn't happen for a while, it can be removed.
> -            (or (file-name-absolute-p localname)
> +            (or (file-name-absolute-p (expand-file-name localname))
>                  (tramp-error
>                   vec 'file-error "File `%s' must be absolute" localname))
>              (tramp-compat-file-name-unquote (directory-file-name localname)))

No, that's not possible. When `tramp-file-name-unify' is invoked, it is
not guaranted that `default-directory' points to a remote file name
corresponding to the relative `localname'. The file name must be
expanded in time.

When applying that patch, I did my best to identify all places in Tramp,
where `expand-file-name' must be used for caching purposes. All Tramp
test suites passed, but obviously there are more use cases in the wild
than Tramp test suite coverage.

So I've added `expand-file-name' to even more places in Tramp in the
hope, that it fits now for all cases. Pushed to master, could you please
check?

> It would be great that tramp development takes in account that there is
> nowaday many alternatives to find-file/dired that are incremental and
> connect automatically as soon as user type.  Most errors come from this
> and have to be fixed more and more frequently as soon there is changes
> in tramp code.

In this case, it was a Tramp error which has been fixed with side
effects. I'm sorry about thgat it hit you, but error fixes must
happen. There is no change in Tramp just for fun, regardless of users.

And we're here in Emacs development, master branch, in order to detect
such regressions early. That's why the sanity message was added to
`tramp-file-name-unify'.

> Thanks.

Best regards, Michael.