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.

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

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
@mneverov
Labels
GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

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.
@gabyhelp
Copy link

Related Issues and Documentation

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

@mneverov
Copy link
Contributor Author

mneverov commented Oct 23, 2024 •
edited
Loading

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
@mneverov
md/go: fail go clean command when failed to find go cache directory
69a6fb5 
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
@mneverov mneverov linked a pull request Nov 16, 2024 that will close this issue
Open
@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mneverov mneverov

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
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

cmd/go: fail go clean command when failed to find go cache directory mneverov/go
5 participants
@cagedmantis @mneverov @gopherbot @matloob @gabyhelp