Skip to content

Commit

Permalink
cmd/gofmt: don't overwrite read-only files
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
griesemer committed Nov 10, 2016
1 parent 0457957 commit 35ea53d
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 22 deletions.
5 changes: 4 additions & 1 deletion src/cmd/gofmt/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
50 changes: 29 additions & 21 deletions src/cmd/gofmt/gofmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -236,39 +252,31 @@ 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
}
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
}

0 comments on commit 35ea53d

Please sign in to comment.