Jonathan Tomer <jktomer@google.com> writes:
Hi Jonathan,
Thanks for the patch.
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -340,6 +340,13 @@ longer.
> ** Multicolor fonts such as "Noto Color Emoji" can be displayed on
> Emacs configured with Cairo drawing and linked with cairo >= 1.16.0.
>
> +---
> +** The file-precious-flag is now respected correctly.
> +A bug previously caused files to be saved twice when
> +`file-precious-flag' or `break-hardlinks-on-save' were specified: once
> +by renaming a temporary file to the destination name, and then again
> +by truncating and rewriting the file, which is exactly what
> +`file-precious-flag' is supposed to avoid.
>
> * Editing Changes in Emacs 27.1
We don't describe bug fixes in etc/NEWS.
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?
> +(ert-deftest files-tests-dont-rewrite-precious-files ()
> + "Test that `file-precious-flag' forces files to be saved by
> +renaming only, rather than modified in-place."
I haven't checked the situation with Tramp. It cares also for
file-precious-flag, bug I don't remember whether it behaves as strict as
you have tested here. Do you want to write a Tramp test for this? It
would fit into tramp-tests.el.
The actual implementation of file-precious-flag's behavior is entirely handled by basic-save-buffer-2 -- TRAMP substitutes different implementations for write-region and rename-file but the decision of which to use comes from outside. The only feature TRAMP adds is that, when file-precious-flag is set, the local and remote temp files are checksummed and the write is considered to have failed if they differ (preventing the final rename into place). I suppose the reason this is done only when file-precious-flag is set is that in the normal case it's too late to do anything about an error.
In other words, I don't believe TRAMP is able to change the strictness of file-precious-flag, unless its implementation of rename-file ends up being nonatomic (which is likely the case for some remotes, but in that case an atomic write is probably impossible anyway). 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.
Best regards, Michael.
Thanks for the fast review!