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 <gri@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

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<number> with permissions perm,
+// with <number randomly chosen such that the file name is unique. backupFile returns
+// the chosen file name.
+func backupFile(filename string, data []byte, perm os.FileMode) (string, error) {
+	// create backup file
+	f, err := ioutil.TempFile(filepath.Dir(filename), filepath.Base(filename))
 	if err != nil {
-		return err
+		return "", err
 	}
-	tmpname := f.Name()
+	bakname := f.Name()
 	err = f.Chmod(perm)
 	if err != nil {
 		f.Close()
-		os.Remove(tmpname)
-		return err
+		os.Remove(bakname)
+		return bakname, err
 	}
 
-	// write data to temp file
+	// write data to backup file
 	n, err := f.Write(data)
 	if err == nil && n < len(data) {
 		err = io.ErrShortWrite
@@ -263,12 +277,6 @@ func writeFile(filename string, data []byte, perm os.FileMode) error {
 	if err1 := f.Close(); err == nil {
 		err = err1
 	}
-	if err == nil {
-		err = os.Rename(tmpname, filename)
-	}
-	if err != nil {
-		os.Remove(tmpname)
-	}
 
-	return err
+	return bakname, err
 }