golang/go

x/tools/cmd/godoc: add module support

dmitshur opened this issue · 17 comments

This is the tracking issue specifically for the golang.org/x/tools/cmd/godoc command to have support for displaying package documentation in module mode. The parent umbrella issue for all “godoc”-like projects is #26827.

At this time, the godoc command is only able to display packages that are located in a GOPATH workspace. When invoked in module mode, it should be able to show documentation for packages in the build list of the main module (as can be determined by running go list -m all), even if those packages are not inside a GOPATH workspace.

The plan is to make this change while maintaining GOPATH support. godoc will use the same logic as the go command to determine whether to use module mode or GOPATH mode (i.e., the value of GO111MODULE environment variable and the presence of a go.mod file).

Scope

There may be future enhancements to add features that weren’t previously viable in GOPATH mode, like being able to select the version via a dropdown UI element, but that’s not in scope for this issue.

This issue is only about maintaining the ability to view the documentation for packages when in module mode.

The -analysis flag is not in scope of this issue, since it's not required to view documentation. It will take more work and is being tracked separately in issue #34473.

Possible Solutions

I have spent some time investigating ways this issue can be resolved. There are many potential long-term solutions that involve making large changes, such as moving towards a newer documentation HTML renderer, making API changes to the godoc library and having it use go/packages to load packages, etc. There is also another solution that requires making a very small, targeted change that reuses much of existing godoc code and functionality. (Another upside is that it allows maintaining -analysis flag support with a small amount of additional effort, tracking issue #34473.) That is the solution I plan to use to resolve this issue.

Godoc functionality is currently spread between the golang.org/x/tools/cmd/godoc command, and the golang.org/x/tools/godoc library. The godoc library API operates on top of a virtual filesystem abstraction, which offers a healthy amount of flexibility. In fact, it's possible to add module support to the godoc command without making any changes to the godoc library API.

Implementation Plan

The goal is to make the godoc command run in module mode whenever the go command would run in module mode. To implement this reliably, godoc will run go env GOMOD with the same environment and working directory, and interpret the result. The -json flag will be used.

If the go env GOMOD result is the empty string, GOPATH mode is being used, and godoc will behave as it currently does: it will find each GOPATH workspace and bind it to the virtual filesystem:

// Bind $GOPATH trees into Go root.
for _, p := range filepath.SplitList(build.Default.GOPATH) {
	fs.Bind("/src", gatefs.New(vfs.OS(p), fsGate), "/src", vfs.BindAfter)
}

If the go env GOMOD result is a non-empty string, module mode is being used. godoc will use go list -m all to determine all modules in the build list, and bind their roots in appropriate locations within the filesystem:

// Determine modules in the build list.
type mod struct {
	Path    string // Module path.
	Version string // Module version.
	Dir     string // Directory holding files for this module, if any.
}
var mods []mod
[... populate mods with output from 'go list -m -json all']

// Bind module trees into Go root.
for _, m := range mods {
	if m.Dir == "" {
		[... either use 'go mod download' to fill module cache, or skip]
	}
	fs.Bind(path.Join("/src", m.Path), gatefs.New(vfs.OS(m.Dir), fsGate), "/", vfs.BindAfter))
}

This way, we outsource the work of doing version selection to the go command, which guarantees consistent results. It will result in a virtual filesystem that accurately contains the source code of Go packages that would be used in the build. It respects local replace directives in the go.mod file, if any. (It will not pick up changes to go.mod file that are made after godoc is started, that will take more changes.)

There may be small changes to the virtual filesystem implementation so that modules are not considered to be inside GOROOT. Some additional care may need to be taken to ensure overlapping paths, in case of nested modules, are supported.

There are tests in the golang.org/x/tools/cmd/godoc package that verify functionality. They will be converted to use packagestest and made to run in both GOPATH mode and module mode.

Decision on automatic downloading of modules not in module cache

Documentation for go list -m notes:

When listing modules, the -f flag still specifies a format template applied to a Go struct, but now a Module struct:

type Module struct {
    [...]
    Dir string // directory holding files for this module, if any
    [...]
}

An important part here is "if any". Most of the time, modules in the build list of the main module are available in the module cache, and the Dir field points to a location on disk where that module is available.

In some cases, for example when a new module has been downloaded but not yet built, some dependencies are not yet available in the module cache. The Dir field will be the empty string, and godoc will not be able to display documentation for that module.

A potential solution here is to simulate what the go command currently does in similar scenarios: automatically start to download the missing modules into the module cache. This can be implemented by invoking go mod download on modules in the build list that are not already present in the module cache.

The alternative solution is to rely on the user to manually run go mod tidy, go test ./..., go build ./..., or any other go command that would populate the missing module cache entries. I am still evaluating which solution to go with here, and currently leaning slightly towards the behavior that is consistent with the go command. Feedback on this decision is welcome.

/cc @andybons

Change https://golang.org/cl/196978 mentions this issue: godoc: remove Corpus.testDir field

Change https://golang.org/cl/196979 mentions this issue: cmd/godoc: check if server exited when waiting

Change https://golang.org/cl/196981 mentions this issue: cmd/godoc: convert tests to packagestest, cover third party packages

Change https://golang.org/cl/196977 mentions this issue: godoc, godoc/vfs: improve documentation of getPageInfo, hasPathPrefix

Change https://golang.org/cl/196980 mentions this issue: go/packages/packagestest: add package use example

Change https://golang.org/cl/196983 mentions this issue: cmd/godoc: add initial support for module mode

The alternative solution is to rely on the user to manually run go mod tidy, go test ./..., go build ./..., or any other go command that would populate the missing module cache entries.

I am in favor of this solution. I think godoc should be just a plain documentation viewer. I wouldn't want it to automatically populate the module cache by itself. If some dependencies are missing from the cache let godoc mark them as empty (or something similar). I can run a command myself to populate it and then run godoc. Perhaps it can recommend a command for me to run in case it finds empty Dir entries, but I personally, wouldn't want it doing stuff by itself.

FWIW I think we should emulate what go doc does:

export GOPATH=$(mktemp -d)
cd $(mktemp -d)
go mod init example.com/hello
go mod edit -require=golang.org/x/tools@latest
go doc golang.org/x/tools/go/packages

Gives:

go: finding golang.org/x/tools latest
go: finding golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2
go: finding golang.org/x/net v0.0.0-20190620200207-3b0461eec859
go: finding golang.org/x/sync v0.0.0-20190423024810-112230192c58
go: finding golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a
go: finding golang.org/x/text v0.3.0
go: finding golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7
package packages // import "golang.org/x/tools/go/packages"

Package packages loads Go packages for inspection and analysis.

...

That's to say, go doc fetches modules as required.

Because if you don't want the underlying go command to do any fetching you can simply set GOPROXY=off.

I agree with the argument Paul made, and plan to include the behavior that will automatically fill the module cache when some of the dependencies of the main module are missing. This behavior can be disabled by setting GOPROXY=off, which is consistent with other tools and doesn't add anything new.

I've considered not doing automatic downloads, but this leads to an inconsistent user experience. Part of the problem is that modifying code in your project can result in previously existing dependencies disappearing. For example, if some dependency is removed, it can result in a different version of other dependencies to be selected via MVS algorithm. And those versions may not be in the module cache. Unlike the GOPATH mode, if the exact version of the required module isn't the in module cache, we can't simply display some other other existing version because that would be misleading.

Let's start with having this behavior on since it's consistent with other similar tools. I've included it in Patch Set 3 of CL 196983. We can adjust it in the future as we learn more.

Change https://golang.org/cl/205661 mentions this issue: godoc/vfs: include dirs needed to reach mount points in Stat

CL 196983, which will resolve this issue, is now ready and I plan to submit it later today. It implements the behavior described above.

This is a nice first cut, but it doesn't play nicely with automatic vendoring in 1.14 (#33848):

~/go/src/cmd$ go version
go version devel +abec5d0879 Wed Nov 6 16:49:40 2019 -0500 linux/amd64

~/go/src/cmd$ go version $(which godoc)
/usr/local/google/home/bcmills/bin/godoc: devel +abec5d0879 Wed Nov 6 16:49:40 2019 -0500

~/go/src/cmd$ godoc
using module mode; GOMOD=/usr/local/google/home/bcmills/go/src/cmd/go.mod
failed to determine the build list of the main module: go command exited unsuccessfully: exit status 1
go list -m: can't compute 'all' using the vendor directory
        (Use -mod=mod or -mod=readonly to bypass.)

I left a few comments on CL 196983 suggesting the points in the implementation that might need to be adapted for vendoring.

Thanks for pointing that out and leaving the comments on the CL Bryan.

I'd like to track that in a separate issue, since it's specific to Go 1.14, which isn't out yet. I want this issue to be available for discussion of module support in the currently released and supported versions of Go. Opened #35429 for it.

Ten years, Go. Ten years without a comprehensive plan for dependency management.

mvdan commented

@mcandre please be nice. Also, I don't see how your comment is relevant to this closed issue.

@dmitshur according to your "Possible solution"

If the go env GOMOD result is a non-empty string, module mode is being used. godoc will use go list -m all to determine all modules in the build list, and bind their roots in appropriate locations within the filesystem:

Does it mean that the module name must represent a real path on the locale filesystem? I faced #26827 (comment) where my module name is my valid VCS name but it can't be mapped according the existing solution.