From 35ea53dcc8d8350898250e87a0b5ffa03e14173e Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 10 Nov 2016 14:40:32 -0800 Subject: [PATCH] cmd/gofmt: don't overwrite read-only files This reverts the changes from https://golang.org/cl/33018: Instead of writing the result of gofmt to a tmp file and then rename that to the original (which doesn't preserve the original file's perm bits, uid, gid, and possibly other properties because it is hard to do in a platform-independent way - see #17869), use the original code that simply overwrites the processed file if gofmt was able to create a backup first. Upon success, the backup is removed, otherwise it remains. Fixes #17873. For #8984. Change-Id: Ifcf2bf1f84f730e6060f3517d63b45eb16215ae1 Reviewed-on: https://go-review.googlesource.com/33098 Run-TryBot: Robert Griesemer Reviewed-by: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/cmd/gofmt/doc.go | 5 ++++- src/cmd/gofmt/gofmt.go | 50 ++++++++++++++++++++++++------------------ 2 files changed, 33 insertions(+), 22 deletions(-) diff --git a/src/cmd/gofmt/doc.go b/src/cmd/gofmt/doc.go index 9d0cd32862..805e5fbdcf 100644 --- a/src/cmd/gofmt/doc.go +++ b/src/cmd/gofmt/doc.go @@ -32,7 +32,8 @@ The flags are: -w Do not print reformatted sources to standard output. If a file's formatting is different from gofmt's, overwrite it - with gofmt's version. + with gofmt's version. If an error occured during overwriting, + the orginal file is restored from an automatic backup. Debugging support: -cpuprofile filename @@ -98,3 +99,5 @@ This may result in changes that are incompatible with earlier versions of Go. package main // BUG(rsc): The implementation of -r is a bit slow. +// BUG(gri): If -w fails, the restored original file may not have some of the +// original file attributes. diff --git a/src/cmd/gofmt/gofmt.go b/src/cmd/gofmt/gofmt.go index 467af87459..88ee75f52d 100644 --- a/src/cmd/gofmt/gofmt.go +++ b/src/cmd/gofmt/gofmt.go @@ -72,13 +72,19 @@ func isGoFile(f os.FileInfo) bool { // If in == nil, the source is the contents of the file with the given filename. func processFile(filename string, in io.Reader, out io.Writer, stdin bool) error { + var perm os.FileMode = 0644 if in == nil { f, err := os.Open(filename) if err != nil { return err } defer f.Close() + fi, err := f.Stat() + if err != nil { + return err + } in = f + perm = fi.Mode().Perm() } src, err := ioutil.ReadAll(in) @@ -116,7 +122,17 @@ func processFile(filename string, in io.Reader, out io.Writer, stdin bool) error fmt.Fprintln(out, filename) } if *write { - err = writeFile(filename, res, 0644) + // make a temporary backup before overwriting original + bakname, err := backupFile(filename+".", src, perm) + if err != nil { + return err + } + err = ioutil.WriteFile(filename, res, perm) + if err != nil { + os.Rename(bakname, filename) + return err + } + err = os.Remove(bakname) if err != nil { return err } @@ -236,26 +252,24 @@ func diff(b1, b2 []byte) (data []byte, err error) { } -// writeFile is a drop-in replacement for ioutil.WriteFile; -// but writeFile writes data to a temporary file first and -// only upon success renames that file to filename. -// TODO(gri) This can be removed if #17869 is accepted and -// implemented. -func writeFile(filename string, data []byte, perm os.FileMode) error { - // open temp file - f, err := ioutil.TempFile(filepath.Dir(filename), "gofmt-") +// backupFile writes data to a new file named filename with permissions perm, +// with