On Wed, May 1, 2019, 10:48 Eli Zaretskii <
eliz@gnu.org> wrote:
> From: Jonathan Tomer <jktomer@google.com>
> Date: Tue, 30 Apr 2019 14:10:32 -0700
> Cc: 35497@debbugs.gnu.org
>
> Btw, your new test in files-tests.el uses file notifications. Possible
> of course. But wouldn't it be more robust to check the inode number of
> the involved files, and how it changes, or not? See file-attributes how
> to retrieve the inode number of a file.
>
> I thought about checking that the inode number changes, but that wouldn't have
> caught this particular bug (where the file is renamed into place with the
> correct contents, and then rewritten in place again); indeed, that doesn't
> appear to be easily caught with any examination of the final state alone,
> since what we're looking for is to prove the *absence* of any write that fails
> to change the inode number. (Perhaps we could check that the modification time
> of the file, after write, is *less* than its inode change time, proving that
> there has been no ordinary write since the rename -- but in my experience,
> inode timestamps are not actually more reliable than inotify, and in
> particular this check is easily defeated by the mode-setting that happens
> after the write is complete, requiring care to make sure that save-buffer will
> not attempt to do so.)
I suggest to make a step back and clearly define what you are trying
to test. Is it that we don't rewrite the file after saving it, or is
it some condition regarding the inodes of the original and the new
file?
These are two different things, and the second one is extremely
platform-dependent (because inodes exist only in certain filesystems,
and are emulated in others, and also because which notifications are
generated in such complex situations is very hard to guess in
advance).
Indeed. What I am testing, as you say, is that the file is not changed by writing to it under its final name (ever, not just after renaming, though the latter happened to be the bug in this case) when file-precious-flag is non-nil.
Any discussion of inodes was only because of the perceived unreliability, and actual relative unportability, of filenotify and the systems underlying it.
It's true that notifications are imperfect, but IMO they are the only possible way to test that particular invariant, and this test implementation is designed to be as strict as the available notification system allows without breaking under any reasonable notification API.