godoc/vfs: fix union logic in NameSpace.ReadDir

ReadDir now returns files from directories in all matching mount
points if no Go files are present in any of them. The behavior now
matches the documentation.

Fixes golang/go#34571

Change-Id: I3a0c8d49a5906ec33ebe9e3efea9d2b9d267506c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197801
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
This commit is contained in:
Jay Conrod 2019-09-27 17:11:20 -04:00
parent 90aeebe843
commit 7c411dea38
3 changed files with 165 additions and 100 deletions

View File

@ -1,58 +0,0 @@
// Copyright 2016 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
package vfs_test
import (
"testing"
"time"
"golang.org/x/tools/godoc/vfs"
"golang.org/x/tools/godoc/vfs/mapfs"
)
func TestNewNameSpace(t *testing.T) {
// We will mount this filesystem under /fs1
mount := mapfs.New(map[string]string{"fs1file": "abcdefgh"})
// Existing process. This should give error on Stat("/")
t1 := vfs.NameSpace{}
t1.Bind("/fs1", mount, "/", vfs.BindReplace)
// using NewNameSpace. This should work fine.
t2 := vfs.NewNameSpace()
t2.Bind("/fs1", mount, "/", vfs.BindReplace)
testcases := map[string][]bool{
"/": {false, true},
"/fs1": {true, true},
"/fs1/fs1file": {true, true},
}
fss := []vfs.FileSystem{t1, t2}
for j, fs := range fss {
for k, v := range testcases {
_, err := fs.Stat(k)
result := err == nil
if result != v[j] {
t.Errorf("fs: %d, testcase: %s, want: %v, got: %v, err: %s", j, k, v[j], result, err)
}
}
}
fi, err := t2.Stat("/")
if err != nil {
t.Fatal(err)
}
if fi.Name() != "/" {
t.Errorf("t2.Name() : want:%s got:%s", "/", fi.Name())
}
if !fi.ModTime().IsZero() {
t.Errorf("t2.Modime() : want:%v got:%v", time.Time{}, fi.ModTime())
}
}

View File

@ -298,66 +298,52 @@ var startTime = time.Now()
func (ns NameSpace) ReadDir(path string) ([]os.FileInfo, error) {
path = ns.clean(path)
// List matching directories and determine whether any of them contain
// Go files.
var (
haveGo = false
haveName = map[string]bool{}
all []os.FileInfo
err error
first []os.FileInfo
dirs [][]os.FileInfo
goDirIndex = -1
readDirErr error
)
for _, m := range ns.resolve(path) {
dir, err1 := m.fs.ReadDir(m.translate(path))
if err1 != nil {
if err == nil {
err = err1
dir, err := m.fs.ReadDir(m.translate(path))
if err != nil {
if readDirErr == nil {
readDirErr = err
}
continue
}
if dir == nil {
dir = []os.FileInfo{}
}
dirs = append(dirs, dir)
if first == nil {
first = dir
}
// If we don't yet have Go files in 'all' and this directory
// has some, add all the files from this directory.
// Otherwise, only add subdirectories.
useFiles := false
if !haveGo {
for _, d := range dir {
if strings.HasSuffix(d.Name(), ".go") {
useFiles = true
haveGo = true
if goDirIndex < 0 {
for _, f := range dir {
if !f.IsDir() && strings.HasSuffix(f.Name(), ".go") {
goDirIndex = len(dirs) - 1
break
}
}
}
}
for _, d := range dir {
name := d.Name()
if (d.IsDir() || useFiles) && !haveName[name] {
// Build a list of files and subdirectories. If a directory contains Go files,
// only include files from that directory. Otherwise, include files from
// all directories. Include subdirectories from all directories regardless
// of whether Go files are present.
haveName := make(map[string]bool)
var all []os.FileInfo
for i, dir := range dirs {
for _, f := range dir {
name := f.Name()
if !haveName[name] && (f.IsDir() || goDirIndex < 0 || goDirIndex == i) {
all = append(all, f)
haveName[name] = true
all = append(all, d)
}
}
}
// We didn't find any directories containing Go files.
// If some directory returned successfully, use that.
if !haveGo {
for _, d := range first {
if !haveName[d.Name()] {
haveName[d.Name()] = true
all = append(all, d)
}
}
}
// Built union. Add any missing directories needed to reach mount points.
// Add any missing directories needed to reach mount points.
for old := range ns {
if hasPathPrefix(old, path) && old != path {
// Find next element after path in old.
@ -374,7 +360,7 @@ func (ns NameSpace) ReadDir(path string) ([]os.FileInfo, error) {
}
if len(all) == 0 {
return nil, err
return nil, readDirErr
}
sort.Sort(byName(all))

137
godoc/vfs/namespace_test.go Normal file
View File

@ -0,0 +1,137 @@
// Copyright 2016 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
package vfs_test
import (
"fmt"
"strings"
"testing"
"time"
"golang.org/x/tools/godoc/vfs"
"golang.org/x/tools/godoc/vfs/mapfs"
)
func TestNewNameSpace(t *testing.T) {
// We will mount this filesystem under /fs1
mount := mapfs.New(map[string]string{"fs1file": "abcdefgh"})
// Existing process. This should give error on Stat("/")
t1 := vfs.NameSpace{}
t1.Bind("/fs1", mount, "/", vfs.BindReplace)
// using NewNameSpace. This should work fine.
t2 := vfs.NewNameSpace()
t2.Bind("/fs1", mount, "/", vfs.BindReplace)
testcases := map[string][]bool{
"/": {false, true},
"/fs1": {true, true},
"/fs1/fs1file": {true, true},
}
fss := []vfs.FileSystem{t1, t2}
for j, fs := range fss {
for k, v := range testcases {
_, err := fs.Stat(k)
result := err == nil
if result != v[j] {
t.Errorf("fs: %d, testcase: %s, want: %v, got: %v, err: %s", j, k, v[j], result, err)
}
}
}
fi, err := t2.Stat("/")
if err != nil {
t.Fatal(err)
}
if fi.Name() != "/" {
t.Errorf("t2.Name() : want:%s got:%s", "/", fi.Name())
}
if !fi.ModTime().IsZero() {
t.Errorf("t2.ModTime() : want:%v got:%v", time.Time{}, fi.ModTime())
}
}
func TestReadDirUnion(t *testing.T) {
for _, tc := range []struct {
desc string
ns vfs.NameSpace
path, want string
}{
{
desc: "no_go_files",
ns: func() vfs.NameSpace {
rootFs := mapfs.New(map[string]string{
"doc/a.txt": "1",
"doc/b.txt": "1",
"doc/dir1/d1.txt": "",
})
docFs := mapfs.New(map[string]string{
"doc/a.txt": "22",
"doc/dir2/d2.txt": "",
})
ns := vfs.NameSpace{}
ns.Bind("/", rootFs, "/", vfs.BindReplace)
ns.Bind("/doc", docFs, "/doc", vfs.BindBefore)
return ns
}(),
path: "/doc",
want: "a.txt:2,b.txt:1,dir1:0,dir2:0",
}, {
desc: "have_go_files",
ns: func() vfs.NameSpace {
a := mapfs.New(map[string]string{
"src/x/a.txt": "",
"src/x/suba/sub.txt": "",
})
b := mapfs.New(map[string]string{
"src/x/b.go": "package b",
"src/x/subb/sub.txt": "",
})
c := mapfs.New(map[string]string{
"src/x/c.txt": "",
"src/x/subc/sub.txt": "",
})
ns := vfs.NameSpace{}
ns.Bind("/", a, "/", vfs.BindReplace)
ns.Bind("/", b, "/", vfs.BindAfter)
ns.Bind("/", c, "/", vfs.BindAfter)
return ns
}(),
path: "/src/x",
want: "b.go:9,suba:0,subb:0,subc:0",
}, {
desc: "empty_mount",
ns: func() vfs.NameSpace {
ns := vfs.NameSpace{}
ns.Bind("/empty", mapfs.New(nil), "/empty", vfs.BindReplace)
return ns
}(),
path: "/",
want: "empty:0",
},
} {
t.Run(tc.desc, func(t *testing.T) {
fis, err := tc.ns.ReadDir(tc.path)
if err != nil {
t.Fatal(err)
}
buf := &strings.Builder{}
sep := ""
for _, fi := range fis {
fmt.Fprintf(buf, "%s%s:%d", sep, fi.Name(), fi.Size())
sep = ","
}
if got := buf.String(); got != tc.want {
t.Errorf("got %q; want %q", got, tc.want)
}
})
}
}