cmd/go: go directive is not only added during go mod init, but also under other conditions that are hard to deduce #30790

dmitshur · 2019-03-12T22:12:08Z

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

$ go version
go version go1.12 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

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

go env Output

$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/dmitshur/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/dmitshur/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/3m/rg2zm24d1jg40wb48wr0hdjw00jwcj/T/go-build766457690=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Run the following commands in one module that has no module requirements:

$ cat go.mod 
module m1

$ go mod tidy

$ cat go.mod 
module m1

go 1.12

Now run the same commands in another module that has 1 or more module requirements:

$ cat go.mod
module m2

require rsc.io/quote v1.0.0

$ go mod tidy

$ cat go.mod
module m2

require rsc.io/quote v1.0.0

Additionally, if the source code in module m2 is modified to no longer import anything from the rsc.io/quote module, and go mod tidy is run, the first time it removes the require rsc.io/quote v1.0.0 line, leaving just module m2. The second time it adds a go directive.

What did you expect to see?

For modules that already exist, and do not already contain a go directive in their go.mod file, the go directive should either be always added automatically (i.e., during any of go build, go test, go mod tidy, etc., operations), or it should never be added.

What did you see instead?

Sometimes it's added, sometimes it isn't added.

Based on investigating the code with @julieqiu and @heschik, we've found that the go directive is always added to go.mod on any go build, go test, go mod tidy, etc., operation whenever the go.mod file has exactly one statement: the module statement. If the go.mod file has at least 1 require, or replace, or exclude directive, in addition to the module statement, then various go operations do not add a go directive.

This behavior is surprising and hard to predict (without looking into the source code to figure it out). I suspect it's an unintentional behavior. The commit message of CL 147281 that implemented this behavior says:

cmd/go: add go statement when initializing go.mod

When creating a go.mod file, add a go statement mentioning the current Go version.

Based on that, I suspect the originally intended behavior was to make it so that a go directive is automatically inserted only when creating a new Go module (via go mod init), but not when working with an existing module that has a go.mod file. I will assign this to @ianlancetaylor to confirm if this is a bug, and perhaps make the decision on how and whether this issue should be resolved.

I think we should fix this and even consider backporting to Go 1.12.1. Doing so should would make the logic of when a go directive is added easier to predict and understand, improving the user experience when in module mode.

/cc @bcmills @matloob

The text was updated successfully, but these errors were encountered:

thepudds · 2019-03-12T22:48:49Z

Likely related: #30043 (comment)

ianlancetaylor · 2019-03-12T22:55:00Z

Before thinking about what should happen, why is the unpredictable go directive "quite disruptive to the user experience when in module mode"? Where does the disruption occur?

ianlancetaylor · 2019-03-12T23:05:04Z

The intent of the CL was to add a "go" statement to a go.mod file, if one did not already exist, when running either go mod init or go mod tidy. Adding a "go" statement when running go mod tidy was suggested by @bcmills during CL review.

So it sounds like there is a bug: if the existing go.mod file contains anything other than just a "module" statement, then running go mod tidy does not add a "go" directive.

dmitshur · 2019-03-12T23:40:27Z

why is the unpredictable go directive "quite disruptive to the user experience when in module mode"?

The behavior I've observed is that various people are either uncertain about when the go directive is actually added (e.g., they describe it as "sometimes"), or they have an incorrect idea of when it's added (e.g., "always added by go mod tidy in Go 1.12" or "only added by go mod init").

If people choose to not add the go directive until figuring out what value it should be (related to #30791), it means they sometimes have to remove the go directive added (e.g., after running go test on a module with 0 required modules) but not other times (e.g., after running go test on a module with 1+ required modules). This encourages people to add the directive with an arbitrary value before understanding the short-term and long-term consequences of doing so.

Where does the disruption occur?

I've observed it occur during code review, and while people are running go commands inside various modules.

Edit: I've also seen it occur during testing of module-related code, as demonstrated by the commit message of CL 168757.

dmitshur · 2019-03-12T23:58:52Z

The intent of the CL was to add a "go" statement to a go.mod file, if one did not already exist, when running either go mod init or go mod tidy. Adding a "go" statement when running go mod tidy was suggested by @bcmills during CL review.

Currently, it also gets added when running go build or go test in a module with a go.mod file that has zero module requirements (and no exclude, replace statements):

$ cat go.mod
module m3
$ go test
PASS
ok  	m3	0.041s
$ cat go.mod
module m3

go 1.12

Is that a bug or intended behavior?

So it sounds like there is a bug: if the existing go.mod file contains anything other than just a "module" statement, then running go mod tidy does not add a "go" directive.

Making both go mod init and go mod tidy add the go directive, if it's not already present, regardless of whether the go.mod file contains just a module statement or more statements, sounds like a valid resolution to this issue.

I've considered suggesting that we change the behavior such that go directive is added only during go mod init and none of the other commands, so that existing modules made in Go 1.11 (without the go directive) don't suddenly start gaining a go directive after users upgrade to Go 1.12. However, if the go directive will ever become mandatory rather than optional, then some version of Go will have to start adding it to existing modules in addition to new modules. So it might as well be Go 1.12.

ianlancetaylor · 2019-03-13T02:27:12Z

Currently, it also gets added when running go build or go test in a module with a go.mod file that has zero module requirements (and no exclude, replace statements):
Is that a bug or intended behavior?

I don't know. I don't really understand how go build and go test are supposed to modify the go.mod file. This likely shouldn't happen. Clearly the inconsistency shouldn't happen, and likely the "go" directive should not be added at all in this case. But I could easily be convinced otherwise.

bcmills · 2019-03-13T13:51:21Z

The empty-requirements behavior is probably a side effect of this case:

go/src/cmd/go/internal/modload/init.go

Lines 376 to 380 in 5f40351

    
           if len(f.Syntax.Stmt) == 1 && f.Module != nil { 
        
           	// Entire file is just a module statement. 
        
           	// Populate require if possible. 
        
           	legacyModInit() 
        
           }

The go directive is added here:

go/src/cmd/go/internal/modload/init.go

Line 424 in 5f40351

addGoStmt()

Perhaps that call should be moved inside the modFile == nil condition.

bcmills · 2019-03-13T13:59:50Z

Or, going in the other direction, perhaps we should always add the go directive if it is missing. That ensures that a module that was successfully built and tested with, say, Go 1.13, continues to build (with the same meaning) with Go 1.14.

thepudds · 2019-03-21T14:53:02Z

CC @jayconrod

gopherbot · 2019-03-22T20:25:37Z

Change https://golang.org/cl/168757 mentions this issue: go/packages/testdata: add go directives to fake module files

Stop fighting the behavior of the go tool when run in these directories. Updates golang/go#30790 Change-Id: I32dfeb0bafa3ed3664500f1768b2293e5257d09b Reviewed-on: https://go-review.googlesource.com/c/tools/+/168757 Run-TryBot: Michael Matloob <matloob@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>

gopherbot · 2019-03-28T17:33:45Z

Change https://golang.org/cl/169877 mentions this issue: cmd/go/internal/modload: do not implicitly add a 'go' directive to an existing go.mod file

bcmills · 2019-03-28T17:37:56Z

@gopherbot, please backport to 1.12: the current behavior is confusing, and the fix is small

gopherbot · 2019-03-28T17:38:02Z

Backport issue(s) opened: #31117 (for 1.12).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

gopherbot · 2019-05-15T14:18:17Z

Change https://golang.org/cl/176925 mentions this issue: cmd/go: always add 'go' directive to the go.mod file if missing

Updates #30790 Fixes #31960 Change-Id: Ib3ac074cf1f98fe69f53af82d2a0441582116570 Reviewed-on: https://go-review.googlesource.com/c/go/+/176925 Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Jay Conrod <jayconrod@google.com>

dmitshur added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. GoCommand cmd/go modules labels Mar 12, 2019

dmitshur added this to the Go1.13 milestone Mar 12, 2019

dmitshur assigned ianlancetaylor Mar 12, 2019

dmitshur mentioned this issue Mar 12, 2019

cmd/go: go directive is insufficiently documented for module authors to be able to make a decision about its value #30791

Closed

dmitshur changed the title ~~cmd/go: go directive is not only added during go mod init, but also under specific and surprising conditions~~ cmd/go: go directive is not only added during go mod init, but also under other conditions that are hard to deduce Mar 13, 2019

bcmills mentioned this issue Mar 28, 2019

cmd/go: builds fail when GOPATH=/dev/null #31100

Closed

gopherbot mentioned this issue Mar 28, 2019

cmd/go: go directive is not only added during go mod init, but also under other conditions that are hard to deduce [1.12 backport] #31117

Closed

gopherbot closed this as completed in a1c481d Apr 19, 2019

bcmills mentioned this issue May 10, 2019

cmd/go: go version stopped getting added to go.mod #31960

Closed

bcmills mentioned this issue Jul 2, 2019

cmd/go: go version is added even when there are no other changes #32889

Closed

golang locked and limited conversation to collaborators May 14, 2020

gopherbot added the FrozenDueToAge label May 14, 2020

rsc unassigned ianlancetaylor Jun 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/go: go directive is not only added during go mod init, but also under other conditions that are hard to deduce #30790

cmd/go: go directive is not only added during go mod init, but also under other conditions that are hard to deduce #30790

dmitshur commented Mar 12, 2019 •

edited

thepudds commented Mar 12, 2019

ianlancetaylor commented Mar 12, 2019

ianlancetaylor commented Mar 12, 2019

dmitshur commented Mar 12, 2019 •

edited

dmitshur commented Mar 12, 2019 •

edited

ianlancetaylor commented Mar 13, 2019

bcmills commented Mar 13, 2019

bcmills commented Mar 13, 2019

thepudds commented Mar 21, 2019

gopherbot commented Mar 22, 2019

gopherbot commented Mar 28, 2019

bcmills commented Mar 28, 2019

gopherbot commented Mar 28, 2019

gopherbot commented May 15, 2019

cmd/go: go directive is not only added during go mod init, but also under other conditions that are hard to deduce #30790

cmd/go: go directive is not only added during go mod init, but also under other conditions that are hard to deduce #30790

Comments

dmitshur commented Mar 12, 2019 • edited

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

Does this issue reproduce with the latest release?

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

What did you do?

What did you expect to see?

What did you see instead?

thepudds commented Mar 12, 2019

ianlancetaylor commented Mar 12, 2019

ianlancetaylor commented Mar 12, 2019

dmitshur commented Mar 12, 2019 • edited

dmitshur commented Mar 12, 2019 • edited

ianlancetaylor commented Mar 13, 2019

bcmills commented Mar 13, 2019

bcmills commented Mar 13, 2019

thepudds commented Mar 21, 2019

gopherbot commented Mar 22, 2019

gopherbot commented Mar 28, 2019

bcmills commented Mar 28, 2019

gopherbot commented Mar 28, 2019

gopherbot commented May 15, 2019

dmitshur commented Mar 12, 2019 •

edited

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

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

dmitshur commented Mar 12, 2019 •

edited

dmitshur commented Mar 12, 2019 •

edited