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: 'go list' should set the 'GoMod' field to reflect -modfile #36220

Closed
bcmills opened this issue Dec 19, 2019 · 4 comments
Closed

cmd/go: 'go list' should set the 'GoMod' field to reflect -modfile #36220

bcmills opened this issue Dec 19, 2019 · 4 comments
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Dec 19, 2019

The -modfile flag added for #34506 allows users to substitute an arbitrary file in place of the go.mod file found at the module root. We (@jayconrod, @matloob, and I) were talking through the implications of that this morning, and realized that that has a very subtle interaction with go env GOMOD, go list, and tools in general.

With Go 1.13, tools may execute either go env GOMOD go list -m -f '{{.GoMod}}' to discover the location of the main module's go.mod file, and (if in a module) may execute dirname $(go env GOMOD) in order to locate the module root.

The go env command does not support the -modfile flag, so if someone sets, say, GOFLAGS=-modfile=/tmp/foo.mod, then go env GOMOD will continue to report the go.mod file found in the module root, rather than the one in use. That seems fine.

However, go list does support the -modfile flag, so it should report the replacement go.mod file in the GoMod field. Today it does not.

(Tools that need to locate the directory containing the main module should already be using the Dir field from go list, rather than the GoMod field.)

Today I see:

~/go/src$ GOFLAGS=-modfile=/tmp/foo.mod go env GOMOD
/usr/local/google/home/bcmills/go/src/go.mod

~/go/src$ GOFLAGS=-modfile=/tmp/foo.mod go list -m -f '{{.GoMod}}'
/usr/local/google/home/bcmills/go/src/go.mod

However, that should probably instead be:

~/go/src$ GOFLAGS=-modfile=/tmp/foo.mod go env GOMOD
/usr/local/google/home/bcmills/go/src/go.mod

~/go/src$ GOFLAGS=-modfile=/tmp/foo.mod go list -m -f '{{.GoMod}}'
/tmp/foo.mod

CC @heschik @dmitshur (in case this affects goimports and/or godoc)

@bcmills bcmills added this to the Go1.14 milestone Dec 19, 2019
@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Dec 19, 2019
@dmitshur
Copy link
Contributor

dmitshur commented Dec 19, 2019

The GoMod field in the template used by go list -m -f is currently documented as:

GoMod string // path to go.mod file for this module, if any

I think with the addition of the -modfile flag, that description becomes ambiguous. It's not verly clear if it means:

GoMod string // path to [original] go.mod file for this module [that may have been replaced via -modfile flag], if any

Or:

GoMod string // path to [potentially replaced via -modfile flag] go.mod file for this module, if any

I think it'd be worth making the documentation of the GoMod field such that it's not ambiguous whether it's affected by a potential -modfile flag value or not.

@heschi
Copy link
Contributor

heschi commented Dec 19, 2019

goimports and gopls should be unaffected by this either way.

@gopherbot
Copy link

Change https://golang.org/cl/212100 mentions this issue: cmd/go: in 'go list -m', print effective go.mod file

@jayconrod
Copy link
Contributor

@dmitshur Good idea. Added to https://golang.org/cl/212100.

@golang golang locked and limited conversation to collaborators Dec 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Development

No branches or pull requests

5 participants