GNU bug report logs - #35497
[PATCH] Don't rewrite buffer contents after saving by rename

Previous Next

Full log

Message #11 received at 35497 <at> debbugs.gnu.org (full text, mbox):

From: Jonathan Tomer <jktomer <at> google.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 35497 <at> debbugs.gnu.org
Subject: Re: bug#35497: [PATCH] Don't rewrite buffer contents after saving by
 rename
Date: Tue, 30 Apr 2019 12:27:55 -0700

[Message part 1 (text/plain, inline)]

On Tue, Apr 30, 2019 at 12:18 AM Michael Albinus <michael.albinus <at> gmx.de>
wrote:

> Jonathan Tomer <jktomer <at> 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!

[Message part 2 (text/html, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.