Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/go: dependencies in go.mod of older versions of modules in require cycles affect the current version's build #36369

Open
nsd20463 opened this issue Jan 3, 2020 · 7 comments

Comments

@nsd20463
Copy link
Contributor

@nsd20463 nsd20463 commented Jan 3, 2020

What version of Go are you using (go version)?

$ go version
go version go1.13.5 linux/amd64

Does this issue reproduce with the latest release?

Yes. 1.13.5 is the latest release at this date.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOOS="linux"

What did you do?

I have three modules, a, b and c. All use go modules. a/go.mod requires b and c, and (due to history) b/go.mod requires a. One (or both) of those cyclic requires refers to an older version than the newest, but it's new enough for both a and b to build.

Now I wanted to downgrade module c to an older version. So I edited a/go.mod, and rebuild a. The c line in a/go.mod was edited to the newer version of c, which I had been using before, and which I did not want to use anymore.

Investigating with go mod graph showed that the newer c was required by an older a, which was required by the current b, which was required by the current a. In other words

a@now -> b@now -> a@older -> c@newer

What did you expect to see?

I did not expect the a@older/go.mod to be of any consequence to the go modules choice of version of c, since in the end it was going to use a@now to build, not a@older.

What did you see instead?

go mod graph showed that because of the cycling in the module dependency graph, a@older/go.mod was consulted, as if a@older wasn't an older version of a module already in the graph, but rather some other independent module.

As a consequence a@older's dependencies changed a@now's build.

@dmitshur dmitshur changed the title dependencies in go.mod of older versions of modules in require cycles affect the current version's build cmd/go: dependencies in go.mod of older versions of modules in require cycles affect the current version's build Jan 3, 2020
@Kurdenes21

This comment was marked as spam.

Copy link

@Kurdenes21 Kurdenes21 commented Jan 3, 2020

  • [ ]

@**__**

@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Jan 6, 2020

Editing go.sum has no effect on the dependency graph: the go.mod file, not the go.sum file, determines the set of dependencies.

@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Jan 6, 2020

Also note that in general you should use go get to upgrade and downgrade, rather than editing the go.mod file directly.

Any time the go command is run, it ensures that the dependencies specified in the go.mod file are consistent with each other's requirements, and it resolves inconsistencies by taking the highest required version. To preserve that property, go get $MODULE@$VERSION downgrades other transitive dependencies as needed until the highest required version of $MODULE is $VERSION.

@jayconrod

This comment has been minimized.

Copy link
Contributor

@jayconrod jayconrod commented Jan 6, 2020

This behavior is a result of how the minimal version selection (MVS) algorithm works. It basically traverses the module version graph and makes a list of the highest versions encountered of each module in the graph. We can't really change this algorithm: doing so would mean different versions of Go would select different versions, possibly breaking builds.

To expand on @bcmills's comment, go get is the right tool for upgrading or downgrading dependencies, but it may not necessarily work with the current module graph. When go get is asked to downgrade c, it will also downgrade anything that transitively depends on it (a and b). Unless there are older versions of a and b that transitively depend on an older version of c (or don't depend on c), go get will report an error. You might need to release additional versions of a or b with the requirements you want.

@toothrot toothrot modified the milestones: Unplanned, Backlog Jan 7, 2020
@nsd20463

This comment has been minimized.

Copy link
Contributor Author

@nsd20463 nsd20463 commented Jan 8, 2020

go.sum

Sorry, my brain was tired. I meant to write that I edited go.mod. I've edited the text to fix this.

[use go get x@v]

Sure. Except in this case it just fails, since there is no version of b old enough. a, b and c all predate go modules.

I expected one version of a to be used in the build. Not two. I understand the MVS algorithm, but I expected it to have a check that if it currently planed to use version X of a, and it saw b needs needs version Y of a, Y<X (and same major versions), that it would not continue down into a@Y's dependencies because it wasn't going to use a@Y in the end.

[release additional versions of a or b...]

That didn't work so easily. Because of the cycle of dependency between a and b goes back long before go modules, there is no version of a or b suitable. Updating a leaves it depending on a b which itself depends on the older a, which depends on the older b, which ... eventually recuses to an a depending on the c I don't want.

What I ended up doing, once I understood what was going on, is breaking the back dependencies by hand editing both a/go.mod and b/go.mod to depend on the future (not-yet-exiting) versions of a and b. I pushed that to source control. Then I created those future versions by tagging a and b. And finally did a build to pull in the go.sum lines, now that those versions of a and b did exist, and pushed again. Now, even though MVS followed the dependencies all the way back, it stops at these releases or a and b, and doesn't go forward.

@jayconrod

This comment has been minimized.

Copy link
Contributor

@jayconrod jayconrod commented Jan 8, 2020

I expected one version of a to be used in the build. Not two. I understand the MVS algorithm, but I expected it to have a check that if it currently planed to use version X of a, and it saw b needs needs version Y of a, Y<X (and same major versions), that it would not continue down into a@Y's dependencies because it wasn't going to use a@Y in the end.

I don't think this can work without some path dependence or non-determinism which would add complication and change results. For example, say the main module M requires A@v1.1.0 and B@v1.0.0, and B@v1.0.0 requires A@v1.0.0. If MVS traverses the graph depth-first with requirements in lexicographic order, it might be able to avoid loading the go.mod file for A@v1.0.0 because it saw A@v1.1.0 earlier. But what if the versions of A are reversed? In order to skip loading A@v1.0.0, MVS would need to load B first to see the requirement on A@v1.1.0.

What I ended up doing, once I understood what was going on, is breaking the back dependencies by hand editing both a/go.mod and b/go.mod to depend on the future (not-yet-exiting) versions of a and b. I pushed that to source control. Then I created those future versions by tagging a and b. And finally did a build to pull in the go.sum lines, now that those versions of a and b did exist, and pushed again. Now, even though MVS followed the dependencies all the way back, it stops at these releases or a and b, and doesn't go forward.

That didn't work so easily. Because of the cycle of dependency between a and b goes back long before go modules, there is no version of a or b suitable. Updating a leaves it depending on a b which itself depends on the older a, which depends on the older b, which ... eventually recuses to an a depending on the c I don't want.

I'm glad this worked out. I'm sorry this wasn't easier though. This is an area where our tooling and documentation could improve a bit.

@bcmills Any thoughts on how this could be better? I think any change here would require changing MVS results, which may break existing builds.

@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Jan 8, 2020

As it so happens, writing a design doc for an algorithm to break cycles using lazy loading is my #1 task for this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.