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: definition of "go" directive in go.mod conflicts between //go:build proposal and other uses #46201

Closed
zikaeroh opened this issue May 17, 2021 · 4 comments
Labels
Documentation FrozenDueToAge GoCommand cmd/go modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@zikaeroh
Copy link
Contributor

zikaeroh commented May 17, 2021

The first definition of the go directive in go.mod is that the version listed version makes a specific set of features available to the module being used. This is consistent with its use to enable //go:embed (#44557 allowed this), lazy loading (#36460), and various other language features. I saw this first well-explained via https://twitter.com/_myitcv/status/1391819772992659461.

Recent CLs have bumped all of the x/* modules up to state 1.17, e.g. CL 315570 with the statement:

Note that this does not prevent users with earlier go versions from successfully building packages from this module.

Under this definition, it's legal for a module to support versions below what is listed in go.mod, so long as the code is properly guarded by build tags or similar. The Go toolchain itself restricts a set of packages to build with 1.4+, even though go.mod always mirrors the current Go version. Other external packages like golang-migrate can safely support io/fs and embed thanks to build tags.


The second definition of the go directive in go.mod comes from the new //go:build changes (#41184), specifically in the transition section of the proposal for "Go 1.(N+1)" (i.e. Go 1.18)

A new fix in go fix will remove // +build stanzas, making sure to leave behind equivalent //go:build lines. The removal only happens when go fix is being run in a module with a go 1.N (or later) line, which is taken as an explicit signal that the developer no longer needs compatibility with Go 1.(N−1).

This definition explicitly conflicts with the first, as it exactly says that the go directive indicates the minimum version supported by a module, and therefore it's safe to start removing things that would be required for older versions to be supported. This proposal is especially interesting, as it is changing build tags themselves, so therefore cannot be protected by build tags.


While at the moment, I'm not sure that the //go:build changes will cause issues, if "N" in the proposal had instead meant that Go 1.17 was the version at which // +build lines started to be removed, then the two would have clashed, and any preemptive go directive bumping would have caused compatibility issues; future changes may not be as lucky with this as precedent. And, as modules move their go version up per definition 1) to obtain new features, trusting that they can still run with older versions of Go, I can see this becoming more of a problem.

Can there be some agreement on what the go directive actually implies, such that there isn't a conflict here? Or, is the definition used in //go:build spec an oversight?

There might be an issue for this ("what does the go directive mean") already, but I couldn't find it.

@dmitshur dmitshur added GoCommand cmd/go modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 17, 2021
@dmitshur
Copy link
Contributor

There might be an issue for this ("what does the go directive mean") already, but I couldn't find it.

That might be #30791.

CC @bcmills, @matloob, @jayconrod, @ianlancetaylor.

@bcmills
Copy link
Contributor

bcmills commented Dec 6, 2021

I think what was actually implemented here was an explicit go fix -fix=buildtag, not an implicit change in the behavior of cmd/gofmt. We should certainly clarify the documentation, though.

@zikaeroh
Copy link
Contributor Author

zikaeroh commented Dec 6, 2021

That matches the quote from the design doc ("A new fix ..."). However, if anyone were to actually use the definition of the go directive mentioned in that design doc (go 1.N implies 1.(N-1) compatibility is no longer required), they'd go up against all of the other precedent set for the other definition, which was the main thing I wanted to note.

@ianlancetaylor
Copy link
Contributor

It's true that running go fix -buildtag will potentially prevent a package from building with earlier versions of Go when it otherwise would. But running go fix -buildtag is entirely voluntary, and in general should only be done by people who no longer care about supporting older versions. The go version in the go.mod file is providing a useful default for people who choose to run go fix -buildtag. I don't see that as a significant conflict.

I'm going to close this as a dup of #30791. Please comment if you disagree.

@golang golang locked and limited conversation to collaborators Jan 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge GoCommand cmd/go modules 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

6 participants