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 <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
diff --git a/src/cmd/go/internal/modget/get.go b/src/cmd/go/internal/modget/get.go
index 171c070..6d83af7 100644
--- a/src/cmd/go/internal/modget/get.go
+++ b/src/cmd/go/internal/modget/get.go
@@ -912,22 +912,15 @@
// 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 e959347..193edfd 100644
--- a/src/cmd/go/internal/modload/import.go
+++ b/src/cmd/go/internal/modload/import.go
@@ -383,7 +383,7 @@
// 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 3927051..2e9d72a 100644
--- a/src/cmd/go/internal/modload/query.go
+++ b/src/cmd/go/internal/modload/query.go
@@ -504,20 +504,39 @@
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 @@
}
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 @@
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 @@
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 @@
}
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 @@
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 @@
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 3531612..181bb3b 100644
--- a/src/cmd/go/internal/work/build.go
+++ b/src/cmd/go/internal/work/build.go
@@ -741,7 +741,7 @@
// 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 49e17e6..543f9f8 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 @@
# 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.
