cmd/go,x/tools/gopls: confusing behavior of go mod when go.work is in play

[frankli0324]

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

$ go version
go version go1.20.4 darwin/amd64

What did you do?

with a go workspace https://github.com/frankli0324/workspace_demo (I committed the go.work file for demonstration purposes)

go.work // uses module_a and module_b
module_a
  - go.mod // that *doesn't* require module_b
  - main.go // imports module_b
module_b
  - go.mod
  - lib.go

module_a can be compiled

now remove go.work, compilation fails with:

main.go:6:2: no required module provides package github.com/frankli0324/workspace_demo/module_b; to add it:
	go get github.com/frankli0324/workspace_demo/module_b

also note that go install github.com/frankli0324/workspace_demo/module_a succeeds, and go install github.com/frankli0324/workspace_demo/module_b@remove-go-work also succeeds.
the remove-go-work branch doesn't have a go.work file and module_a doesn't require module_b

What did you expect to see?/What did you see instead?

I expect either gopls to stop complaining about missing modules in go.mod and "you must go mod tidy" or go build/install fail to build the module if go.mod is incorrectly constructed (missing require)

Conclusion

From what I observe, I believe the essence of go work is forcing submodules to build as if there's an replace $declared_module_name => ./$local_path in each of the used go.mods:

in persudo-code:

used = use_directives_from(go.work)
replacements = []
for mod in used:
    mod_name = get_module_name_from(path.join(mod, go.mod))
    replacements.append(mod, mod_name)
for mod in used:
    for m, mod_name in replacements:
        set_replacement_for(mod, mod_name => m)

However, normally if module_a doesn't require module_b, if module_b is replaced anyway, go build still fails:

main.go:6:2: module github.com/frankli0324/workspace_demo/module_b provides package github.com/frankli0324/workspace_demo/module_b and is replaced but not required; to add it:
	go get github.com/frankli0324/workspace_demo/module_b

this is where I think is inconsistent and erroneous.

I'm aware that #52962 raised the issue already, but I believe it's not a duplicate. Also the issue seems wasted since no one is getting anywhere closer to an answer.

[frankli0324]

actually this also raises a question: how should versions be managed if we could build the module_a without requiring module_b? or is this "undefined behavior"? Is this some legacy from the GOPATH era?

[frankli0324]

also related: #53502

[findleyr]

Slotted this for gopls@v0.13.0, when we are revisiting UX of modules/workspaces/etc.

[findleyr]

This should have been fixed by #57979. Now gopls should just do the right thing when you open a Go file, auto-detecting the applicable workspace. Please comment if that is not the case.

[frankli0324]

@findleyr so the behavior of go build and go install is expected? may I ask for an explanation?

[findleyr]

@frankli0324 sorry, let me make sure I understand: you are asking about the behavior of go build with/without a go.work file?

In general, go.work files are only used for development: for example they are not used by go install. That's why go.mod files must require all the modules they use. I can see an argument for making it an error for module_a to import module_b, even if a go.work file uses both, but apparently that's not how it works (probably for simplicity). I don't see a strong reason to allow unused replace directives, so it seems correct to me that this is an error.

CC @bcmills @matloob for the go.work behavior, though as far as I understand it, this is working as intended.

I agree that the behavior of gopls was confusing, since gopls should be able to figure out how to build your workspace with and without a go.work file. That is what was fixed by #57979.

[matloob]

Yes, from what I've read it seems like it's working as intended. go.work will mask missing modules errors but each module should have the proper requirements to be able to build without a go.work file.

[frankli0324]

however this(wrong branch previously, updated) builds without errors with or without a go.work @matloob .
It doesn't have a go.work but builds successfully. module_a doesn't require module_b while importing it, I'm expecting an error for this case.

➜  ~ go install github.com/frankli0324/workspace_demo/module_a@remove-go-work
go: downloading github.com/frankli0324/workspace_demo v0.0.0-20230525143326-5f566b988c3e
go: downloading github.com/frankli0324/workspace_demo/module_a v0.0.0-20230525143326-5f566b988c3e
go: finding module for package github.com/frankli0324/workspace_demo/module_b
go: downloading github.com/frankli0324/workspace_demo/module_b v0.0.0-20230525151631-f91535a39128
go: downloading github.com/frankli0324/workspace_demo v0.0.0-20230525151631-f91535a39128
go: found github.com/frankli0324/workspace_demo/module_b in github.com/frankli0324/workspace_demo/module_b v0.0.0-20230525151631-f91535a39128
➜  ~ module_a
test

So this is a seperate issue then?

[frankli0324]

also, as a matter of fact, CI actions and workflows build the release builds with go build. go build respects go.work by default, go install doesn't. I think this is why many mistook GOWORK for prod purposes.
So is it expected to GOWORK=off before packaging? sounds weird, if go.work is used for dev purposes only, isn't it more reasonable to disable it by default, and users should turn them on in dev environments, similar to how we configure other GOXX variables?

[findleyr]

See also #53502. We could have clearer documentation about best practices for go.work, such as whether they should be checked in to VCS.