On Tue, Apr 30, 2019 at 1:47 PM Michael Albinus wrote: > Jonathan Tomer writes: > > Hi Jonathan, > > > Thanks, I'll fix. What's the preferred mechanism for sending an > > updated patch -- send an entirely new patch (relative to upstream) on > > a new thread, or on this thread, or a delta to apply as an additional > > commit on top of my previous patch? > > Please reply to these messages, keeping 35497@debbugs.gnu.org in To: or > Cc:. > This archives your message in the bug tracker. > > Usually, we appreciate a new patch relative to upstream. But in this > case, not changing etc/NEWS, I believe it isn't necessary to send a new > patch until there are other changes you want to show. > OK, will send along with the patch adding the TRAMP test shortly. > That said, I'm happy to add a test to tramp-tests.el as well; at the > > very least, with the mock tramp method we should see that the > > destination file is renamed-to rather than overwritten as well. > > Pls do. > > 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.) Best, Jonathan