Note, this issue was discovered in CL 387014 because the Go build system currently runs go test {module-root}/... invocations in each module root, rather than the shorter (and I expect more commonly used by users) go test ./... invocation. (CC @nigeltao.)

I'm not sure which format is better for TryBots to use (I thought they'd be pretty much interchangeable), and it's not hard for us to change it if desired. But that should be a separate issue.

Also worth noting a difference in error text between when go list is given an import path pattern vs the import path corresponding to the module root (which happens not to have a Go package in it):

$ go list golang.org/x/exp/shiny/...
pattern golang.org/x/exp/shiny/...: golang.org/x/exp@v0.0.0-20190731235908-ec7cb31e5a56: missing go.sum entry

$ go list golang.org/x/exp/shiny    
missing go.sum entry for module providing package golang.org/x/exp/shiny; to add:
	go mod download golang.org/x/exp

Notice the helpful go mod download golang.org/x/exp suggestion is included only in the latter case. Running it re-adds the go.sum entry that tidy removed.

The Go 1.18 draft release notes mention:

The go mod tidy command now retains additional checksums in the go.sum file for modules whose source code is needed to verify that each imported package is provided by only one module in the build list.

Since x/exp/shiny only recently became a separate module (previously all of its packages were inside x/exp), and neither x/exp nor x/exp/shiny modules provide a package at import path golang.org/x/exp/shiny, this makes me think that's possibly related. Maybe the fix for that tidy issue (#47738) is not handling this edge case?

go list ./... stops at module boundaries, whereas go list $(GOWORK=off go list -m)/... does not.
(#43733 proposes to change that, but that's still deep in the backlog and may or may not actually happen.)

Since x/exp/shiny only recently became a separate module (previously all of its packages were inside x/exp), and neither x/exp nor x/exp/shiny modules provide a package at import path golang.org/x/exp/shiny, this makes me think that's possibly related.

Indeed. The reason go list golang.org/x/exp/shiny/... fails is because the go command is trying to determine whether the required version of the x/exp module (which may be arbitrarily different from the version contained in the parent of the working directory) provides any package matching golang.org/x/exp/shiny/....

However, go mod tidy does not preserve the checksum for golang.org/x/exp because it isn't needed: all of the x/exp packages actually imported within x/exp/shiny/... are contained within the shiny module itself, and x/exp is only in the module graph of x/exp/shiny at all as an otherwise-irrelevant transitive dependency of x/mobile. Because x/exp isn't explicitly required and x/exp/shiny is at go 1.18, lazy module loading takes effect and the irrelevant dependency is not used at all during package loading, so its missing checksum doesn't cause any problems when building packages within the module.

So unfortunately, I'm not entirely sure about the right path forward here. It seems wasteful in the general case to add a checksum for the selected version of x/exp, since that module isn't actually relevant to the build and (especially if the repo is private) the checksum could potentially be fairly expensive to compute. On the other hand, it also seems wrong for go list to potentially under-report the set of packages matching the requested pattern. 🤔

go list ./... stops at module boundaries, whereas go list $(GOWORK=off go list -m)/... does not.

Ah, this distinction is very helpful to be aware of and certainly explains this, thanks.

Do you know if it is already documented somewhere? I tried looking at https://pkg.go.dev/cmd/go#hdr-Relative_import_paths (and the rest of that page) and https://go.dev/ref/mod, but couldn't spot it so far. (Perhaps documenting it is a part of future work, if so, that's not a problem.)

I expected the difference to be mentioned in https://pkg.go.dev/cmd/go@master#hdr-Package_lists_and_patterns, but it doesn't appear to be there. 🤔

So is this all Working As Intended?

Note, this issue was discovered in CL 387014

Getting back to this CL, if "go mod tidy" is still a no-op in my x/exp/shiny directory, should I manually edit the go.sum file, in order to pass the trybots?

@nigeltao Take a look at #51283 (comment). I suggest using go mod download golang.org/x/exp to update the go.sum file, instead of manually editing it.

Since this issue is still open, I understand there's a chance something will change in the future, but existing Go versions are working as described above.

I've opened #51455 to consider changing the TryBot behavior to test packages in each module via the "./..." import path pattern instead of "{module-root}/...".

I think #51455 is clearly the correct short-term fix.

I do not see manual edits to the go.sum file as a viable workflow, and I also don't think it's viable to make drastic changes to the cmd/go pattern-match semantics at this time — especially not that could be backported to Go 1.18 and earlier.

For a longer-term fix, we could consider adding a go.work file at the root of x/exp and including all of its modules.

However, that would lose some test fidelity: it would test each package against the MVS solution for the x/exp workspace overall instead of the requirements declared in each x/exp module's go.mod file individually, and the selected versions for the workspace overall may be substantially upgraded as compared to the selected versions for each individual module.

I think if we did that we would also need the TryBots to check that go work sync is a no-op for the workspace.

@bcmills @matloob This issue is marked for 1.19. Should it move to Backlog? Thanks.

I think both the x/exp specifics and coordinator testing strategy (#51455) are in an okay shape by now, and not in scope of the problem described in the initial report here. Since it has the Thinking label, I think it's okay to be in Backlog so there's no timing pressure to find the optimal cmd/go solution here. I'll tentatively move it there as a next step (since I filed the issue), and Bryan or Michael can feel free to refine the milestone further as needed.

Change https://go.dev/cl/455916 mentions this issue: shiny: update mtl version