Unfixable failure when the dep tree includes a dependency circle and one of those deps at one time referenced a broken module

[jeffwidman]

A particular go library has a very convoluted dep tree:

• A > B > A
• A > C > D > A & B (and B of course depends on A)

Unfortunately C also depends on another repo that released a version of their library that they later removed, so it broke the go.sum checks: grpc/grpc-go#3945

Breaking this cycle of dependabot death has been a royal pain. It's easy to bump the broken version within C. But then when dependabot tries to walk the deps of the current (fixed) C, it finds D, then finds A, which relies on an older version of C which is broken.

Literally I'm seeing error messages in repo A that look like:

go: github.com/(redacted)/C@v0.5.0 requires
	github.com/(redacted)/D@v0.4.0 requires
	github.com/(redacted)/A@v0.17.0 requires
	github.com/(redacted)/C@v0.1.5 requires
	github.com/(redacted)/D@v0.3.0 requires
	github.com/(redacted)/B@v0.0.57 requires
	github.com/(redacted)/A@v0.0.92 requires
	google.golang.org/grpc@v1.33.0: reading google.golang.org/grpc/go.mod at revision v1.33.0: unknown revision

I've been manually unrolling the loop by slowly bumping things and cutting new versions, but I was wondering if there was a more efficient way to resolve this?

I was a bit surprised in the above error message to see Dependabot ping-ponging between older and older versions of the same library... shouldn't it be smart enough to cache the first time it sees a library version pin and if it later sees older pins of that library to ignore them and eliminate that subtree as completely irrelevant?

Additionally, go.mod supports the concept of // indirect whereby I can specify a minimum version for a dep of a dep... If dependabot prunes subtrees for older versions, then in the top-level repo I could add some // indirect pins which would force dependabot's tree walker to simply skip old/broken versions of underlying deps.

In addition to making it faster to unbreak things, it'd also speed up the dep runs significantly.

I am unclear if this is a bug, or desired behavior...

The only reason I could see it being expected behavior is to guard against an edge case where a dep pins a dep which is incompatible with another dep... but again, if the parent dep has already pinned a library version, then somewhere a human has already explicitly said "this is okay". I suppose there may be some really weird edge case where an incompatibility is added after the fact to a newer release of an underlying dep... but there's still got to be some cleaner way to workaround this.

[jeffwidman]

Here's another error I just saw in repo A:

go: github.com/redacted/B@v0.22.0 requires
	github.com/redacted/A@v0.15.0 requires
	github.com/redacted/B@v0.21.0 requires
	github.com/redacted/A@v0.13.0 requires
	github.com/redacted/B@v0.15.0 requires
	github.com/redacted/C@v0.1.5 requires
	github.com/redacted/D@v0.3.0 requires
	github.com/redacted/E@v0.3.0 requires
	github.com/redacted/B@v0.2.0 requires
	github.com/redacted/A@v0.0.57 requires
	github.com/redacted/B@v0.0.92 requires
	google.golang.org/grpc@v1.33.0: reading google.golang.org/grpc/go.mod at revision v1.33.0:

Again, note how many times the tree walking yo-yo's between A and B... why isn't that cycle cut-short given that we already know we're in repo A? What am I missing?

[jurre]

I think we just shell out to go mod for this behavior? Could you paste more of the error / logs that you're seeing for this case?

[jeffwidman]

We are a GitHub enterprise customer, and I have to be careful about what I can post publicly, so this may be easier to debug via a GitHub private support ticket between you and my @jeffwidman-ns account where I don't have to be paranoid about redacting everything... that way you may also be able to directly see the relevant go.mod files... I don't know what's easiest.

Here's some examples:

• https://github.com/netSkope/hive/network/updates/111846175
• https://github.com/netSkope/phoenix-test-lib/network/updates/111846375

Basically, if I'm understanding this correctly, because of the circular dependency, that broken dep will always be reachable because dependabot just keeps yo-yo'ing between older and older versions that depend on each other until it finally finds the version that pulls in the broken dep which causes go mod tidy to bomb out.

We would really like to get to the bottom of this and get it fixed, so just let me know what I can do to assist here.

[jurre]

We are a GitHub enterprise customer, and I have to be careful about what I can post publicly, so this may be easier to debug via a GitHub private support ticket between you and my @jeffwidman-ns account where I don't have to be paranoid about redacting everything... that way you may also be able to directly see the relevant go.mod files... I don't know what's easiest.

Here's some examples:

• https://github.com/netSkope/hive/network/updates/111846175
• https://github.com/netSkope/phoenix-test-lib/network/updates/111846375

Basically, if I'm understanding this correctly, because of the circular dependency, that broken dep will always be reachable because dependabot just keeps yo-yo'ing between older and older versions that depend on each other until it finally finds the version that pulls in the broken dep which causes go mod tidy to bomb out.

We would really like to get to the bottom of this and get it fixed, so just let me know what I can do to assist here.

Thanks, yeah that error comes straight from go get -d from what I can see from browsing the code, but please feel free to open a ticket with support (and reference this thread), it'll make it easier to dig into private details 👍

[jeffwidman]

Sounds good, I opened ticket 1117886.

[jeffwidman]

So the upshot of our email discussion was that the error was being thrown by go get -d, which seemed very weird because I couldn't repro the issue locally on my mac. I can run go get -d and it just works. Every time. Even when I mutate the go.mod file (exactly like dependabot is doing) by bumping the version there and then following up with go get -d.

However, I was able to replicate the issue using the docker development container. From there, I noticed a few things. Dependabot today manually edits the go.mod file, then calls go get -d to catch more updates to go.mod and also pickup the go.sum changes. That appeared to be the root of the problem because go get tries to walk the dep tree and always finds the failing/broken dep.

But if I ran go get -d <dep>@<version> before go.mod was edited by dependabot, then it all worked fine. My guess is that there's error-checking logic that only kicks in when go.mod mentions a version that has no accompanying go.sum entry.

Additionally, once the error started appearing, it would pretty much always start appearing. I ran go clean -modcache -cache -i -r and even that command would intermittently fail with the error. I was never quite sure if this was an issue with the go command machinery, or with Docker's overlay file system (I've had problems with that before), or something else.

I tried a lot of variants to "pre-warm" the module cache such as running go mod tidy or go list all etc because I thought that was originally the problem causing go get to walk the tree, but all those were fruitless. Which makes sense if the issue is the go.mod / go.sum mismatch, rather than being a difference between whether or not go get -d can use one of the local caches.

Anyway, I'm 100% confident I don't fully understand the go get machinery, but it's also not worth deeply investigating for upstream bugs because the guts of it will be changing quite a bit in go 1.17 with the new lazy module loading.

And also, it was very clear that manually editing go.mod is a bit of an anti-pattern. Albeit one that every go programmer I know does, but still the recommendation when hitting bugs with the go command (go get, go mod, etc) was to use go get and let it handle making the necessary edits to go.mod/go.sum.

I applied that as a fix in #3590, and it completely solves the problem. I tried multiple repos, all of which are currently failing on dependabot github native, and they all started working with that fix.

[jurre]

Hi @jeffwidman, where are we at with this issue, has this been resolved by the changes you've submitted? It's been a minute since we looked into this so I am slightly lost 😄

[jeffwidman]

I think the underlying problem here is unfixable on the dependabot side and will be resolved at this point by simply bumping to go 1.17 with the new go dependency resolving:

• https://go.dev/doc/go1.17#go-command
• cmd/go: module graph pruning and lazy module loading golang/go#36460

However, I was waiting to close this until i could verify... unfortunately the problematic repos where we are hitting this are still on 1.16... we actually planned to bump them this week.

So let's hold this open a few more days until I have more info.

[jeffwidman]

So as best I can tell, after bumping to go 1.17 this is now working, but we are hitting #4423 in those repos, and I'm not sure which error is triggered first...

I'll close for now, but may re-open if resolving #4423 makes this re-appear.

[jurre]

@jeffwidman I think you meant to link to a different issue, maybe #4423?