Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/gofmt: go fmt eats my source code if it gets ENOSPC #8984

Closed
gopherbot opened this issue Oct 23, 2014 · 6 comments
Closed

cmd/gofmt: go fmt eats my source code if it gets ENOSPC #8984

gopherbot opened this issue Oct 23, 2014 · 6 comments
Assignees
Milestone

Comments

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 23, 2014

by tycho@tycho.ws:

What does 'go version' print?

go version go1.2.1 linux/amd64

What steps reproduce the problem?
If possible, include a link to a program on play.golang.org.

1. write some code
2. fill up your disk
3. run `go fmt`

What happened?

go fmt deletes my code, then tells me ENOSPC. go fmt should ideally be smart enough to
realize that (preferably by keeping the old file around until the write of the new file
succeeds) and not delete my source code.
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Oct 23, 2014

Comment 1:

Labels changed: added repo-tools, release-none.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Oct 23, 2014

Comment 2:

go fmt calls gofmt, which reads the file, reformats it in memory, and then does:
                if *write {
                        err = ioutil.WriteFile(filename, res, 0644)
            if err != nil {
                                return err
                    }
            }
WriteFile is just:
// otherwise WriteFile truncates it before writing.
func WriteFile(filename string, data []byte, perm os.FileMode) error {
        f, err := os.OpenFile(filename, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, perm)
        if err != nil {
                return err
        }
        n, err := f.Write(data)
        if err == nil && n < len(data) {
                err = io.ErrShortWrite
        }
        if err1 := f.Close(); err == nil {
                err = err1
        }
        return err
}
So yes, we're overwriting it in place.  I suppose we could write it to a temp file in
the same dir and then try to automatically rename it into place, at least on Unix (see
Windows issue #8914).  But I think we should make a helper function for that rather than
ask each tool to try to do it.

Status changed to Accepted.

@griesemer
Copy link
Contributor

@griesemer griesemer commented Oct 23, 2014

Comment 3:

Perhaps WriteFile should be smart enough and do it for everybody?
@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@rsc rsc removed release-none labels Apr 10, 2015
@rsc rsc changed the title cmd/gofmt: go fmt eats my source code if it gets ENOSPC x/tools/cmd/gofmt: go fmt eats my source code if it gets ENOSPC Apr 14, 2015
@rsc rsc changed the title x/tools/cmd/gofmt: go fmt eats my source code if it gets ENOSPC cmd/gofmt: go fmt eats my source code if it gets ENOSPC Apr 14, 2015
@rsc rsc removed the repo-tools label Apr 14, 2015
@griesemer griesemer self-assigned this Nov 9, 2016
@gopherbot
Copy link
Author

@gopherbot gopherbot commented Nov 9, 2016

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

@bradfitz bradfitz modified the milestones: Go1.8Maybe, Unplanned Nov 10, 2016
@gopherbot gopherbot closed this in b188b4c Nov 10, 2016
@gopherbot
Copy link
Author

@gopherbot gopherbot commented Nov 10, 2016

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

gopherbot pushed a commit that referenced this issue Nov 10, 2016
Follow-up on https://golang.org/cl/33018.

For #8984.

Change-Id: I6655a5537a60d4ea3ee13029a56a75b150f8c8f8
Reviewed-on: https://go-review.googlesource.com/33020
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@gopherbot
Copy link
Author

@gopherbot gopherbot commented Nov 10, 2016

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

gopherbot pushed a commit that referenced this issue Nov 10, 2016
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>
@golang golang locked and limited conversation to collaborators Nov 10, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.