Skip to content

Commit

Permalink
cmd/go/internal/modload: avoid using the global build list in QueryPa…
Browse files Browse the repository at this point in the history
…ttern

The Query function allows the caller to specify the current version of
the requested module, but the QueryPattern function is missing that
parameter: instead, it always assumes that the current version is the
one selected from the global build list.

This change removes that assumption, instead adding a callback
function to determine the current version. (The callback is currently
invoked once per candidate module, regardless of whether that module
exists, but in a future change we can refactor it to invoke the
callback only when needed.)

For #36460
For #40775

Change-Id: I001a4a8ab24f5b4fcc66a670d9bd305b47e948ba
Reviewed-on: https://go-review.googlesource.com/c/go/+/261640
Trust: Bryan C. Mills <[email protected]>
Run-TryBot: Bryan C. Mills <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Jay Conrod <[email protected]>
Reviewed-by: Michael Matloob <[email protected]>
  • Loading branch information
Bryan C. Mills committed Oct 16, 2020
1 parent ff05273 commit 41162be
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 16 deletions.
2 changes: 1 addition & 1 deletion src/cmd/go/internal/modget/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,7 @@ 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, allowed)
results, 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.
Expand Down
15 changes: 15 additions & 0 deletions src/cmd/go/internal/modload/buildlist.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,21 @@ func LoadedModules() []module.Version {
return buildList
}

// Selected returns the selected version of the module with the given path, or
// the empty string if the given module has no selected version
// (either because it is not required or because it is the Target module).
func Selected(path string) (version string) {
if path == Target.Path {
return ""
}
for _, m := range buildList {
if m.Path == path {
return m.Version
}
}
return ""
}

// SetBuildList sets the module build list.
// The caller is responsible for ensuring that the list is valid.
// SetBuildList does not retain a reference to the original list.
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/go/internal/modload/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,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", CheckAllowed)
candidates, err := QueryPattern(ctx, path, "latest", Selected, CheckAllowed)
if err != nil {
if errors.Is(err, os.ErrNotExist) {
// Return "cannot find module providing package […]" instead of whatever
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/go/internal/modload/modfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func checkRetractions(ctx context.Context, m module.Version) error {
// Ignore exclusions from the main module's go.mod.
// We may need to account for the current version: for example,
// v2.0.0+incompatible is not "latest" if v1.0.0 is current.
rev, err := Query(ctx, path, "latest", findCurrentVersion(path), nil)
rev, err := Query(ctx, path, "latest", Selected(path), nil)
if err != nil {
return &entry{nil, err}
}
Expand Down
15 changes: 3 additions & 12 deletions src/cmd/go/internal/modload/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ type QueryResult struct {
// 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, allowed AllowedFunc) ([]QueryResult, error) {
func QueryPattern(ctx context.Context, pattern, query string, current func(string) string, allowed AllowedFunc) ([]QueryResult, error) {
ctx, span := trace.StartSpan(ctx, "modload.QueryPattern "+pattern+" "+query)
defer span.Done()

Expand Down Expand Up @@ -591,9 +591,9 @@ func QueryPattern(ctx context.Context, pattern, query string, allowed AllowedFun
ctx, span := trace.StartSpan(ctx, "modload.QueryPattern.queryModule ["+proxy+"] "+path)
defer span.Done()

current := findCurrentVersion(path)
pathCurrent := current(path)
r.Mod.Path = path
r.Rev, err = queryProxy(ctx, proxy, path, query, current, allowed)
r.Rev, err = queryProxy(ctx, proxy, path, query, pathCurrent, allowed)
if err != nil {
return r, err
}
Expand Down Expand Up @@ -649,15 +649,6 @@ func modulePrefixesExcludingTarget(path string) []string {
return prefixes
}

func findCurrentVersion(path string) string {
for _, m := range buildList {
if m.Path == path {
return m.Version
}
}
return ""
}

type prefixResult struct {
QueryResult
err error
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/go/internal/work/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, allowed)
qrs, err := modload.QueryPattern(ctx, patterns[0], version, modload.Selected, allowed)
if err != nil {
base.Fatalf("go install %s: %v", args[0], err)
}
Expand Down

0 comments on commit 41162be

Please sign in to comment.