bug#66546: 30.0.50; save-buffer to write-protected file without backup fails

[Eli Zaretskii]

> From: Jens Schmidt <jschmidt4gnu@vodafonemail.de>
> Cc: 66546@debbugs.gnu.org
> Date: Tue, 17 Oct 2023 22:12:58 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> Do we agree that this bug is all about the "no-backup" case (*C-0* C-x
> C-s)?

The bug is, but basic-save-buffer-2 isn't.

> For me that means: I want to save to file "foo", and I explicitly do not
> want Emacs to create or touch a backup file "foo~" for that.

While true, I'm not sure what does this have to do with the issue we
are discussing.

> As a consequence, during the whole operation, there is only _one_ file
> being involved, and not some second one, both as far as Emacs and the
> operating system are concerned.

Only if this condition in basic-save-buffer-2 is NOT true:

    (let* ((dir (file-name-directory buffer-file-name))
           (dir-writable (file-writable-p dir)))
      (if (or (and file-precious-flag dir-writable)
              (and break-hardlink-on-save
                   (file-exists-p buffer-file-name)
                   (> (file-nlinks buffer-file-name) 1)
                   (or dir-writable
                       (error (concat "Directory %s write-protected; "
                                      "cannot break hardlink when saving")
                              dir))))

Again, I'm not sure why is this relevant.

> If I were to write a function replacing `basic-save-buffer-2' just for
> that special "no-backup" case, then this way:
> 
>   (defun basic-save-buffer-2-no-backup ()
>     (interactive)
>     ;; ... user confirmation elided here ...
>     (setq setmodes (list (file-modes buffer-file-name)
>                          (file-extended-attributes buffer-file-name)
>                          buffer-file-name))
>     ;; No need to call `set-file-extended-attributes' here, since
>     ;; we only have one file, and we just got the extended
>     ;; attributes from that file.
>     (set-file-modes buffer-file-name
>                     (logior (car setmodes) 128))
>     (let (success)
>       (unwind-protect
>           (progn
>             (write-region nil nil
>                           buffer-file-name nil t buffer-file-truename)
>             (setq success t))
>         (and setmodes (not success)
>              (progn
>                ;; No sense in calling `rename-file' here as done
>                ;; in `basic-save-buffer-2', since we only have
>                ;; one file.
>                (set-file-extended-attributes buffer-file-name
>                                              (nth 1 setmodes))
>                (setq buffer-backed-up nil)))))
>     setmodes)
> 
> 
> >> And wouldn't that be, in this context, just a no-op?
> >
> > Which part of the above would be a no-op?
> 
> Exactly that:
> 
>   (set-file-extended-attributes
>    buffer-file-name
>    (file-extended-attributes buffer-file-name))
> 
> We set the extended file attributes on the same file
> (`buffer-file-name') where we just got them from (`buffer-file-name').

I think you have an inaccurate mental model of what
set-file-extended-attributes does.  In particular, you seem to think
that

   (set-file-extended-attributes
    buffer-file-name
    (file-extended-attributes buffer-file-name))

leaves the file with the same unchanged extended attributes, as if it
were a set-file-modes call.  But that is not true in general,
especially if the original owner of the file was not the same user as
the one who runs the Emacs session that makes these calls.  Depending
on the OS, the actual privileges of the user running Emacs, and the
file-access setup of the underlying system, the user might not be
allowed to set some of the attributes, or might become the owner of
the file, or the OS could add some extended attributes to the original
ones.  So the above is not necessarily a no-op, although in simple
cases it probably is.

> >> I fully understand that the extended attributes stored in `setmodes' are
> >> required later to restore the attributes of the file after it has been
> >> written to.  And in that context I understand why we call
> >> `set-file-extended-attributes'.  But here not really, yet.
> >
> > Well, then let me turn the table and ask you: why do you think we need
> > to restore the extended attributes later? what is the purpose of doing
> > that?
> 
> To restore them after we (possibly) have made the file writable.  Which
> we need to do a) when the call to `write-region' has failed (then in
> function `basic-save-buffer-2' itself), but also b) when the call has
> succeeded (then further up the stack in `basic-save-buffer').

As I tried to explain above, this is not the case.  We set the
extended attributes so that the access rights to the file would be as
close as possible to the original ones, as much as the underlying
filesystem and the user privileges allow.