internal/fetch: get source info from module getter

Make it the ModuleGetter's responsibility to provide links to source
files, instead of passing around a separate source.Client.

This will allow getters that fetch from disk to serve those local
files.

For golang/go#47982

Change-Id: I6c7cd7890b03ad61d9410850a510c1c9a63f1c9a
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/347749
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>

cmd/frontend/main.go

@@ -78,10 +78,9 @@ func main() {
 
 	if *directProxy {
 		ds := fetchdatasource.Options{
-			Getters:              []fetch.ModuleGetter{fetch.NewProxyModuleGetter(proxyClient)},
+			Getters:              []fetch.ModuleGetter{fetch.NewProxyModuleGetter(proxyClient, source.NewClient(1*time.Minute))},
 			ProxyClientForLatest: proxyClient,
 			BypassLicenseCheck:   *bypassLicenseCheck,
-			SourceClient:         source.NewClient(1 * time.Minute),
 		}.New()
 		dsg = func(context.Context) internal.DataSource { return ds }
 	} else {

cmd/pkgsite/main.go

@@ -150,11 +150,10 @@ func newServer(ctx context.Context, paths []string, gopathMode bool, downloadDir
 		getters = append(getters, fetch.NewFSProxyModuleGetter(downloadDir))
 	}
 	if prox != nil {
-		getters = append(getters, fetch.NewProxyModuleGetter(prox))
+		getters = append(getters, fetch.NewProxyModuleGetter(prox, source.NewClient(time.Second)))
 	}
 	lds := fetchdatasource.Options{
 		Getters:              getters,
-		SourceClient:         source.NewClient(time.Second),
 		ProxyClientForLatest: prox,
 		BypassLicenseCheck:   true,
 	}.New()

internal/fetch/fetch.go

@@ -21,7 +21,6 @@ import (
 	"golang.org/x/pkgsite/internal/licenses"
 	"golang.org/x/pkgsite/internal/log"
 	"golang.org/x/pkgsite/internal/proxy"
-	"golang.org/x/pkgsite/internal/source"
 	"golang.org/x/pkgsite/internal/stdlib"
 )
 
@@ -49,14 +48,14 @@ type FetchResult struct {
 // *internal.Module and related information.
 //
 // Even if err is non-nil, the result may contain useful information, like the go.mod path.
-func FetchModule(ctx context.Context, modulePath, requestedVersion string, mg ModuleGetter, sourceClient *source.Client) (fr *FetchResult) {
+func FetchModule(ctx context.Context, modulePath, requestedVersion string, mg ModuleGetter) (fr *FetchResult) {
 	fr = &FetchResult{
 		ModulePath:       modulePath,
 		RequestedVersion: requestedVersion,
 	}
 	defer derrors.Wrap(&fr.Error, "FetchModule(%q, %q)", modulePath, requestedVersion)
 
-	err := fetchModule(ctx, fr, mg, sourceClient)
+	err := fetchModule(ctx, fr, mg)
 	fr.Error = err
 	if err != nil {
 		fr.Status = derrors.ToStatus(fr.Error)
@@ -67,7 +66,7 @@ func FetchModule(ctx context.Context, modulePath, requestedVersion string, mg Mo
 	return fr
 }
 
-func fetchModule(ctx context.Context, fr *FetchResult, mg ModuleGetter, sourceClient *source.Client) error {
+func fetchModule(ctx context.Context, fr *FetchResult, mg ModuleGetter) error {
 	info, err := GetInfo(ctx, fr.ModulePath, fr.RequestedVersion, mg)
 	if err != nil {
 		return err
@@ -123,7 +122,7 @@ func fetchModule(ctx context.Context, fr *FetchResult, mg ModuleGetter, sourceCl
 		}
 	}
 
-	mod, pvs, err := processModuleContents(ctx, fr.ModulePath, fr.ResolvedVersion, fr.RequestedVersion, commitTime, contentDir, sourceClient)
+	mod, pvs, err := processModuleContents(ctx, fr.ModulePath, fr.ResolvedVersion, fr.RequestedVersion, commitTime, contentDir, mg)
 	if err != nil {
 		return err
 	}
@@ -184,7 +183,7 @@ func getGoModPath(ctx context.Context, modulePath, resolvedVersion string, mg Mo
 
 // processModuleContents extracts information from the module filesystem.
 func processModuleContents(ctx context.Context, modulePath, resolvedVersion, requestedVersion string,
-	commitTime time.Time, contentDir fs.FS, sourceClient *source.Client) (_ *internal.Module, _ []*internal.PackageVersionState, err error) {
+	commitTime time.Time, contentDir fs.FS, mg ModuleGetter) (_ *internal.Module, _ []*internal.PackageVersionState, err error) {
 	defer derrors.Wrap(&err, "processModuleContents(%q, %q)", modulePath, resolvedVersion)
 
 	ctx, span := trace.StartSpan(ctx, "fetch.processModuleContents")
@@ -194,7 +193,7 @@ func processModuleContents(ctx context.Context, modulePath, resolvedVersion, req
 	if modulePath == stdlib.ModulePath && stdlib.SupportedBranches[requestedVersion] {
 		v = requestedVersion
 	}
-	sourceInfo, err := source.ModuleInfo(ctx, sourceClient, modulePath, v)
+	sourceInfo, err := mg.SourceInfo(ctx, modulePath, v)
 	if err != nil {
 		log.Infof(ctx, "error getting source info: %v", err)
 	}

internal/fetch/getters.go

@@ -24,6 +24,7 @@ import (
 	"golang.org/x/mod/module"
 	"golang.org/x/pkgsite/internal/derrors"
 	"golang.org/x/pkgsite/internal/proxy"
+	"golang.org/x/pkgsite/internal/source"
 	"golang.org/x/pkgsite/internal/version"
 )
 
@@ -41,14 +42,19 @@ type ModuleGetter interface {
 	// to have according to the zip archive layout specification at
 	// https://golang.org/ref/mod#zip-files.
 	ContentDir(ctx context.Context, path, version string) (fs.FS, error)
+
+	// SourceInfo returns information about where to find a module's repo and
+	// source files.
+	SourceInfo(ctx context.Context, path, version string) (*source.Info, error)
 }
 
 type proxyModuleGetter struct {
 	prox *proxy.Client
+	src  *source.Client
 }
 
-func NewProxyModuleGetter(p *proxy.Client) ModuleGetter {
-	return &proxyModuleGetter{p}
+func NewProxyModuleGetter(p *proxy.Client, s *source.Client) ModuleGetter {
+	return &proxyModuleGetter{p, s}
 }
 
 // Info returns basic information about the module.
@@ -71,6 +77,11 @@ func (g *proxyModuleGetter) ContentDir(ctx context.Context, path, version string
 	return fs.Sub(zr, path+"@"+version)
 }
 
+// SourceInfo gets information about a module's repo and source files by calling source.ModuleInfo.
+func (g *proxyModuleGetter) SourceInfo(ctx context.Context, path, version string) (*source.Info, error) {
+	return source.ModuleInfo(ctx, g.src, path, version)
+}
+
 // Version and commit time are pre specified when fetching a local module, as these
 // fields are normally obtained from a proxy.
 var (
@@ -143,6 +154,11 @@ func (g *directoryModuleGetter) ContentDir(ctx context.Context, path, version st
 	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
+}
+
 // An fsProxyModuleGetter gets modules from a directory in the filesystem
 // that is organized like the proxy, with paths that correspond to proxy
 // URLs. An example of such a directory is $(go env GOMODCACHE)/cache/download.
@@ -225,6 +241,11 @@ func (g *fsProxyModuleGetter) ContentDir(ctx context.Context, path, vers string)
 	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
+}
+
 // latestVersion gets the latest version that is in the directory.
 func (g *fsProxyModuleGetter) latestVersion(modulePath string) (_ string, err error) {
 	defer derrors.Wrap(&err, "fsProxyModuleGetter.latestVersion(%q)", modulePath)

internal/fetch/helper_test.go

@@ -148,7 +148,7 @@ func proxyFetcher(t *testing.T, withLicenseDetector bool, ctx context.Context, m
 		Files:      mod.Files,
 	}})
 	defer teardownProxy()
-	got := FetchModule(ctx, modulePath, fetchVersion, NewProxyModuleGetter(proxyClient), source.NewClientForTesting())
+	got := FetchModule(ctx, modulePath, fetchVersion, NewProxyModuleGetter(proxyClient, source.NewClientForTesting()))
 	if !withLicenseDetector {
 		return got, nil
 	}
@@ -174,7 +174,7 @@ func localFetcher(t *testing.T, withLicenseDetector bool, ctx context.Context, m
 	if err != nil {
 		t.Fatal(err)
 	}
-	got := FetchModule(ctx, modulePath, LocalVersion, g, source.NewClientForTesting())
+	got := FetchModule(ctx, modulePath, LocalVersion, g)
 	if !withLicenseDetector {
 		return got, nil
 	}

internal/fetchdatasource/fetchdatasource.go

@@ -22,7 +22,6 @@ import (
 	"golang.org/x/pkgsite/internal/fetch"
 	"golang.org/x/pkgsite/internal/log"
 	"golang.org/x/pkgsite/internal/proxy"
-	"golang.org/x/pkgsite/internal/source"
 	"golang.org/x/pkgsite/internal/version"
 )
 
@@ -40,7 +39,6 @@ type Options struct {
 	// If set, this will be used for latest-version information. To fetch modules from the proxy,
 	// include a ProxyModuleGetter in Getters.
 	ProxyClientForLatest *proxy.Client
-	SourceClient         *source.Client
 	BypassLicenseCheck   bool
 }
 
@@ -132,7 +130,7 @@ func (ds *FetchDataSource) fetch(ctx context.Context, modulePath, version string
 		log.Infof(ctx, "FetchDataSource: fetched %s@%s in %s with error %v", modulePath, version, time.Since(start), err)
 	}()
 	for _, g := range ds.opts.Getters {
-		fr := fetch.FetchModule(ctx, modulePath, version, g, ds.opts.SourceClient)
+		fr := fetch.FetchModule(ctx, modulePath, version, g)
 		if fr.Error == nil {
 			m := fr.Module
 			if ds.opts.BypassLicenseCheck {

internal/fetchdatasource/fetchdatasource_test.go

@@ -125,12 +125,11 @@ func setup(t *testing.T, testModules []*proxytest.Module, bypassLicenseCheck boo
 
 	getters := localGetters
 	if testModules != nil {
-		getters = append(getters, fetch.NewProxyModuleGetter(client))
+		getters = append(getters, fetch.NewProxyModuleGetter(client, source.NewClientForTesting()))
 	}
 
 	return ctx, Options{
 			Getters:              getters,
-			SourceClient:         source.NewClientForTesting(),
 			ProxyClientForLatest: client,
 			BypassLicenseCheck:   bypassLicenseCheck,
 		}.New(), func() {
@@ -340,7 +339,9 @@ func TestLocalGetUnitMeta(t *testing.T) {
 	ctx, ds, teardown := setup(t, defaultTestModules, true)
 	defer teardown()
 
-	sourceInfo := source.NewGitHubInfo("https://github.com/my/module", "", "v0.0.0")
+	// TODO(golang/go#47982): use the correct sourceInfo here.
+	var sourceInfo *source.Info
+
 	for _, test := range []struct {
 		path, modulePath string
 		want             *internal.UnitMeta

internal/frontend/fetch.go

@@ -556,7 +556,7 @@ func FetchAndUpdateState(ctx context.Context, modulePath, requestedVersion strin
 		derrors.Wrap(&err, "FetchAndUpdateState(%q, %q)", modulePath, requestedVersion)
 	}()
 
-	fr := fetch.FetchModule(ctx, modulePath, requestedVersion, fetch.NewProxyModuleGetter(proxyClient), sourceClient)
+	fr := fetch.FetchModule(ctx, modulePath, requestedVersion, fetch.NewProxyModuleGetter(proxyClient, sourceClient))
 	if fr.Error == nil {
 		// Only attempt to insert the module into module_version_states if the
 		// fetch process was successful.

internal/testing/integration/frontend_test.go

@@ -99,7 +99,7 @@ func processVersions(ctx context.Context, t *testing.T, testModules []*proxytest
 
 func fetchAndInsertModule(ctx context.Context, t *testing.T, tm *proxytest.Module, proxyClient *proxy.Client) {
 	sourceClient := source.NewClient(1 * time.Second)
-	res := fetch.FetchModule(ctx, tm.ModulePath, tm.Version, fetch.NewProxyModuleGetter(proxyClient), sourceClient)
+	res := fetch.FetchModule(ctx, tm.ModulePath, tm.Version, fetch.NewProxyModuleGetter(proxyClient, sourceClient))
 	if res.Error != nil {
 		t.Fatal(res.Error)
 	}

internal/worker/fetch.go

@@ -292,7 +292,7 @@ func (f *Fetcher) fetchAndInsertModule(ctx context.Context, modulePath, requeste
 		return ft
 	}
 
-	proxyGetter := fetch.NewProxyModuleGetter(f.ProxyClient)
+	proxyGetter := fetch.NewProxyModuleGetter(f.ProxyClient, f.SourceClient)
 	// Fetch the module, and the current @main and @master version of this module.
 	// The @main and @master version will be used to update the version_map
 	// target if applicable.
@@ -301,7 +301,7 @@ func (f *Fetcher) fetchAndInsertModule(ctx context.Context, modulePath, requeste
 	go func() {
 		defer wg.Done()
 		start := time.Now()
-		fr := fetch.FetchModule(ctx, modulePath, requestedVersion, proxyGetter, f.SourceClient)
+		fr := fetch.FetchModule(ctx, modulePath, requestedVersion, proxyGetter)
 		if fr == nil {
 			panic("fetch.FetchModule should never return a nil FetchResult")
 		}