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: many subcommands take nonsensical build flags #62250

Open
aclements opened this issue Aug 23, 2023 · 1 comment
Open

cmd/go: many subcommands take nonsensical build flags #62250

aclements opened this issue Aug 23, 2023 · 1 comment
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

@aclements
Copy link
Member

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

$ go version
go version go1.21.0 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What did you do?

go clean -n -gcflags=-S

What did you expect to see?

I expected the go command to reject the -gcflags flag because it makes no sense for that subcommand.

What did you see instead?

It quietly succeeded.

Many of the go subcommands are sensitive not just to which packages match the package pattern, but the specific set of source files selected for a build configuration. These call work.AddBuildFlags to register the flags that affect source selection, but this function also registers many flags that only affect actual compilation and linking.

  • The clean command is sensitive to source file selection because it finds all source files in the a package that have a correspondingly named binary in the directory (but are not part of package main) to delete the binaries.
  • The fix command uses work.AddBuildFlags, but appears not to be sensitive to any build flags, because it uses pkg.InternalAllGoFiles() to list source files, which should returns all source files regardless of source file selection.
  • The generate command only searches for go:generate directives in matched source files.

The full set of build flags makes sense for build, install, get, run, test, vet, and list because they all build source files (vet and list -export both invoke the toolchain to create up-to-date export files).

cc @bcmills

@bcmills bcmills added GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 23, 2023
@bcmills bcmills added this to the Backlog milestone Aug 23, 2023
@rsc
Copy link
Contributor

rsc commented Aug 28, 2023

Yes some of the flags are unnecessary, but I am not sure we want to tease apart exactly which ones.

In theory any command that loads packages should support flags like -compiler and -tags, because, like GOOS and GOARCH, those can change which packages a particular wildcard like all matches (if you have packages that only contain //go:build-restricted files for a specific configuration).

Clean used AddBuildFlags because of #7302 in https://go.dev/cl/99130043. That is no longer a concern so it makes sense to remove them.

Fix uses AddBuildFlags because I added it in https://go.dev/cl/240611. I didn't document why but since it was the buildtag fix I think it was to be able to run the fix on packages that only exist in certain configurations. Or maybe I just wanted -n and -x.

Generate used AddBuildFlags for -v, -n, and -x in https://golang.org/cl/125580044.

It's posssible we should just add the build flags unconditionally to every command and stop trying to pick and choose commands, let alone flags.

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

No branches or pull requests

3 participants