x/build/cmd/coordinator: improve error message when build fails due to a module not being tidy (and causing failed requests to sum.golang.org)

[dmitshur]

The fix to #32528 was implemented by finding nested modules, and then invoking go test once for each one, in sequence.

This isn't compatible with GO_DISABLE_OUTBOUND_NETWORK=1 being set and converted to a permanent outbound network restriction as soon as the first set of tests run, because the second go test invocation may need to talk to the module mirror and/or checksum database to fetch/verify additional modules.

Edit: More accurately, the network gets restricted early on, before any of the tests run. The network restriction only affects the default checksum database at https://sum.golang.org. The module proxy is not affected because it's treated specially. See #34356 (comment) for more details.

/cc @bradfitz @bcmills

[bradfitz]

The go.sum files are stale.

This doesn't have anything to do with it being the first or second test in a run. The firewall is applied before the command execution regardless.

[dmitshur]

I've looked, and indeed, the firewall is being restricted very early on, when the bc.Exec call is made to invoke go env -json GOMOD here:

https://github.com/golang/build/blob/da876aaec3965bc391792c76f5ccca997637a4ee/cmd/coordinator/coordinator.go#L2521-L2527

However, it is not a problem for fetching modules as I originally thought, those go through the non-restricted internal proxy (i.e., here).

But it is a problem if some go.sum files are missing entries and the Go 1.13 command decides to check with the default checksum database at https://sum.golang.org, it will be blocked:

:: Running [...] "go" "test" "-short" "golang.org/x/build/maintner/cmd/maintserve/..." [...]

verifying google.golang.org/genproto@v0.0.0-20190502173448-54afdca5d873/go.mod: google.golang.org/genproto@v0.0.0-20190502173448-54afdca5d873/go.mod: Get "https://sum.golang.org/lookup/google.golang.org/genproto@v0.0.0-20190502173448-54afdca5d873": dial tcp 108.177.111.141:443: connect: no route to host
verifying github.com/hashicorp/golang-lru@v0.5.1/go.mod: github.com/hashicorp/golang-lru@v0.5.1/go.mod: Get "https://sum.golang.org/lookup/github.com/hashicorp/golang-lru@v0.5.1": dial tcp 108.177.111.141:443: connect: no route to host

In a way, this is a feature, because it helps find missing go.sum entries. But maybe we should be detecting and reporting that in a more user-friendly way? Or maybe we should be tunneling the checksum database through the module proxy?

Moving this to NeedsDecision to figure out how to proceed here. It's certainly not as big of a problem as I originally mistakenly thought.

[bradfitz]

It's failing in a pretty readable way, IMO:

Get "https://sum.golang.org/lookup/google.golang.org/genproto@v0.0.0-20190502173448-54afdca5d873": dial tcp 108.177.111.141:443: connect: no route to host

But if there's an easy way to detect that and fail with a different message, that's fine too.

[dmitshur]

The current error message can be very confusing when one is not familiar with implementation details of x/build (which is most of Go contributors). For a recent example, see https://golang.org/cl/221102.

Edit: I should note that the error message can be more confusing when a repository contains more than 1 module.

I've retitled this issue to make it more clear we want to improve the error text to be more readable.

[dmitshur]

I'm not working on this right now, so unassigning, but we should fix this.

[bcmills]

If you expect the modules to be tidy (or at least complete), it may suffice to set GOFLAGS=-mod=readonly in the builder environment.

[dmitshur]

it may suffice to set GOFLAGS=-mod=readonly in the builder environment

That has become the default in Go 1.16 (#40728), so the error message there looks like this:

... in golang.org/x/tools/gopls

go: downloading github.com/sergi/go-diff v1.1.0
go: downloading honnef.co/go/tools v0.0.1-2020.1.5
go: downloading mvdan.cc/gofumpt v0.0.0-20200802201014-ab5a8192947d
go: downloading mvdan.cc/xurls/v2 v2.2.0
go: downloading github.com/BurntSushi/toml v0.3.1
go: updates to go.sum needed, disabled by -mod=readonly

(From CL 262765.)

[bcmills]

CL 262341 (#41934) should substantially improve the error message generated by cmd/go.