x/exp/cmd/gorelease: avoid using -mod=mod and support lazy module loading

[jayconrod]

gorelease uses go list $modpath/... to build a list of packages it needs to load and compare (where $modpath is the path of the module being released). Go 1.16 has -mod=readonly on by default (#40728), so if the module is missing one or more sums, this may result in an error like

go: updates to go.sum needed, disabled by -mod=readonly

We can work around this temporarily by explicitly using -mod=mod.

Go 1.16 will also support lazy module loading (#36460). This may require changes to the way gorelease generates temporary go.mod files for loading packages in downloaded modules, since nothing is imported in those temporary modules. We can work around this temporarily by using go 1.16 in temporary go.mod files.

A possible solution to both problems is to replace the go list call with a walk of the directory tree, listing non-main, non-internal packages (importable by other modules). Once we have such a list, we can generate a temporary .go file that imports those packages, then run go get -d on that temporary package. That will ensure it's safe to call packages.Load later.

cc @jadekler @bcmills

[gopherbot]

Change https://golang.org/cl/255597 mentions this issue: cmd/gorelease: use -mod=mod when listing packages

[bcmills]

FWIW, I think the temporary-go.mod approach will continue to work after lazy loading. (The dependencies of the target module will still all be present in the deepening scan, as if the synthetic module imported something from the target module in some package that we just don't happen to be looking at right now.)

[bcmills]

I think you can fix the -mod=readonly issue by running go get $modpath@$version before go list, unless $modpath is a local replacement (in which case you can copy over its go.sum file).

[bcmills]

Ah, but I guess the go.sum file for the module itself may be incomplete if it has any replace directives. 😞

go mod download should do the trick, though! (It just might download more than is strictly necessary.)

[jayconrod]

I thought about go get $modpath@$version; that would add the sum for the module itself, but we can get that already from go mod download -json, which runs earlier. We copy over the go.sum file already as well.

The bigger problem is if the module's go.sum file is missing or incomplete. We can't use go list -mod=readonly $modpath/... to make a list of importable packages in that case. go mod tidy won't work in the temporary module since there are no imports. go get $modpath/... would have other side effects if there are nested modules.

[bcmills]

Yeah, considering all the alternatives I suspect that the go mod tidy approach you describe in the CL is probably the most robust. (It's also the best approximation of what real users of the module-to-be-released will do, so a more faithful test in that regard.)

[jeanbza]

Quick check - is this discussion centered around making gorelease fetch to file modules that are in the dependency graph but not yet downloaded to file? (so that gorelease can go inspect them)

I ask because I was wondering why we're talking about go.mod / go.sum changing - seems like things that shouldn't happen. And, if it does happen, seems like things that cause gorelease to emit a requirements incomplete message.

---

(sorry for the potentially unintelligent question, just catching up :) )

[jayconrod]

@jadekler Right, this is mainly about gorelease fetching modules that are in the dependency graph but not in go.sum. It also happens due to the way we load the base module (we create a go.mod in a temp temporary, then require the base module version), though we could work around that.

With -mod=mod (the default until now), when we load packages with go list (either directly or via packages.Load), the go command will update go.sum automatically, and we can check whether it changed after the fact.

With -mod=readonly (the new default in Go 1.16), the go command reports a hard error if something is missing from go.sum and doesn't load anything. We can still print a diagnostic in that case, but we won't be able to report API changes.

[jeanbza]

Ahhh. Thank you! This helps greatly.