Skip to content

path/filepath: go test fails when GOROOT isn't writeable #23881

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
Closed
dmitshur opened this issue Feb 16, 2018 · 3 comments
Closed

path/filepath: go test fails when GOROOT isn't writeable #23881

dmitshur opened this issue Feb 16, 2018 · 3 comments
Labels
FrozenDueToAge

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Feb 16, 2018 •
edited
Loading

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

$ go version
go version go1.10 darwin/amd64

Does this issue reproduce with the latest release?

Yes, with Go 1.10 (final).

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


GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/Dmitri/Library/Caches/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/Dmitri/Dropbox/Work/2013/GoLanding:/Users/Dmitri/Dropbox/Work/2013/GoLand"
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/tw/kgz4v2kn4n7d7ryg5k_z3dk40000gn/T/go-build654935185=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

  1. Install Go 1.10 via go1.10.darwin-amd64.pkg installer on a Mac.
  2. Observe that GOROOT isn't writeable by normal user. (I'm happy about this, it helps me avoid accidentally changing its contents.)
  3. Run go test path/filepath.

What did you expect to see?

$ go test path/filepath
ok  	path/filepath	0.027s

What did you see instead?

~ $ go test path/filepath
--- FAIL: TestWalk (0.00s)
	path_test.go:367: makeTree: open testdata/a: no such file or directory
	path_test.go:367: makeTree: open testdata/c: no such file or directory
	path_test.go:367: makeTree: open testdata/d/x: no such file or directory
	path_test.go:367: makeTree: open testdata/d/z/u: no such file or directory
	path_test.go:367: makeTree: open testdata/d/z/v: no such file or directory
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0x10f69c6]

goroutine 17 [running]:
testing.tRunner.func1(0xc4200acc30)
	/usr/local/go/src/testing/testing.go:742 +0x29d
panic(0x11247e0, 0x11fdfd0)
	/usr/local/go/src/runtime/panic.go:505 +0x229
path/filepath_test.mark(0x0, 0x0, 0x1165a00, 0xc420088720, 0xc42000a320, 0x1, 0x1165a00, 0xc420088720)
	/usr/local/go/src/path/filepath/path_test.go:392 +0x26
path/filepath_test.TestWalk.func1(0x114b64f, 0x8, 0x0, 0x0, 0x1165a00, 0xc420088720, 0x20, 0x112eee0)
	/usr/local/go/src/path/filepath/path_test.go:440 +0x5d
path/filepath.Walk(0x114b64f, 0x8, 0xc42000a340, 0xc4200ca000, 0x0)
	/usr/local/go/src/path/filepath/path.go:401 +0x6c
path/filepath_test.TestWalk(0xc4200acc30)
	/usr/local/go/src/path/filepath/path_test.go:443 +0x136
testing.tRunner(0xc4200acc30, 0x11545c8)
	/usr/local/go/src/testing/testing.go:777 +0xd0
created by testing.(*T).Run
	/usr/local/go/src/testing/testing.go:824 +0x2e0
FAIL	path/filepath	0.012s

However, adding sudo, it works okay:

$ sudo go test path/filepath
ok  	path/filepath	0.027s

This happens because TestWalk calls makeTree, which tries to create new files in GOROOT/src/path/filepath directory. Since the GOROOT directory isn't writeable by normal user, creating the test tree fails. Then, trying to walk the tree results in a nil pointer dereference panic.

I'm not sure why TestWalk tries to write in GOROOT at all. Shouldn't it create a temporary directory and use that?
@dgryski
Copy link
Contributor

dgryski commented Feb 17, 2018

I wonder if there should be a "read-only builder"? If we're committed to supporting a RO GOROOT maybe?

@dmitshur
Copy link
Contributor Author

dmitshur commented Feb 17, 2018 •
edited
Loading

For what it's worth, I think it's fine if such an issue comes up occasionally because of no "read-only builder". If it comes up, it can be reported, and then fixed. If no one notices for some time, there's no harm. Running a dedicated builder for the sole purpose of catching this may not be worth the cost of resources (but I'm not in a position to decide, of course).

This is assuming that all such issues are trivial to resolve by writing to a temporary directory rather than GOROOT.

@dmitshur dmitshur mentioned this issue Feb 17, 2018
Merged
mostynb added a commit to mostynb/go that referenced this issue Mar 4, 2018
@mostynb
test: use a temp dir in path/filepath/path_test.go
dab3568 
We should avoid writing temp files to GOROOT, since it might be readonly.

Fixes golang#23881

Change-Id: Iaa38ec404b303f0cf27fdfb7daf1ddd60fd5d1c9
mostynb added a commit to mostynb/go that referenced this issue Mar 4, 2018
@mostynb
path/filepath: use a temp dir in path_test.go
5083e7e 
We should avoid writing temp files to GOROOT, since it might be readonly.

Fixes golang#23881

Change-Id: Iaa38ec404b303f0cf27fdfb7daf1ddd60fd5d1c9
@mostynb mostynb mentioned this issue Mar 4, 2018
Closed
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/98517 mentions this issue: path/filepath: use a temp dir in path_test.go

mostynb added a commit to mostynb/go that referenced this issue Mar 4, 2018
@mostynb
path/filepath: use a temp dir in path_test.go
6ea846c 
We should avoid writing temp files to GOROOT, since it might be readonly.

Fixes golang#23881

Change-Id: Iaa38ec404b303f0cf27fdfb7daf1ddd60fd5d1c9
mostynb added a commit to mostynb/go that referenced this issue Mar 4, 2018
@mostynb
path/filepath: use a temp dir in path_test.go
62d17f3 
We should avoid writing temp files to GOROOT, since it might be readonly.

Fixes golang#23881

Change-Id: I500ca0e0944b6053fd8fd2879ff88f5636424dab
mostynb added a commit to mostynb/go that referenced this issue Mar 5, 2018
@mostynb
path/filepath: use a temp dir in path_test.go
de0211d 
We should avoid writing temp files to GOROOT, since it might be readonly.

Fixes golang#23881

Change-Id: I500ca0e0944b6053fd8fd2879ff88f5636424dab
@golang golang locked and limited conversation to collaborators Mar 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@dgryski @dmitshur @gopherbot