From: Bryan C. Mills Date: Fri, 30 Oct 2020 16:24:51 +0000 (-0400) Subject: cmd/go/internal/modload: return a module-only result from QueryPattern X-Git-Tag: go1.16beta1~319 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=04b5b4f740a34a95a10253a0e34779bb259ec9c4;p=gostls13.git cmd/go/internal/modload: return a module-only result from QueryPattern This allows a single QueryPattern call to resolve a path that could be either a package or a module. It is important to be able to make a single QueryPattern call — rather than a QueryPattern followed by a Query for the specific module path — to provide appropriate fallback behavior: if the proxy returns package results but does not contain a module result, we don't want to fall back to the next proxy to look for the (probably-nonexistent) module. For #37438 Change-Id: I419b8bb3ab4565f443bb5cee9a8b206f453b9801 Reviewed-on: https://go-review.googlesource.com/c/go/+/266657 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot Reviewed-by: Jay Conrod --- diff --git a/src/cmd/go/internal/modget/get.go b/src/cmd/go/internal/modget/get.go index 171c070ab3..6d83af710f 100644 --- a/src/cmd/go/internal/modget/get.go +++ b/src/cmd/go/internal/modget/get.go @@ -912,22 +912,15 @@ func getQuery(ctx context.Context, path, vers string, prevM module.Version, forc // If it turns out to only exist as a module, we can detect the resulting // PackageNotInModuleError and avoid a second round-trip through (potentially) // all of the configured proxies. - results, err := modload.QueryPattern(ctx, path, vers, modload.Selected, allowed) + results, modOnly, err := modload.QueryPattern(ctx, path, vers, modload.Selected, allowed) if err != nil { - // If the path doesn't contain a wildcard, check whether it was actually a - // module path instead. If so, return that. - if !strings.Contains(path, "...") { - var modErr *modload.PackageNotInModuleError - if errors.As(err, &modErr) && modErr.Mod.Path == path { - if modErr.Mod.Version != vers { - logOncef("go: %s %s => %s", path, vers, modErr.Mod.Version) - } - return modErr.Mod, nil - } - } - return module.Version{}, err } + if len(results) == 0 { + // The path doesn't contain a wildcard, but was actually a + // module path instead. Return that. + return modOnly.Mod, nil + } m := results[0].Mod if m.Path != path { diff --git a/src/cmd/go/internal/modload/import.go b/src/cmd/go/internal/modload/import.go index e959347020..193edfdd20 100644 --- a/src/cmd/go/internal/modload/import.go +++ b/src/cmd/go/internal/modload/import.go @@ -383,7 +383,7 @@ func queryImport(ctx context.Context, path string) (module.Version, error) { // and return m, dir, ImpportMissingError. fmt.Fprintf(os.Stderr, "go: finding module for package %s\n", path) - candidates, err := QueryPattern(ctx, path, "latest", Selected, CheckAllowed) + candidates, err := QueryPackages(ctx, path, "latest", Selected, CheckAllowed) if err != nil { if errors.Is(err, fs.ErrNotExist) { // Return "cannot find module providing package […]" instead of whatever diff --git a/src/cmd/go/internal/modload/query.go b/src/cmd/go/internal/modload/query.go index 3927051015..2e9d72a6bf 100644 --- a/src/cmd/go/internal/modload/query.go +++ b/src/cmd/go/internal/modload/query.go @@ -504,20 +504,39 @@ type QueryResult struct { Packages []string } +// QueryPackages is like QueryPattern, but requires that the pattern match at +// least one package and omits the non-package result (if any). +func QueryPackages(ctx context.Context, pattern, query string, current func(string) string, allowed AllowedFunc) ([]QueryResult, error) { + pkgMods, modOnly, err := QueryPattern(ctx, pattern, query, current, allowed) + + if len(pkgMods) == 0 && err == nil { + return nil, &PackageNotInModuleError{ + Mod: modOnly.Mod, + Replacement: Replacement(modOnly.Mod), + Query: query, + Pattern: pattern, + } + } + + return pkgMods, err +} + // QueryPattern looks up the module(s) containing at least one package matching // the given pattern at the given version. The results are sorted by module path -// length in descending order. +// length in descending order. If any proxy provides a non-empty set of candidate +// modules, no further proxies are tried. // -// QueryPattern queries modules with package paths up to the first "..." -// in the pattern. For the pattern "example.com/a/b.../c", QueryPattern would -// consider prefixes of "example.com/a". If multiple modules have versions -// that match the query and packages that match the pattern, QueryPattern -// picks the one with the longest module path. +// For wildcard patterns, QueryPattern looks in modules with package paths up to +// the first "..." in the pattern. For the pattern "example.com/a/b.../c", +// QueryPattern would consider prefixes of "example.com/a". // // If any matching package is in the main module, QueryPattern considers only // the main module and only the version "latest", without checking for other // possible modules. -func QueryPattern(ctx context.Context, pattern, query string, current func(string) string, allowed AllowedFunc) ([]QueryResult, error) { +// +// QueryPattern always returns at least one QueryResult (which may be only +// modOnly) or a non-nil error. +func QueryPattern(ctx context.Context, pattern, query string, current func(string) string, allowed AllowedFunc) (pkgMods []QueryResult, modOnly *QueryResult, err error) { ctx, span := trace.StartSpan(ctx, "modload.QueryPattern "+pattern+" "+query) defer span.Done() @@ -531,6 +550,7 @@ func QueryPattern(ctx context.Context, pattern, query string, current func(strin } var match func(mod module.Version, root string, isLocal bool) *search.Match + matchPattern := search.MatchPattern(pattern) if i := strings.Index(pattern, "..."); i >= 0 { base = pathpkg.Dir(pattern[:i+3]) @@ -558,20 +578,29 @@ func QueryPattern(ctx context.Context, pattern, query string, current func(strin if HasModRoot() { m := match(Target, modRoot, true) if len(m.Pkgs) > 0 { - if query != "latest" { - return nil, fmt.Errorf("can't query specific version for package %s in the main module (%s)", pattern, Target.Path) + if query != "latest" && query != "upgrade" && query != "patch" { + return nil, nil, fmt.Errorf("can't query version %q for package %s in the main module (%s)", query, pattern, Target.Path) } if err := allowed(ctx, Target); err != nil { - return nil, fmt.Errorf("internal error: package %s is in the main module (%s), but version is not allowed: %w", pattern, Target.Path, err) + return nil, nil, fmt.Errorf("internal error: package %s is in the main module (%s), but version is not allowed: %w", pattern, Target.Path, err) } return []QueryResult{{ Mod: Target, Rev: &modfetch.RevInfo{Version: Target.Version}, Packages: m.Pkgs, - }}, nil + }}, nil, nil } if err := firstError(m); err != nil { - return nil, err + return nil, nil, err + } + + if query != "latest" && query != "upgrade" && query != "patch" && matchPattern(Target.Path) { + if err := allowed(ctx, Target); err == nil { + modOnly = &QueryResult{ + Mod: Target, + Rev: &modfetch.RevInfo{Version: Target.Version}, + } + } } } @@ -580,14 +609,17 @@ func QueryPattern(ctx context.Context, pattern, query string, current func(strin candidateModules = modulePrefixesExcludingTarget(base) ) if len(candidateModules) == 0 { - return nil, &PackageNotInModuleError{ - Mod: Target, - Query: query, - Pattern: pattern, + if modOnly == nil { + return nil, nil, &PackageNotInModuleError{ + Mod: Target, + Query: query, + Pattern: pattern, + } } + return nil, modOnly, nil } - err := modfetch.TryProxies(func(proxy string) error { + err = modfetch.TryProxies(func(proxy string) error { queryModule := func(ctx context.Context, path string) (r QueryResult, err error) { ctx, span := trace.StartSpan(ctx, "modload.QueryPattern.queryModule ["+proxy+"] "+path) defer span.Done() @@ -606,7 +638,7 @@ func QueryPattern(ctx context.Context, pattern, query string, current func(strin } m := match(r.Mod, root, isLocal) r.Packages = m.Pkgs - if len(r.Packages) == 0 { + if len(r.Packages) == 0 && !matchPattern(path) { if err := firstError(m); err != nil { return r, err } @@ -620,12 +652,19 @@ func QueryPattern(ctx context.Context, pattern, query string, current func(strin return r, nil } - var err error - results, err = queryPrefixModules(ctx, candidateModules, queryModule) + allResults, err := queryPrefixModules(ctx, candidateModules, queryModule) + results = allResults[:0] + for _, r := range allResults { + if len(r.Packages) == 0 { + modOnly = &r + } else { + results = append(results, r) + } + } return err }) - return results, err + return results[:len(results):len(results)], modOnly, err } // modulePrefixesExcludingTarget returns all prefixes of path that may plausibly @@ -651,11 +690,6 @@ func modulePrefixesExcludingTarget(path string) []string { return prefixes } -type prefixResult struct { - QueryResult - err error -} - func queryPrefixModules(ctx context.Context, candidateModules []string, queryModule func(ctx context.Context, path string) (QueryResult, error)) (found []QueryResult, err error) { ctx, span := trace.StartSpan(ctx, "modload.queryPrefixModules") defer span.Done() diff --git a/src/cmd/go/internal/work/build.go b/src/cmd/go/internal/work/build.go index 3531612dc6..181bb3b874 100644 --- a/src/cmd/go/internal/work/build.go +++ b/src/cmd/go/internal/work/build.go @@ -741,7 +741,7 @@ func installOutsideModule(ctx context.Context, args []string) { // Don't check for retractions if a specific revision is requested. allowed = nil } - qrs, err := modload.QueryPattern(ctx, patterns[0], version, modload.Selected, allowed) + qrs, err := modload.QueryPackages(ctx, patterns[0], version, modload.Selected, allowed) if err != nil { base.Fatalf("go install %s: %v", args[0], err) } diff --git a/src/cmd/go/testdata/script/mod_get_downgrade_missing.txt b/src/cmd/go/testdata/script/mod_get_downgrade_missing.txt index 49e17e6507..543f9f8111 100644 --- a/src/cmd/go/testdata/script/mod_get_downgrade_missing.txt +++ b/src/cmd/go/testdata/script/mod_get_downgrade_missing.txt @@ -3,14 +3,24 @@ cp go.mod go.mod.orig # getting a specific version of a module along with a pattern # not yet present in that module should report the version mismatch # rather than a "matched no packages" warning. + ! go get example.net/pkgadded@v1.1.0 example.net/pkgadded/subpkg/... stderr '^go get: conflicting versions for module example\.net/pkgadded: v1\.1\.0 and v1\.2\.0$' ! stderr 'matched no packages' cmp go.mod.orig go.mod -! go get example.net/pkgadded/...@v1.0.0 -stderr '^go get example\.net/pkgadded/\.\.\.@v1\.0\.0: module example\.net/pkgadded@v1\.0\.0 found, but does not contain packages matching example\.net/pkgadded/\.\.\.$' -cmp go.mod.orig go.mod + +# A wildcard pattern should match a package in a module with that path. + +go get example.net/pkgadded/...@v1.0.0 +go list -m all +stdout '^example.net/pkgadded v1.0.0' +cp go.mod.orig go.mod + + +# If we need to resolve a transitive dependency of a package, +# and another argument constrains away the version that provides that +# package, then 'go get' should fail with a useful error message. ! go get example.net/pkgadded@v1.0.0 . stderr -count=1 '^go: found example.net/pkgadded/subpkg in example.net/pkgadded v1\.2\.0$' # TODO: We shouldn't even try v1.2.0.