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>

diff --git a/src/cmd/gofmt/doc.go b/src/cmd/gofmt/doc.go
index 9d0cd328623cb406c470f10fdea230d5a6fa08f2..805e5fbdcfd8637bde81128d08aa0f035442d978 100644 (file)
--- 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 467af87459f51264437f3aae9269057cc8ab86e2..88ee75f52d9e79e8b7657c9bb88574981094c259 100644 (file)
--- 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
 }