bug#68976: 30.0.50; Tramp: unexpected error when calling (file-remote-p "/dav:localhost#8000:/foo")

bug#68976: 30.0.50; Tramp: unexpected error when calling (file-remote-p "/dav:localhost#8000:/foo")

[Ihor Radchenko]

Hello,

Today, I tried to execute an innocent test checking if a file is remote
or not:

(file-remote-p "/dav:localhost#8000:/foo")

Unexpectedly, I encountered user error
tramp-error: Package `tramp-gvfs' not supported

I did not expect that `file-remote-p' should ever throw an error other
than when passed non-string argument.

Looking at the code, (setq tramp-gvfs-enabled t) would make the error
disappear. (The default value is nil)

I believe that it is inappropriate that throw an error in such
situation. If "gvfs" is not supported, but still claimed to be remote by
TRAMP, I expect non-nil return value when calling `file-remote-p'; not
an error.

In GNU Emacs 30.0.50 (build 6, x86_64-pc-linux-gnu, GTK+ Version
 3.24.41, cairo version 1.18.0) of 2024-02-04 built on localhost
Repository revision: ac3b44daf09cf723687664f21ff557d9d5ebc19b
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101011
System Description: Gentoo Linux

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#68976: 30.0.50; Tramp: unexpected error when calling (file-remote-p "/dav:localhost#8000:/foo")

[Eli Zaretskii]

> Cc: michael.albinus@gmx.de
> From: Ihor Radchenko <yantar92@posteo.net>
> Date: Wed, 07 Feb 2024 16:08:20 +0000
> 
> Today, I tried to execute an innocent test checking if a file is remote
> or not:
> 
> (file-remote-p "/dav:localhost#8000:/foo")
> 
> Unexpectedly, I encountered user error
> tramp-error: Package `tramp-gvfs' not supported
> 
> I did not expect that `file-remote-p' should ever throw an error other
> than when passed non-string argument.
> 
> Looking at the code, (setq tramp-gvfs-enabled t) would make the error
> disappear. (The default value is nil)
> 
> I believe that it is inappropriate that throw an error in such
> situation. If "gvfs" is not supported, but still claimed to be remote by
> TRAMP, I expect non-nil return value when calling `file-remote-p'; not
> an error.

What if the "dav:" part was deliberate, i.e. not an accident?  That
ism the caller did want to access a gvfs remote filesystem.  Returning
nil in that case would be exactly the wrong thing to do.  IOW, please
keep in mind that file-remote-p is called internally by many APIs, and
its main use case is to be called at such a low level, where silently
failing is not an option.

So I'm not sure we want to fix this, but maybe Michael will suggest
some solution.

bug#68976: 30.0.50; Tramp: unexpected error when calling (file-remote-p "/dav:localhost#8000:/foo")

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> (file-remote-p "/dav:localhost#8000:/foo")
> ...> 
>> I believe that it is inappropriate that throw an error in such
>> situation. If "gvfs" is not supported, but still claimed to be remote by
>> TRAMP, I expect non-nil return value when calling `file-remote-p'; not
>> an error.
>
> What if the "dav:" part was deliberate, i.e. not an accident?  That
> ism the caller did want to access a gvfs remote filesystem.  Returning
> nil in that case would be exactly the wrong thing to do.

I do not suggest returning nil. I suggest to return non-nil rather than
throwing an error. It would be more appropriate to throw an error when
opening the file were attempted, not merely checking if that file is
remote.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#68976: 30.0.50; Tramp: unexpected error when calling (file-remote-p "/dav:localhost#8000:/foo")

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: 68976@debbugs.gnu.org, michael.albinus@gmx.de
> Date: Wed, 07 Feb 2024 16:40:12 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> (file-remote-p "/dav:localhost#8000:/foo")
> > ...> 
> >> I believe that it is inappropriate that throw an error in such
> >> situation. If "gvfs" is not supported, but still claimed to be remote by
> >> TRAMP, I expect non-nil return value when calling `file-remote-p'; not
> >> an error.
> >
> > What if the "dav:" part was deliberate, i.e. not an accident?  That
> > ism the caller did want to access a gvfs remote filesystem.  Returning
> > nil in that case would be exactly the wrong thing to do.
> 
> I do not suggest returning nil. I suggest to return non-nil rather than
> throwing an error. It would be more appropriate to throw an error when
> opening the file were attempted, not merely checking if that file is
> remote.

I don't know what you mean: Emacs doesn't have any "open file"
operations, it has a "visit file" operation, and that makes all kinds
of tests before actually trying to read the file's contents, to
provide user-friendly diagnostics instead of the cryptic errors
generated by the OS.  One of those tests is file-remote-p.

But let's wait for Michael to chime in.

bug#68976: 30.0.50; Tramp: unexpected error when calling (file-remote-p "/dav:localhost#8000:/foo")

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Ihor Radchenko <yantar92@posteo.net> writes:

> Hello,

Hi Ihor,

> Today, I tried to execute an innocent test checking if a file is remote
> or not:
>
> (file-remote-p "/dav:localhost#8000:/foo")
>
> Unexpectedly, I encountered user error
> tramp-error: Package `tramp-gvfs' not supported
>
> I did not expect that `file-remote-p' should ever throw an error other
> than when passed non-string argument.

You are right. It should have returned nil instead of the
error. `file-remote-p' claims in its docstring

--8<---------------cut here---------------start------------->8---
Test whether FILE specifies a location on a remote system.
--8<---------------cut here---------------end--------------->8---

> Looking at the code, (setq tramp-gvfs-enabled t) would make the error
> disappear. (The default value is nil)

If tramp-gvfs-enabled is nil (default value if D-Bus is not enabled),
the example given above does not specify "a location on a remote system".

> I believe that it is inappropriate that throw an error in such
> situation. If "gvfs" is not supported, but still claimed to be remote by
> TRAMP, I expect non-nil return value when calling `file-remote-p'; not
> an error.

Tramp does not claim it is remote. However, it should return nil.

If you want to know, whether a file name has remote file name syntax,
you should call

(string-match-p tramp-file-name-regexp "/dav:localhost#8000:/foo")

This variable is documented in the Tramp manual, node "(tramp) Change
file name syntax".

I will fix it on master (returning nil but an error).

Best regards, Michael.

bug#68976: 30.0.50; Tramp: unexpected error when calling (file-remote-p "/dav:localhost#8000:/foo")

[Ihor Radchenko]

Michael Albinus <michael.albinus@gmx.de> writes:

> You are right. It should have returned nil instead of the
> error. `file-remote-p' claims in its docstring
> ...
> --8<---------------cut here---------------start------------->8---
> Test whether FILE specifies a location on a remote system.
> --8<---------------cut here---------------end--------------->8---
> ...
>
> If you want to know, whether a file name has remote file name syntax,
> you should call
>
> (string-match-p tramp-file-name-regexp "/dav:localhost#8000:/foo")
>
> This variable is documented in the Tramp manual, node "(tramp) Change
> file name syntax".

Are you saying that I shouldn't use `file-remote-p'?

For me, the goal is to determine whether file is coming from trusted
place or not. By default, anything non-local (Urls, TRAMP paths, etc)
should not be trusted.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#68976: 30.0.50; Tramp: unexpected error when calling (file-remote-p "/dav:localhost#8000:/foo")

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Version: 30.1

Ihor Radchenko <yantar92@posteo.net> writes:

Hi Ihor,

>> If you want to know, whether a file name has remote file name syntax,
>> you should call
>>
>> (string-match-p tramp-file-name-regexp "/dav:localhost#8000:/foo")
>>
>> This variable is documented in the Tramp manual, node "(tramp) Change
>> file name syntax".
>
> Are you saying that I shouldn't use `file-remote-p'?

No.

> For me, the goal is to determine whether file is coming from trusted
> place or not. By default, anything non-local (Urls, TRAMP paths, etc)
> should not be trusted.

I stand corrected. Other Tramp backends do not check whether there is a
valid method. So we even have

--8<---------------cut here---------------start------------->8---
(file-remote-p "/foo:localhost#8000:/foo") => "/foo:localhost#8000:"
--8<---------------cut here---------------end--------------->8---

I've fixed tramp-gvfs.el to behave similar, and to return non-nil for
your example even if tramp-gvfs-enabled is nil. Pushed to master,
closing the bug.

Best regards, Michael.