golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.22k stars 17.7k forks source link

cmd/go: go clean -cache silently ignores GOCACHE relative path #69997

Open mneverov opened 1 month ago

mneverov commented 1 month ago

Go version

go version go1.23.1 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/mneverov/.cache/go-build'
GOENV='/home/mneverov/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/mneverov/go/pkg/mod'
GOOS='linux'
GOPATH='/home/mneverov/go'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR='/home/mneverov/tmp'
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.23.1'
GODEBUG=''
GOTELEMETRY='on'
GOTELEMETRYDIR='/home/mneverov/.config/go/telemetry'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/mneverov/go/src/github.com/mneverov/ingress-nginx/go.mod'
GOWORK='/home/mneverov/go/src/github.com/mneverov/ingress-nginx/go.work'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/home/mneverov/tmp/go-build82578650=/tmp/go-build -gno-record-gcc-switches'

What did you do?

GOCACHE='.cache' go clean --cache

What did you see happen?

No output. Neither default gocache directory nor ".cache" directory cleaned up.

What did you expect to see?

Error or empty .cache directory.

gabyhelp commented 1 month ago

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

mneverov commented 1 month ago

I see a couple of issues here:

I'd like to work on this. I see a couple of ways to address the second point.

DefaultDir should return string, bool, error

Pros

  1. Consistent with go convention if a function during execution encounters an error it should return the error
  2. Function with this signature already exists: cfg.EnvFile

    Cons

  3. Looks ugly to me
  4. Change should be propagated to 8 places that uses the function

Introduce func DefaultDirError() error { return defaultDirErr }

Pros

  1. Can be used only where needed (go clean command) to check the error

    Cons

  2. Implicit dependency: should be used only after DefaultDir is called. Can be fixed with the proper comment.
  3. Introduces new public method.
cagedmantis commented 3 weeks ago

cc @matloob @samthanawalla

matloob commented 1 week ago

I don't think we need to document the absolute path requirement in the "Build and test caching" section, but that we should in the GOCACHE entry in the https://pkg.go.dev/cmd/go#hdr-Environment_variables section.

I agree that there's no nice way to address the second point, though my preference is to return string, bool, error. (We already had to change the usages to ignore the ok value when we added it and this is a similar addition)

gopherbot commented 6 days ago

Change https://go.dev/cl/628596 mentions this issue: cmd/go: fail go clean command when failed to find go cache directory