[Ecls-list] open :supersede and rename-file advise

[Geo Carncross]

Sorry for replying to all of these at once. If this causes you any
problems, let me know and I'll split it up.

On Nov 26, 2007 4:56 PM, Juan Jose Garcia-Ripoll
<jjgarcia at users.sourceforge.net> wrote:
...

> CLISP, on the other hand, does complain
>
> $ touch foo
> $ touch faa
> $ clisp -x '(progn (rename-file "foo" "faa") (quit))'
> [...]
> *** - RENAME-FILE: file #P"/Users/jjgarcia/faa" already exists

Sadly, on clisp (at least 2.41), there's no RENAME-FILE that can
overwrite an existing file.

I presently do this on clisp:

#+clisp
(ffi:def-call-out
      clisp-unix-rename (:name "rename") (:language :stdc)
      (:library "libc.so.6")
      (:arguments (b ffi:c-string) (a ffi:c-string))
      (:return-type ffi:int))

Disgusting, isn't it? :)

> I just saw that the new semantics was implemented in your patch as an
> optional, keyword argument :error, specifying whether to signal an
> error whether the file exists.

That's right. The idea is that this can be used to directly expose a
rename() that clobbers existing files, and one that doesn't. OpenMCL
does the same thing.

> I also saw that you had specific code for the Windows port. Is this
> required? I mean, ECL uses rename(), a posix function. Does the
> Windows implementation of rename() have a different behavior than what
> posix demands, namely that it atomically replaces the destination?

Yes, the Windows rename() does infact have a different behavior. It's
also different based on the filesystem used, and the version of
Windows- although in subtle ways. The patch I included goes through
all the necessary contortions to have posix-like semantics at least as
far back as Windows 98 on local filesystems, and Windows 2000 (IIRC)
on networked file systems.

You probably ran into this when you did this:

1.43         (jjgarcia 07-Dec-04): #ifdef _MSC_VER
1.43         (jjgarcia 07-Dec-04):      /* MSVC rename doesn't remove
an existing file */
1.43         (jjgarcia 07-Dec-04):      if (access(backupfilename,
F_OK) == 0 && unlink(backupfilename))
1.52         (jgarcia  29-May-06):              FElibc_error("Cannot
remove the file ~S", 1, make_simple_base_string(backupfilename));
1.43         (jjgarcia 07-Dec-04): #endif

This "fix" just hadn't found its way everywhere yet. I think my method
is better because there's no opportunity between the unlink() and the
rename() to lose data.

> This is a different issue, isn't it? I mean, the supersede semantics
> is going to replace the original file when the new one is closed, and
> rename() will then simply wipe out the old one and replace it with the
> new one in an atomic fashion.

Yes, it is a different issue, I suppose.

To implement the :supersede semantics, I'd use the patched
cl_rename_file() routine, and set up its arguments in
ecl_open_stream() in the struct ecl_stream to be dealt with in
cl_close()

If this is a fine way to go, I'll continue submitting more patches
along this. The next one would be modifying ecl_force_output() to
fsync() the file descriptor (and fdatasync() the directory its in on
Linux).

This way, each chunk can be reviewed separately, and all the
intermediate functionality remains exposed so that it can be used for
different things.

> CL:RENAME-FILE does this for you and the Hyperspec does not specify
> whether the rename operation has to fail on an existing file or not
>
> Indeed, if it fails, I find it disturbing since it removes, not adds
> functionality.

The ability to overwrite the file is included. It depends on the
specified value of :IF-EXISTS - the idea being that if you didn't
expect the destination file to exist, this could overwrite it.

Ideally, I'd like to make a restart that included REPLACE as an
option, but that'd extend RENAME-FILE. By keeping the logic there, the
(future) patch to OPEN could benefit from that.

> With a RENAME-FILE that replaces existing files, I can
> include the check if I want to avoid such operations.  The operation remains atomic.

No you cannot. PROBE-FILE might indicate that the file does not exist,
but another process might create it before rename() has an opportunity
to run.

> If I _always_ forbid to replace existing files then I will have to go through
> serious twists (check for existing file, delete it then, call RENAME-FILE again)
> and will lose atomicity.

Or set :IF-EXISTS to T. Or we could change it to provide a restart and
that way if the user was controlling it interactively they could
specify whether to overwrite. Consider that situation as being
especially useful if the "save" operation takes a while- the user gets
an error at the end that the file exists, and do they want to
overwrite? A REPLACE restart could do it right there at the end,
easily.

Deleting the file before rename-file just introduces an extra sequence
point. If the system fails there (or ECL dies, or garbage collects, or
runs out of storage, or someone yanks the plug or whatever) then data
gets destroyed. The point behind this patch is to reduce the number of
places where data can be destroyed -in a manner that won't be
completely incompatible with the existing expectations.

That said, I'd be happy to rewrite the patch to use CEerror() when the
file exists. That way, we could make the default be the current
(clobber) behavior, but if the error isn't specifically handled (say
during batch processing) the user could be prompted.

What do you think about this?

To summarize, the patch I've provided includes the following features:

1. Same behavior on Windows and POSIX systems
2. Predictable behavior on Windows 98 for local filesystems
3. Predictable behavior on Windows 2000 for remote filesystems
4. The ability to prevent a file from being overwritten accidentally

It also has an extension over OpenMCL's behavior by controlling how
the error is propagated. None of these things are *present now*.

I noted that ECL doesn't presently seem to use rename-file for
anything so it's difficult for me to tell whether changing
rename-file's defaults would negatively affect anyone. If you think
it's too controversial, then I'd suggest changing the default to T
instead of NIL. That way, at least does rename() semantics will be
done correctly.