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 get doesn't validate module arguments #71437

Closed
andig opened this issue Jan 25, 2025 · 18 comments
Closed

cmd/go get doesn't validate module arguments #71437

andig opened this issue Jan 25, 2025 · 18 comments
Labels
BugReport Issues describing a possible bug in the Go implementation.

Comments

@andig
Copy link
Contributor

andig commented Jan 25, 2025

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

$ go version
go version go1.24rc1 darwin/arm64

Does this issue reproduce with the latest release?

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

go env Output
$ go env
AR='ar'
CC='clang'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='clang++'
GCCGO='gccgo'
GO111MODULE=''
GOARCH='arm64'
GOARM64='v8.0'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/Users/andig/Library/Caches/go-build'
GODEBUG=''
GOENV='/Users/andig/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/sv/rs_453y57xj86xsbz3kw1mbc0000gn/T/go-build209847769=/tmp/go-build -gno-record-gcc-switches -fno-common'
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMOD='/Users/andig/htdocs/evcc/go.mod'
GOMODCACHE='/Users/andig/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/andig/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/Users/andig/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24rc1.darwin-arm64'
GOSUMDB='sum.golang.org'
GOTELEMETRY='on'
GOTELEMETRYDIR='/Users/andig/Library/Application Support/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/andig/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24rc1.darwin-arm64/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.24rc1'
GOWORK=''
PKG_CONFIG='pkg-config'
GOROOT/bin/go version: go version go1.24rc1 darwin/arm64
GOROOT/bin/go tool compile -V: compile version go1.24rc1
uname -v: Darwin Kernel Version 24.2.0: Fri Dec  6 18:40:14 PST 2024; root:xnu-11215.61.5~2/RELEASE_ARM64_T8103
ProductName:		macOS
ProductVersion:		15.2
BuildVersion:		24C101
lldb --version: lldb-1600.0.39.109
Apple Swift version 6.0.3 (swiftlang-6.0.3.1.10 clang-1600.0.30.1)

What did you do?

Accidentally run (using either 1.24 or gotip)

go get tool github.com/gokrazy/tools/cmd/gok@main

What did you expect to see?

An error message since it should be

go get -tool github.com/gokrazy/tools/cmd/gok@main

and "tool" cannot be added to the go.mod.

What did you see instead?

No error

@andig andig changed the title cmd/go get doesn't validate arguments cmd/go get doesn't validate module arguments Jan 25, 2025
@seankhliao
Copy link
Member

seankhliao commented Jan 25, 2025

tool is a meta package, and go get accepts one or more packages.

closing as working as intended

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Jan 25, 2025
@gabyhelp gabyhelp added the BugReport Issues describing a possible bug in the Go implementation. label Jan 25, 2025
@zigo101
Copy link

zigo101 commented Jan 25, 2025

meta modules should not be go got.

I suggest to reopen this issue.

@zigo101
Copy link

zigo101 commented Jan 25, 2025

@aclements @ianlancetaylor

@seankhliao
Copy link
Member

you can go get all to update all you dependencies, tool is no different

@andig
Copy link
Contributor Author

andig commented Jan 25, 2025

Imho if I can‘t get/update a meta package, trying to do so should result in an error.

@zigo101
Copy link

zigo101 commented Jan 25, 2025

At least tool should not be allowed to be mixed with other ones.

BTW, tools is a better name in my opinion.

@seankhliao
Copy link
Member

go get tool will update the set of tools you have installed (maybe 0).


tool was an explicit decision. #48429 (comment)

@zigo101
Copy link

zigo101 commented Jan 25, 2025

The name is unfortunate in my opinion.

And again, tool should not be allowed to be mixed with other module names.

@zigo101
Copy link

zigo101 commented Jan 27, 2025

Kindly ping @aclements @ianlancetaylor

Do you think it's a good idea to allow meta module names to be mixed with regular module paths?

@seankhliao
Copy link
Member

seankhliao commented Jan 27, 2025

you may want it to set dependencies at specific versions.

go get tool dont.upgrade/this@v0.1.0

@andig
Copy link
Contributor Author

andig commented Jan 27, 2025

Wouldnt that be the exact mistake I have made?

-tool

it should be?

@zigo101
Copy link

zigo101 commented Jan 29, 2025

@seankhliao

go get tool dont.upgrade/this@v0.1.0

Is dont.upgrade/this@v0.1.0 expected to be a tool module or a non-tool module here?

@zigo101
Copy link

zigo101 commented Jan 30, 2025

@aclements @ianlancetaylor @seankhliao

It looks to me that using go get tool to update all tool dependencies is a bad idea.
It should be go get -tool all instead. How do you think?

@zigo101
Copy link

zigo101 commented Jan 30, 2025

@seankhliao

Did you hide my comment in #48429 (comment)?

@zigo101
Copy link

zigo101 commented Feb 1, 2025

It looks that after running

go get -tool github.com/gokrazy/tools/cmd/gok@main

the following two commands are equivalent:

go get -tool github.com/gokrazy/tools/cmd/gok
go get github.com/gokrazy/tools/cmd/gok

In other words, tool dependencies and main dependencies are not distinguishable.
And tools and main code can't depend on different versions of a dependency module.

@zigo101
Copy link

zigo101 commented Feb 1, 2025

It looks the tool meta module name is not much meaningful. All its uses should be used -tool without other module paths instead.

@aclements
Copy link
Member

I can definitely see the potential for confusion between go get -tool x and go get tool x. It follows from usage of go get that has been in place for a long time, but it's still unfortunate. However, it does not follow that "the tool meta module name is not much meaningful." It's just as meaningful as all.

It would not be viable to add a blanket rule that you can't mix meta-module names and regular module paths, since that's sure to break at least some existing usage. So the best we could do is a special case that disallows mixing specifically tool with regular module paths (as suggested by @zigo101).

I'm inclined to leave this as is and not add any more complexity or special cases for this. The consequences of mistaking go get -tool x and go get tool x are surprising and perhaps annoying, but do not seem particularly bad. Since a tool is almost certainly a "main" module, go get tool x doesn't actually add x to your go.mod at all. You're probably going to run go tool x shortly thereafter and it'll tell you "no such tool x", at which point you'll realize something went wrong. It's not exactly an ideal user journey.

Maybe trying to go get a main package should print a useful message. I was surprised I couldn't find any existing issues about this. If we were to do that, I think it would be reasonable to notice the user also said tool and hint that maybe they meant -tool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BugReport Issues describing a possible bug in the Go implementation.
Projects
None yet
Development

No branches or pull requests

5 participants