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

go/build: doesn't implement new GO111MODULE=auto logic for 1.13 #32799

Closed
dmitshur opened this issue Jun 27, 2019 · 2 comments
Closed

go/build: doesn't implement new GO111MODULE=auto logic for 1.13 #32799

dmitshur opened this issue Jun 27, 2019 · 2 comments
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@dmitshur
Copy link
Contributor

There was limited module support added to go/build in #26504.

There is logic to determine whether it's appropriate to look at modules in Context.importGo method:

// If modules are not enabled, then the in-process code works fine and we should keep using it.
// TODO(bcmills): This assumes that the default is "auto" instead of "on".
switch os.Getenv("GO111MODULE") {
case "off":
	return errNoModules
case "on":
	// ok
default: // "", "auto", anything else
	// Automatic mode: no module use in $GOPATH/src.
	for _, root := range gopath {
		sub, ok := ctxt.hasSubdir(root, srcDir)
		if ok && strings.HasPrefix(sub, "src/") {
			return errNoModules
		}
	}
}

(Source: src/go/build/build.go#L1004-L1019)

This logic is out of date for 1.13 given the change to GO111MODULE=auto in #31857.

Further below, it looks to see if there is a go.mod file in the source directory or any of its parent directories. That needs to be checked before return errNoModules can be done when srcDir is inside GOPATH/src.

I know the support for modules in go/build is limited, so we need to decide if we should fix this or not. Given that having the wrong logic here can produce incorrect/inconsistent results and the fix should be small, I think it makes sense to do it. Added NeedsDecision label.

/cc @bcmills @jayconrod @rsc @matloob

@dmitshur dmitshur added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. modules labels Jun 27, 2019
@dmitshur dmitshur added this to the Go1.13 milestone Jun 27, 2019
@jayconrod
Copy link
Contributor

I think we should fix this. We don't need to add any new support for modules, just new interpretation of GO111MODULE. Shouldn't be too complicated.

@jayconrod jayconrod self-assigned this Jun 27, 2019
@jayconrod jayconrod added NeedsFix The path to resolution is known, but the work has not been done. release-blocker and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Jun 27, 2019
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/184098 mentions this issue: go/build: don't check if srcDir in GOPATH when deciding to use modules

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

3 participants