Skip to content

os: documentation of Rename doesn't specify the behavior of trying to rename a file to an existing directory #68690

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

Closed
naughtyGitCat opened this issue Aug 1, 2024 · 6 comments
Labels
Documentation Issues describing a change to documentation. FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@naughtyGitCat
Copy link

naughtyGitCat commented Aug 1, 2024

Go version

go version go1.22.2 darwin/arm64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/naughtyGitCat/Library/Caches/go-build'
GOENV='/Users/naughtyGitCat/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/naughtyGitCat/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/naughtyGitCat/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.com.cn,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.2'
GCCGO='gccgo'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/naughtyGitCat/development/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/8g/_t9d_0693kz1pmsfkcb72hj40000gn/T/go-build3876076188=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

code in https://go.dev/play/p/GmHflbjn1uG
same as below

// You can edit this code!
// test file move
// file a.conf, dir a.bak, move `a.conf` to `a.bak`
package main

import (
	"fmt"
	"os"
	"os/exec"
)

func moveFileByUnixMove() {
	fmt.Println("now move file to dir by unix mv....")
	_, err := exec.Command("touch", "b.conf").CombinedOutput()
	if err != nil {
		panic(err)
	}
	err = os.Mkdir("b.bak", 0700)
	if err != nil {
		panic(err)
	}
	_, err = exec.Command("mv", "b.conf", "b.bak").CombinedOutput()
	if err != nil {
		panic(err)
	}
	fmt.Println("now check files in dir...")
	dir, err := os.Open("b.bak")
	if err != nil {
		panic(err)
	}
	defer dir.Close()
	names, _ := dir.Readdirnames(2)
	if len(names) > 0 {
		for _, name := range names {
			fmt.Println(fmt.Sprintf("b.bak dir have file: %s", name))
		}
	} else {
		fmt.Println("dir have no files")
	}
}

func moveFileByGoRename() {
	fmt.Println("now move file to dir by go, os.Rename....")
	_, err := exec.Command("touch", "a.conf").CombinedOutput()
	if err != nil {
		panic(err)
	}
	err = os.Mkdir("a.bak", 0700)
	if err != nil {
		panic(err)
	}
	err = os.Rename("a.conf", "a.bak")
	if err != nil {
		fmt.Println(fmt.Sprintf("move file by os.Rename failed, %v", err))
	}
	fmt.Println("now check files in dir...")
	dir, err := os.Open("a.bak")
	if err != nil {
		panic(err)
	}
	defer dir.Close()
	names, _ := dir.Readdirnames(2)
	if len(names) > 0 {
		for _, name := range names {
			fmt.Println(fmt.Sprintf("a.bak dir have file: %s", name))
		}
	} else {
		fmt.Println("dir have no files")
	}
}

func main() {
	moveFileByUnixMove()
	fmt.Println()
	moveFileByGoRename()
}

What did you see happen?

// test file move
// file: a.conf, dir: a.bak, move a.conf to a.bak

program output:

now move file to dir by unix mv....
now check files in dir...
b.bak dir have file: b.conf

now move file to dir by go, os.Rename....
move file by os.Rename failed, rename a.conf a.bak: file exists
now check files in dir...
dir have no files

What did you expect to see?

move file to dir without error, and

now check files in dir...
b.bak dir have file: b.conf

what did you want

add a dedicate os.Move() command just like mv (i know mv is utility of rename), or add comment to os.Rename suggest developer use os.Rename(file.conf, file.bak/file.conf) is this circumstance

@naughtyGitCat naughtyGitCat changed the title import/path: os move file with Os.Rename failed in some case Aug 1, 2024
@naughtyGitCat naughtyGitCat changed the title move file with Os.Rename failed in some case [BUG] move file with Os.Rename failed in some case Aug 1, 2024
@naughtyGitCat naughtyGitCat changed the title [BUG] move file with Os.Rename failed in some case os: [BUG] move file with Os.Rename failed in some case Aug 1, 2024
@naughtyGitCat naughtyGitCat changed the title os: [BUG] move file with Os.Rename failed in some case os: move file to dir with Os.Rename failed Aug 1, 2024
@naughtyGitCat
Copy link
Author

naughtyGitCat commented Aug 1, 2024

other language supports file move in build-in library

https://docs.python.org/3/library/shutil.html
https://learn.microsoft.com/en-us/dotnet/api/system.io.file.move
https://docs.oracle.com/javase/tutorial/essential/io/move.html

Other language does not support file move, but have more clear comments
https://doc.rust-lang.org/std/fs/fn.rename.html

On Unix, if from is a directory, to must also be an (empty) directory. 
If from is not a directory, to must also be not a directory.

and here is go's comment
https://pkg.go.dev/os#Rename

Rename renames (moves) oldpath to newpath. If newpath already exists and is not a directory, Rename replaces it. OS-specific restrictions may apply when oldpath and newpath are in different directories. .....

@ianlancetaylor
Copy link
Member

The mv command and os.Rename are not the same thing. On Unix systems os.Rename calls the rename system call.

@naughtyGitCat
Copy link
Author

naughtyGitCat commented Aug 1, 2024

The mv command and os.Rename are not the same thing. On Unix systems os.Rename calls the rename system call.

yes, but the comment of os.Rename is unclear,you can see the rust rename comment, which is more helpful

On Unix, if from is a directory, to must also be an (empty) directory. 
If from is not a directory, to must also be not a directory.

@ianlancetaylor
Copy link
Member

Thanks, I sent https://go.dev/cl/602178.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/602178 mentions this issue: os: clarify Rename docs for renaming to a directory

@mknyszek mknyszek added Documentation Issues describing a change to documentation. NeedsFix The path to resolution is known, but the work has not been done. labels Aug 1, 2024
@mknyszek mknyszek changed the title os: move file to dir with Os.Rename failed os: documentation of Rename doesn't specify the behavior of trying to rename a file to an existing directory Aug 1, 2024
@mknyszek mknyszek added this to the Backlog milestone Aug 1, 2024
@dmitshur dmitshur modified the milestones: Backlog, Go1.24 Aug 2, 2024
@dmitshur dmitshur added the FixPending Issues that have a fix which has not yet been reviewed or submitted. label Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Issues describing a change to documentation. FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants