backup method

backup method

[Han Boetes]

Hi,

The default backup method is mv and then create a new file. That's
nice if you really want to keep the timestamp intact. But if you
use emacs with crontab -e you remove the tmpfile and since the
filedescriptor is still open your update to your cron is lost --
without warning. It was pretty hard to discover why this happened.

So I would like to suggest to  make backup by copying the default.

Perhaps it's also an idea to disable backups altogether if the
file is in /tmp



# Han

Re: backup method

[Luc Teirlinck]

Han Boetes wrote:

   So I would like to suggest to  make backup by copying the default.

I agree.  Backup by renaming is a very confusing default.  I had to
change it instantly after I first started using Emacs.  I had a
process appending to a file with auto-revert enabled.  I edited the
file and saved my changes.  The process started appending to the
backup file.  Very confusing.  (It actually was so confusing that I
still clearly remember it.)  At that time I was not knowledgeable
about Emacs at all (I had never used it before) and without the help
of my sysadmin, I would not have been able to figure out at all what
to do.  (Which, of course, was to set `backup-by-copying' to true.)

Then of course there is the silent breaking of hard links.

Sincerely,

Luc.

Re: backup method

[Miles Bader]

On Wed, 26 Jan 2005 18:45:18 -0600 (CST), Luc Teirlinck
<teirllm@dms.auburn.edu> wrote:
> Han Boetes wrote:
> 
>    So I would like to suggest to  make backup by copying the default.
> 
> I agree.  Backup by renaming is a very confusing default.

No it's not.  It's by _far_ the most natural way to do it.

> I had to
> change it instantly after I first started using Emacs.  I had a
> process appending to a file with auto-revert enabled.  I edited the
> file and saved my changes.  The process started appending to the
> backup file.  Very confusing

The problem here is that you're doing something Very Weird: editing
and saving a file currently being written by a process is _not_ a
common thing to do, for good reason -- no matter what your editor
does, it's pretty much random luck whether the right thing happens or
not.

Backup-by-renaming is the best method for general use; there are
specific exceptional cases where it doesn't work well.  If some of
these exceptional cases are easy to detect, Emacs should detect them.

-Miles
-- 
Do not taunt Happy Fun Ball.

Re: backup method

[Luc Teirlinck]

Miles Bader wrote:

   The problem here is that you're doing something Very Weird: editing
   and saving a file currently being written by a process is _not_ a
   common thing to do, for good reason -- no matter what your editor
   does, it's pretty much random luck whether the right thing happens or
   not.

The right thing always happens if the other program only appends.  The
only problem occurs if there is program output before you are able to
save your changes.  Auto Revert warns you about that.  So you save
often enough that loosing your changes is no big deal, especially
since these changes are just killing some junk output by the program
and you only edit if you know the program is going to be busy for
hours without output anyway.  I have enjoyed 100% random luck for many
years now.

If a program is going to run for weeks or maybe months, I can not wait
for weeks or months before starting to edit its output.

Sincerely,

Luc.

Re: backup method

[Luc Teirlinck]

Han Boetes wrote:

   Perhaps it's also an idea to disable backups altogether if the
   file is in /tmp

Is this not already the case?  From `(elisp)Making Backups':

 -- Variable: backup-enable-predicate
     This variable's value is a function to be called on certain
     occasions to decide whether a file should have backup files.  The
     function receives one argument, an absolute file name to consider.
     If the function returns `nil', backups are disabled for that
     file.  Otherwise, the other variables in this section say whether
     and how to make backups.

     The default value is `normal-backup-enable-predicate', which checks
     for files in `temporary-file-directory' and
     `small-temporary-file-directory'.

`temporary-file-directory' is "/tmp/" by default.

Sincerely,

Luc.

Re: backup method

[Han Boetes]

Miles Bader wrote:
> Backup-by-renaming is the best method for general use; there are
> specific exceptional cases where it doesn't work well. If some
> of these exceptional cases are easy to detect, Emacs should
> detect them.

If the file is open ie another program has fopen'ed the file then
you have an exceptional case.

lsof and fstat ( for BSD ) can detect open file descriptors. I
suppose emacs can do the same.



# Han

Re: backup method

[Han Boetes]

Luc Teirlinck wrote:
> Han Boetes wrote:
>
>    Perhaps it's also an idea to disable backups altogether if the
>    file is in /tmp
>
> Is this not already the case?  From `(elisp)Making Backups':
>
>  -- Variable: backup-enable-predicate

I do not yet seem to have this variable in the multitty branch,
which is now over a month old. Must be something new.



# Han

Re: backup method

[Han Boetes]

Han Boetes wrote:
> Luc Teirlinck wrote:
> > Han Boetes wrote:
> >
> >    Perhaps it's also an idea to disable backups altogether if the
> >    file is in /tmp
> >
> > Is this not already the case?  From `(elisp)Making Backups':
> >
> >  -- Variable: backup-enable-predicate
>
> I do not yet seem to have this variable in the multitty branch,
> which is now over a month old. Must be something new.

Ow wait. I got it!!

I normally set TMP and TMPDIR to ~/.tmp so all tmpfiles are made
in those dirs.

Very good measure against programs that use unsafe tmpfile
creation and what not.

I just tested it. If I ``unset TMP TMPDIR'' and then set backup by
moving I get no problems with crontab because c-h v
temporary-file-directory is "/tmp/"

But not all programs honour TMP and TMPDIR. So I suggest that that
variable should always include /tmp/



# Han

Re: backup method

[Miles Bader]

On Thu, 27 Jan 2005 03:08:24 +0100, Han Boetes <han@mijncomputer.nl> wrote:
> >  -- Variable: backup-enable-predicate
> 
> I do not yet seem to have this variable in the multitty branch,
> which is now over a month old. Must be something new.

The earliest ChangeLog entry for it is from 1990, so maybe look again... :-)

-Miles
-- 
Do not taunt Happy Fun Ball.

Re: backup method

[Han Boetes]

Miles Bader wrote:
> Han Boetes wrote:
> > >  -- Variable: backup-enable-predicate
> >
> > I do not yet seem to have this variable in the multitty
> > branch, which is now over a month old. Must be something new.
>
> The earliest ChangeLog entry for it is from 1990, so maybe look
> again... :-)

Yes, I was looking for the customize-variable :-)



# Han

Re: backup method

[Han Boetes]

So this is the normal code in files.el:

(defun normal-backup-enable-predicate (name)
  "Default `backup-enable-predicate' function.
Checks for files in `temporary-file-directory' or
`small-temporary-file-directory'."
  (not (or (let ((comp (compare-strings temporary-file-directory 0 nil
                                        name 0 nil)))
             ;; Directory is under temporary-file-directory.
             (and (not (eq comp t))
                  (< comp (- (length temporary-file-directory)))))
           (if small-temporary-file-directory
               (let ((comp (compare-strings small-temporary-file-directory
                                            0 nil
                                            name 0 nil)))
                 ;; Directory is under small-temporary-file-directory.
                 (and (not (eq comp t))
                      (< comp (- (length small-temporary-file-directory)))))))))


And that should look like:

(defun normal-backup-enable-predicate (name)
  "Default `backup-enable-predicate' function.
Checks for files in `temporary-file-directory' or
`small-temporary-file-directory'."
  (not (or (let ((comp (compare-strings temporary-file-directory 0 nil
                                        name 0 nil)))
             ;; Directory is under temporary-file-directory.
             (and (not (eq comp t))
                  (< comp (- (length temporary-file-directory))))
	     ;; Nor under /tmp/
	     ;; Insert magic and portable lispcode here.
		  )
           (if small-temporary-file-directory
               (let ((comp (compare-strings small-temporary-file-directory
                                            0 nil
                                            name 0 nil)))
                 ;; Directory is under small-temporary-file-directory.
                 (and (not (eq comp t))
                      (< comp (- (length small-temporary-file-directory)))))))))


Except that I don't understand how this should be written down in
lisp :-\



# Han

Re: backup method

[Eli Zaretskii]

> Date: Thu, 27 Jan 2005 01:01:48 +0059
> From: Han Boetes <han@mijncomputer.nl>
> 
> The default backup method is mv and then create a new file. That's
> nice if you really want to keep the timestamp intact. But if you
> use emacs with crontab -e you remove the tmpfile and since the
> filedescriptor is still open your update to your cron is lost --
> without warning. It was pretty hard to discover why this happened.
> 
> So I would like to suggest to  make backup by copying the default.

So you had a very specific problem, and you are suggesting to change
the defaults of _all_ users just because of that one problem?

I agree with Miles: the default backup method is a very natural thing
to do.  I don't think your special case is a reason good enough to
change the default.  At best, it's a reason for you to customize your
Emacs.

Re: backup method

[Han Boetes]

Eli Zaretskii wrote:
> > Date: Thu, 27 Jan 2005 01:01:48 +0059
> > From: Han Boetes <han@mijncomputer.nl>
> >
> > The default backup method is mv and then create a new file. That's
> > nice if you really want to keep the timestamp intact. But if you
> > use emacs with crontab -e you remove the tmpfile and since the
> > filedescriptor is still open your update to your cron is lost --
> > without warning. It was pretty hard to discover why this happened.
> >
> > So I would like to suggest to  make backup by copying the default.
>
> So you had a very specific problem, and you are suggesting to change
> the defaults of _all_ users just because of that one problem?

You are absolutely right, that's exactly what I suggested. I also
suggest you make a study on rhetorical questions and I hope you
find out why you should avoid them.


> I agree with Miles: the default backup method is a very natural thing
> to do.  I don't think your special case is a reason good enough to
> change the default.

If you would have read the rest of the thread you would have
discovered I came up with something better.


>  At best, it's a reason for you to customize your Emacs.

No I really found a problem. Not a bug in emacs, but it does need
a workaround.



# Han

Re: backup method

[Kim F. Storm]

"Eli Zaretskii" <eliz@gnu.org> writes:

>> Date: Thu, 27 Jan 2005 01:01:48 +0059
>> From: Han Boetes <han@mijncomputer.nl>
>> 
>> The default backup method is mv and then create a new file. That's
>> nice if you really want to keep the timestamp intact. But if you
>> use emacs with crontab -e you remove the tmpfile and since the
>> filedescriptor is still open your update to your cron is lost --
>> without warning. It was pretty hard to discover why this happened.
>> 
>> So I would like to suggest to  make backup by copying the default.
>
> So you had a very specific problem, and you are suggesting to change
> the defaults of _all_ users just because of that one problem?
>
> I agree with Miles: the default backup method is a very natural thing
> to do.  I don't think your special case is a reason good enough to
> change the default.  At best, it's a reason for you to customize your
> Emacs.

I agree!

I don't normally use auto-revert mode, but perhaps the default could
change if you enable auto-revert mode?

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: backup method

[Kevin Rodgers]

Luc Teirlinck wrote:
> Then of course there is the silent breaking of hard links.

Which is why backup-by-copying-when-linked should be t by default.

-- 
Kevin Rodgers

Re: backup method

[Eli Zaretskii]

> Date: Thu, 27 Jan 2005 06:27:12 +0100
> From: Han Boetes <han@mijncomputer.nl>
> 
> > So you had a very specific problem, and you are suggesting to change
> > the defaults of _all_ users just because of that one problem?
> 
> You are absolutely right, that's exactly what I suggested. I also
> suggest you make a study on rhetorical questions and I hope you
> find out why you should avoid them.

It wasn't clear to me that you understood it was a rhetorical
question.

As long as we agree that your suggestion is based on a specific
problem of a single user, I'm happy, since your arrogant attitude
suggests that we have little hope of having a useful discussion.

Re: backup method

[Luc Teirlinck]

Kim Storm wrote:

   I don't normally use auto-revert mode, but perhaps the default could
   change if you enable auto-revert mode?

It has actually nothing _directly_ to do with Auto Revert mode, except
that the reason to enable Auto Revert is that you expect something
else to be writing to the file.  This does not interfere with your
editing as long as it only appends to the file.  You do not want to
have the process start to append to the backup file.

I guess that it would be sufficient to set
`backup-by-copying-when-mismatch' to t to avoid the problem.  (And
`backup-by-copying-when-linked' to t to avoid silently breaking hard
links, an unrelated but nasty problem.)

Sincerely,

Luc.

Re: backup method

[Luc Teirlinck]

Kim Storm wrote:

   I don't normally use auto-revert mode, but perhaps the default could
   change if you enable auto-revert mode?

In my previous reply, I failed to make clear that I would _not_
consider it a good idea to make any of this depend on Auto Revert
mode.  Many people use Global Auto Revert mode and that applies to all
file visiting buffers.

Sincerely,

Luc.

Re: backup method

[Reiner Steib]

On Thu, Jan 27 2005, Kevin Rodgers wrote:

> Luc Teirlinck wrote:
>> Then of course there is the silent breaking of hard links.
>
> Which is why backup-by-copying-when-linked should be t by default.

ACK.  I never understood why it is nil by default.  Is there a strong
reason for the current default?

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: backup method

[Luc Teirlinck]

>From my previous reply:
   
   You do not want to have the process start to append to the backup
   file.

   I guess that it would be sufficient to set
   `backup-by-copying-when-mismatch' to t to avoid the problem.

Actually, I tried that and it did not help.  I am the owner of the
files that my processes write to, so `backup-by-copying-when-mismatch'
has no effect, because there is no "mismatch".  So I really need to
set backup-by-copying itself to t to be able to use Emacs for my work.

Sincerely,

Luc.

Re: backup method

[Richard Stallman]

    The default backup method is mv and then create a new file. That's
    nice if you really want to keep the timestamp intact. But if you
    use emacs with crontab -e you remove the tmpfile and since the
    filedescriptor is still open your update to your cron is lost --
    without warning. It was pretty hard to discover why this happened.

    So I would like to suggest to  make backup by copying the default.

I won't change it in general, but we could probably arrange to use
backup by copying for *this specific case*.  Can you give a precise
recipe for distinguishing this case--a regexp for these file names,
perhaps?

    If the file is open ie another program has fopen'ed the file then
    you have an exceptional case.

    lsof and fstat ( for BSD ) can detect open file descriptors. I
    suppose emacs can do the same.

It is not the case in general that the existence of other descriptors
means it is better to backup by copying.  That's what you want in this
particular case, for crontab files.  However, in other cases where
another program is reading the old version, it should be able to
continue reading the old version, and editing the file should not
interfere with it.

Re: backup method

[Richard Stallman]

    I don't normally use auto-revert mode, but perhaps the default could
    change if you enable auto-revert mode?

I don't use it either, and I am not sure what's best here, but it is
not out of the question for auto-revert mode to set this variable.

However, what does auto-revert mode have to do with the problem case?

Re: backup method

[Miles Bader]

On Thu, 27 Jan 2005 11:25:10 -0700, Kevin Rodgers <ihs_4664@yahoo.com> wrote:
> Luc Teirlinck wrote:
> > Then of course there is the silent breaking of hard links.
> 
> Which is why backup-by-copying-when-linked should be t by default.

Um, the "silent breaking of hard links" is in many case a _good thing_
-- it's quite common to use hard-link trees to "snapshot" a
directories, and depend on your editor to snap the link if you change
a file.

It would be a nasty, nasty, shock (as in, lots of corrupted files) to
those people who depend on this feature to have the default suddenly
change out from under them!

-Miles
-- 
Do not taunt Happy Fun Ball.

Re: backup method

[Miles Bader]

On Thu, 27 Jan 2005 21:25:45 +0100, Reiner Steib
<reinersteib+gmane@imap.cc> wrote:
> On Thu, Jan 27 2005, Kevin Rodgers wrote:
> > Which is why backup-by-copying-when-linked should be t by default.
> 
> ACK.  I never understood why it is nil by default.  Is there a strong
> reason for the current default?

See my reply to Kevin.

Basically it depends on what the hard-link is being used for as to
whether you want it to be broken on save or not.  One could argue that
the "native" unix behavior is to not break the link (since that's
e.g., what "cat > file" does), but a lot of people have become
accustomed to  using emacs' link breaking to make a sort of cheap COW
(copy-on-write) facility for hard-linked trees; other
developer-oriented utilities, most notably the "patch" command, have
the same behavior (breaking the link).

In any case, I think it's far too late to change the default.

-Miles
-- 
Do not taunt Happy Fun Ball.

Re: backup method

[Richard Stallman]

    > Then of course there is the silent breaking of hard links.

    Which is why backup-by-copying-when-linked should be t by default.

You may be thinking of cases where you want the other name to continue
to refer to the latest version of the file.  Those cases exist.  But
there are also cases where the contents of other name should remain
unchanged.  Backup by renaming does that job right.

If you want the other name to always refer to the latest version,
you could get that result with a symlink.  So I think the current
defaults are best.  They make it possible to get both behaviors.

Re: backup method

[Han Boetes]

Richard Stallman wrote:
> Han Boetes wrote:
> > The default backup method is mv and then create a new file.
> > That's nice if you really want to keep the timestamp intact.
> > But if you use emacs with crontab -e you remove the tmpfile
> > and since the filedescriptor is still open your update to your
> > cron is lost -- without warning. It was pretty hard to
> > discover why this happened.
> >
> > So I would like to suggest to make backup by copying the
> > default.
>
> I won't change it in general, but we could probably arrange to
> use backup by copying for *this specific case*. Can you give a
> precise recipe for distinguishing this case--a regexp for these
> file names, perhaps?

My initial suggestion to do backup by copying is not the right
solution for the problem at all. I explained the right solution in
another message in this thread.

My prove is:

  unset TMP TMPDIR
  export VISUAL='emacs -q' # which does backup by moving by default
  crontab -e

And you can edit your crontab fine since emacs won't make a backup
of files in /tmp

  export TMP=~/.tmp; export TMPDIR=~/.tmp
  export VISUAL='emacs -q' # which does backup by moving by default
  crontab -e

And you can no longer edit crontabs since emacs will make a backup
of files in /tmp.

This problem is caused by the fact that not all programs honour
TMPDIR and still make tmpfiles in /tmp. Or that emacs does not
anticipate this.

I described the solution in another message in this thread.




# Han

Re: backup method

[Han Boetes]

You pointed out to me that my initial idea was not the right one
by formulating it as a rhetorical question. Which is a terribly
unforgiving way to point out someone made a mistake.

You didn't even bother to notice my further corrected findings.

I _admitted_ I was wrong and _suggested_ you should not use
rhetorical questions to point out peoples mistakes.

And now you answer with this! Some cheek!


Eli Zaretskii wrote:
> > From: Han Boetes <han@mijncomputer.nl>
> >
> > > So you had a very specific problem, and you are suggesting
> > > to change the defaults of _all_ users just because of that
> > > one problem?
> >
> > You are absolutely right, that's exactly what I suggested. I
> > also suggest you make a study on rhetorical questions and I
> > hope you find out why you should avoid them.
>
> It wasn't clear to me that you understood it was a rhetorical
> question.
>
> As long as we agree that your suggestion is based on a specific
> problem of a single user, I'm happy, since your arrogant
> attitude suggests that we have little hope of having a useful
> discussion.



# Han

Re: backup method

[Richard Stallman]

    My prove is:

      unset TMP TMPDIR
      export VISUAL='emacs -q' # which does backup by moving by default
      crontab -e

    And you can edit your crontab fine since emacs won't make a backup
    of files in /tmp

So you're not editing the crontab files under their real names, but
rather, editing copies that were made in /tmp?

If so, I understand the situation now.  Thanks.  Does this fix it?


*** files.el	24 Jan 2005 20:08:31 -0500	1.743
--- files.el	28 Jan 2005 09:33:33 -0500	
***************
*** 2844,2856 ****
  
  (defun normal-backup-enable-predicate (name)
    "Default `backup-enable-predicate' function.
! Checks for files in `temporary-file-directory' or
! `small-temporary-file-directory'."
    (not (or (let ((comp (compare-strings temporary-file-directory 0 nil
  					name 0 nil)))
  	     ;; Directory is under temporary-file-directory.
  	     (and (not (eq comp t))
  		  (< comp (- (length temporary-file-directory)))))
  	   (if small-temporary-file-directory
  	       (let ((comp (compare-strings small-temporary-file-directory
  					    0 nil
--- 2844,2861 ----
  
  (defun normal-backup-enable-predicate (name)
    "Default `backup-enable-predicate' function.
! Checks for files in `temporary-file-directory',
! `small-temporary-file-directory', and /tmp."
    (not (or (let ((comp (compare-strings temporary-file-directory 0 nil
  					name 0 nil)))
  	     ;; Directory is under temporary-file-directory.
  	     (and (not (eq comp t))
  		  (< comp (- (length temporary-file-directory)))))
+ 	   (let ((comp (compare-strings "/tmp" 0 nil
+ 					name 0 nil)))
+ 	     ;; Directory is under /tmp.
+ 	     (and (not (eq comp t))
+ 		  (< comp (- (length "/tmp")))))
  	   (if small-temporary-file-directory
  	       (let ((comp (compare-strings small-temporary-file-directory
  					    0 nil

Re: backup method

[Han Boetes]

Richard Stallman wrote:
> So you're not editing the crontab files under their real names,
> but rather, editing copies that were made in /tmp?

crontab -e keeps the file-descriptor open while editing the
tmpfile. So you would edit the right filename but the wrong file.


> If so, I understand the situation now.  Thanks.  Does this fix it?

Yes it does. At least for default emacs. Not for the multitty-
branch. But that's not your concern of course. I'll poke the maintainer.

I do have one question left though: Is this patch sufficiently portable?

Looking through files.el I found this description of what a
temporary-file-directory looks like:


(defcustom temporary-file-directory
  (file-name-as-directory
   (cond ((memq system-type '(ms-dos windows-nt))
          (or (getenv "TEMP") (getenv "TMPDIR") (getenv "TMP") "c:/temp"))
         ((memq system-type '(vax-vms axp-vms))
          (or (getenv "TMPDIR") (getenv "TMP") (getenv "TEMP") "SYS$SCRATCH:"))
         (t
          (or (getenv "TMPDIR") (getenv "TMP") (getenv "TEMP") "/tmp"))))
  "The directory for writing temporary files."
  :group 'files
  :type 'directory)


And assuming this problem only needs fixing on unix doesn't feel
right IMHO.



# Han

Re: backup method

[Luc Teirlinck]

Han Boetes wrote:

   I do have one question left though: Is this patch sufficiently portable?

   Looking through files.el I found this description of what a
   temporary-file-directory looks like:

I am not _sure_, but I have the impression that `temporary-file-directory'
was exactly intended for portability reasons.  I believe that it was
intended to be the "normal" place for the OS to put temporary files.
You normally do not have to customize it because the code you quoted
normally gets it right.  If it gets it wrong, you have to customize it.

I have the impression that to get the portability you want, you would
need two new variables, that would do what I believe
`temporary-file-directory' and  `small-temporary-file-directory' are
actually intended to do, `system-temporary-file-directory' and
`system-small-temporary-file-directory'.  I do not really know whether
that would really be the correct solution.

Note that we are discussing whether or not to make backup files while
discussing a problem that has nothing at all to do with whether it is
useful or not to make backup files.  The only thing we are worried
about is whether the file should be copied or overwritten on save.
Some people _want_ and _expect_ Emacs to make backups in /temp/,
because they do make changes there that they might regret.  They then
try to use the backup file, which is not there.  I have seen "bug"
reports by people who where very inconvenienced by this (for them)
unexpected behavior.  "How come Emacs always makes backup files
_except_ when I really need them?".

Default Emacs is not an overwrite-on-save system and it is not a
copy-on-save system either.  By default, Emacs copies on save if and
only if a backup is made.  This links two unrelated questions, "Do I
need a copy in case I regret my edits" and "Does this file need to be
copied or overwritten on save".  Obviously, there are going to be
situations where this creates problems.

I believe that the correct solution in this particular case is to make
`normal-backup-enable-predicate' use a customizable list of
directories in which no backups are made, say `no-backup-directories'.
We could then tell people that if they want remove "/temp/" or
equivalent from the list, they should set `backup-by-copying' to t, or
we could use a second customizable variable
`backup-by-copying-directories', to unlink the two problems (at least
in this particular case).

Sincerely,

Luc.

Re: backup method

[Luc Teirlinck]

In my original reply, I forgot that you actually set TMP and TMPDIR
and did not customize `temporary-file-directory' itself.  I still
believe that to get the portability you want, the user should be able
to correct wrong guesses we made about his operating system defaults.
To achieve this and to unlink the two unrelated problems of whether
the user needs a backup in case he might regret his changes and
whether processes need to use the edited version or not, I still
believe it is best to make `normal-backup-enable-predicate' use a
customizable list of directories in which no backups should be made.
Also, as I already said in my earlier reply, maybe a customizable
variable `backup-by-copying-directories' might be useful.

Sincerely,

Luc.

Re: backup method

[Stefan Monnier]

> I am not _sure_, but I have the impression that `temporary-file-directory'
> was exactly intended for portability reasons.  I believe that it was
> intended to be the "normal" place for the OS to put temporary files.
> You normally do not have to customize it because the code you quoted
> normally gets it right.  If it gets it wrong, you have to customize it.

No, temporary-file-directory points to the directory where *Emacs* should
put its temporary files.  What Han's problem suggests is that we may want to
add some variable that points to the director*ies* whose files should be
considered "temporary".


        Stefan

Re: backup method

[Luc Teirlinck]

Stefan Monnier wrote:

   What Han's problem suggests is that we may want to add some
   variable that points to the director*ies* whose files should be
   considered "temporary".

Well, I guess that Emacs needs _one_ directory in which it puts
temporary files.

In as far as the other directories where files "should be considered
temporary", it is not clear what conclusions should be drawn out of
that.  Apparently the conclusions drawn out of it are that one should
make no backup files there (something with which some users seem to
strongly disagree, as I have noticed in some bug reports) and that
these files should keep using the edited version of the file, if the
file is edited.  The latter happens automatically if backup-by-copying
is true and if and only if a backup is made if backup-by-copying is nil.

I personally believe that the best solution would be to have a
customizable list of directories in which no backups are made.  The
default _could_ be the list of directories in which files should be
considered temporary.  Actually, the principle of least surprise would
suggest the empty list as default.  (Apparently some users have been
surprised and heavily inconvenienced when they found out that the
backup file they badly needed was not there in tmp.)

One could also use a customizable list of directories where backups
are made by copying, even if backup-by-copying is nil.  That one would
consist by default of the files in those temporary directories.

Sincerely,

Luc.

Re: backup method

[Luc Teirlinck]

>From my previous message:

    and that these files should keep using the edited version of the file,
    if the file is edited.

I meant:

and that the programs that use these files should keep using the
edited version of the file, if the file is edited.

Sincerely,

Luc.

Re: backup method

[Luc Teirlinck]

I am apparently in typo-mode.  From my previous message:

    and that these files should keep using the edited version of the file,
    if the file is edited.  The latter happens automatically if
    backup-by-copying is true and if and only if a backup is made if
    backup-by-copying is nil.

The opposite, of course.  It happens if and only if _no_ backup is
made if backup-by-copying is nil.

Sincerely,

Luc.

Re: backup method

[Luc Teirlinck]

>From my previous message:

   (something with which some users seem to
   strongly disagree, as I have noticed in some bug reports)

and:

   Actually, the principle of least surprise would suggest the empty
   list as default.  (Apparently some users have been surprised and
   heavily inconvenienced when they found out that the backup file
   they badly needed was not there in tmp.)

Actually, I found the bug report I remembered back and what happened
was that, for some strange reason, the OP's Linux trees were mounted
on /tmp/linux, indeed a very strange thing to do.

So maybe it is indeed better to keep the list of all temporary
directories as a default for the list of no backup directories.  But I
myself have also seen tmp misused in other ways and it might be good
to at least leave the user the option to have backups made there.

I personally do not routinely hand edit files in tmp.

Sincerely,

Luc.

Re: backup method

[Luc Teirlinck]

>From my previous message:

   One could also use a customizable list of directories where backups
   are made by copying, even if backup-by-copying is nil.  That one would
   consist by default of the files in those temporary directories.

Maybe that is not necessary.  If implemented, it would also need to
apply to recursive subdirectories.  But if we continue to disable
backups by default in temporary directories, we can just tell people
that if they want backups made in those directories, they have to set
`backup-by-copying' to t.

Sincerely,

Luc.

Re: backup method

[Han Boetes]

Luc Teirlinck wrote:
> I believe that the correct solution in this particular case is
> to make `normal-backup-enable-predicate' use a customizable list
> of directories in which no backups are made, say
> `no-backup-directories'.
> We could then tell people that if they want remove "/temp/" or
> equivalent from the list, they should set `backup-by-copying' to
> t, or we could use a second customizable variable
> `backup-by-copying-directories', to unlink the two problems (at
> least in this particular case).

Well at least we got a clearer picture of what the problem is. But
let me bring some additional factors in to make it even more
complicated.

/tmp is word-writable

Ow yes... It's trivial to setup a little script that checks for
tmpfiles from cron and other software and then make symlinks to
the backupfiles from the editor and get nice copies of whatever is
in them.

So I suggest not making backupfiles in world-writable directories.
At least not in the world-writable directory itself. I setup
~/.tmp/backups as my backups dir here.

Another thing to improve is that if ~/.tmp/backups doesn't exist
it is created with permissions defined by umask.

~% ls -ld ~/.tmp/backups 
drwxr-xr-x  2 han users 4096 jan 29 16:57 /mega/home/han/.tmp/backups/

IMHO it would be better if that directory was created with 700
permissions.



# Han

Re: backup method

[Stefan Monnier]

> So I suggest not making backupfiles in world-writable directories.

Agreed.  It's a security hole since the backup file name is predictable.

> At least not in the world-writable directory itself. I setup
> ~/.tmp/backups as my backups dir here.

Of course the deciding factor is whether the directory where the backup file
will be put is world-writable.


        Stefan

Re: backup method

[Richard Stallman]

    And assuming this problem only needs fixing on unix doesn't feel
    right IMHO.

Supporting Emacs on VMS and Windows is something I leave to others.
I don't want them to take my time away from the GNU system and
free software.

Re: backup method

[Han Boetes]

Richard Stallman wrote:
>     And assuming this problem only needs fixing on unix doesn't feel
>     right IMHO.
>
> Supporting Emacs on VMS and Windows is something I leave to others.
> I don't want them to take my time away from the GNU system and
> free software.

:-)



# Han

Re: backup method

[Han Boetes]

Stefan Monnier wrote:
> Han Boetes wrote:
> > So I suggest not making backupfiles in world-writable
> > directories.
>
> Agreed. It's a security hole since the backup file name is
> predictable.
>
> > At least not in the world-writable directory itself. I setup
> > ~/.tmp/backups as my backups dir here.
>
> Of course the deciding factor is whether the directory where the
> backup file will be put is world-writable.

Of course this problem applies to multiple editors. I also
mentioned this problem to the OpenBSD-project regarding ``mg'' a
mini emacs-clone. And they also recognized the problem. Todd
Miller had the following to say:

Todd Miller wrote:
> I don't think this is the best way to do it. I think what we
> really want is to use mkstemp() to create the temp file and then
> just rename() it to the predictable name. If the rename fails,
> we know someone is trying to attach us. If not, we are safe.

Which is of course an elegant way to solve the whole business.

Check this URL for the whole thread:

  http://marc.theaimsgroup.com/?t=110708237600001&r=1&w=2



# Han

Re: backup method

[Richard Stallman]

    I believe that the correct solution in this particular case is to make
    `normal-backup-enable-predicate' use a customizable list of
    directories in which no backups are made, say `no-backup-directories'.

I have no objection, but note that there is already a hook to replace
the backup predicate entirely--one could replace it with `ignore', for
instance.  (This is in addition to the possibility of editing the
code.)  There is no need for everything in Emacs to be customizable in
several different ways.

But why avoid making backups for /tmp?  Is there some specific reason
why it is bad to make backups for /tmp?  Or was the idea nothing more
than "these files are unimportant anyway"?

Sometimes a program writes a file into /tmp just for it to be
edited, and after you exit Emacs, the program will delete it.
In might be pointless to have backups for these files, but not
disastrous.  With disk space what it is, why should the default
be not to make backups for them?

Re: backup method

[Richard Stallman]

    > So I suggest not making backupfiles in world-writable directories.

This change would be a disastrous surprise.

(My home directory used to be world-writable.  Maybe it still is, on
fencepost--I can't check from here.)

Re: backup method

[Stefan Monnier]

>> So I suggest not making backupfiles in world-writable directories.
> This change would be a disastrous surprise.

> (My home directory used to be world-writable.  Maybe it still is, on
> fencepost--I can't check from here.)

In a world-writable directory, editing with a backup file is dangerous:

- if backup-by-copying is t, then when writing the backup file we may follow
  a symlink (planted by some other user) to some important file.

- if backup-by-copying is nil, the rename-file to create the backup is safe
  (as long as it doesn't do a copy behind the scenes, of course), but the
  main file will be written to in a way that will happily follow a symlink
  (planted by some other user) to some important file.

Here are the possible answers I can think of:

- who cares, we lived with it for so long already.
- I trust other users not to try to play such nasty games.
- don't make backups in world-writable directories (we already do that for
  the most common world-writable directory: /tmp).
- take special precautions when making a backup in
  a world-writable directory (use make-temp-file & rename, for example, or
  save to some other place).


        Stefan

Re: backup method

[Han Boetes]

Richard Stallman wrote:
> But why avoid making backups for /tmp? Is there some specific
> reason why it is bad to make backups for /tmp? Or was the idea
> nothing more than "these files are unimportant anyway"?

Symlink attacks.

If you are going open /tmp/tmpfile, you will also create
/tmp/tmpfile~ as a backup. Now I can do two things:

1)
  touch /home/mine/foo
  chmod 666 /home/mine/foo
  ln -s /home/mine/foo /tmp/tmpfile~

And after the tmpfile creation of your file has been done I can
read the contents of the tempfile.

2)
  ln -s /home/you/.shellrc /tmp/tmpfile~

And making the backup will overwrite your .shellrc.



# Han

Re: backup method

[Richard Stallman]

    - if backup-by-copying is t, then when writing the backup file we may follow
      a symlink (planted by some other user) to some important file.

Simply writing a file in such a directory would raise the same issue,
right?  If so, it really has nothing to do with backups.

I think the fix is to treat files in such directories as precious.
Not because they really are precious, but because the handling of a
precious file might avoid the problem.

However, I think the existing code for basic-save-buffer-2 that handles
file-precious-flag is not entirely correct for this.  It tries to
find a name that does not exist, but doesn't protect against the
possibility that someone might create the name after it tests
but before it uses the name.

I wrote this patch to try to fix it.  I also tried fixing
backup-copy-buffer in a similar way, but isn't perfect;
someone could delete the file and create a symlink in between
the call to write-region and the call to copy-file.
So we would need an "exclusive" option in copy-file too.



*** files.el	28 Jan 2005 09:33:33 -0500	1.744
--- files.el	31 Jan 2005 08:33:15 -0500	
***************
*** 3312,3350 ****
  	  ;; This requires write access to the containing dir,
  	  ;; which is why we don't try it if we don't have that access.
  	  (let ((realname buffer-file-name)
! 		tempname nogood i succeed
  		(old-modtime (visited-file-modtime)))
! 	    (setq i 0)
! 	    (setq nogood t)
! 	    ;; Find the temporary name to write under.
! 	    (while nogood
! 	      (setq tempname (format
! 			      (if (and (eq system-type 'ms-dos)
! 				       (not (msdos-long-file-names)))
! 				  "%s#%d.tm#" ; MSDOS limits files to 8+3
! 				(if (memq system-type '(vax-vms axp-vms))
! 				    "%s$tmp$%d"
! 				  "%s#tmp#%d"))
! 			      dir i))
! 	      (setq nogood (file-exists-p tempname))
! 	      (setq i (1+ i)))
  	    (unwind-protect
! 		(progn (clear-visited-file-modtime)
! 		       (write-region (point-min) (point-max)
! 				     tempname nil realname
! 				     buffer-file-truename)
! 		       (setq succeed t))
! 	      ;; If writing the temp file fails,
! 	      ;; delete the temp file.
! 	      (or succeed
! 		  (progn
! 		    (condition-case nil
! 			(delete-file tempname)
! 		      (file-error nil))
! 		    (set-visited-file-modtime old-modtime))))
! 	    ;; Since we have created an entirely new file
! 	    ;; and renamed it, make sure it gets the
! 	    ;; right permission bits set.
  	    (setq setmodes (or setmodes (cons (file-modes buffer-file-name)
  					      buffer-file-name)))
  	    ;; We succeeded in writing the temp file,
--- 3314,3354 ----
  	  ;; This requires write access to the containing dir,
  	  ;; which is why we don't try it if we don't have that access.
  	  (let ((realname buffer-file-name)
! 		tempname succeed
! 		(umask (default-file-modes))
  		(old-modtime (visited-file-modtime)))
! 	    ;; Create temp files with strict access rights.  It's easy to
! 	    ;; loosen them later, whereas it's impossible to close the
! 	    ;; time-window of loose permissions otherwise.
  	    (unwind-protect
! 		(progn
! 		  (clear-visited-file-modtime)
! 		  (set-default-file-modes ?\700)
! 		  ;; Try various temporary names.
! 		  ;; This code follows the example of make-temp-file,
! 		  ;; but it calls write-region in the appropriate way
! 		  ;; for saving the buffer.
! 		  (while (condition-case ()
! 			     (progn
! 			       (setq tempname
! 				     (make-temp-name
! 				      (expand-file-name "tmp" dir)))
! 			       (write-region (point-min) (point-max)
! 					     tempname nil  realname
! 					     buffer-file-truename 'excl)
! 			       nil)
! 			   (file-already-exists t))
! 		    ;; The file was somehow created by someone else between
! 		    ;; `make-temp-name' and `write-region', let's try again.
! 		    nil)
! 		  (setq succeed t))
! 	      ;; Reset the umask.
! 	      (set-default-file-modes umask)
! 	      ;; If we failed, restore the buffer's modtime.
! 	      (unless succeed
! 		(set-visited-file-modtime old-modtime)))
! 	    ;; Since we have created an entirely new file,
! 	    ;; make sure it gets the right permission bits set.
  	    (setq setmodes (or setmodes (cons (file-modes buffer-file-name)
  					      buffer-file-name)))
  	    ;; We succeeded in writing the temp file,

Re: backup method

[Stefan Monnier]

> Simply writing a file in such a directory would raise the same issue,
> right?  If so, it really has nothing to do with backups.

Agreed.

> I think the fix is to treat files in such directories as precious.
> Not because they really are precious, but because the handling of a
> precious file might avoid the problem.

OK.

> ! 		  (set-default-file-modes ?\700)
> ! 		  ;; Try various temporary names.
> ! 		  ;; This code follows the example of make-temp-file,
> ! 		  ;; but it calls write-region in the appropriate way
> ! 		  ;; for saving the buffer.
> ! 		  (while (condition-case ()
> ! 			     (progn
> ! 			       (setq tempname
> ! 				     (make-temp-name
> ! 				      (expand-file-name "tmp" dir)))
> ! 			       (write-region (point-min) (point-max)
> ! 					     tempname nil  realname
> ! 					     buffer-file-truename 'excl)
> ! 			       nil)
> ! 			   (file-already-exists t))
> ! 		    ;; The file was somehow created by someone else between
> ! 		    ;; `make-temp-name' and `write-region', let's try again.
> ! 		    nil)

Any reason why you didn't use make-temp-file?


        Stefan

Re: backup method

[Richard Stallman]

    Any reason why you didn't use make-temp-file?

make-temp-file creates the file empty.  Between that and writing the
data into it, someone else could delete the file and put in a symlink.
The code I wrote solves that problem.

Re: backup method

[David Kastrup]

Richard Stallman <rms@gnu.org> writes:

>     Any reason why you didn't use make-temp-file?
>
> make-temp-file creates the file empty.  Between that and writing the
> data into it, someone else could delete the file and put in a symlink.

The permissions in world-writable temp directories are almost always
1777 which means that nobody but the owner can delete such a file.
Symlink attacks are only possible when the file name can be guessed by
an outside attacker _before_ the file is created.

It is probably just prehistoric and/or misconfigured systems (and
Windows?) where anybody can delete a file from somebody else in
temporary directories.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: backup method

[Han Boetes]

David Kastrup wrote:
> The permissions in world-writable temp directories are almost
> always 1777 which means that nobody but the owner can delete
> such a file. Symlink attacks are only possible when the file
> name can be guessed by an outside attacker _before_ the file is
> created.

Just a few thoughts.

Perhaps it's an idea to add a function at the C-level that uses
mkstemp?

Since creating a tmp-file with a random name and then moving it to
the proper name is not expensive I see no reason simply do that
for all files.

The mktemp(1) manpage explains very well how normal programs can
create secure files in world-writable dirs.



# Han

Re: backup method

[Richard Stallman]

    The permissions in world-writable temp directories are almost always
    1777 which means that nobody but the owner can delete such a file.

I had forgotten about that feature, so thanks for reminding me.
However, not all world-writable directories are temp directories.

Re: backup method

[David Kastrup]

Richard Stallman <rms@gnu.org> writes:

>     The permissions in world-writable temp directories are almost always
>     1777 which means that nobody but the owner can delete such a file.
>
> I had forgotten about that feature, so thanks for reminding me.
> However, not all world-writable directories are temp directories.

If the directory permissions are 777, then you can't do any further
operation using the file _name_ after doing the creat system call (at
which point its name is known and accessible).  All you want to do
with the file, you have to do via the file descriptor returned by
creat.  The only thing you can safely do with the file name after
creat is deleting it again by name.  Everything else is open to
symlink attacks.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: backup method

[Richard Stallman]

    If the directory permissions are 777, then you can't do any further
    operation using the file _name_ after doing the creat system call (at
    which point its name is known and accessible).  All you want to do
    with the file, you have to do via the file descriptor returned by
    creat.

I designed the patch on that basis.  The file is written using
write-region, and nothing else is written after that.

However, it's true that the modes are set separately later on.  There
is no way to avoid that without further changes in write-region.
I don't feel like writing such changes now, but in principle they
seem to be needed here.