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
Labels
GoCommand cmd/go modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@nsd20463
Copy link
Contributor

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
@bcmills
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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 added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 7, 2020
@toothrot toothrot modified the milestones: Unplanned, Backlog Jan 7, 2020
@nsd20463
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor

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.

@gopherbot
Copy link

Change https://golang.org/cl/220080 mentions this issue: design/36460: add design for lazy module loading

gopherbot pushed a commit to golang/proposal that referenced this issue Mar 5, 2020
Updates golang/go#36460
Updates golang/go#27900
Updates golang/go#26955
Updates golang/go#30831
Updates golang/go#32058
Updates golang/go#32380
Updates golang/go#32419
Updates golang/go#33370
Updates golang/go#33669
Updates golang/go#36369

Change-Id: I1d4644e3e8b4e688c2fc5a569312495e5072b7d7
Reviewed-on: https://go-review.googlesource.com/c/proposal/+/220080
Reviewed-by: Russ Cox <rsc@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GoCommand cmd/go modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

7 participants
@toothrot @jayconrod @dmitshur @bcmills @nsd20463 @gopherbot and others