On Tue, Apr 30, 2019 at 2:21 PM Michael Albinus wrote: > Jonathan Tomer writes: > > Hi Jonathan, > > > 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 see. But pls keep in mind, that inotify is not the only file > notification backend. Currently, we have six different beasts for this. > I was a bit worried about that; I considered disabling the test when file-notify--library is not inotify, but instead designed the test so that *missed* notifications would not cause it to fail, and I believe the other implementations at least should not record a spurious "changed" notification. It's still not ideal but it is the best regression coverage I can think of here. I don't have BSD or win32 machines available to test on, so if you can point me to a way to test this change on those targets I'm happy to run the new tests. Updated patch incoming.