natefinch/atomic

atomic is a go package for atomic file writing

atomic's Issues

Doesn't work on Docker or Vagrant shared volume

Calling atomic.FileWrite has no effect if the provided filename is in a Docker shared --volume or a Vagrant synced_folder. In other directories inside Docker and/or Vagrant (or both, or neither), the same code behaves as expected.

Fails if your temp directory is on a different physical drive

The code currently writes a temp file in your temp dir and then tries to rename it over to the real filename. However, this fails if your temp directory is on a different physical drive (this makes sense, there's no atomic way to actually transfer bytes from one disk to another).

Will require a fix to write the temp file to the same directory as the real file, with a unique name, and then rename.

Allow user to wrap an existing write function

First off, thank you @natefinch and all for putting this tool together!

I have the following usecase and wonder if this project can be extended to support it: today, atomic.WriteFile() uses io.Copy() to do the actual I/O to the temporary file. io.Copy is called with os.File and io.Reader, which makes a lot of sense for most cases. I have a particular case where I would like to override the I/O behavior, but still have the file write be atomic.

Can we consider introducing a new function NewWriter() (or WrapWriter()), like so:

func NewWriter(w func(filename string, r io.Reader) error) func(string, io.Reader) error {
  return func(...) error {
    // Create temp file
    // Call w
    // Sync and rename resultant file
}

Is this within the purview of the project? If so and if people are happy with this proposal, I am happy to submit a PR for this. Thanks!

Consider tagging the master branch (ir)regulary

... so (important) changes (like #5) reach apps using this lib via e.g. @dependabot.

Question: why is MOVEFILE_WRITE_THROUGH needed?

In the current os.Rename implementationm MoveFileEx is used but with a different flags: https://cs.opensource.google/go/go/+/refs/tags/go1.18.3:src/internal/syscall/windows/syscall_windows.go;l=293

So the difference is whether MOVEFILE_WRITE_THROUGH is used or not. I was wondering why this is needed. If this is needed, should we commit this flag to the Go standard library?

Thanks,

Handling long filenames on Windows

It appears that long file names (longer than MAX_PATH which is 260 characters) passed to atomic.WriteFile are not automatically prefixed with \\?\ which makes them fail.

I have a workaround in my package:
https://github.com/kopia/kopia/blob/59b0464931158fb0fdd436717788ee646327dd65/internal/atomicfile/atomicfile.go

Would it make sense to make similar change here?

No way to control file permissions on created files

Since this library uses ioutil.TempFile, all files are created with 0600 permission (https://golang.org/src/io/ioutil/tempfile.go?s=1419:1477#L65)

[question]don't the atomic package need to call fsync?

Hi, I'm reading this package.

Modern OSes probably has dirty buffer, so I think WriteFile needs to flush the buffer with fsync or fdatasync.
Windows might need to replace these syscalls with FlushFileBuffers.