Add support for Go 1.16

[nacx]

Adds support for Go 1.16 by getting rid of the missing go.sum errors:

bingo get github.com/bufbuild/buf/cmd/buf@v0.36.0
Error: get command failed: get: buf.mod: getting github.com/bufbuild/buf/cmd/buf@v0.36.0: install: go: github.com/bufbuild/buf@v0.36.0: missing go.sum entry; to add it:
	go mod download github.com/bufbuild/buf
: exit 1

In Go 1.16 the go-modules behavior changed and it no longer updates the go.mod and go.sum automatically when building or running go list (https://blog.golang.org/go116-module-changes).
To overcome this, in Go >= 1.16 we run go mod downlaod to make sure it populates what's missing so that go list and go build succeed.

/cc @codefromthecrypt

[codefromthecrypt]

started with a typo pass

[bwplotka]

Nice! Thanks for help. Do you mind if I propose a PR to your PR or send commit? I am playing with it to fix the skipped tests 🤗

[nacx]

Do you mind if I propose a PR to your PR or send commit?

Totally! Whatever works best to get Go 1.16 supported. I'm not the most versed in how Go modules work internally, so the approach in this PR may not be the best. Any help is very welcome!

[bwplotka]

Added PR nacx#1 if you want to merge/check. We can then iterate over other errors. @nacx 🤗

[bwplotka]

Thanks for merging, let's iterate, I will get back to this tomorrow (focusing on book writing today)

[nacx]

Ok, so now the only test that seems to be failing is the thanos one, and the error produced does not contain a hint on the version. Playing a bit manually with some commands, I've found that:

We could try the go mod download as json to resolve the Version field:

$ go mod download -json github.com/envoyproxy/protoc-gen-validate@d6164de4910977d3c3c8dbd9299b5064ea9e7c2b
{
	"Path": "github.com/envoyproxy/protoc-gen-validate",
	"Version": "v0.0.15-0.20190405222122-d6164de49109",
	"Info": "/Users/nacx/src/go/pkg/mod/cache/download/github.com/envoyproxy/protoc-gen-validate/@v/v0.0.15-0.20190405222122-d6164de49109.info",
	"GoMod": "/Users/nacx/src/go/pkg/mod/cache/download/github.com/envoyproxy/protoc-gen-validate/@v/v0.0.15-0.20190405222122-d6164de49109.mod",
	"Zip": "/Users/nacx/src/go/pkg/mod/cache/download/github.com/envoyproxy/protoc-gen-validate/@v/v0.0.15-0.20190405222122-d6164de49109.zip",
	"Dir": "/Users/nacx/src/go/pkg/mod/github.com/envoyproxy/protoc-gen-validate@v0.0.15-0.20190405222122-d6164de49109",
	"Sum": "h1:U1RyeEwqO++6RvIA7bY98AcYIFPQMwrOeWNmw+dvq9w=",
	"GoModSum": "h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c="
}

For the Thanos one, though, since it is using /cmd/thanos and not the root the command fails, but the error still gives a hint on the resolved version:

$ go mod download -json  github.com/thanos-io/thanos/cmd/thanos@f85e4003ba51f0592e42c48fdfdf0b800a23ba74
{
	"Path": "github.com/thanos-io/thanos/cmd/thanos",
	"Version": "f85e4003ba51f0592e42c48fdfdf0b800a23ba74",
	"Error": "github.com/thanos-io/thanos/cmd/thanos@v0.0.0-20210108102609-f85e4003ba51: invalid version: missing github.com/thanos-io/thanos/cmd/thanos/go.mod at revision f85e4003ba51"
}

Is this something we could use when handling these errors? I haven't found a direct way to get the modules pseudo-version given a URL.

[bwplotka]

Thanks, that sounds like some solution, I can look on it only in few hours.

Ideally we don't need to extra calls that's my only worry 🤗

[nacx]

I've been playing around with this a bit, and added a commit with an idea. Basically, when the error does not contain any hint about the module version, we can try find it in the local module cache and resolve that version.
I've added a small (a bit dirty) function that implements this to see if it fixes the remaining e2e tests.

[bwplotka]

Thanks so much, I got out from meetings so looking on this. I wonder if we can do that by default (: Wil push commit in second.

[bwplotka]

I think the fix might be easier then we thought, trying here: #75 (:

[bwplotka]

Pushed potential "fix".

[bwplotka]

Nope, not yet done (:

[bwplotka]

Last standing error is this: https://github.com/golang/go/blob/master/src/cmd/go/internal/modload/query.go#L849

[bwplotka]

golang/go@06538fa

[bwplotka]

Wow. it's so unreliable at this point:

   unexpected error: error while running command "/tmp/bingo-tmpgoenv444360868/bingo get github.com/thanos-io/thanos/cmd/thanos@f85e4003ba51f0592e42c48fdfdf0b800a23ba74"; out: Error: get command failed: get: thanos.mod: getting github.com/thanos-io/thanos/cmd/thanos@f85e4003ba51f0592e42c48fdfdf0b800a23ba74: go get did not found the package (or none of our regexps matches: go: found github.com/thanos-io/thanos/cmd/thanos in (\S*) (\S*),go: downloading (github.com/thanos-io/thanos/cmd) (\S*),go: downloading (github.com/thanos-io/thanos) (\S*),go: (\S*) upgrade => (\S*)): go get: github.com/thanos-io/thanos@v0.11.0 updating to

Time for go mod download -json solution I suppose (the one you proposed @nacx)

[nacx]

Would it be more efficient to walk the local cache as in the latter attempt instead of doing a go mod download? That could save some network calls and be potentially faster? One caveat there is that with that approach, it failed on the upgrade test. For the thanos example it resolved the version properly but the upgrade test failed (I still haven't had time to fully investigate that failure).

[bwplotka]

Yes, that was my worry is that this kind of works, but then we don't know which version "would be" actually chosen by go get, if you have many dirs cached. The only way to know is to... call go mod download again. This is so funky, I wished the internal Go code was not under internal.... (:

[bwplotka]

And I think extra network call is not that bad solution - better than nothing 🙃

[bwplotka]

If you are worried by network calls, check -x and what go get is doing ((: (dozen of calls)

[nacx]

Hmmm Looking at the thanos example in the go mod download:

{
	"Path": "github.com/thanos-io/thanos/cmd/thanos",
	"Version": "f85e4003ba51f0592e42c48fdfdf0b800a23ba74",
	"Error": "github.com/thanos-io/thanos/cmd/thanos@v0.0.0-20210108102609-f85e4003ba51: invalid version: missing github.com/thanos-io/thanos/cmd/thanos/go.mod at revision f85e4003ba51"
}

Note that the error message (and the path) refer to the given input (/cmd/thanos) and not to the directory that contains the actual go.mod (github.com/thanos-io/thanos). Would this be an issue? Because in the code that walks the cache we could properly detect that, but not in this case...

[bwplotka]

Yea I would do the same as I did now:

	// Since we don't know which part of full path is package, which part is module, start from longest and go until we find one.
		for i := len(strings.Split(target.Path(), "/")); i > 3; i-- {
			modulePath = filepath.Dir(modulePath)

			attempted = append(attempted, fmt.Sprintf(`go: downloading (%v) (\S*)`, modulePath))
			re, err = regexp.Compile(attempted[len(attempted)-1])
			if err != nil {
				return errors.Wrapf(err, "regexp compile %v", attempted[len(attempted)-1])
			}
			if re.MatchString(gerr.Error()) {
				break
			}
		}
		if !re.MatchString(gerr.Error()) {
			attempted = append(attempted, `go: (\S*) upgrade => (\S*)`)
			re = regexp.MustCompile(attempted[len(attempted)-1])
		}
	}

Just with go mod download. WDYT?

[bwplotka]

But then again, this solution is short on one more thing: When you don't specify any version, and if go-get fails then we don't know updated version too sooooo, we need something else. ):

[nacx]

Thinking out loud now:
With the local cache thing, we should always have a version. If the version is explicitly set in the command line, we should be good.
If not, given that we're creating empty go.mod files with just one line, we can assume that if no version is specified in the command line go get would resolve to the latest one, and we should be able to get that latest version from the cache as well by inspecting: $(GOMODCACHE)/cache/download/github.com/thanos-io/thanos/@v/list
Is this something that could work?

[bwplotka]

I think that is a solid workaround. Sorry for killing your gomodcache try out - you can recover it from 4cccf28 🤗

[bwplotka]

I think it might not match the go get update logic 1:1 but we can have something very simplified - we could even try to remove -u and -upatch and just have -u in bingo get options - which is very trivial - just get latest. I like this idea, thanks!

[bwplotka]

It's 11pm and I have been working from 9am, so time for me to stop - I will fix this tomorrow if you will not do that earlier. Thanks for help!

And btw, we can then remove output matching totally IMO (for all Go versions)

[nacx]

NP! Let's see if we can make this work :) I have to go AFK now, but I'll give it a try tomorrow if you don't beat me at it (feel free to continue experimenting in this same PR!)

[bwplotka]

Ok, focusing on it now, let's go.

[bwplotka]

[nacx]

And it's working indeed! Yay!

[bwplotka]

Thanks for great idea! Releasing 🤗

[nacx]

If I'm not wrong, this returns the last version found in the file, which seems to be how the file is laid out. Given that the semver library already has functions to compare, is it worth sorting the versions based on semver instead of just returning the last one?
The modules reference does not say anything about how versions are sorted in that file: https://golang.org/ref/mod#module-cache

[bwplotka]

Hm there are those custom versions too (with sha and date). But maybe it follows semver (: Then yes.

In practice what I checked this file is sorted, but you are right it would be better to ensure this just in case as this behavior is not stated anywhere. Let's do in separate PR