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: go clean -cache silently ignores GOCACHE relative path #69997

Open
mneverov opened this issue Oct 23, 2024 · 5 comments · May be fixed by #70392
Open

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

mneverov opened this issue Oct 23, 2024 · 5 comments · May be fixed by #70392
Assignees
Labels
GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@mneverov
Copy link
Contributor

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.

@mneverov
Copy link
Contributor Author

mneverov commented Oct 23, 2024

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

  1. Looks ugly to me
  2. 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

  1. Implicit dependency: should be used only after DefaultDir is called. Can be fixed with the proper comment.
  2. Introduces new public method.

@cagedmantis cagedmantis added GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Oct 28, 2024
@cagedmantis cagedmantis added this to the Backlog milestone Oct 28, 2024
@cagedmantis
Copy link
Contributor

cc @matloob @samthanawalla

@matloob
Copy link
Contributor

matloob commented Nov 12, 2024

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)

mneverov added a commit to mneverov/go that referenced this issue Nov 16, 2024
Currently, if computing of the go cache directory fails it does not expose the error. Commands like go clean, exec, modindex that use go cache directory continue execution producing incorrect or no result.
This patch adds an error to the return values such that it can be validated on call sites. It also introduces such validation in go clean -cache command to fail execution in case when error occurred.

Fixes golang#69997
@gopherbot
Copy link
Contributor

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

mneverov added a commit to mneverov/go that referenced this issue Dec 10, 2024
Currently, if computing of the go cache directory fails it does not expose the error. Commands like go clean, exec, modindex that use go cache directory continue execution producing incorrect or no result.
This patch adds an error to the return values such that it can be validated on call sites. It also introduces such validation in go clean -cache command to fail execution in case when error occurred.

Fixes golang#69997
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants