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

cmd/go: leave $GOPATH/pkg/mod directories writable (removable) #27161

Closed
jasonkeene opened this issue Aug 22, 2018 · 10 comments

Comments

Projects
None yet
10 participants
@jasonkeene
Copy link
Contributor

commented Aug 22, 2018

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

$ go version
go version go1.11rc1 linux/amd64

Does this issue reproduce with the latest release?

yeap

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

$ uname -a
Linux theia 4.15.0-32-generic #35-Ubuntu SMP Fri Aug 10 17:58:07 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/pivotal/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/pivotal/workspace/mod-experiment/go"
GOPROXY=""
GORACE=""
GOROOT="/home/pivotal/sdk/go1.11rc1"
GOTMPDIR=""
GOTOOLDIR="/home/pivotal/sdk/go1.11rc1/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/pivotal/workspace/mod-experiment/mod/go.mod"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build771004922=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I tried to do rm -r $GOPATH/pkg/mod to reset my modules cache. It failed with a permissions error. Write permission bit is not set. Even rm -rf does not work.

Steps to reproduce:

$ mkdir ~/workspace/mod-experiment
$ cd ~/workspace/mod-experiment
$ export GOPATH=$PWD/go
$ mkdir go mod
$ cd mod
$ go mod init foo
go: creating new go.mod: module foo
$ go get rsc.io/quote
go: finding rsc.io/quote v1.5.2
go: finding rsc.io/sampler v1.3.0
go: finding golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c
go: downloading rsc.io/quote v1.5.2
go: downloading rsc.io/sampler v1.3.0
go: downloading golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c
$ rm -rf $GOPATH/pkg/mod
rm: cannot remove '/home/pivotal/workspace/mod-experiment/go/pkg/mod/golang.org/x/text@v0.0.0-20170915032832-14c0d48ead0c/AUTHORS': Permission denied
...
rm: cannot remove '/home/pivotal/workspace/mod-experiment/go/pkg/mod/rsc.io/sampler@v1.3.0/hello_test.go': Permission denied
$ chmod -R +w $GOPATH/pkg
$ rm -rf $GOPATH/pkg/mod

What did you expect to see?

The $GOPATH/pkg/mod dir should be deleted.

What did you see instead?

It wasn't.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Aug 22, 2018

@bradfitz bradfitz changed the title Can not remove $GOPATH/pkg cmd/go: can not remove $GOPATH/pkg Aug 22, 2018

@jasonkeene

This comment has been minimized.

Copy link
Contributor Author

commented Aug 23, 2018

Interesting, I wasn't aware of that command. I think most folks are more familiar with rm -r though. Especially when deleting the entire $GOPATH it seems silly to require users to remember to do go clean -modcache and most likely forget that command and just do sudo rm -r $GOPATH.

@josharian josharian changed the title cmd/go: can not remove $GOPATH/pkg cmd/go: cannot remove $GOPATH/pkg Aug 23, 2018

@FiloSottile

This comment has been minimized.

Copy link
Member

commented Sep 5, 2018

Read-only modules are useful to add friction to local modifications which cause "works on my machine" issues (which I'm often guilty of in Python for example).

However, we'd probably get most of the benefit by just marking the files read-only, and not the folders, such that modifying files still fails but removing them succeeds.

$ mkdir dir
$ touch dir/file
$ chmod 400 dir/file
$ rm -rf dir

vs

$ mkdir dir
$ touch dir/file
$ chmod -R 500 dir
$ rm -rf dir
rm: dir/file: Permission denied
rm: dir: Directory not empty
@wfernandes

This comment has been minimized.

Copy link
Contributor

commented Sep 6, 2018

So if my understanding is correct, everything should be acceptable if we had the directories within the mod folder have write permissions but the files with read only permissions.

@jasonkeene

This comment has been minimized.

Copy link
Contributor Author

commented Sep 6, 2018

This would work for me.

@agnivade

This comment has been minimized.

Copy link
Member

commented Oct 11, 2018

Paging @rsc for a decision on this. Other folks are stumbling on this too.

What do you think of @FiloSottile's suggestion ?

@rsc

This comment has been minimized.

Copy link
Contributor

commented Oct 25, 2018

I am still concerned that "write a temp file and mv it into the target file" will succeed at the system call level unless the directory is marked read-only (that's definitely true).

It may be that all the popular editors that use this trick are also smart enough to check the writability of the target file themselves.

I am also very surprised that rm -r is somehow too dumb to do the right thing.

But fine, we can leave the directories writable to make up for rm -r's inadequacy.

@rsc rsc changed the title cmd/go: cannot remove $GOPATH/pkg cmd/go: leave $GOPATH/pkg/mod directories writable Oct 25, 2018

@rsc rsc changed the title cmd/go: leave $GOPATH/pkg/mod directories writable cmd/go: leave $GOPATH/pkg/mod directories writable (removable) Oct 25, 2018

@rsc rsc added the NeedsFix label Oct 25, 2018

@rsc

This comment has been minimized.

Copy link
Contributor

commented Oct 25, 2018

I remembered the other reason the directories are unwritable. Tests are by definition run in the directory containing the source code for the package, so that they can access testdata directories as relative paths, and so on. But we absolutely do not want tests to be writing in those directories. Marking the directory unwritable is how we ensure that.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Oct 25, 2018

Working as intended. Will not change directories to be writable.

@ddevault

This comment has been minimized.

Copy link

commented Apr 21, 2019

I'd like to object to this. Making them temporarily unwritable during tests seems fine, but leaving it that way is pretty silly.

The integrity of pkg/mod/** is important

via #27455

No, it's really not. People don't go around rm -r'ing things unless they mean to, and even so - it's just a cache. It's not important.

freeformz added a commit to heroku/heroku-buildpack-go that referenced this issue May 7, 2019

Make files in the go-path readable if they aren't.
Go modules writes a bunch of stuff in $GOPATH/pkg/mod as read-only.
Standard rm -rf throws an error when it encounters read-only files.
So find the files that are read-only and make them not read-only.

See also: golang/go#27161

freeformz added a commit to heroku/heroku-buildpack-go that referenced this issue May 7, 2019

Make files in the go-path readable if they aren't.
Go modules writes a bunch of stuff in $GOPATH/pkg/mod as read-only.
Standard rm -rf throws an error when it encounters read-only files.
So find the files that are read-only and make them not read-only.

See also: golang/go#27161
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.