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

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

Comments

@zeebo
Copy link
Contributor

@zeebo 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

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Mar 21, 2017

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")
@bradfitz bradfitz closed this Mar 21, 2017
@zeebo

This comment has been minimized.

Copy link
Contributor Author

@zeebo zeebo commented Mar 21, 2017

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

This comment has been minimized.

Copy link
Contributor Author

@zeebo 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

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Mar 21, 2017

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 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

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Mar 21, 2017

@WGH-

This comment has been minimized.

Copy link

@WGH- 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

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Mar 21, 2017

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.

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc 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 and removed NeedsDecision labels Apr 5, 2017
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Apr 13, 2017

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

@gopherbot gopherbot closed this in 0f0a51f May 18, 2017
gopherbot pushed a commit that referenced this issue May 18, 2017
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.
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.