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.

Sign up for GitHub

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

Jump to bottom

os: Rename error behavior changed from Go 1.7 to Go 1.8 when source file doesn't exist #19647

Closed
zeebo opened this issue Mar 21, 2017 · 9 comments
Closed

os: Rename error behavior changed from Go 1.7 to Go 1.8 when source file doesn't exist #19647

zeebo opened this issue Mar 21, 2017 · 9 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.9

Comments

@zeebo
Copy link
Contributor

zeebo commented Mar 21, 2017

What version of Go are you using (go version)?

go1.8

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH=""
GORACE=""
GOROOT="/Users/jeff/external/golang"
GOTOOLDIR="/Users/jeff/external/golang/pkg/tool/darwin_amd64"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/tn/09dglcts0b9gc5111hnf0_dm0000gn/T/go-build647286002=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"

What did you do?

run this program under go1.7 and go1.8

package main

import (
	"fmt"
	"os"
)

func main() {
	os.Mkdir("destination", 0755)
	os.Create("destination/nonempty")
	err := os.Rename("doesnt-exist", "destination")
	fmt.Println(os.IsNotExist(err), err)
}

https://play.golang.org/p/v4gGML3VCP

What did you expect to see?

the same output

What did you see instead?

$ go1.7 run main.go
true rename doesnt-exist destination: no such file or directory
$ go1.8 run main.go
false rename doesnt-exist destination: file exists

Fixing issue #14527 in CL https://golang.org/cl/31358 caused this (it checks if the destination is a directory before attempting the rename). There are many places in my code base where I optimistically attempt to migrate some old directory path to a new one, and all of them will have to include a check that the old directory exists first to maintain idempotency.
@bradfitz
Copy link
Contributor

Yes, this was intentional.

https://golang.org/doc/go1.8#os says:

On Unix systems, os.Rename will now return an error when used to rename a directory to an existing empty directory. Previously it would fail when renaming to a non-empty directory but succeed when renaming to an empty directory. This makes the behavior on Unix correspond to that of other systems.

The fix that works on both Go 1.7 and Go 1.8 is to write instead:

	err := os.Rename("doesnt-exist", "destination/doesnt-exist")

@zeebo
Copy link
Contributor Author

zeebo commented Mar 21, 2017 •
edited

You misunderstand. The difference is that in go1.7 the error that is returned is that the source does not exist, and in go1.8 the error that is returned is that the destination already exists. That semantic change caused bugs for me, and so I was hoping to maintain the property that if you attempt to rename with a source that doesn't exist, that check is performed first. The mv command has this property, for example.

@zeebo
Copy link
Contributor Author

zeebo commented Mar 21, 2017

To be clear, I'm fine with go1.8 erroring because the destination already exists, it would just be good if it errored that the source does not exist first to maintain compatibility with go1.7.

@bradfitz
Copy link
Contributor

Gotcha.

But now that the behavior has changed in a released version of Go, I'm not sure how much good (or harm) it does to change it again. We've never documented the precedence of errors between the two possible failure cases.

The only reliable thing to do is stat your source file first, before the rename.

I can reopen for a decision, but I don't see a great answer.

@bradfitz bradfitz reopened this Mar 21, 2017
@bradfitz bradfitz added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 21, 2017
@bradfitz bradfitz changed the title Rename error result change when source does not exist os: Rename error behavior changed from Go 1.7 to Go 1.8 when source file doesn't exist Mar 21, 2017
@bradfitz bradfitz added this to the Go1.8.1 milestone Mar 21, 2017
@bradfitz
Copy link
Contributor

/cc @rsc @ianlancetaylor

@WGH-
Copy link

WGH- commented Mar 21, 2017

The only reliable thing to do is stat your source file first, before the rename.

Doesn't sound very reliable due to potential race condition, though.

@bradfitz
Copy link
Contributor

Then do it afterwards, or afterwards also to disambiguate between the two possible errors. Like I said, we've never documented which of the src error vs dest error is returned first if both are bogus.

@bcmills bcmills mentioned this issue Apr 2, 2017
Closed
@rsc
Copy link
Contributor

rsc commented Apr 5, 2017

I think this is not important enough to fix for Go 1.8.1. I don't mind if someone feels strongly and wants to add an Lstat of oldname for Go 1.9.

@rsc rsc modified the milestones: Go1.9, Go1.8.1 Apr 5, 2017
@bradfitz bradfitz added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Apr 5, 2017
@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue May 18, 2017
@bradfitz
os: add some comments and remove an unused variable in rename func
ca598e3 
This slightly clarifies the just-submitted CL 40577.

Updates #19647

Change-Id: I5584ad0e1abbc31796e3e5752351857f2a13d6d7
Reviewed-on: https://go-review.googlesource.com/43625
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators May 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.9
Development

No branches or pull requests
5 participants
@bradfitz @zeebo @rsc @WGH- @gopherbot