x/tools: define module boundaries

[bcmills]

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 from x/tools/cmd/tip (via a dependency on golang.org/x/build/autocertcache), and are only needed when the autocert 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 a replace directive from the latter to the former.

That gives the following structure.

golang.org/x/tools/go.mod:

module golang.org/x/tools

require (
        golang.org/x/crypto v0.0.0-20180910181607-0e37d006457b
        golang.org/x/net v0.0.0-20180911220305-26e67e76b6c3
        golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f // indirect
        google.golang.org/appengine v1.2.0
)

go list -m all for golang.org/x/tools:

golang.org/x/tools
github.com/golang/protobuf v1.2.0
golang.org/x/crypto v0.0.0-20180910181607-0e37d006457b
golang.org/x/net v0.0.0-20180911220305-26e67e76b6c3
golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f
golang.org/x/text v0.3.0
google.golang.org/appengine v1.2.0

---

golang.org/x/tools/cmd/tip/go.mod:

module golang.org/x/tools/cmd/tip

require (
        cloud.google.com/go v0.28.0
        golang.org/x/build v0.0.0-20180925162740-ff91e0e28ab0
        golang.org/x/crypto v0.0.0-20180910181607-0e37d006457b
)

replace golang.org/x/tools => ./../..

go list -m all for golang.org/x/tools/cmd/tip:

golang.org/x/tools/cmd/tip
cloud.google.com/go v0.28.0
contrib.go.opencensus.io/exporter/stackdriver v0.5.0
dmitri.shuralyov.com/html/belt v0.0.0-20180602232347-f7d459c86be0
dmitri.shuralyov.com/state v0.0.0-20180228185332-28bcc343414c
github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239
github.com/bradfitz/go-smtpd v0.0.0-20170404230938-deb6d6237625
github.com/coreos/go-systemd v0.0.0-20180705093442-88bfeed483d3
github.com/davecgh/go-spew v1.1.0
github.com/dustin/go-humanize v0.0.0-20180713052910-9f541cc9db5d
github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568
github.com/fsnotify/fsnotify v1.4.7
github.com/gliderlabs/ssh v0.1.1
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
github.com/golang/protobuf v1.2.0
github.com/google/go-cmp v0.2.0
github.com/google/go-github v16.0.0+incompatible
github.com/google/go-querystring v0.0.0-20170111101155-53e6ce116135
github.com/google/martian v2.0.0-beta.2.0.20180813215018-c223d6f7955e+incompatible
github.com/googleapis/gax-go v2.0.0+incompatible
github.com/gopherjs/gopherjs v0.0.0-20180628210949-0892b62f0d9f
github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7
github.com/jellevandenhooff/dkim v0.0.0-20150330215556-f50fe3d243e1
github.com/kisielk/gotool v1.0.0
github.com/kr/pretty v0.1.0
github.com/kr/pty v1.1.2
github.com/kr/text v0.1.0
github.com/microcosm-cc/bluemonday v1.0.0
github.com/neelance/astrewrite v0.0.0-20160511093645-99348263ae86
github.com/neelance/sourcemap v0.0.0-20151028013722-8c68805598ab
github.com/pmezard/go-difflib v1.0.0
github.com/russross/blackfriday v1.5.1
github.com/sergi/go-diff v1.0.0
github.com/shurcooL/component v0.0.0-20170202220835-f88ec8f54cc4
github.com/shurcooL/events v0.0.0-20180517181903-37636e399bf5
github.com/shurcooL/github_flavored_markdown v0.0.0-20180602233135-8913699a52e3
github.com/shurcooL/go v0.0.0-20180423040247-9e1955d9fb6e
github.com/shurcooL/go-goon v0.0.0-20170922171312-37c2f522c041
github.com/shurcooL/gofontwoff v0.0.0-20180329035133-29b52fc0a18d
github.com/shurcooL/gopherjslib v0.0.0-20160914041154-feb6d3990c2c
github.com/shurcooL/highlight_diff v0.0.0-20170515013008-09bb4053de1b
github.com/shurcooL/highlight_go v0.0.0-20170515013102-78fb10f4a5f8
github.com/shurcooL/htmlg v0.0.0-20170918183704-d01228ac9e50
github.com/shurcooL/httperror v0.0.0-20170206035902-86b7830d14cc
github.com/shurcooL/httpfs v0.0.0-20171119174359-809beceb2371
github.com/shurcooL/httpgzip v0.0.0-20180522190206-b1c53ac65af9
github.com/shurcooL/issues v0.0.0-20180509033703-c5ffda838306
github.com/shurcooL/issuesapp v0.0.0-20180602232740-048589ce2241
github.com/shurcooL/notifications v0.0.0-20180509033327-dff011de8119
github.com/shurcooL/octicon v0.0.0-20180602230221-c42b0e3b24d9
github.com/shurcooL/reactions v0.0.0-20180602233045-253d879cae26
github.com/shurcooL/sanitized_anchor_name v0.0.0-20170918181015-86672fcb3f95
github.com/shurcooL/users v0.0.0-20180125191416-49c67e49c537
github.com/shurcooL/webdavfs v0.0.0-20170829043945-18c3829fa133
github.com/sourcegraph/annotate v0.0.0-20160123013949-f4cad6c6324d
github.com/sourcegraph/syntaxhighlight v0.0.0-20170531221838-bd320f5d308e
github.com/stretchr/testify v1.2.2
github.com/tarm/serial v0.0.0-20180114052751-eaafced92e96
go.opencensus.io v0.14.0
go4.org v0.0.0-20180417224846-9599cf28b011
golang.org/x/build v0.0.0-20180925162740-ff91e0e28ab0
golang.org/x/crypto v0.0.0-20180910181607-0e37d006457b
golang.org/x/net v0.0.0-20180911220305-26e67e76b6c3
golang.org/x/oauth2 v0.0.0-20180724155351-3d292e4d0cdc
golang.org/x/perf v0.0.0-20180704124530-6e6d33e29852
golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f
golang.org/x/sys v0.0.0-20180807162357-acbc56fc7007
golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2
golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2
golang.org/x/tools v0.0.0-20180807205940-ca6481ae5650 => ./../..
google.golang.org/api v0.0.0-20180808000436-6e1e03fd226b
google.golang.org/appengine v1.2.0
google.golang.org/genproto v0.0.0-20180731170733-daca94659cb5
google.golang.org/grpc v1.14.0
gopkg.in/inf.v0 v0.9.1
grpc.go4.org v0.0.0-20170609214715-11d0a25b4919

Edit: I think x/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.

[ianthehat]

My questions are more about which API's do we want to individually version, and how are we going to lay that out.
For instance, the go/packages API is far more likely to need a v2 than some of the other ones, whereas the go/buildutil needs to be deprecated.
Are we just going to do major versions with directories, which means a checkout has every major version of every package in it, for ever more? Or should we be separating packages into their own modules so we can version them and then prune them.
There is a lot of code in there that I would like to version stamp and then delete so we don't have to worry about it but we don't break people that are already using it.

[bradfitz]

Therefore, I propose that we split 'x/tools' into two modules

That seems quite a bit more coarse than I was imagining, not that I've written anything down.

[bcmills]

There is a lot of code in there that I would like to version stamp and then delete so we don't have to worry about it but we don't break people that are already using it.

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 master branch until all of the supported Go releases also support modules. (We could prune stuff out on a v2 branch and leave the master branch on v1, but that seems more awkward than just retaining some packages we would rather not have around.)

For instance, the go/packages API is far more likely to need a v2 than some of the other ones,

If we need a v2 for one or two packages, I think we can do those packages individually. It should be fine to have a /v2 package inside a v1 module: the repo tag applies to the module, but module paths an package paths don't necessarily have to be coupled. (That is, golang.org/x/tools/go/packages/v2 is just a package inside golang/x/tools, whereas golang.org/x/tools/v2/go/packages is presumably a package in the golang.org/x/tools/v2 module.)

The goforward tool I'm prototyping in https://golang.org/cl/137076 is intended to support exactly that sort of API versioning: if we can write the v1 API in terms of the v2 API, then there is basically no cost to keeping v1 around (because it contains only trivial code).

[bcmills]

That seems quite a bit more coarse than I was imagining

It's more coarse than I was expecting too, but there isn't really a narrower split that does much good.

cmd/ is too heterogeneous to split out, but module-per-command seems too tedious to version separately.
Similarly, go/ is too heterogeneous, but splitting out subdirectories of go/ provides no clear benefit.

We could possibly drop the google.golang.org/appengine dependency by splitting out a bunch of individual tools (godoc, playground, blog, etc.), but that only prunes out the one module (it induces no transitive dependencies) and does so at the cost of adding a bunch of individual modules that would be a pain to wire back together.

[bcmills]

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 require the appropriate version of the new module that contains that path (so that we don't end up with two otherwise-compatible modules trying to provide the same package).

[hyangah]

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.

cmd hosts binaries only and changing the go get path is pretty undesirable, so except dependency management purpose, I think it's better to keep them in v1 as long as possible.

Most top-level directories can be possibly in a separate module assuming their life cycles are independent from each other. The internal directories also complicate splitting the repo to finer grained modules.

[bcmills]

The internal directories also complicate splitting the repo to finer grained modules.

FWIW, internal directories should mostly not be a problem: packages in one module are allowed to import internal packages from another as long as they have the same path prefix (up to the internal/ path component).

[bcmills]

Most top-level directories can be possibly in a separate module assuming their life cycles are independent from each other.

That's true, but bear in mind that any subpackage that depends on golang.org/x/tools itself will need a replace directive during editing, and those directives make it much more likely that we will accidentally introduce version skew.

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.

[bcmills]

Note that golang.org/x/tools is (de facto) currently a single module, since any go.mod entry pointing to a commit from before we add a go.mod file (whenever that occurs) provides all of the packages in the repository as it stands today.

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).

[hyangah]

The top-most internal/ directory is in the version 1(or 0) (yes, since we are currently in a single module), if a v2 of submodule dependes on the internal/... in the future, that creates dependency of v2 on the v1 module, so prevents migrating the v1 to use any of v2 for implementation.

[bcmills]

if a v2 of submodule [depends] on the internal/... in the future, that creates dependency of v2 on the v1 module, so prevents migrating the v1 to use any of v2 for implementation.

If everything is working as intended, v1 and v2 should be able to have a cyclic dependency: v1 can import v2 for implementation, and golang.org/x/tools/v2 can implement golang.org/x/tools/internal from v1. The versions you actually end up with are the fix-point of the cycle.

The only hard constraint is that the same import path can't appear in more than one module in the cycle, but since v1 and v2 have separate import paths that shouldn't be an issue.

[bcmills]

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 x/tools, it probably belongs in the standard library or toolchain.

Are there any packages within x/tools that are stable and not candidates for the standard toolchain?