proposal: provide ioutil.SafeWriteFile writing to a tmp file before overwriting an existing file

[griesemer]

If ioutil.WriteFile fails for some reason during writing/closing, it may destroy an existing file with the same name before reporting an error.

It's not hard to handle this on a case-by-case basis, but I suspect this is a common scenario for which to protect against and perhaps should be supported by ioutil.

See #8984 for a use case.

Implementation:

// SafeWriteFile is a drop-in replacement for ioutil.WriteFile;
// but SafeWriteFile writes data to a temporary file first and
// only upon success renames that file to filename.
func SafeWriteFile(filename string, data []byte, perm os.FileMode) error {
	// open temp file
	f, err := ioutil.TempFile(filepath.Dir(filename), "tmp")
	if err != nil {
		return err
	}
	err = f.Chmod(perm)
	if err != nil {
		return err
	}
	tmpname := f.Name()

	// write data to temp file
	n, err := f.Write(data)
	if err == nil && n < len(data) {
		err = io.ErrShortWrite
	}
	if err1 := f.Close(); err == nil {
		err = err1
	}
	if err != nil {
		return err
	}

	return os.Rename(tmpname, filename)
}

Better func name welcome.

[bradfitz]

See https://go-review.googlesource.com/c/33018/2/src/cmd/gofmt/gofmt.go#244

[mvdan]

This seems like a good idea. I would prefer it to have a WriteFile prefix, but WriteFileSafe is confusing.

[bradfitz]

Using an adverb would clarify what part is safe (the writing or the file): WriteFileSafely.

But maybe better to just replace the verb: ReplaceFile or something.

[minux]

In light of #17873, we also need to decide whether the proposed function should: 1. avoid rewrite the destination if it's readonly 2. preserve the destination's permission bits and other file system properties like ACL, xattr and the like. Solve the latter point is notoriously non-trivial, and it's something to think about. I think it's probably one of the reason there is no CopyFile function in the std.

[mattn]

I think the temporary file prefer on same disk-partition, same filesytem. How about to put the temporary file into same directory? ex: filepath.Join(filepath.Dir(filename), filepath.Base(filename)) + "~~~".

[mattn]

Sorry, already mentioned by grieseme.

[0xmohit]

In addition to what @minux mentioned, the proposed function should also preserve owner and group information.

[minux]

Ahh, preserving ownership and group is hard. I'm almost thinking that this is too big a problem to fix in std and gofmt needs to choose another fix for ENOSPC problem, namely, make a backup of the original file and then write to the original file unconditionally, and if the write succeed, remove the backup.

[griesemer]

@minux Yes, that's the other approach I was thinking of. It will make updates much slower if there's a lot of files to change. What I have now preserves permissions, but that is only half the story.

[gopherbot]

CL https://golang.org/cl/33098 mentions this issue.

[slrz]

I wonder if the "Safe" name prefix will raise crash safety expectations that won't be fulfilled by the function as proposed. Brad's ReplaceFile nicely avoids this connotation.

[griesemer]

@slrz Yes, as I said in the original comment, we need a better name. Here's a few more ideas:
WriteFileAtomically
WriteFileTransaction
WriteFileAndReplace (or ReplaceFile per @bradfitz)

[0xmohit]

Maybe AtomicWrite or AtomicWriteFile.

[minux]

The word Replace here still doesn't make it clear whether it replaces the content only (i.e. preserving ownership/permission/etc.) or the file as a whole (which might not preserve ownership, permission, etc.)

[dsnet]

It seems that we're heading into the area of bikeshedding, but any name with the words 'Safe' or 'Atomic' in them seems to imply to me heavier guarantees of data persistence than the current proposed implementation gives. I would expect f.Sync() to be called in it's implementation, but that incurs serious performance costs.

[danp]

Probably relevant commentary around atomic writing in #8868 and https://go-review.googlesource.com/c/1591/.

[kardianos]

I haven't seen this mentioned before, but I've used: https://godoc.org/github.com/dchest/safefile before.

[rsc]

This seems very hard and maybe beyond the scope of the library.

[rsc]

We don't see how to do a 100% safe implementation of this on most systems. Gofmt changed to copy the old file to a temp file, then overwrite the old file, and then if the overwrite fails, attempt to rename the tempfile into the old file's place, which is actually different from the initial proposal.

You can have the one that clears permission bits / ACLs on success.
You can have the one that clears permission bits / ACLs on failure.
There doesn't seem to be one that is 100% "overwrite the file (and do nothing else) or else do nothing else." You have to pick which compromise you want that means "safe" in this case. Your choice may not be another person's choice.

If the community converges on a particular semantics that is the "right" one, we can certainly revisit this. But right now we don't know what this should do, especially well enough to put in the standard library.

Objections to declining this proposal?

[mvdan]

You have to pick which compromise you want that means "safe" in this case. Your choice may not be another person's choice.

Would it be sensible for the standard library to include one or many of these functions that compromise in one way or another? From what I understand, you'd want the "right" one (as in not many at once) or at least for most people to agree on which is the better one.

[dsnet]

I would rather see a third party package provide a set of functions that emphasis different design priorities rather than seeing the standard library provide a hodgepodge of different implementations.