[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#35497: [PATCH] Don't rewrite buffer contents after saving by rename

From: Jonathan Tomer
Subject: bug#35497: [PATCH] Don't rewrite buffer contents after saving by rename
Date: Tue, 30 Apr 2019 12:27:55 -0700

On Tue, Apr 30, 2019 at 12:18 AM Michael Albinus <address@hidden> wrote:
Jonathan Tomer <address@hidden> 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!

reply via email to

[Prev in Thread] Current Thread [Next in Thread]