cmd/pkgsite,internal: serve source
- Set up a /files endpoint on the server that can serve
files from fs.FS implementations.
- Add source.FilesInfo, which returns a source.Info that links to
/files paths. Use it to implement the SourceInfo method of the local
ModuleGetters.
- Add a SourceFS method to the local MethodGetters so they can tell
the server how to serve their files.
- Improve the tests in cmd/pkgsite to verify the source links.
Fixes golang/go#47982
Change-Id: Iff42bad15c4abaf408a364e58b7c0f0dad60b40d
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/347932
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Julie Qiu <julie@golang.org>
diff --git a/cmd/pkgsite/main.go b/cmd/pkgsite/main.go
index 3ec5bf8..31e55c8 100644
--- a/cmd/pkgsite/main.go
+++ b/cmd/pkgsite/main.go
@@ -81,16 +81,16 @@
paths = []string{"."}
}
- var downloadDir string
+ var modCacheDir string
if *useCache {
- downloadDir = *cacheDir
- if downloadDir == "" {
+ modCacheDir = *cacheDir
+ if modCacheDir == "" {
var err error
- downloadDir, err = defaultCacheDir()
+ modCacheDir, err = defaultCacheDir()
if err != nil {
die("%v", err)
}
- if downloadDir == "" {
+ if modCacheDir == "" {
die("empty value for GOMODCACHE")
}
}
@@ -116,7 +116,7 @@
stdlib.SetGoRepoPath(*goRepoPath)
}
- server, err := newServer(ctx, paths, *gopathMode, downloadDir, prox)
+ server, err := newServer(ctx, paths, *gopathMode, modCacheDir, prox)
if err != nil {
die("%s\nMaybe you need to provide the location of static assets with -static.", err)
}
@@ -144,7 +144,11 @@
func newServer(ctx context.Context, paths []string, gopathMode bool, downloadDir string, prox *proxy.Client) (*frontend.Server, error) {
getters := buildGetters(ctx, paths, gopathMode)
if downloadDir != "" {
- getters = append(getters, fetch.NewFSProxyModuleGetter(downloadDir))
+ g, err := fetch.NewFSProxyModuleGetter(downloadDir)
+ if err != nil {
+ return nil, err
+ }
+ getters = append(getters, g)
}
if prox != nil {
getters = append(getters, fetch.NewProxyModuleGetter(prox, source.NewClient(time.Second)))
@@ -161,6 +165,12 @@
if err != nil {
return nil, err
}
+ for _, g := range getters {
+ p, fsys := g.SourceFS()
+ if p != "" {
+ server.InstallFS(p, fsys)
+ }
+ }
return server, nil
}
diff --git a/cmd/pkgsite/main_test.go b/cmd/pkgsite/main_test.go
index 021a56e..a938eb3 100644
--- a/cmd/pkgsite/main_test.go
+++ b/cmd/pkgsite/main_test.go
@@ -9,17 +9,40 @@
"flag"
"net/http"
"net/http/httptest"
+ "os"
+ "path"
"path/filepath"
- "strings"
+ "regexp"
"testing"
"github.com/google/go-cmp/cmp"
+ "golang.org/x/net/html"
"golang.org/x/pkgsite/internal/proxy/proxytest"
+ "golang.org/x/pkgsite/internal/testing/htmlcheck"
+)
+
+var (
+ in = htmlcheck.In
+ hasText = htmlcheck.HasText
+ attr = htmlcheck.HasAttr
+
+ // href checks for an exact match in an href attribute.
+ href = func(val string) htmlcheck.Checker {
+ return attr("href", "^"+regexp.QuoteMeta(val)+"$")
+ }
)
func Test(t *testing.T) {
repoPath := func(fn string) string { return filepath.Join("..", "..", fn) }
+ abs := func(dir string) string {
+ a, err := filepath.Abs(dir)
+ if err != nil {
+ t.Fatal(err)
+ }
+ return a
+ }
+
localModule := repoPath("internal/fetch/testdata/has_go_mod")
cacheDir := repoPath("internal/fetch/testdata/modcache")
flag.Set("static", repoPath("static"))
@@ -34,15 +57,37 @@
mux := http.NewServeMux()
server.Install(mux.Handle, nil, nil)
+ modcacheChecker := in("",
+ in(".Documentation", hasText("var V = 1")),
+ sourceLinks(path.Join(abs(cacheDir), "modcache.com@v1.0.0"), "a.go"))
+
for _, test := range []struct {
- name string
- url string
- wantInBody string
+ name string
+ url string
+ want htmlcheck.Checker
}{
- {"local", "example.com/testmod", "There is no documentation for this package."},
- {"modcache", "modcache.com@v1.0.0", "var V = 1"},
- {"modcache2", "modcache.com", "var V = 1"},
- {"proxy", "example.com/single/pkg", "G is new in v1.1.0"},
+ {
+ "local",
+ "example.com/testmod",
+ in("",
+ in(".Documentation", hasText("There is no documentation for this package.")),
+ sourceLinks(path.Join(abs(localModule), "example.com/testmod"), "a.go")),
+ },
+ {
+ "modcache",
+ "modcache.com@v1.0.0",
+ modcacheChecker,
+ },
+ {
+ "modcache latest",
+ "modcache.com",
+ modcacheChecker,
+ },
+ {
+ "proxy",
+ "example.com/single/pkg",
+ hasText("G is new in v1.1.0"),
+ },
} {
t.Run(test.name, func(t *testing.T) {
w := httptest.NewRecorder()
@@ -50,14 +95,28 @@
if w.Code != http.StatusOK {
t.Fatalf("got status code = %d, want %d", w.Code, http.StatusOK)
}
- body := w.Body.String()
- if !strings.Contains(body, test.wantInBody) {
- t.Fatalf("body is missing %q\n%s", test.wantInBody, body)
+ doc, err := html.Parse(w.Body)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err := test.want(doc); err != nil {
+ if testing.Verbose() {
+ html.Render(os.Stdout, doc)
+ }
+ t.Error(err)
}
})
}
}
+func sourceLinks(dir, filename string) htmlcheck.Checker {
+ filesPath := path.Join("/files", dir) + "/"
+ return in("",
+ in(".UnitMeta-repo a", href(filesPath)),
+ in(".UnitFiles-titleLink a", href(filesPath)),
+ in(".UnitFiles-fileList a", href(filesPath+filename)))
+}
+
func TestCollectPaths(t *testing.T) {
got := collectPaths([]string{"a", "b,c2,d3", "e4", "f,g"})
want := []string{"a", "b", "c2", "d3", "e4", "f", "g"}
diff --git a/internal/fetch/getters.go b/internal/fetch/getters.go
index b1a0113..02f6742 100644
--- a/internal/fetch/getters.go
+++ b/internal/fetch/getters.go
@@ -16,6 +16,7 @@
"io/fs"
"io/ioutil"
"os"
+ "path"
"path/filepath"
"strings"
"time"
@@ -46,6 +47,12 @@
// SourceInfo returns information about where to find a module's repo and
// source files.
SourceInfo(ctx context.Context, path, version string) (*source.Info, error)
+
+ // SourceFS returns the path to serve the files of the modules loaded by
+ // this ModuleGetter, and an FS that can be used to read the files. The
+ // returned values are intended to be passed to
+ // internal/frontend.Server.InstallFiles.
+ SourceFS() (string, fs.FS)
}
type proxyModuleGetter struct {
@@ -82,6 +89,12 @@
return source.ModuleInfo(ctx, g.src, path, version)
}
+// SourceFS is unimplemented for modules served from the proxy, because we
+// link directly to the module's repo.
+func (g *proxyModuleGetter) SourceFS() (string, fs.FS) {
+ return "", nil
+}
+
// Version and commit time are pre specified when fetching a local module, as these
// fields are normally obtained from a proxy.
var (
@@ -93,11 +106,12 @@
// a module's files.
type directoryModuleGetter struct {
modulePath string
- dir string
+ dir string // absolute path to direction
}
// NewDirectoryModuleGetter returns a ModuleGetter for reading a module from a directory.
func NewDirectoryModuleGetter(modulePath, dir string) (*directoryModuleGetter, error) {
+
if modulePath == "" {
goModBytes, err := ioutil.ReadFile(filepath.Join(dir, "go.mod"))
if err != nil {
@@ -108,8 +122,12 @@
return nil, fmt.Errorf("go.mod in %q has no module path: %w", dir, derrors.BadModule)
}
}
+ abs, err := filepath.Abs(dir)
+ if err != nil {
+ return nil, err
+ }
return &directoryModuleGetter{
- dir: dir,
+ dir: abs,
modulePath: modulePath,
}, nil
}
@@ -154,23 +172,41 @@
return os.DirFS(g.dir), nil
}
-// TODO(golang/go#47982): implement.
-func (g *directoryModuleGetter) SourceInfo(ctx context.Context, path, version string) (*source.Info, error) {
- return nil, nil
+// SourceInfo returns a source.Info that will link to the files in the
+// directory. The files will be under /files/directory/modulePath, with no
+// version.
+func (g *directoryModuleGetter) SourceInfo(ctx context.Context, _, _ string) (*source.Info, error) {
+ return source.FilesInfo(g.fileServingPath()), nil
+}
+
+// SourceFS returns the absolute path to the directory along with a
+// filesystem FS for serving the directory.
+func (g *directoryModuleGetter) SourceFS() (string, fs.FS) {
+ return g.fileServingPath(), os.DirFS(g.dir)
+}
+
+func (g *directoryModuleGetter) fileServingPath() string {
+ return path.Join(filepath.ToSlash(g.dir), g.modulePath)
}
// An fsProxyModuleGetter gets modules from a directory in the filesystem that
// is organized like the module cache, with a cache/download directory that has
-// paths that correspond to proxy URLs. An example of such a directory is $(go
-// env GOMODCACHE).
+// paths that correspond to proxy URLs. An example of such a directory is
+// $(go env GOMODCACHE).
type fsProxyModuleGetter struct {
dir string
}
// NewFSModuleGetter return a ModuleGetter that reads modules from a filesystem
// directory organized like the proxy.
-func NewFSProxyModuleGetter(dir string) ModuleGetter {
- return &fsProxyModuleGetter{dir: dir}
+func NewFSProxyModuleGetter(dir string) (_ *fsProxyModuleGetter, err error) {
+ derrors.Wrap(&err, "NewFSProxyModuleGetter(%q)", dir)
+
+ abs, err := filepath.Abs(dir)
+ if err != nil {
+ return nil, err
+ }
+ return &fsProxyModuleGetter{dir: abs}, nil
}
// Info returns basic information about the module.
@@ -182,6 +218,7 @@
if err != nil {
return nil, err
}
+ fmt.Printf("#### resolved latest to %q\n", vers)
}
// Check for a .zip file. Some directories in the download cache have .info and .mod files but no .zip.
@@ -242,9 +279,16 @@
return fs.Sub(zr, path+"@"+vers)
}
-// TODO(golang/go#47982): implement.
-func (g *fsProxyModuleGetter) SourceInfo(ctx context.Context, path, version string) (*source.Info, error) {
- return nil, nil
+// SourceInfo returns a source.Info that will create /files links to modules in
+// the cache.
+func (g *fsProxyModuleGetter) SourceInfo(ctx context.Context, mpath, version string) (*source.Info, error) {
+ return source.FilesInfo(path.Join(g.dir, mpath+"@"+version)), nil
+}
+
+// SourceFS returns the absolute path to the cache, and an FS that retrieves
+// files from it.
+func (g *fsProxyModuleGetter) SourceFS() (string, fs.FS) {
+ return filepath.ToSlash(g.dir), os.DirFS(g.dir)
}
// latestVersion gets the latest version that is in the directory.
diff --git a/internal/fetch/getters_test.go b/internal/fetch/getters_test.go
index 7c4d528..92c47ff 100644
--- a/internal/fetch/getters_test.go
+++ b/internal/fetch/getters_test.go
@@ -8,6 +8,7 @@
"context"
"errors"
"io/ioutil"
+ "path/filepath"
"testing"
"time"
@@ -46,13 +47,20 @@
"dir/cache/download/github.com/a!bc/@v/v2.3.4.zip",
},
} {
- g := NewFSProxyModuleGetter("dir").(*fsProxyModuleGetter)
+ g, err := NewFSProxyModuleGetter("dir")
+ if err != nil {
+ t.Fatal(err)
+ }
got, err := g.escapedPath(test.path, test.version, test.suffix)
if err != nil {
t.Fatal(err)
}
- if got != test.want {
- t.Errorf("%s, %s, %s: got %q, want %q", test.path, test.version, test.suffix, got, test.want)
+ want, err := filepath.Abs(test.want)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got != want {
+ t.Errorf("%s, %s, %s: got %q, want %q", test.path, test.version, test.suffix, got, want)
}
}
}
@@ -68,7 +76,10 @@
if err != nil {
t.Fatal(err)
}
- g := NewFSProxyModuleGetter("testdata/modcache")
+ g, err := NewFSProxyModuleGetter("testdata/modcache")
+ if err != nil {
+ t.Fatal(err)
+ }
t.Run("info", func(t *testing.T) {
got, err := g.Info(ctx, modulePath, vers)
if err != nil {
diff --git a/internal/fetchdatasource/fetchdatasource_test.go b/internal/fetchdatasource/fetchdatasource_test.go
index b274da1..7d5ed75 100644
--- a/internal/fetchdatasource/fetchdatasource_test.go
+++ b/internal/fetchdatasource/fetchdatasource_test.go
@@ -10,6 +10,7 @@
"fmt"
"log"
"os"
+ "path"
"testing"
"time"
@@ -339,8 +340,7 @@
ctx, ds, teardown := setup(t, defaultTestModules, true)
defer teardown()
- // TODO(golang/go#47982): use the correct sourceInfo here.
- var sourceInfo *source.Info
+ sourceInfo := source.FilesInfo("XXX")
for _, test := range []struct {
path, modulePath string
@@ -441,9 +441,21 @@
if err != nil {
t.Fatal(err)
}
- if diff := cmp.Diff(test.want, got, cmp.AllowUnexported(source.Info{})); diff != "" {
+ var gotURL string
+ if got.SourceInfo != nil {
+ gotURL = got.SourceInfo.RepoURL()
+ }
+ wantURL := "/files/*/*/github.com/my/module/"
+ m, err := path.Match(wantURL, gotURL)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if !m {
+ t.Errorf("RepoURL: got %q, want match of %q", gotURL, wantURL)
+ }
+ diff := cmp.Diff(test.want, got, cmp.AllowUnexported(source.Info{}), cmpopts.IgnoreFields(source.Info{}, "repoURL"))
+ if diff != "" {
t.Errorf("mismatch (-want +got):\n%s", diff)
-
}
}
})
diff --git a/internal/frontend/server.go b/internal/frontend/server.go
index 69d41e9..7e32c4f 100644
--- a/internal/frontend/server.go
+++ b/internal/frontend/server.go
@@ -11,6 +11,7 @@
"errors"
"fmt"
"io"
+ "io/fs"
"net/http"
"strings"
"sync"
@@ -48,6 +49,7 @@
googleTagManagerID string
serveStats bool
reportingClient *errorreporting.Client
+ fileMux *http.ServeMux
mu sync.Mutex // Protects all fields below
templates map[string]*template.Template
@@ -92,6 +94,7 @@
googleTagManagerID: scfg.GoogleTagManagerID,
serveStats: scfg.ServeStats,
reportingClient: scfg.ReportingClient,
+ fileMux: http.NewServeMux(),
}
errorPageBytes, err := s.renderErrorPage(context.Background(), http.StatusInternalServerError, "error", nil)
if err != nil {
@@ -148,6 +151,7 @@
http.Redirect(w, r, "/cmd/cgo", http.StatusMovedPermanently)
}))
handle("/golang.org/x", s.staticPageHandler("subrepo", "Sub-repositories"))
+ handle("/files/", http.StripPrefix("/files", s.fileMux))
handle("/", detailHandler)
if s.serveStats {
handle("/detail-stats/",
@@ -165,6 +169,11 @@
}))
}
+// InstallFS adds path under the /files handler, serving the files in fsys.
+func (s *Server) InstallFS(path string, fsys fs.FS) {
+ s.fileMux.Handle(path+"/", http.StripPrefix(path, http.FileServer(http.FS(fsys))))
+}
+
const (
// defaultTTL is used when details tab contents are subject to change, or when
// there is a problem confirming that the details can be permanently cached.
diff --git a/internal/frontend/server_test.go b/internal/frontend/server_test.go
index 771fea4..d359a48 100644
--- a/internal/frontend/server_test.go
+++ b/internal/frontend/server_test.go
@@ -1682,3 +1682,18 @@
}
}
}
+
+func TestInstallFS(t *testing.T) {
+ s, handler, teardown := newTestServer(t, nil, nil)
+ defer teardown()
+ s.InstallFS("/dir", os.DirFS("."))
+ // Request this file.
+ w := httptest.NewRecorder()
+ handler.ServeHTTP(w, httptest.NewRequest("GET", "/files/dir/server_test.go", nil))
+ if w.Code != http.StatusOK {
+ t.Errorf("got status code = %d, want %d", w.Code, http.StatusOK)
+ }
+ if want := "TestInstallFS"; !strings.Contains(w.Body.String(), want) {
+ t.Errorf("body does not contain %q", want)
+ }
+}
diff --git a/internal/source/source.go b/internal/source/source.go
index be09983..e66ef4d 100644
--- a/internal/source/source.go
+++ b/internal/source/source.go
@@ -921,3 +921,21 @@
}
return info
}
+
+// FilesInfo returns an Info that links to a path in the server's /files
+// namespace. The same path needs to be installed via frontend.Server.InstallFS.
+func FilesInfo(dir string) *Info {
+ // The repo and directory patterns need a final slash. Without it,
+ // http.FileServer redirects instead of serving the directory contents, with
+ // confusing results.
+ return &Info{
+ repoURL: path.Join("/files", dir),
+ templates: urlTemplates{
+ Repo: "{repo}/",
+ Directory: "{repo}/{dir}/",
+ File: "{repo}/{file}",
+ Line: "{repo}/{file}#L{line}", // not supported now, but maybe someday
+ Raw: "{repo}/{file}",
+ },
+ }
+}
diff --git a/internal/source/source_test.go b/internal/source/source_test.go
index 0650527..6d7c5ad 100644
--- a/internal/source/source_test.go
+++ b/internal/source/source_test.go
@@ -1020,3 +1020,18 @@
}
}
}
+
+func TestFilesInfo(t *testing.T) {
+ info := FilesInfo("/Users/bob")
+
+ check := func(got, want string) {
+ t.Helper()
+ if got != want {
+ t.Errorf("got %q, want %q", got, want)
+ }
+ }
+
+ check(info.RepoURL(), "/files/Users/bob/")
+ check(info.ModuleURL(), "/files/Users/bob/")
+ check(info.FileURL("dir/a.go"), "/files/Users/bob/dir/a.go")
+}
