mirror of
https://github.com/golang/go.git
synced 2025-05-22 07:59:19 +00:00
cmd/go/internal/modfetch: remove error return from Lookup
We generally don't care about errors in resolving a repo if the result we're looking for is already in the module cache. Moreover, we can avoid some expense in initializing the repo if all of the methods we plan to call on it hit in the cache — especially when using GOPROXY=direct. This also incidentally fixes a possible (but rare) bug in Download: we had forgotten to reset the downloaded file in case the Zip method returned an error after writing a nonzero number of bytes. For #37438 Change-Id: Ib64f10f763f6d1936536b8e1f7d31ed1b463e955 Reviewed-on: https://go-review.googlesource.com/c/go/+/259158 Trust: Bryan C. Mills <bcmills@google.com> Trust: Jay Conrod <jayconrod@google.com> Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
This commit is contained in:
parent
3a65abfbda
commit
c9211577eb
@ -14,6 +14,7 @@ import (
|
|||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
|
"sync"
|
||||||
|
|
||||||
"cmd/go/internal/base"
|
"cmd/go/internal/base"
|
||||||
"cmd/go/internal/cfg"
|
"cmd/go/internal/cfg"
|
||||||
@ -155,16 +156,30 @@ func SideLock() (unlock func(), err error) {
|
|||||||
type cachingRepo struct {
|
type cachingRepo struct {
|
||||||
path string
|
path string
|
||||||
cache par.Cache // cache for all operations
|
cache par.Cache // cache for all operations
|
||||||
r Repo
|
|
||||||
|
once sync.Once
|
||||||
|
initRepo func() (Repo, error)
|
||||||
|
r Repo
|
||||||
}
|
}
|
||||||
|
|
||||||
func newCachingRepo(r Repo) *cachingRepo {
|
func newCachingRepo(path string, initRepo func() (Repo, error)) *cachingRepo {
|
||||||
return &cachingRepo{
|
return &cachingRepo{
|
||||||
r: r,
|
path: path,
|
||||||
path: r.ModulePath(),
|
initRepo: initRepo,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (r *cachingRepo) repo() Repo {
|
||||||
|
r.once.Do(func() {
|
||||||
|
var err error
|
||||||
|
r.r, err = r.initRepo()
|
||||||
|
if err != nil {
|
||||||
|
r.r = errRepo{r.path, err}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
return r.r
|
||||||
|
}
|
||||||
|
|
||||||
func (r *cachingRepo) ModulePath() string {
|
func (r *cachingRepo) ModulePath() string {
|
||||||
return r.path
|
return r.path
|
||||||
}
|
}
|
||||||
@ -175,7 +190,7 @@ func (r *cachingRepo) Versions(prefix string) ([]string, error) {
|
|||||||
err error
|
err error
|
||||||
}
|
}
|
||||||
c := r.cache.Do("versions:"+prefix, func() interface{} {
|
c := r.cache.Do("versions:"+prefix, func() interface{} {
|
||||||
list, err := r.r.Versions(prefix)
|
list, err := r.repo().Versions(prefix)
|
||||||
return cached{list, err}
|
return cached{list, err}
|
||||||
}).(cached)
|
}).(cached)
|
||||||
|
|
||||||
@ -197,7 +212,7 @@ func (r *cachingRepo) Stat(rev string) (*RevInfo, error) {
|
|||||||
return cachedInfo{info, nil}
|
return cachedInfo{info, nil}
|
||||||
}
|
}
|
||||||
|
|
||||||
info, err = r.r.Stat(rev)
|
info, err = r.repo().Stat(rev)
|
||||||
if err == nil {
|
if err == nil {
|
||||||
// If we resolved, say, 1234abcde to v0.0.0-20180604122334-1234abcdef78,
|
// If we resolved, say, 1234abcde to v0.0.0-20180604122334-1234abcdef78,
|
||||||
// then save the information under the proper version, for future use.
|
// then save the information under the proper version, for future use.
|
||||||
@ -224,7 +239,7 @@ func (r *cachingRepo) Stat(rev string) (*RevInfo, error) {
|
|||||||
|
|
||||||
func (r *cachingRepo) Latest() (*RevInfo, error) {
|
func (r *cachingRepo) Latest() (*RevInfo, error) {
|
||||||
c := r.cache.Do("latest:", func() interface{} {
|
c := r.cache.Do("latest:", func() interface{} {
|
||||||
info, err := r.r.Latest()
|
info, err := r.repo().Latest()
|
||||||
|
|
||||||
// Save info for likely future Stat call.
|
// Save info for likely future Stat call.
|
||||||
if err == nil {
|
if err == nil {
|
||||||
@ -258,7 +273,7 @@ func (r *cachingRepo) GoMod(version string) ([]byte, error) {
|
|||||||
return cached{text, nil}
|
return cached{text, nil}
|
||||||
}
|
}
|
||||||
|
|
||||||
text, err = r.r.GoMod(version)
|
text, err = r.repo().GoMod(version)
|
||||||
if err == nil {
|
if err == nil {
|
||||||
if err := checkGoMod(r.path, version, text); err != nil {
|
if err := checkGoMod(r.path, version, text); err != nil {
|
||||||
return cached{text, err}
|
return cached{text, err}
|
||||||
@ -277,26 +292,11 @@ func (r *cachingRepo) GoMod(version string) ([]byte, error) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (r *cachingRepo) Zip(dst io.Writer, version string) error {
|
func (r *cachingRepo) Zip(dst io.Writer, version string) error {
|
||||||
return r.r.Zip(dst, version)
|
return r.repo().Zip(dst, version)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Stat is like Lookup(path).Stat(rev) but avoids the
|
// InfoFile is like Lookup(path).Stat(version) but returns the name of the file
|
||||||
// repository path resolution in Lookup if the result is
|
// containing the cached information.
|
||||||
// already cached on local disk.
|
|
||||||
func Stat(proxy, path, rev string) (*RevInfo, error) {
|
|
||||||
_, info, err := readDiskStat(path, rev)
|
|
||||||
if err == nil {
|
|
||||||
return info, nil
|
|
||||||
}
|
|
||||||
repo, err := Lookup(proxy, path)
|
|
||||||
if err != nil {
|
|
||||||
return nil, err
|
|
||||||
}
|
|
||||||
return repo.Stat(rev)
|
|
||||||
}
|
|
||||||
|
|
||||||
// InfoFile is like Stat but returns the name of the file containing
|
|
||||||
// the cached information.
|
|
||||||
func InfoFile(path, version string) (string, error) {
|
func InfoFile(path, version string) (string, error) {
|
||||||
if !semver.IsValid(version) {
|
if !semver.IsValid(version) {
|
||||||
return "", fmt.Errorf("invalid version %q", version)
|
return "", fmt.Errorf("invalid version %q", version)
|
||||||
@ -307,10 +307,7 @@ func InfoFile(path, version string) (string, error) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
err := TryProxies(func(proxy string) error {
|
err := TryProxies(func(proxy string) error {
|
||||||
repo, err := Lookup(proxy, path)
|
_, err := Lookup(proxy, path).Stat(version)
|
||||||
if err == nil {
|
|
||||||
_, err = repo.Stat(version)
|
|
||||||
}
|
|
||||||
return err
|
return err
|
||||||
})
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@ -336,11 +333,7 @@ func GoMod(path, rev string) ([]byte, error) {
|
|||||||
rev = info.Version
|
rev = info.Version
|
||||||
} else {
|
} else {
|
||||||
err := TryProxies(func(proxy string) error {
|
err := TryProxies(func(proxy string) error {
|
||||||
repo, err := Lookup(proxy, path)
|
info, err := Lookup(proxy, path).Stat(rev)
|
||||||
if err != nil {
|
|
||||||
return err
|
|
||||||
}
|
|
||||||
info, err := repo.Stat(rev)
|
|
||||||
if err == nil {
|
if err == nil {
|
||||||
rev = info.Version
|
rev = info.Version
|
||||||
}
|
}
|
||||||
@ -357,11 +350,8 @@ func GoMod(path, rev string) ([]byte, error) {
|
|||||||
return data, nil
|
return data, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
err = TryProxies(func(proxy string) error {
|
err = TryProxies(func(proxy string) (err error) {
|
||||||
repo, err := Lookup(proxy, path)
|
data, err = Lookup(proxy, path).GoMod(rev)
|
||||||
if err == nil {
|
|
||||||
data, err = repo.GoMod(rev)
|
|
||||||
}
|
|
||||||
return err
|
return err
|
||||||
})
|
})
|
||||||
return data, err
|
return data, err
|
||||||
|
@ -60,7 +60,6 @@ var altVgotests = map[string]string{
|
|||||||
type codeRepoTest struct {
|
type codeRepoTest struct {
|
||||||
vcs string
|
vcs string
|
||||||
path string
|
path string
|
||||||
lookErr string
|
|
||||||
mpath string
|
mpath string
|
||||||
rev string
|
rev string
|
||||||
err string
|
err string
|
||||||
@ -332,9 +331,9 @@ var codeRepoTests = []codeRepoTest{
|
|||||||
// package in subdirectory - custom domain
|
// package in subdirectory - custom domain
|
||||||
// In general we can't reject these definitively in Lookup,
|
// In general we can't reject these definitively in Lookup,
|
||||||
// but gopkg.in is special.
|
// but gopkg.in is special.
|
||||||
vcs: "git",
|
vcs: "git",
|
||||||
path: "gopkg.in/yaml.v2/abc",
|
path: "gopkg.in/yaml.v2/abc",
|
||||||
lookErr: "invalid module path \"gopkg.in/yaml.v2/abc\"",
|
err: "invalid module path \"gopkg.in/yaml.v2/abc\"",
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
// package in subdirectory - github
|
// package in subdirectory - github
|
||||||
@ -440,16 +439,7 @@ func TestCodeRepo(t *testing.T) {
|
|||||||
testenv.MustHaveExecPath(t, tt.vcs)
|
testenv.MustHaveExecPath(t, tt.vcs)
|
||||||
}
|
}
|
||||||
|
|
||||||
repo, err := Lookup("direct", tt.path)
|
repo := Lookup("direct", tt.path)
|
||||||
if tt.lookErr != "" {
|
|
||||||
if err != nil && err.Error() == tt.lookErr {
|
|
||||||
return
|
|
||||||
}
|
|
||||||
t.Errorf("Lookup(%q): %v, want error %q", tt.path, err, tt.lookErr)
|
|
||||||
}
|
|
||||||
if err != nil {
|
|
||||||
t.Fatalf("Lookup(%q): %v", tt.path, err)
|
|
||||||
}
|
|
||||||
|
|
||||||
if tt.mpath == "" {
|
if tt.mpath == "" {
|
||||||
tt.mpath = tt.path
|
tt.mpath = tt.path
|
||||||
@ -685,10 +675,7 @@ func TestCodeRepoVersions(t *testing.T) {
|
|||||||
testenv.MustHaveExecPath(t, tt.vcs)
|
testenv.MustHaveExecPath(t, tt.vcs)
|
||||||
}
|
}
|
||||||
|
|
||||||
repo, err := Lookup("direct", tt.path)
|
repo := Lookup("direct", tt.path)
|
||||||
if err != nil {
|
|
||||||
t.Fatalf("Lookup(%q): %v", tt.path, err)
|
|
||||||
}
|
|
||||||
list, err := repo.Versions(tt.prefix)
|
list, err := repo.Versions(tt.prefix)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("Versions(%q): %v", tt.prefix, err)
|
t.Fatalf("Versions(%q): %v", tt.prefix, err)
|
||||||
@ -763,10 +750,7 @@ func TestLatest(t *testing.T) {
|
|||||||
testenv.MustHaveExecPath(t, tt.vcs)
|
testenv.MustHaveExecPath(t, tt.vcs)
|
||||||
}
|
}
|
||||||
|
|
||||||
repo, err := Lookup("direct", tt.path)
|
repo := Lookup("direct", tt.path)
|
||||||
if err != nil {
|
|
||||||
t.Fatalf("Lookup(%q): %v", tt.path, err)
|
|
||||||
}
|
|
||||||
info, err := repo.Latest()
|
info, err := repo.Latest()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
if tt.err != "" {
|
if tt.err != "" {
|
||||||
|
@ -233,12 +233,28 @@ func downloadZip(ctx context.Context, mod module.Version, zipfile string) (err e
|
|||||||
}
|
}
|
||||||
}()
|
}()
|
||||||
|
|
||||||
|
var unrecoverableErr error
|
||||||
err = TryProxies(func(proxy string) error {
|
err = TryProxies(func(proxy string) error {
|
||||||
repo, err := Lookup(proxy, mod.Path)
|
if unrecoverableErr != nil {
|
||||||
if err != nil {
|
return unrecoverableErr
|
||||||
return err
|
|
||||||
}
|
}
|
||||||
return repo.Zip(f, mod.Version)
|
repo := Lookup(proxy, mod.Path)
|
||||||
|
err := repo.Zip(f, mod.Version)
|
||||||
|
if err != nil {
|
||||||
|
// Zip may have partially written to f before failing.
|
||||||
|
// (Perhaps the server crashed while sending the file?)
|
||||||
|
// Since we allow fallback on error in some cases, we need to fix up the
|
||||||
|
// file to be empty again for the next attempt.
|
||||||
|
if _, err := f.Seek(0, io.SeekStart); err != nil {
|
||||||
|
unrecoverableErr = err
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
if err := f.Truncate(0); err != nil {
|
||||||
|
unrecoverableErr = err
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return err
|
||||||
})
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
|
@ -188,27 +188,26 @@ type lookupCacheKey struct {
|
|||||||
//
|
//
|
||||||
// A successful return does not guarantee that the module
|
// A successful return does not guarantee that the module
|
||||||
// has any defined versions.
|
// has any defined versions.
|
||||||
func Lookup(proxy, path string) (Repo, error) {
|
func Lookup(proxy, path string) Repo {
|
||||||
if traceRepo {
|
if traceRepo {
|
||||||
defer logCall("Lookup(%q, %q)", proxy, path)()
|
defer logCall("Lookup(%q, %q)", proxy, path)()
|
||||||
}
|
}
|
||||||
|
|
||||||
type cached struct {
|
type cached struct {
|
||||||
r Repo
|
r Repo
|
||||||
err error
|
|
||||||
}
|
}
|
||||||
c := lookupCache.Do(lookupCacheKey{proxy, path}, func() interface{} {
|
c := lookupCache.Do(lookupCacheKey{proxy, path}, func() interface{} {
|
||||||
r, err := lookup(proxy, path)
|
r := newCachingRepo(path, func() (Repo, error) {
|
||||||
if err == nil {
|
r, err := lookup(proxy, path)
|
||||||
if traceRepo {
|
if err == nil && traceRepo {
|
||||||
r = newLoggingRepo(r)
|
r = newLoggingRepo(r)
|
||||||
}
|
}
|
||||||
r = newCachingRepo(r)
|
return r, err
|
||||||
}
|
})
|
||||||
return cached{r, err}
|
return cached{r}
|
||||||
}).(cached)
|
}).(cached)
|
||||||
|
|
||||||
return c.r, c.err
|
return c.r
|
||||||
}
|
}
|
||||||
|
|
||||||
// lookup returns the module with the given module path.
|
// lookup returns the module with the given module path.
|
||||||
@ -228,7 +227,7 @@ func lookup(proxy, path string) (r Repo, err error) {
|
|||||||
|
|
||||||
switch proxy {
|
switch proxy {
|
||||||
case "off":
|
case "off":
|
||||||
return nil, errProxyOff
|
return errRepo{path, errProxyOff}, nil
|
||||||
case "direct":
|
case "direct":
|
||||||
return lookupDirect(path)
|
return lookupDirect(path)
|
||||||
case "noproxy":
|
case "noproxy":
|
||||||
@ -407,6 +406,23 @@ func (l *loggingRepo) Zip(dst io.Writer, version string) error {
|
|||||||
return l.r.Zip(dst, version)
|
return l.r.Zip(dst, version)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// errRepo is a Repo that returns the same error for all operations.
|
||||||
|
//
|
||||||
|
// It is useful in conjunction with caching, since cache hits will not attempt
|
||||||
|
// the prohibited operations.
|
||||||
|
type errRepo struct {
|
||||||
|
modulePath string
|
||||||
|
err error
|
||||||
|
}
|
||||||
|
|
||||||
|
func (r errRepo) ModulePath() string { return r.modulePath }
|
||||||
|
|
||||||
|
func (r errRepo) Versions(prefix string) (tags []string, err error) { return nil, r.err }
|
||||||
|
func (r errRepo) Stat(rev string) (*RevInfo, error) { return nil, r.err }
|
||||||
|
func (r errRepo) Latest() (*RevInfo, error) { return nil, r.err }
|
||||||
|
func (r errRepo) GoMod(version string) ([]byte, error) { return nil, r.err }
|
||||||
|
func (r errRepo) Zip(dst io.Writer, version string) error { return r.err }
|
||||||
|
|
||||||
// A notExistError is like os.ErrNotExist, but with a custom message
|
// A notExistError is like os.ErrNotExist, but with a custom message
|
||||||
type notExistError struct {
|
type notExistError struct {
|
||||||
err error
|
err error
|
||||||
|
@ -77,11 +77,7 @@ func versions(ctx context.Context, path string, allowed AllowedFunc) ([]string,
|
|||||||
// so there's no need for us to add extra caching here.
|
// so there's no need for us to add extra caching here.
|
||||||
var versions []string
|
var versions []string
|
||||||
err := modfetch.TryProxies(func(proxy string) error {
|
err := modfetch.TryProxies(func(proxy string) error {
|
||||||
repo, err := modfetch.Lookup(proxy, path)
|
allVersions, err := modfetch.Lookup(proxy, path).Versions("")
|
||||||
if err != nil {
|
|
||||||
return err
|
|
||||||
}
|
|
||||||
allVersions, err := repo.Versions("")
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
@ -229,14 +229,15 @@ func queryProxy(ctx context.Context, proxy, path, query, current string, allowed
|
|||||||
// If the identifier is not a canonical semver tag — including if it's a
|
// If the identifier is not a canonical semver tag — including if it's a
|
||||||
// semver tag with a +metadata suffix — then modfetch.Stat will populate
|
// semver tag with a +metadata suffix — then modfetch.Stat will populate
|
||||||
// info.Version with a suitable pseudo-version.
|
// info.Version with a suitable pseudo-version.
|
||||||
info, err := modfetch.Stat(proxy, path, query)
|
repo := modfetch.Lookup(proxy, path)
|
||||||
|
info, err := repo.Stat(query)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
queryErr := err
|
queryErr := err
|
||||||
// The full query doesn't correspond to a tag. If it is a semantic version
|
// The full query doesn't correspond to a tag. If it is a semantic version
|
||||||
// with a +metadata suffix, see if there is a tag without that suffix:
|
// with a +metadata suffix, see if there is a tag without that suffix:
|
||||||
// semantic versioning defines them to be equivalent.
|
// semantic versioning defines them to be equivalent.
|
||||||
if canonicalQuery != "" && query != canonicalQuery {
|
if canonicalQuery != "" && query != canonicalQuery {
|
||||||
info, err = modfetch.Stat(proxy, path, canonicalQuery)
|
info, err = repo.Stat(canonicalQuery)
|
||||||
if err != nil && !errors.Is(err, os.ErrNotExist) {
|
if err != nil && !errors.Is(err, os.ErrNotExist) {
|
||||||
return info, err
|
return info, err
|
||||||
}
|
}
|
||||||
@ -266,10 +267,7 @@ func queryProxy(ctx context.Context, proxy, path, query, current string, allowed
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Load versions and execute query.
|
// Load versions and execute query.
|
||||||
repo, err := modfetch.Lookup(proxy, path)
|
repo := modfetch.Lookup(proxy, path)
|
||||||
if err != nil {
|
|
||||||
return nil, err
|
|
||||||
}
|
|
||||||
versions, err := repo.Versions(prefix)
|
versions, err := repo.Versions(prefix)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
|
Loading…
x
Reference in New Issue
Block a user