mirror of
https://github.com/golang/go
synced 2024-11-26 05:48:05 -07:00
cmd/go/internal/modload: use (*loadPkg).mod only to indicate the module from which the package was loaded
The (*loadPkg).mod field normally indicates the module from which the package was loaded. However, if the package was missing, we previously used the mod field to instead store the module from which we intend to load the package next time around. That sort of dual use makes the semantics (and synchronization) of the mod field much more complex to reason about. For example, it would be nice to have the invariant that the mod field is always one of the modules in the overall build list, or one of the modules selected in the overall module graph. Similarly, it would be nice to have the invariant that the version indicated by the mod field can coexist with (without upgrading) all of the other versions indicated in the mod fields of other packages. This repurposing of the mod field appears to be solely in the service of storing the module when resolving missing imports. To keep conceptually-separate fields separate, I have changed resolveMissingImports to store a slice of package–module pairs, instead of just packages that need to be revisited. This may increase allocation pressure slightly if we have many unresolved packages, but most packages are not unresolved, and it seems worth the cost to use a little extra memory if it means we can reason more clearly about the (quite complex) behaviors of the module loader. For #36460 Change-Id: Ic434df0f38185c6e9e892c5e9ba9ff53b3efe01f Reviewed-on: https://go-review.googlesource.com/c/go/+/312930 Trust: Bryan C. Mills <bcmills@google.com> Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Michael Matloob <matloob@golang.org>
This commit is contained in:
parent
0d1280c685
commit
a53dc4c1ce
@ -1057,7 +1057,11 @@ func (ld *loader) updateRequirements(ctx context.Context, add []module.Version)
|
||||
// resolveMissingImports returns a map from each new module version to
|
||||
// the first missing package that module would resolve.
|
||||
func (ld *loader) resolveMissingImports(ctx context.Context) (modAddedBy map[module.Version]*loadPkg) {
|
||||
var needPkgs []*loadPkg
|
||||
type pkgMod struct {
|
||||
pkg *loadPkg
|
||||
mod *module.Version
|
||||
}
|
||||
var pkgMods []pkgMod
|
||||
for _, pkg := range ld.pkgs {
|
||||
if pkg.err == nil {
|
||||
continue
|
||||
@ -1072,24 +1076,47 @@ func (ld *loader) resolveMissingImports(ctx context.Context) (modAddedBy map[mod
|
||||
continue
|
||||
}
|
||||
|
||||
needPkgs = append(needPkgs, pkg)
|
||||
|
||||
pkg := pkg
|
||||
var mod module.Version
|
||||
ld.work.Add(func() {
|
||||
pkg.mod, pkg.err = queryImport(ctx, pkg.path, ld.requirements)
|
||||
var err error
|
||||
mod, err = queryImport(ctx, pkg.path, ld.requirements)
|
||||
if err != nil {
|
||||
// pkg.err was already non-nil, so we can reasonably attribute the error
|
||||
// for pkg to either the original error or the one returned by
|
||||
// queryImport. The existing error indicates only that we couldn't find
|
||||
// the package, whereas the query error also explains why we didn't fix
|
||||
// the problem — so we prefer the latter.
|
||||
pkg.err = err
|
||||
}
|
||||
|
||||
// err is nil, but we intentionally leave pkg.err non-nil and pkg.mod
|
||||
// unset: we still haven't satisfied other invariants of a
|
||||
// successfully-loaded package, such as scanning and loading the imports
|
||||
// of that package. If we succeed in resolving the new dependency graph,
|
||||
// the caller can reload pkg and update the error at that point.
|
||||
//
|
||||
// Even then, the package might not be loaded from the version we've
|
||||
// identified here. The module may be upgraded by some other dependency,
|
||||
// or by a transitive dependency of mod itself, or — less likely — the
|
||||
// package may be rejected by an AllowPackage hook or rendered ambiguous
|
||||
// by some other newly-added or newly-upgraded dependency.
|
||||
})
|
||||
|
||||
pkgMods = append(pkgMods, pkgMod{pkg: pkg, mod: &mod})
|
||||
}
|
||||
<-ld.work.Idle()
|
||||
|
||||
modAddedBy = map[module.Version]*loadPkg{}
|
||||
for _, pkg := range needPkgs {
|
||||
if pkg.err != nil {
|
||||
for _, pm := range pkgMods {
|
||||
pkg, mod := pm.pkg, *pm.mod
|
||||
if mod.Path == "" {
|
||||
continue
|
||||
}
|
||||
|
||||
fmt.Fprintf(os.Stderr, "go: found %s in %s %s\n", pkg.path, pkg.mod.Path, pkg.mod.Version)
|
||||
if modAddedBy[pkg.mod] == nil {
|
||||
modAddedBy[pkg.mod] = pkg
|
||||
fmt.Fprintf(os.Stderr, "go: found %s in %s %s\n", pkg.path, mod.Path, mod.Version)
|
||||
if modAddedBy[mod] == nil {
|
||||
modAddedBy[mod] = pkg
|
||||
}
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user