x/exp: reconsider using single module for all unrelated experimental packages of various maturity levels #37175
Comments
dmitshur
added
NeedsInvestigation
|
It seems like But separate |
|
I'd argue that packages in @bcmills is working on a change that would reduce the cost of a large module graph (both in load time and stability). Placeholder issue is #36460. So this may not be a justification for splitting modules in the future. In general, multi-module repos complicate development in a lot of ways, and I think we should avoid that if we can. |
|
Nothing should depend on x/exp. If lots of things need cmd/apidiff, it sounds like it should move. In general, x/ repos should not depend on non-golang repos. x/exp is an exception because it's not supposed to be important for people (see previous paragraph). It would be fine to put a go.mod in x/exp/shiny today to contain the dependencies and not pollute the rest of x/exp that people should not be depending on. In the longer term we need to work out the plan for x/ repos generally. I'm OK with a band-aid in x/exp because, again, people should not be depending on it. x/tools is a bit of a dumping ground but maybe is the right home for cmd/apidiff at this point. It at least tends dependencies better. The right fix for today is to put a go.mod in x/exp/shiny. In the longer term, lazy loading of go.mods will make this situation not bother looking at shiny, because it doesn't matter for the build at hand. |
|
To speak specifically to the point about lazy module loading: note that the module dependencies that provide |
I'm not familiar with all the intricacies of go.mod, but as the original shiny author (and shiny was created at a time before go.mod or even before github.com/golang IIRC), I'd also be fine if moving shiny out of x/exp would help (e.g. to github.com/golang/shiny similar to github.com/golang/snappy being a stand-alone project). |
|
Change https://golang.org/cl/383197 mentions this issue: |
The shiny package is not related to other exp packages. Put it in a separate module. This package uses a modified copy of github.com/BurntSushi/xgb. Move that out of the vendor directory, so that current go commands work. For golang/go#37175 Change-Id: Ib3bad0823d2651ff90a9651e405ed7bcd9de94b6 Reviewed-on: https://go-review.googlesource.com/c/exp/+/383197 Trust: Ian Lance Taylor <[email protected]> Run-TryBot: Ian Lance Taylor <[email protected]> Trust: Nigel Tao <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]>
Clean up dependencies after moving shiny, event, and jsonrpc2 to separate modules. For golang/go#37175 Change-Id: Ie72d0eb4b99a625f41615d6ce3bb5c8935eb11c4 Reviewed-on: https://go-review.googlesource.com/c/exp/+/386795 Trust: Ian Lance Taylor <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]>
|
Change https://go.dev/cl/386795 mentions this issue: |
|
With recent changes, x/exp is down to requiring just 2 golang.org/x modules directly, and 2 more indirectly. There are a number of nested modules that contain distinct parts of x/exp and their dependencies. I don't think there's anything more left to do in this issue, so closing. |
dmitshur
added
NeedsFix
The
golang.org/x/exprepository is described in its README as:It currently contains various unrelated experimental packages that are not related to each other. Some of them are more experimental and less stable, while others are more stable and less experimental.
Back when GOPATH mode was the only build mode available, there was not a high cost to having many unrelated packages in one repository.
In module mode, having many unrelated packages in one module can contribute to increasing the size of the module graph.
For example, the
cloud.google.com/gomodule requiresgolang.org/x/expbecause it usescmd/apidiff, which means all transitive module requirements of all packages ingolang.org/x/expare added to anyone's module that uses at least one Google Cloud Client library fromcloud.google.com/go.The module mirror (https://proxy.golang.org) helps protect users from individual dependencies being unavailable at the origin server, but at this time there are still some users that haven't started using it for various reasons. I expect that number will continue to go down over time. See various user feedback from a recent outage affecting one of
golang.org/x/expdependencies:It can be argued that perhaps the
cloud.google.com/gomodule should not be requiringgolang.org/x/exp(that can still be investigated in a separate issue at https://github.com/googleapis/google-cloud-go/issues), but maybe there can be a better solution ingolang.org/x/expthat would make it possible to keep separate packages and their dependencies more isolated.One possible idea is to move the
shinyproject into a nested module atgolang.org/x/exp/shiny, which would remove its dependencies from the main module. It's unclear how scaleable such a solution is or how applicable it would be in the general case, but it would likely help people who want to importcmd/apidiff.Opening this issue to discuss and look for ways to improve the current situation if possible.
Other related issues:
/cc @bcmills @jayconrod @matloob @jba @dsymonds @robpike
The text was updated successfully, but these errors were encountered: