google / renameio Goto Github PK

WriteFile uses an empty string as the TempFile Directory. Please add this as parameter to WriteFile.

I have a program that used to overwrite input files in-place, and that was lacking atomicity. When switching to renameio, the biggest question for me was - what perm bits should I use? The old os.OpenFile(path, os.O_WRONLY|os.O_TRUNC, 0) call didn't need to change any permissions, so it's not obvious what should I do here.

I don't think there's a way to replace a file and keep its permission bits in an atomic way. For now, I'm just using os.Lstat first to grab the bits, then using them for renameio.WriteFile. I realise that's racy, for example if the permissions change between the two operations, but it seems like an OK tradeoff while keeping it impossible to lose data.

Maybe I'm alone in seeing this as a gap in the README or docs. Do you reckon any tips would be a good addition?

In some situations I need to set modification time for the new file.

Having an Option WithPermissions

func WithModificationTime(time time.Time) Option {
	return optionFunc(func(cfg *config) {
		cfg.modTime = &time
	})
}

that takes provided time into account and updates the resulting file would make it possible.

Moving from Twitter at https://twitter.com/mvdan_/status/1057289983340544002.

https://golang.org/pkg/io/ioutil/#WriteFile suffices for quite a lot of simple cases where one wants to create or overwrite a file with some contents. That generally is four lines of code, including error handling.

Comparatively, the simplest TempFile example takes nine lines, and requires a defer. This makes it less suitable for these simpler use cases, and makes replacing ioutil.WriteFile with this package harder.

It shouldn't be necessary to strictly stick to ioutil.WriteFile's signature, but one advantage of doing that would be the simplicity of porting code over.

This package was developed/tested on Linux.

We should add windows-specific code which does the closest thing to the rename/fsync combination there is.

Contributions very welcome.

The text of package maybe implies that it could run on Windows:

if runtime.GOOS == "windows" {
        return ioutil.WriteFile(filename, data, perm)
}
return renameio.WriteFile(filename, data, perm)

But it wont, since renameio.WriteFile doesn't exist when building on Windows. It should probably use build tags (and go:build directives) instead of runtime.GOOS.

Moved from Twitter at https://twitter.com/mvdan_/status/1057290763254546434

I'm not a big fan of code like:

tf, err := write.TempFile("", "foo")

Reading it for the first time, it seems to say "write a temporary file called foo", similar to how ioutil.TempFile creates a temporary file named after a string.

I think the name should convey "write a file called foo". If there's only going to be this implementation through a temporary file, then I'd just call it write.File.

If there are going to be other PendingFile constructors, I do understand the need to add something to this one's name. Some ideas that come to mind are write.WithTemp, write.ViaTemp, write.TempTo, or even write.NewTemp, since the constructor doesn't actually write anything.

Considering that the package is quite low-level at times, would it be at all possible to add support for the fs.FS interface? For example, to have versions of TempDir and TempFile that accept fs.FS? And perhaps the latter would return a modified version of fs.File?

The README file states the following under subtleties which must be
consider when using rename(3) for atomic file writes:

On POSIX operating systems, the `fsync` system call must be used
to ensure that the `os.Rename()` call will not result in a
0-length file.

And tempfile.go does indeed use fsync(3) through File.Sync() (from
the os package) in the CloseAtomicallyReplace function. This
performs an fsync for the underlying file descriptor of the regular
PendingFile.

I was, however, wondering if this is sufficient. Unfortunately, the
definition of fsync in POSIX.1-2008 seems very vague to me. The POSIX
specification of fsync states the following:

The fsync() function shall request that all data for the open
file descriptor named by fildes is to be transferred to the
storage device associated with the file described by fildes.

But doesn't explicitly mentioned which data this applies to. For
instance, the name of the file is usually (not sure if POSIX mandates
this) not part of the file metadata (e.g. the inode) but instead stored
in the directory file.

The linux man page for fsync is more specific on this issue and states
the following:

Calling fsync() does not necessarily ensure that the entry in
the directory containing the file has also reached disk. For
that an explicit fsync() on a file descriptor for the directory
is also needed.

I would therefore assume that it is necessary to also perform an
fsync(3) operation on the file descriptor for the directory
containing the file (at least on Linux).

To further investigate this I looked at the C implementation of an atomic
write in the text editor vis which seems to
perform an fsync operation on both: the directory containing the file
and the file itself.

I was wondering if this should also be implemented in renameio. If not:
The reasons for not doing so should be documented since this doesn't
seem obvious to me.

On Unix systems the renameio.WriteFile function does not respect the umask(2). This is directly caused by ioutil.TempFile always setting the file mode to 0600, in contrast to ioutil.WriteFile which passes the desired mode to os.OpenFile where it's forwarded to the underlying syscall creating the file after applying the umask.

The active umask can't be looked up without changing it and, optionally, restoring the previous value (see syscall.Umask and man page linked above). This is no good when multiple file operations may be ongoing concurrently. As such I don't see an immediate solution without avoiding ioutil.TempFile.

Reproduction with a umask of 0077 (drop all permissions for the group and others):

$ umask 0077; go run umask.go /tmp/umask0077.txt; stat -c '%A %a %n' /tmp/umask0077.txt*
-rw------- 600 /tmp/umask0077.txt.ioutil
-rw-r--r-- 644 /tmp/umask0077.txt.renameio

Test code:

package main

import (
        "io/ioutil"
        "log"
        "os"

        "github.com/google/renameio"
)

func main() {
        if err := renameio.WriteFile(os.Args[1]+".renameio", nil, 0644); err != nil {
                log.Fatal(err)
        }

        if err := ioutil.WriteFile(os.Args[1]+".ioutil", nil, 0644); err != nil {
                log.Fatal(err)
        }
}

This is not necessarily safe—a new tempdir might have taken place of the old one in between the regular os.RemoveAll call and the deferred one.

chezmoi uses renameio to ensure that files in the user's home directory are replaced atomically.

renameio.TempDir takes the name of the directory where the file replacement will take place so that it can determine a suitable location for the interim file with which the final destination file will be replaced.

chezmoi calls renameio.TempDir once based on the destination directory, but this implicitly assumes that the user's home directory is on a single filesystem. If the user's home directory spans multiple filesystems then different temporary directories will be needed. If the user's home directory is on a single filesystem then the same temporary directory can be re-used.

The safe path is to call renameio.TempDir for every file replacement, but this is very slow (chezmoi might change thousands of files, there's no need to create thousands of temporary directories when only one is needed).

I think renameio needs a function like:

// MaybeNewTempDir returns either one of existingTempDirs if a member (any of
// the keys) can be used as a temporary directory for replacing dest, or a new
// temporary directory otherwise. It is the caller's responsibility to add the
// return value to future calls to MaybeNewTempDir if needed, i.e.
// MaybeNewTempDir does not modify existingTempDirs.
func MaybeNewTempDir(dest string, existingTempDirs map[string]struct{}) string {
  // ...
}

I'm replacing some code with renameio and it relies on knowing that tempfiles are named xxx.tmp. On startup it can delete any .tmp files knowing that they were left over from an interrupted rename. It would be nice to have a way to name my files a similar way with renameio.

With ioutil.TempFile, if I use a pattern of foo.txt the tempfile will be named like foo.txt12345789. (renameio also adds a . prefix, so with renameio.TempFile("", "foo.txt") I would get .foo.txt12345789.) However, ioutil.TempFile has a feature for controlling the name:

The filename is generated by taking pattern and adding a random string to the end. If pattern includes a "*", the random string replaces the last "*".

So in my code I could use a TempFile pattern of foo.txt-*.tmp and get temp files like foo.txt-123456789.tmp. However, renameio is unaware of this ioutil.TempFile feature, so if I do renameio.WriteFile("", "foo.txt-*.tmp"), then renameio will create a target file named foo.txt-*.tmp after the rename.

Separately, but relatedly, I would really prefer that my tempfiles not be named with a leading .. For most of my applications, having them generate hidden files is not what I want.

Here are some preliminary ideas that fix one or more of these issues.

Adding parallel APIs

func WriteFile2(dest, pattern string) string
func TempFileNamed(dir, path, pattern string) (*PendingFile, error)

This would let the user control the pattern that is passed to ioutil.TempFile. (So I could use renameio.TempFileNamed("", "foo.txt", "foo.txt-*.tmp").)

(This would obviously need better names.)

Handling * patterns explicitly

If the user passed * which is handled specially by ioutil.TempFile anyway, let's assume they do not want a file named with *. At minimum, we could just remove the * from the final filename (option 1), or we could do something more, such as removing the * and anything afterwards (option 2).

So foo*.tmp would become:

Additionally, we could imagine dropping the leading . if the user provided a path containing *, on the presumption that they want more control over the tempfile name.

I don't think that this is possible, but I suspect that you'll be able to find the definitive answer.

I want to remove an existing file and replace it with a new file with new contents in a single atomic operation.

The specific case is (chezmoi again...):

I have a file called foo. I want to atomically replace it with encrypted_foo where the contents of encrypted_foo are the previous contents of foo but encrypted with some key. At no time do I want both foo and encrypted_foo to exist (because then the state is ambiguous, with two sources of truth) and neither do I want neither to exist (because then foo is lost).

It is possible that duplicating the directory containing foo and then replacing that directory with a new directory containing encrypted_foo might do the job, but maybe there's an easier method?

Thanks, and feel free to close this issue if it's not possible.

Pretty please :) No need to start tagging releases of course, but I don't think there's a need to avoid Go modules.

There are many methods in the stdlib and in the wild that accept a io.Writer argument. A signature like func Write(w io.Writer) error is idiomatic.
Currently, the WriteFile function of this library accepts a []byte slice. Basically the entire content of the file must be read in memory before writing it.
Are you open to add a function like the one below to solve this problem?

func Write(filename string, wr func(io.Writer) error, perm os.FileMode) error {
	t, err := TempFile("", filename)
	if err != nil {
		return err
	}
	defer t.Cleanup()

	if err := t.Chmod(perm); err != nil {
		return err
	}

	if err := wr(t); err != nil {
		return err
	}

	return t.CloseAtomicallyReplace()
}

There have been a few updates since v0.1.0 was tagged. Could you tag a new version please?

The documentation for renameio.TempDir states:

TempDir checks whether os.TempDir() can be used as a temporary directory for later atomically replacing files within dest.

On macOS, renameio.TempDir can return an unusable temporary directory. For example, my home directory is /Users/twp, and calling renameio.TempDir("/Users/twp") returns "/Users", which I do not have permission to write to. I haven't tested on other operating systems.

This program, adapted from the Many Example demonstrates the problem:

package main

import (
	"fmt"
	"os"
	"path/filepath"

	"github.com/google/renameio"
)

func run() error {
	homeDir := os.Getenv("HOME")
	dir := renameio.TempDir(homeDir)
	t, err := renameio.TempFile(dir, filepath.Join(homeDir, "hello.txt"))
	if err != nil {
		return err
	}
	defer t.Cleanup()
	if _, err := t.Write([]byte("Hello, world!\n")); err != nil {
		return err
	}
	return t.CloseAtomicallyReplace()
}

func main() {
	if err := run(); err != nil {
		fmt.Println(err)
		os.Exit(1)
	}
}

Running this on my system yields:

$ go run main.go
open /Users/.hello.txt549952214: permission denied
exit status 1

Note that renameio.WriteFile works fine - likely because the temporary directory is computed for the single file being written, rather than using a cached value for many writes to the same directory.

I've read a couple issues on Windows support, and I'm not sure what the current support state is.

Anyway, the WriteFile function is broken on Windows, as *os.File.Chmod() used in the function is not supported on Windows:

O_TMPFILE is a Linux-specific flag that seems suited for exactly the purpose of atomic file creation + renaming.

From the manpage:

There are two main use cases for O_TMPFILE:

• Improved tmpfile(3) functionality: race-free creation of temporary files that (1) are automatically deleted when closed; (2) can never be reached via any pathname; (3) are not subject to symlink attacks; and (4) do not require the caller to devise unique names.
• Creating a file that is initially invisible, which is then populated with data and adjusted to have appropriate filesystem attributes (fchown(2), fchmod(2), fsetxattr(2), etc.) before being atomically linked into the filesystem in a fully formed state (using linkat(2) as described above).