Go: cmd/go: go get -u in module mode can fail and produce output that's not easy to diagnose

[dmitshur]

_This issue is related to issue https://github.com/golang/lint/issues/436. That issue is about a problem in the lint repository. This issue is about a problem in cmd/go._

We added a go.mod file to the lint repository in CL 162913 today, following the process described in https://golang.org/issue/28136#issuecomment-462971974. We quickly learned via an issue report at https://github.com/golang/lint/issues/436 that doing so caused go get -u golang.org/x/lint to begin to fail when in module mode. Previously, go get -u golang.org/x/lint succeeded in module mode. go get golang.org/x/lint, without -u continues to work.

The go.mod file specifies the module path to be "golang.org/x/lint", which matches the import path comment of the lint package. The import path comment means no current Go package can import the lint package at an import path other than golang.org/x/lint successfully. However, older versions of modules can and do.

As @thepudds shared in https://github.com/golang/lint/issues/436#issuecomment-468110230, there is an old module version that refers to lint repository via an incorrect github.com/golang/lint path:

• google.golang.org/[email protected] - https://github.com/grpc/grpc-go/blob/v1.16.0/go.mod#L7

That latest version of the github.com/openzipkin/zipkin-go module is currently v0.1.5. That version requires the bad old grpc version:

https://github.com/openzipkin/zipkin-go/blob/v0.1.5/go.mod#L27

What ends up happening is that installing the lint module ends up pulling in many modules incidentally (x/tools module -> appengine module -> ... -> x/build module -> ...). Some module ends up pulling in the latest module of zipkin-go, which in turn pulls in the bad grpc version. Since -u flag is used, go get fetches the latest pseudo-version of the github.com/golang/lint module, which is now v0.0.0-20190227174305-5b3e6a55c961 and includes a go.mod file stating the module path to be golang.org/x/lint, causing the error:

go: github.com/golang/[email protected]: parsing go.mod: unexpected module path "golang.org/x/lint"

There are a few issues here that should be investigated. For example, the error message from go get -u can be improved to make diagnosing the situation better. Perhaps more.

/cc @bcmills @jayconrod @matloob @thepudds

[bcmills]

the error message from go get -u can be improved to make diagnosing the situation better.

That's #28489.

[bcmills]

Some module ends up pulling in the latest module of zipkin-go, which in turn pulls in the bad grpc version.

One workaround might be to explicitly upgrade google.golang.org/grpc above v1.16.0 in the requirements of golang.org/x/lint. That way, when someone runs go get -u, the invalid path is no longer in the transitive import graph and its module should not be included.

[dmitshur]

@bcmills That sounds like a valid quick fix for golang.org/x/lint. Do you think we should go ahead and implement it (say, tomorrow), or shall we wait a bit for https://github.com/openzipkin/zipkin-go/pull/115 to get merged (and tagged), making it no longer necessary?

[thepudds]

@dmitshur yet another possible solution might be merging
https://golang.org/cl/160837, but (a) I haven't looked closely enough to know if that is true, and (b) that might be more work than other possible solutions already on the table.

edit: sorry, scratch that, I think that is actually not the right CL... but I had thought there was another pending CL that would further cut down on more of the golang.org/x/... dependencies, but not sure.

[dmitshur]

@thepudds I had the same idea. I plan to prioritize getting that CL submitted tomorrow, as I think it will be very helpful for the x/tools module and everything that requires it. /cc @matloob

Edit: I think it is the right CL, is it not? It removes appengine and x/sync from required modules, leaving only x/net.

[thepudds]

@dmitshur if you tell me it is the right CL, I would believe you. ;-)

It does seem to be the one: https://go-review.googlesource.com/c/tools/+/160837/2/go.mod

[thepudds]

@bcmills in https://github.com/golang/lint/issues/436#issuecomment-468127788 you said in part:

go get -u should only upgrade modules in the transitive import graph, so even if the problematic github.com/golang/lint module is in the module graph, as long as it is not in the import graph it should not cause any trouble. (If it does, that's a bug.)

I am not sure how to think about the transitive import graph when the main module is empty (go.mod file, but no *.go files), such as in https://github.com/golang/lint/issues/436#issuecomment-468110230, which then fails go get -u golang.org/x/lint. Probably the transative import graph at that point effectively has pulled in golang.org/x/lint and friends?

Separately, if you start with an empty module and do go get golang.org/x/lint (no -u), things like the module build list and go list -deps seem pretty small:

$ go list -m all
tempmod
github.com/golang/protobuf v1.2.0
golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961
golang.org/x/net v0.0.0-20190213061140-3a22650c66bd
golang.org/x/sync v0.0.0-20181108010431-42b317875d0f
golang.org/x/text v0.3.0
golang.org/x/tools v0.0.0-20190226205152-f727befe758c
google.golang.org/appengine v1.4.0

$ go list -deps -f '{{if not .Standard}}{{.ImportPath}}{{end}}' golang.org/x/lint
golang.org/x/tools/go/ast/astutil
golang.org/x/tools/go/internal/gcimporter
golang.org/x/tools/go/gcexportdata
golang.org/x/lint

But admittedly those two sample commands above alone do not provide a complete view... and also that is not in the context of appengine build tags, so maybe that explains it or maybe test dependencies explain it, but I will confess I am not sure if there is another bug here or not.

go get -u should only upgrade modules in the transitive import graph

I don't know if the fact that so much gets upgraded if you do go get -u after doing go get golang.org/x/lint on an otherwise empty module is #29702, or a variation of that, or a separate bug, or "expected" because there are actually a bunch of modules in the transitive import graph...

[bcmills]

It sounds like the cmd/go part of this issue is mostly covered by #28489. The only other thing I would expect cmd/go to do is perhaps try older tags to see if the go.mod path is different in them, but in this case github.com/golang/lint is untagged.

Closing as duplicate of #28489, and let's keep the lint-specific discussion on golang/lint#436.

[bcmills]

I am not sure how to think about the transitive import graph when the main module is empty (go.mod file, but no *.go files), such as in golang/lint#436 (comment), which then fails go get -u golang.org/x/lint.

go get -u golang.org/x/lint says “add the module containing package golang.org/x/lint to the main module and upgrade the modules that provide the transitive imports of that package”. That's why it still affects the main module.

[thepudds]

Hi @bcmills

go get -u should only upgrade modules in the transitive import graph, so even if the problematic github.com/golang/lint module is in the module graph, as long as it is not in the import graph it should not cause any trouble. (If it does, that's a bug.)

What is an easy-ish way to see the transitive import graph, and does that "transitive import graph" as used in your quote at the top of this comment include test-only dependencies? (For example, something like go list -deps -f '{{if not .Standard}}{{.ImportPath}}{{end}}' golang.org/x/lint I think does not include test-only dependencies).

It seems more clear how the the _module_ graph gets so large for the current golang.org/x/lint... but when you say " go get -u should only upgrade modules in the transitive import graph", is it clear what in golang.org/x/lint is causing the transitive _import_ graph to be so large? (Maybe I missed something obvious; less fun to look at these types of questions when some of the standard commands error out).

[bcmills]

What is an easy-ish way to see the transitive import graph […]?

go list -deps -test $pkg, with whatever -f flags you like.

does that "transitive import graph" as used in your quote at the top of this comment include test-only dependencies?

Yes, I believe so.

[bcmills]

is it clear what in golang.org/x/lint is causing the transitive _import_ graph to be so large?

Yes: https://github.com/golang/protobuf/issues/806.

[bcmills]

Well, that was one source. There are apparently others.

[thepudds]

FWIW, go list -deps -test also produces a small list:

$ go list -deps -test -f '{{if not .Standard}}{{.ImportPath}}{{end}}' golang.org/x/lint/...
golang.org/x/tools/go/ast/astutil
golang.org/x/tools/go/internal/gcimporter
golang.org/x/tools/go/gcexportdata
golang.org/x/lint
golang.org/x/lint/golint
golang.org/x/lint [golang.org/x/lint.test]
golang.org/x/lint.test

Separately, Bryan mentioned #26902 is potentially related.

[thepudds]

@dmitshur FYI, re-running the replace-based workaround that worked for me two days ago in https://github.com/golang/lint/issues/436#issuecomment-468126102 now fails with:

go: github.com/go-resty/[email protected]: parsing go.mod: unexpected module path "gopkg.in/resty.v1"

(Perhaps better for this comment to be on golang/lint#436, but adding it here because golang/lint#436 is now a fairly high volume issue).

Commands

mkdir /tmp/scratchpad/lint-test-workaround-rerun-1

cd /tmp/scratchpad/lint-test-workaround-rerun-1

export GOPATH=/tmp/scratchpad/gopath-for-lint-workaround-rerun-1

go mod init tempmod

echo 'replace google.golang.org/grpc => google.golang.org/grpc v1.19.0' >> go.mod

echo 'replace github.com/openzipkin/zipkin-go => github.com/openzipkin/zipkin-go b722d64 // current master' >> go.mod

go get -u golang.org/x/lint

[thepudds]

Haven't looked at this carefully, but the resty mismatch might now be coming in via grpc-gateway:

github.com/grpc-ecosystem/grpc-gateway/@v/v1.8.0.mod:   github.com/go-resty/resty v1.9.0
github.com/grpc-ecosystem/grpc-gateway/@v/v1.8.0.mod:replace github.com/go-resty/resty => gopkg.in/resty.v1 v1.9.0

It looks like https://github.com/grpc-ecosystem/grpc-gateway released v1.8.0 about 5 hours ago. It looks like that might be the first release with go.mod. Note that it uses github.com/go-resty/resty in the require section of its go.mod, but has a replace github.com/go-resty/resty => gopkg.in/resty.v1 v1.9.0.

[thepudds]

Question: could this be viewed as a non-fatal error? In the original example reported in golang/lint#436, I think there were other require directives for google.golang.org/grpc for versions higher than the problematic google.golang.org/[email protected]. In other words, the problematic
google.golang.org/[email protected] I think was not going to end up in the build list if the build list had been allowed to be constructed and the build had been allowed to happen instead of a fatal error.