-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
x/tools: define module boundaries #27858
Comments
My questions are more about which API's do we want to individually version, and how are we going to lay that out. |
That seems quite a bit more coarse than I was imagining, not that I've written anything down. |
Pruning is the interesting part, to be sure. Pruning is (necessarily) a breaking change, which means a new module path. So we can't prune anything stable out of the
If we need a The |
It's more coarse than I was expecting too, but there isn't really a narrower split that does much good.
We could possibly drop the |
Also note that we can adjust module boundaries to some extent in the future, subject to the caveat that the module containing any existing package path should |
I am still compiling my thought about how to split this repo - but bummer is that my original motivation on adopting modules in this repo was to tidy up the master branch and freeze the old, unsupported packages as the form of v1 in the separate branch. But as @bcmills pointed out, this is a breaking change and can't be done until all supported go tools can recognize modules. On the other hand, having sources in /v2 directory is not helping for me whose driving motivation is tidyup.
Most top-level directories can be possibly in a separate module assuming their life cycles are independent from each other. The |
FWIW, |
That's true, but bear in mind that any subpackage that depends on Until we have some mechanism in place to detect and/or avoid that version skew (possibly #26420; possibly #27542), I would prefer to keep the subdirectories in the same module. There doesn't seem to be a compelling reason to break them out yet, and we can always do that later if we discover one. |
Note that We can't move any of those packages to other modules without either adding a cyclic module requirement (which is fine, but makes any dependency pruning kind of moot) or introducing an ambiguous import path (which will break builds). |
The top-most |
If everything is working as intended, The only hard constraint is that the same import path can't appear in more than one module in the cycle, but since |
A further thought. We might want separate submodules for parts of the tree that we are ready to declare “stable” (v1) instead of “unstable” (v0). But if we're ready to commit to API stability for anything in Are there any packages within |
What do you mean by toolchain or standard toolchain? |
I mean the tools accessible via |
Ah, gotcha. When I hear "toolchain" I typically think assembler+compiler+linker. Note that things accessible by |
Come to think of it, I don't think we want (or need) a Ideally we want |
Change https://golang.org/cl/137815 mentions this issue: |
Adding a few thoughts/questions.
The fact that
Isn't our decision in this regard more a function of what people depending on these commands need/expect? For example, if all of the commands in We could postpone a decision here until such a time as any given command needs to make a breaking change, but at that point I think the creation of a submodule represents a breaking change in the root module does it not (it's equivalent to the removal of a package)? In any case, tooling can surely help here. Because that same tooling would surely be useful to consumers of any modules in
I don't think there can be any argument against adding a
Regardless of whether we add a root
This doesn't feel like a good reason to introduce a module boundary to my mind. Because when something becomes "stable" we're not really going to move it, are we? Instead, I'd advocate creating modules out of sets of stable, related packages.
I'm not quite sure what you mean here?
The discovery of module versions |
Recall that each major version needs a new import path: we can define a new module for I think the distinction between So perhaps the way to proceed is to first decide which packages are stable, then break the repo into modules based on the stable/unstable split. |
Creating a submodule is not a breaking change as long as the module and submodule have a cyclic requirement. (This is illustrated in the The procedure is:
Testing that commit is a very hard problem, but actually producing it should be straightforward. |
Perhaps, but it's not clear that we can do that with the existing package layout. |
In this specific example, the top module is still v0 - without any backwards-compatibility guarantee. Is that the reason that this is not considered as a breaking change? Or is it because v0 and v1 are indistinguishable w.r.t. import paths? Once the module reaches v1+, removing a package can be like removing exported names
I couldn't manage to get the package splitting working yet after several attempts. Either it's not as straightforward as it sounds, or it's very error-prone. Unless there is a tool and well-defined documentation I am reluctant about the top-level go.mod. Some of the go commands including |
That's the one, yeah: as long as the v0 and v1 modules have a dependency cycle, we can ensure that the user still gets exactly one copy of any given package.
Right: you can't remove a package that way, you can only move it back and forth across the submodule boundary. The cyclic dependency decouples the versioning a bit. The submodule only needs to |
Note that that problem is independent of whether the submodules are defined from the start or factored out later. It's hard to test multi-module changes period. (That's part of why I'm leaning toward fewer modules: if we fix the multi-module testing problem in general, we'll presumably also fix it for the factoring-out-submodules use-case.) |
Some users may set GO111MODULE=on, and we will eventually want to be able to build x/tools itself in module mode. Updates golang/go#27858 Updates golang/go#27852 Change-Id: Iaf488b2a89e6526471530245cb580f1f0391a770 Reviewed-on: https://go-review.googlesource.com/137815 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org>
Given #27858 (comment), I now believe that we should define We can reconsider the module boundaries if and when we want to tag something in |
This seems like a compensation for a flaw in the tool. After a |
I've filed proposal #28835 for one possible solution. |
Hi @bcmills, in #27858 (comment) you wrote:
A few comments/questions.
Apologies if some (or all) of this is too vague, or simply off base... |
Yes, exactly. (That's what the
I don't think they do: you could certainly try out the structure with pre-release tags, but at some point you still have to replace them with release tags in the
Indeed. That's what leads to the practice of adding cyclic requirements when refactoring module boundaries: if the requirements form a cycle, then the path scan produces the same set of results no matter where you start, so the decision collapses to just one dimension (versions).
I don't think we need to favor one use-case over the others: for example, I would expect merges of
That seems like the complement of @jba's suggestion above (#27858 (comment)). It's certainly worth considering. |
(Perhaps naive question) Can we move cmd/tip out of tools to avoid needing to create a submodule in tools? I'm thinking we should be moving a couple of other things too. |
Maybe it should've lived elsewhere to begin with but I'd prefer not to mess with its git history at this point by moving it. Is there something gross about making cmd/tip its own module within x/tools? |
We've found (on the tools team) that multi-module repositories lead to a lot of user confusion and we'd prefer to completely avoid them. Can't we relatively easily make a new repo and copy the version history of the cmd/tip directory to that new repo? |
That seems like a pretty severe reaction to confusion. No learning opportunity there instead?
Now we're making a whole new repo too? Not just moving to x/build? Relative to what? If we wanted to keep git revisions hashes unchanged we'd need to include a full copy of x/tool's git history (+ size) in the new repo. It's possible, but relative to adding a go.mod file to x/tools, I don't think it's easier. |
I agree that we should avoid splitting user-facing packages across modules when possible, and should avoid splitting out modules unless there is a clear need. However, in this case, (And bear in mind that many the complexities of multi-module repos, such as difficulty editing multiple modules in concert, also apply to modules stored in separate repos.) |
Our repos provide an example to users: From everything I've seen, multi-module repositories should be avoided whenever possible, and I don't think it's something we would want our users learning from.
I don't know whether we should put tip in build or its own repo, but I'm fairly convinced it should be moved out. I'd like to better understand the problems with doing so: what's the benefit of keeping git revision hashes unchanged?
I agree, but I think it's a much easier to understand the implications of editing multiple modules in concert compared to modules in submodules. |
My understanding is that it was in scope of #29206 to move Making its history more difficult (depending on what tools one uses) to follow is unfortunate, but I don't see it as a blocker. The website code is already being moved, and Edit: I am in agreement with striving to not introduce multi-modules in |
We've moved cmd/tip to x/build, and plan to move cmd/godoc to x/website. Once cmd/godoc is moved, there won't be any dependencies outside of x/net and x/crypto. Now that the question of cmd/tip is gone, and after the above changes are mode, we should be able to turn x/tools into a single-module repository. We can hold off on tagging versions for now. |
Is there anything left to do for this issue? cmd/tip long ago moved out. |
I don't think there's anything left to do for this issue. I don't remember what our plans for godoc were when this issue was created, but it seems like #59056 tracks our plans for godoc now. I'm going to close this issue. |
I'm looking at the package structure of the
x/tools
repo in response to some comments from @ianthehat and an in-depth experience report from @hyangah.The most extensive dependencies in
x/tools
come fromx/tools/cmd/tip
(via a dependency ongolang.org/x/build/autocertcache
), and are only needed when theautocert
build tag is set.Therefore, I propose that we split 'x/tools' into two modules: one at the repo root, and one at
cmd/tip
, with areplace
directive from the latter to the former.That gives the following structure.
golang.org/x/tools/go.mod
:go list -m all
forgolang.org/x/tools
:golang.org/x/tools/cmd/tip/go.mod
:go list -m all
forgolang.org/x/tools/cmd/tip
:Edit: I thinkx/tools
should be a single module for now. See #27858 (comment).Edit 2: In light of #29935, I'm back to wanting to keep
cmd/tip
as a separate module.Edit 3 by @dmitshur:
cmd/tip
has moved out from x/tools as of CL 160817. This is compatible with it being a separate module from x/tools, as @bcmills suggested in edit 2.The text was updated successfully, but these errors were encountered: