common icon indicating copy to clipboard operation
common copied to clipboard

Subpackage version imports repo client_golang which imports this repo…

Open puellanivis opened this issue 5 years ago • 8 comments

This does not cause an import cycle while compiling, so no one’s builds have broken, and go does a decent job at caching the go.mod downloads, so no one has really noticed the stress yet. But just importing the most recent version of client_golang (v1.4.1) ends up cascading through three different versions of client_golang and two versions of common.

https://github.com/prometheus/client_golang/blob/v1.4.1/go.mod#L11 depends on common v0.9.1 https://github.com/prometheus/common/blob/v0.9.1/go.mod#L13 depends on client_golang v1.0.0 https://github.com/prometheus/client_golang/blob/v1.0.0/go.mod#L10 depends on common v0.4.1 https://github.com/prometheus/common/blob/v0.4.1/go.mod#L17 depends on client_golang v0.9.1 https://github.com/prometheus/client_golang/blob/v0.9.1 has no go.mod so this is the end of the line.

As interdependent versions progress, this cyclic dependency is likely to only grow longer and longer, potentially exponentially exploding the results of go mod graph in turn. It might be a good idea to consider how to deal with it now, rather than later.

puellanivis avatar Feb 18 '20 17:02 puellanivis

See https://github.com/prometheus/client_golang/issues/701

brian-brazil avatar Feb 18 '20 17:02 brian-brazil

It would be interesting to know if this actually causes problems in practice. As concluded in https://github.com/prometheus/client_golang/issues/701#issuecomment-571131978 , it looks like the behavior is by design and not really problematic.

If it ever grows into an actual problem, we already know the solution: Replicate https://github.com/prometheus/common/blob/master/version/info.go (or move it to another module).

beorn7 avatar Feb 18 '20 17:02 beorn7

So, the chief problem that I’ve seen is that just by importing client_golang the go mod graph becomes so awash in noise that it is basically unusable without extensive replication of logic from go mod to resolve which version of a package actually gets brought in, and then ignoring the others. Sure the resolution logic is not SAT, but it’s still not really trivial.

That the go mod dependency solver handles this reasonably well so far, does not mean that it could not still be seen as pathological behavior, whether now or in the future. Often in CIs and Dockerfiles, people are building in environments without persistent caches and end up having to repeatedly resolve the dependency chain, which now when it’s only the 3 version is not so bad, but imagine when multiple interdependent packages are stairstepping down say 10 or 30 different versions each in the future? And then with enough of those versions pointing to different versions of even non-cyclic packages, you can end up with an extraordinary number of versions for even those packages, perhaps even some that will have never have been actually depended upon for years, but keep getting dragged in as part of the eternal dependency chain. (Example, imagine that 10 years from now, people still need to resolve github.com/pkg/[email protected].)

So, sure, go mod uses a polynomial algorithm over the NP-complete SAT algorithm, but :woman_shrugging: one should probably not treat it like it cannot be overloaded. Perhaps like Kessler Syndrome, it might not ever actually be a problem, until it is an insurmountable problem?

But of course moving the version package to client_golang would require removing the package from common, which would be a breaking change, which by semver, technically should be behind a major version bump. This is not an insurmountable issue, but it is not really trivial. So… :cry: I don’t know if there really is any good solution here.

puellanivis avatar Feb 19 '20 10:02 puellanivis

which would be a breaking change, which by semver

This is internal-only code, and we're not 1.0 so no need to worry about semver. The issue is more that version does belong in this repo as it's not a public API. Appropriate fixes to this have been discussed in the issue I linked, but none are trivial.

brian-brazil avatar Feb 19 '20 10:02 brian-brazil

The issue is more that version does belong in this repo as it’s not a public API.

Unfortunately, while that might be the intent, it is being used publicly: https://github.com/search?l=Go&q=%22version.NewCollector%22&type=Code

puellanivis avatar Feb 19 '20 11:02 puellanivis

If developers have chosen to use an internal API that's their choice, it's not much use without our (regularly changing) build infrastructure though.

brian-brazil avatar Feb 19 '20 11:02 brian-brazil