close_issue_1
/
tools
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

cmd/godoc, godoc: implement build.Context.IsDir, update expected error string 

Prior to this change, handlerServer.GetPageInfo created a build.Context
starting with build.Default, and provided custom implementations for
its IsAbsPath, ReadDir, and OpenFile funcs. Those funcs would operate
on h.c.fs virtual filesystem.

https://godoc.org/go/build#Context.IsDir is documented as:

	// IsDir reports whether the path names a directory.
	// If IsDir is nil, Import calls os.Stat and uses the result's IsDir method.
	IsDir func(path string) bool

IsDir was left as nil, and so the default implementation was used.
The default implementation uses os.Stat and isn't aware of the h.c.fs
virtual filesystem.

This appears to have been harmless in the past, but after the change to
go/build in https://golang.org/cl/33158, it started to interfere with
the operation of godoc. The command godoc began to fail to resolve
relative import path "." in directories that exist, because the
incorrect IsDir implementation was looking in real filesystem, rather
than the virtual one:

	$ ./godoc fmt
	2017/03/04 18:59:50 cannot find package "." in:
		/target
	$ ./godoc -http=localhost:6060
	2017/03/04 19:00:07 cannot find package "." in:
		/src/fmt

Providing a custom implementation of IsDir that is aware of the h.c.fs
virtual filesystem, and performs a stat operation on the correct path
there resolves that problem. It also fixes the failing tests.

Additionally, because the exact error string returned from
Context.Import has changed after https://golang.org/cl/33158,
and now contains the text "package not found" rather than the
underlying error text from filesystem caused by a failed ReadDir
operation, the expected error message from "./godoc nonexistingpkg"
in a test needed to be updated to "cannot find package". It's no longer
dependent on the operating system.

It might be desirable to provide more relevant detail in the error
message from cmd/godoc when a package is not found, but that should be
determined and done in a followup CL. The scope of this one is to fix
normal functionality.

This change follows update to go/build in https://golang.org/cl/33158.

Helps golang/go#19401.

Change-Id: I00e2f746ec4a2fe7e640218adce75f15bdf29aaf
Reviewed-on: https://go-review.googlesource.com/37768
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This commit is contained in:
Dmitri Shuralyov 2017-03-04 19:06:07 -05:00 committed by Brad Fitzpatrick
parent 8ffd8eda92
commit f595fb5c96
2 changed files with 5 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
cmd/godoc/godoc_test.go
View File

@ -44,9 +44,7 @@ var godocTests = []struct {
{ {
args: []string{"nonexistingpkg"}, args: []string{"nonexistingpkg"},
matches: []string{ matches: []string{
// The last pattern (does not e) is for plan9: `cannot find package`,
// http://build.golang.org/log/2d8e5e14ed365bfa434b37ec0338cd9e6f8dd9bf
`no such file or directory|does not exist|cannot find the file|(?:' does not e)`,
}, },
}, },
{ {

4
godoc/server.go
View File

@ -65,6 +65,10 @@ func (h *handlerServer) GetPageInfo(abspath, relpath string, mode PageInfoMode,
// are used. // are used.
ctxt := build.Default ctxt := build.Default
ctxt.IsAbsPath = pathpkg.IsAbs ctxt.IsAbsPath = pathpkg.IsAbs
ctxt.IsDir = func(path string) bool {
fi, err := h.c.fs.Stat(filepath.ToSlash(path))
return err == nil && fi.IsDir()
}
ctxt.ReadDir = func(dir string) ([]os.FileInfo, error) { ctxt.ReadDir = func(dir string) ([]os.FileInfo, error) {
f, err := h.c.fs.ReadDir(filepath.ToSlash(dir)) f, err := h.c.fs.ReadDir(filepath.ToSlash(dir))
filtered := make([]os.FileInfo, 0, len(f)) filtered := make([]os.FileInfo, 0, len(f))