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: CopyFS overwrites existing file in destination. #68895

Closed
mkch opened this issue Aug 15, 2024 · 8 comments
Closed

os: CopyFS overwrites existing file in destination. #68895

mkch opened this issue Aug 15, 2024 · 8 comments
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@mkch
Copy link

mkch commented Aug 15, 2024

Go version

go version go1.23.0 darwin/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/Users/user/Library/Caches/go-build'
GOENV='/Users/user/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT='rangefunc'
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/user/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/user/go'
GOPRIVATE=''
GOPROXY='https://goproxy.cn,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/darwin_amd64'
GOVCS=''
GOVERSION='go1.23.0'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/user/Library/Application Support/go/telemetry'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/dev/null'
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 x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/4v/g652j2zn37lg22crrdwmcqdm0000gn/T/go-build1911013595=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

package main

import (
	"fmt"
	"os"
)

func main() {
	os.Mkdir("src", 0o777)
	os.WriteFile("src/file", ([]byte)("src"), 0o666)
	os.Mkdir("dest", 0o777)
	os.WriteFile("dest/file", ([]byte)("dest"), 0o666)
	os.CopyFS("dest", os.DirFS("src")) // dest/file should not be overwritten.
	file, _ := os.ReadFile("dest/file")
	fmt.Printf("%s", file)
}

The doc of os.CopyFS says: CopyFS will not overwrite existing files, and returns an error if a file name in fsys already exists in the destination.. So dest/file should not be overwritten.

What did you see happen?

src

What did you expect to see?

dest

@seankhliao
Copy link
Member

cc @panjf2000

@seankhliao seankhliao changed the title os: os.CopyFS overwrites existing file in destination. os: CopyFS overwrites existing file in destination. Aug 15, 2024
@seankhliao
Copy link
Member

https://cs.opensource.google/go/go/+/refs/tags/go1.23.0:src/os/dir.go;l=179;drc=559c77592f182a2f77f2d70328cb649609517bd3

should O_TRUNC instead be O_EXCL ?

@panjf2000
Copy link
Member

See also #62484 (comment)

@panjf2000
Copy link
Member

I missed that discussion at the time, but now as far as I'm concerned we should replace O_TRUNC with O_EXCL in order to match the new function comment. @neild @ianlancetaylor @rsc

@panjf2000 panjf2000 added the NeedsFix The path to resolution is known, but the work has not been done. label Aug 16, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/606095 mentions this issue: os: use O_EXCL instead of O_TRUNC in CopyFS to disallow rewriting existing files

@panjf2000
Copy link
Member

@gopherbot please create a backport to 1.23, it's a mismatch of doc and actual behavior.

@gopherbot
Copy link
Contributor

Backport issue(s) opened: #68907 (for 1.23).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@panjf2000 panjf2000 added this to the Go1.23.1 milestone Aug 16, 2024
@dmitshur dmitshur modified the milestones: Go1.23.1, Go1.24 Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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