Skip to content

Commit

Permalink
cmd/go: when resolving packages, try all module paths before falling …
Browse files Browse the repository at this point in the history 
…back to the next proxy

Since we're mucking with error-propagation in modload.Query* anyway,
simplify the classification logic. Ensure that “module not found”
errors are reported as such in various places, since non-“not found”
errors terminate the module search.

Fixes #31785

Change-Id: Ie3ca5f4eec10a5f2a6037ec7e1c2cf47bd37a232
Reviewed-on: https://go-review.googlesource.com/c/go/+/177958
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
  • Loading branch information
@rsc
Bryan C. Mills authored and rsc committed May 24, 2019
1 parent 496b8db commit c7385e2
Show file tree
Hide file tree
Showing 14 changed files with 305 additions and 299 deletions.
2 changes: 1 addition & 1 deletion src/cmd/go/internal/modconv/convert_test.go
View file
Expand Up @@ -28,7 +28,7 @@ func TestMain(m *testing.M) {
}

func testMain(m *testing.M) int {
modfetch.SetProxy("direct")
cfg.GOPROXY = "direct"

if _, err := exec.LookPath("git"); err != nil {
fmt.Fprintln(os.Stderr, "skipping because git binary not found")
Expand Down
55 changes: 43 additions & 12 deletions src/cmd/go/internal/modfetch/cache.go
View file
Expand Up @@ -258,12 +258,12 @@ func (r *cachingRepo) Zip(dst io.Writer, version string) error {
// Stat is like Lookup(path).Stat(rev) but avoids the
// repository path resolution in Lookup if the result is
// already cached on local disk.
func Stat(path, rev string) (*RevInfo, error) {
func Stat(proxy, path, rev string) (*RevInfo, error) {
_, info, err := readDiskStat(path, rev)
if err == nil {
return info, nil
}
repo, err := Lookup(path)
repo, err := Lookup(proxy, path)
if err != nil {
return nil, err
}
Expand All @@ -276,9 +276,22 @@ func InfoFile(path, version string) (string, error) {
if !semver.IsValid(version) {
return "", fmt.Errorf("invalid version %q", version)
}
if _, err := Stat(path, version); err != nil {

if file, _, err := readDiskStat(path, version); err == nil {
return file, nil
}

err := TryProxies(func(proxy string) error {
repo, err := Lookup(proxy, path)
if err == nil {
_, err = repo.Stat(version)
}
return err
})
if err != nil {
return "", err
}

// Stat should have populated the disk cache for us.
file, _, err := readDiskStat(path, version)
if err != nil {
Expand All @@ -294,21 +307,39 @@ func GoMod(path, rev string) ([]byte, error) {
// Convert commit hash to pseudo-version
// to increase cache hit rate.
if !semver.IsValid(rev) {
info, err := Stat(path, rev)
if err != nil {
return nil, err
if _, info, err := readDiskStat(path, rev); err == nil {
rev = info.Version
} else {
err := TryProxies(func(proxy string) error {
repo, err := Lookup(proxy, path)
if err != nil {
return err
}
info, err := repo.Stat(rev)
if err == nil {
rev = info.Version
}
return err
})
if err != nil {
return nil, err
}
}
rev = info.Version
}

_, data, err := readDiskGoMod(path, rev)
if err == nil {
return data, nil
}
repo, err := Lookup(path)
if err != nil {
return nil, err
}
return repo.GoMod(rev)

err = TryProxies(func(proxy string) error {
repo, err := Lookup(proxy, path)
if err == nil {
data, err = repo.GoMod(rev)
}
return err
})
return data, err
}

// GoModFile is like GoMod but returns the name of the file containing
Expand Down
8 changes: 4 additions & 4 deletions src/cmd/go/internal/modfetch/coderepo_test.go
View file
Expand Up @@ -25,7 +25,7 @@ func TestMain(m *testing.M) {
}

func testMain(m *testing.M) int {
SetProxy("direct")
cfg.GOPROXY = "direct"

// The sum database is populated using a released version of the go command,
// but this test may include fixes for additional modules that previously
Expand Down Expand Up @@ -360,7 +360,7 @@ func TestCodeRepo(t *testing.T) {
return func(t *testing.T) {
t.Parallel()

repo, err := Lookup(tt.path)
repo, err := Lookup("direct", tt.path)
if tt.lookerr != "" {
if err != nil && err.Error() == tt.lookerr {
return
Expand Down Expand Up @@ -561,7 +561,7 @@ func TestCodeRepoVersions(t *testing.T) {
tt := tt
t.Parallel()

repo, err := Lookup(tt.path)
repo, err := Lookup("direct", tt.path)
if err != nil {
t.Fatalf("Lookup(%q): %v", tt.path, err)
}
Expand Down Expand Up @@ -616,7 +616,7 @@ func TestLatest(t *testing.T) {
tt := tt
t.Parallel()

repo, err := Lookup(tt.path)
repo, err := Lookup("direct", tt.path)
if err != nil {
t.Fatalf("Lookup(%q): %v", tt.path, err)
}
Expand Down
11 changes: 7 additions & 4 deletions src/cmd/go/internal/modfetch/fetch.go
View file
Expand Up @@ -205,13 +205,16 @@ func downloadZip(mod module.Version, zipfile string) (err error) {
}
}()

repo, err := Lookup(mod.Path)
err = TryProxies(func(proxy string) error {
repo, err := Lookup(proxy, mod.Path)
if err != nil {
return err
}
return repo.Zip(f, mod.Version)
})
if err != nil {
return err
}
if err := repo.Zip(f, mod.Version); err != nil {
return err
}

// Double-check that the paths within the zip file are well-formed.
//
Expand Down
180 changes: 33 additions & 147 deletions src/cmd/go/internal/modfetch/proxy.go
View file
Expand Up @@ -84,16 +84,6 @@ cached module versions with GOPROXY=https://example.com/proxy.
`,
}

var proxyURL = cfg.Getenv("GOPROXY")

// SetProxy sets the proxy to use when fetching modules.
// It accepts the same syntax as the GOPROXY environment variable,
// which also provides its default configuration.
// SetProxy must not be called after the first module fetch has begun.
func SetProxy(url string) {
proxyURL = url
}

var proxyOnce struct {
sync.Once
list []string
Expand All @@ -102,13 +92,25 @@ var proxyOnce struct {

func proxyURLs() ([]string, error) {
proxyOnce.Do(func() {
for _, proxyURL := range strings.Split(proxyURL, ",") {
if cfg.GONOPROXY != "" && cfg.GOPROXY != "direct" {
proxyOnce.list = append(proxyOnce.list, "noproxy")
}
for _, proxyURL := range strings.Split(cfg.GOPROXY, ",") {
proxyURL = strings.TrimSpace(proxyURL)
if proxyURL == "" {
continue
}
if proxyURL == "off" {
// "off" always fails hard, so can stop walking list.
proxyOnce.list = append(proxyOnce.list, "off")
break
}
if proxyURL == "direct" {
proxyOnce.list = append(proxyOnce.list, "direct")
continue
// For now, "direct" is the end of the line. We may decide to add some
// sort of fallback behavior for them in the future, so ignore
// subsequent entries for forward-compatibility.
break
}

// Check that newProxyRepo accepts the URL.
Expand All @@ -125,32 +127,30 @@ func proxyURLs() ([]string, error) {
return proxyOnce.list, proxyOnce.err
}

func lookupProxy(path string) (Repo, error) {
list, err := proxyURLs()
// TryProxies iterates f over each configured proxy (including "noproxy" and
// "direct" if applicable) until f returns an error that is not
// equivalent to os.ErrNotExist.
//
// TryProxies then returns that final error.
//
// If GOPROXY is set to "off", TryProxies invokes f once with the argument
// "off".
func TryProxies(f func(proxy string) error) error {
proxies, err := proxyURLs()
if err != nil {
return nil, err
return err
}
if len(proxies) == 0 {
return f("off")
}

var repos listRepo
for _, u := range list {
var r Repo
if u == "direct" {
// lookupDirect does actual network traffic.
// Especially if GOPROXY="http://mainproxy,direct",
// avoid the network until we need it by using a lazyRepo wrapper.
r = &lazyRepo{setup: lookupDirect, path: path}
} else {
// The URL itself was checked in proxyURLs.
// The only possible error here is a bad path,
// so we can return it unconditionally.
r, err = newProxyRepo(u, path)
if err != nil {
return nil, err
}
for _, proxy := range proxies {
err = f(proxy)
if !errors.Is(err, os.ErrNotExist) {
break
}
repos = append(repos, r)
}
return repos, nil
return err
}

type proxyRepo struct {
Expand Down Expand Up @@ -342,117 +342,3 @@ func (p *proxyRepo) Zip(dst io.Writer, version string) error {
func pathEscape(s string) string {
return strings.ReplaceAll(url.PathEscape(s), "%2F", "/")
}

// A lazyRepo is a lazily-initialized Repo,
// constructed on demand by calling setup.
type lazyRepo struct {
path string
setup func(string) (Repo, error)
once sync.Once
repo Repo
err error
}

func (r *lazyRepo) init() {
r.repo, r.err = r.setup(r.path)
}

func (r *lazyRepo) ModulePath() string {
return r.path
}

func (r *lazyRepo) Versions(prefix string) ([]string, error) {
if r.once.Do(r.init); r.err != nil {
return nil, r.err
}
return r.repo.Versions(prefix)
}

func (r *lazyRepo) Stat(rev string) (*RevInfo, error) {
if r.once.Do(r.init); r.err != nil {
return nil, r.err
}
return r.repo.Stat(rev)
}

func (r *lazyRepo) Latest() (*RevInfo, error) {
if r.once.Do(r.init); r.err != nil {
return nil, r.err
}
return r.repo.Latest()
}

func (r *lazyRepo) GoMod(version string) ([]byte, error) {
if r.once.Do(r.init); r.err != nil {
return nil, r.err
}
return r.repo.GoMod(version)
}

func (r *lazyRepo) Zip(dst io.Writer, version string) error {
if r.once.Do(r.init); r.err != nil {
return r.err
}
return r.repo.Zip(dst, version)
}

// A listRepo is a preference list of Repos.
// The list must be non-empty and all Repos
// must return the same result from ModulePath.
// For each method, the repos are tried in order
// until one succeeds or returns a non-ErrNotExist (non-404) error.
type listRepo []Repo

func (l listRepo) ModulePath() string {
return l[0].ModulePath()
}

func (l listRepo) Versions(prefix string) ([]string, error) {
for i, r := range l {
v, err := r.Versions(prefix)
if i == len(l)-1 || !errors.Is(err, os.ErrNotExist) {
return v, err
}
}
panic("no repos")
}

func (l listRepo) Stat(rev string) (*RevInfo, error) {
for i, r := range l {
info, err := r.Stat(rev)
if i == len(l)-1 || !errors.Is(err, os.ErrNotExist) {
return info, err
}
}
panic("no repos")
}

func (l listRepo) Latest() (*RevInfo, error) {
for i, r := range l {
info, err := r.Latest()
if i == len(l)-1 || !errors.Is(err, os.ErrNotExist) {
return info, err
}
}
panic("no repos")
}

func (l listRepo) GoMod(version string) ([]byte, error) {
for i, r := range l {
data, err := r.GoMod(version)
if i == len(l)-1 || !errors.Is(err, os.ErrNotExist) {
return data, err
}
}
panic("no repos")
}

func (l listRepo) Zip(dst io.Writer, version string) error {
for i, r := range l {
err := r.Zip(dst, version)
if i == len(l)-1 || !errors.Is(err, os.ErrNotExist) {
return err
}
}
panic("no repos")
}

0 comments on commit c7385e2

Please sign in to comment.