Re: master 9dc306b1: Improve reporting of I/O, access errors

Re: master 9dc306b1: Improve reporting of I/O, access errors

[Michael Albinus]

eggert@cs.ucla.edu (Paul Eggert) writes:

Hi,

> diff --git a/etc/NEWS b/etc/NEWS
> index 9aec8da..dce4903 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -2005,6 +2005,16 @@ file name if there is no user named "foo".
>  ** The FILENAME argument to 'file-name-base' is now mandatory and no
>  longer defaults to 'buffer-file-name'.
>
> ++++
> +** File metadata primitives now signal an error if I/O, access, or
> +other serious errors prevent them from determining the result.
> +Formerly, these functions often (though not always) returned nil.
> +For example, if the directory /etc/firewalld is not searchable,
> +(file-symlink-p "/etc/firewalld/firewalld.conf") now signals an error
> +instead of returning nil, because file-symlink-p cannot determine
> +whether a symbolic link exists there.  These functions still behave as
> +before if the only problem is that the file does not exist.

That is a serious API change, and I fear it will break existing
code. For example, Tramp trusts on file-attributes returning nil, when a
file does not exist or is not accessible for whatever reason. This is
true also for local files.

Furthermore, Tramp does not report anything but nil if a remote file is
not accessible due to I/O errors. So the promise of this change cannot
be kept for all situations.

I didn't follow the discussion for this, but is it really worth to break
existing code?

(And at least, this paragraph belongs to "Incompatible Lisp Changes in
Emacs 27.1" in NEWS)

Best regards, Michael.

Re: master 9dc306b1: Improve reporting of I/O, access errors

[Michael Albinus]

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

> (And at least, this paragraph belongs to "Incompatible Lisp Changes in
> Emacs 27.1" in NEWS)

Please forget this, it is there. I should learn to read carefully.

Best regards, Michael.

Re: master 9dc306b1: Improve reporting of I/O, access errors

[Paul Eggert]

On 9/18/19 12:41 AM, Michael Albinus wrote:
> Tramp trusts on file-attributes returning nil, when a
> file does not exist or is not accessible for whatever reason. This is
> true also for local files.

Could you give an example of where Tramp does this? That is, suppose 
there is a serious local-file error (I/O error, ancestor-directory 
permissions failure, etc.) when one uses Tramp to access a file - when 
would it be wrong for Tramp to signal such an error?

You suggested that there might be such an example in Bug#37202; I didn't 
quite understand that suggestion, and have followed up there.

> Furthermore, Tramp does not report anything but nil if a remote file is
> not accessible due to I/O errors. So the promise of this change cannot
> be kept for all situations.

It would be nicer if serious errors were also signaled for remote files, 
but that's not essential; such a feature can be delayed until a later 
version (if ever).

Although the jury might still be out on the 
ancestor-directory-permissions aspect of the change (certainly on 
MS-Windows, which reports EACCES for missing directories), surely Emacs 
should not simply ignore I/O errors and the like. And the permissions 
change has been helpful for testing, as it's uncovered a configuration 
bug in how Emacs starts up (Bug#37445, still being investigated). As 
that bug is possibly security-relevant, I'd like to shake it out (along 
with any similar bugs that crop up) before deciding any withdrawal of 
the ancestor-directory permissions aspect of the change.

Re: master 9dc306b1: Improve reporting of I/O, access errors

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Thu, 19 Sep 2019 17:21:09 -0700
> Cc: emacs-devel@gnu.org
> 
> Although the jury might still be out on the 
> ancestor-directory-permissions aspect of the change (certainly on 
> MS-Windows, which reports EACCES for missing directories), surely Emacs 
> should not simply ignore I/O errors and the like.

I think the jury is still out on the latter part as well.  I, for one,
am not convinced.  Emacs is not a file- or directory-management
utility, it's an editor, so its file I/O aspects are secondary to the
primary goal of modifying files' contents.  IMO, we need to closely
watch the results of these changes, which from my POV are still
experimental, and carefully draw conclusions from the user reports.
(What's worse, these changes might significantly delay the release of
Emacs 27.1, but that ship has sailed.)

> And the permissions change has been helpful for testing

That could mean these changes should be only activated by some
build-time option, or maybe a run-time option, by default off.  It
isn't an evidence in favor of the changes being active by default, at
least not a significant evidence.

Re: master 9dc306b1: Improve reporting of I/O, access errors

[Michael Albinus]

Paul Eggert <eggert@cs.ucla.edu> writes:

Hi Paul,

>> Tramp trusts on file-attributes returning nil, when a
>> file does not exist or is not accessible for whatever reason. This is
>> true also for local files.
>
> Could you give an example of where Tramp does this? That is, suppose
> there is a serious local-file error (I/O error, ancestor-directory 
> permissions failure, etc.) when one uses Tramp to access a file - when
> would it be wrong for Tramp to signal such an error?

For such signals we have `access-file'. We shouldn't move its
functionality to `file-attributes'.

>> Furthermore, Tramp does not report anything but nil if a remote file is
>> not accessible due to I/O errors. So the promise of this change cannot
>> be kept for all situations.
>
> It would be nicer if serious errors were also signaled for remote
> files, but that's not essential; such a feature can be delayed until a
> later version (if ever).

Tramp signals errors via ´access-file'. Its current implementation is
not perfect, and should be improved, but that's another story.

Best regards, Michael.

Re: master 9dc306b1: Improve reporting of I/O, access errors

[Paul Eggert]

On 9/20/19 5:43 AM, Michael Albinus wrote:

> For such signals we have `access-file'. We shouldn't move its
> functionality to `file-attributes'.

access-file is different: it signals an error if you cannot access (actually, 
read) a file, and if you cannot access the file because you cannot search its 
parent directory then that's simply another reason that you cannot access the 
file. In contrast, a predicate like file-exists-p is supposed to tell you 
whether the file exists regardless of whether you can access it.

Re: master 9dc306b1: Improve reporting of I/O, access errors

[Michael Albinus]

Paul Eggert <eggert@cs.ucla.edu> writes:

Hi Paul,

>> For such signals we have `access-file'. We shouldn't move its
>> functionality to `file-attributes'.
>
> access-file is different: it signals an error if you cannot access
> (actually, read) a file, and if you cannot access the file because you
> cannot search its parent directory then that's simply another reason
> that you cannot access the file. In contrast, a predicate like
> file-exists-p is supposed to tell you whether the file exists
> regardless of whether you can access it.

I'm speaking about file-attributes, not file-exists-p, which should not
raise an error when a file cannot be accessed.

My point is that file-attributes shall return nil, if it cannot provide
the information, for *whatever* reason. That's the contract of
file-attributes for decades.

If I want to know whether a file exists, or whether a file is
accessible, I use file-exists-p or access-file.


Best regards, Michael.