From: Bryan C. Mills Date: Tue, 23 Mar 2021 04:26:39 +0000 (-0400) Subject: cmd/go: attribute direct imports from indirect dependencies to the importing package X-Git-Tag: go1.17beta1~986 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=adb037d67a;p=gostls13.git cmd/go: attribute direct imports from indirect dependencies to the importing package For #36460 Updates #40775 Change-Id: I833ee8ee733151aafcff508bf91d0e6e37c50032 Reviewed-on: https://go-review.googlesource.com/c/go/+/303869 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot Reviewed-by: Jay Conrod --- diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go index c9619f1b28..66b6d0dc46 100644 --- a/src/cmd/go/internal/load/pkg.go +++ b/src/cmd/go/internal/load/pkg.go @@ -29,6 +29,7 @@ import ( "cmd/go/internal/base" "cmd/go/internal/cfg" "cmd/go/internal/fsys" + "cmd/go/internal/imports" "cmd/go/internal/modinfo" "cmd/go/internal/modload" "cmd/go/internal/par" @@ -478,6 +479,7 @@ var ( _ ImportPathError = (*importError)(nil) _ ImportPathError = (*modload.ImportMissingError)(nil) _ ImportPathError = (*modload.ImportMissingSumError)(nil) + _ ImportPathError = (*modload.DirectImportFromImplicitDependencyError)(nil) ) type importError struct { @@ -675,6 +677,8 @@ func loadImport(ctx context.Context, pre *preload, path, srcDir string, parent * stk.Push(path) defer stk.Pop() } + // TODO(bcmills): Why are we constructing Error inline here instead of + // calling setLoadPackageDataError? return &Package{ PackagePublic: PackagePublic{ ImportPath: path, @@ -840,6 +844,27 @@ func loadPackageData(ctx context.Context, path, parentPath, parentDir, parentRoo data.p.Root = info.Dir } } + if r.err != nil { + if data.err != nil { + // ImportDir gave us one error, and the module loader gave us another. + // We arbitrarily choose to keep the error from ImportDir because + // that's what our tests already expect, and it seems to provide a bit + // more detail in most cases. + } else if errors.Is(r.err, imports.ErrNoGo) { + // ImportDir said there were files in the package, but the module + // loader said there weren't. Which one is right? + // Without this special-case hack, the TestScript/test_vet case fails + // on the vetfail/p1 package (added in CL 83955). + // Apparently, imports.ShouldBuild biases toward rejecting files + // with invalid build constraints, whereas ImportDir biases toward + // accepting them. + // + // TODO(#41410: Figure out how this actually ought to work and fix + // this mess. + } else { + data.err = r.err + } + } } else if r.err != nil { data.p = new(build.Package) data.err = r.err diff --git a/src/cmd/go/internal/modload/import.go b/src/cmd/go/internal/modload/import.go index 508db2f247..1d321bb24b 100644 --- a/src/cmd/go/internal/modload/import.go +++ b/src/cmd/go/internal/modload/import.go @@ -128,6 +128,23 @@ func (e *AmbiguousImportError) Error() string { return buf.String() } +// A DirectImportFromImplicitDependencyError indicates a package directly +// imported by a package or test in the main module that is satisfied by a +// dependency that is not explicit in the main module's go.mod file. +type DirectImportFromImplicitDependencyError struct { + ImporterPath string + ImportedPath string + Module module.Version +} + +func (e *DirectImportFromImplicitDependencyError) Error() string { + return fmt.Sprintf("package %s imports %s from implicitly required module; to add missing requirements, run:\n\tgo get %s@%s", e.ImporterPath, e.ImportedPath, e.Module.Path, e.Module.Version) +} + +func (e *DirectImportFromImplicitDependencyError) ImportPath() string { + return e.ImporterPath +} + // ImportMissingSumError is reported in readonly mode when we need to check // if a module contains a package, but we don't have a sum for its .zip file. // We might need sums for multiple modules to verify the package is unique. diff --git a/src/cmd/go/internal/modload/load.go b/src/cmd/go/internal/modload/load.go index d89dc67028..386b53938c 100644 --- a/src/cmd/go/internal/modload/load.go +++ b/src/cmd/go/internal/modload/load.go @@ -940,32 +940,51 @@ func loadFromRoots(ctx context.Context, params loaderParams) *loader { } base.ExitIfErrors() // TODO(bcmills): Is this actually needed? + rs := ld.requirements + // Compute directly referenced dependency modules. direct := make(map[string]bool) for _, pkg := range ld.pkgs { - if pkg.mod == Target { - for _, dep := range pkg.imports { - if dep.mod.Path != "" && dep.mod.Path != Target.Path && index != nil { - _, explicit := index.require[dep.mod] - if allowWriteGoMod && cfg.BuildMod == "readonly" && !explicit { - // TODO(#40775): attach error to package instead of using - // base.Errorf. Ideally, 'go list' should not fail because of this, - // but today, LoadPackages calls WriteGoMod unconditionally, which - // would fail with a less clear message. - base.Errorf("go: %[1]s: package %[2]s imported from implicitly required module; to add missing requirements, run:\n\tgo get %[2]s@%[3]s", pkg.path, dep.path, dep.mod.Version) + if pkg.mod != Target { + continue + } + for _, dep := range pkg.imports { + if dep.mod.Path == "" || dep.mod.Path == Target.Path { + continue + } + + if pkg.err == nil && cfg.BuildMod != "mod" { + if v, ok := rs.rootSelected(dep.mod.Path); !ok || v != dep.mod.Version { + // dep.mod is not an explicit dependency, but needs to be. + // Because we are not in "mod" mod, we will not be able to update it. + // Instead, mark the importing package with an error. + // + // TODO(#41688): The resulting error message fails to include the file + // position of the erroneous import (because that information is not + // tracked by the module loader). Figure out how to plumb the import + // position through. + pkg.err = &DirectImportFromImplicitDependencyError{ + ImporterPath: pkg.path, + ImportedPath: dep.path, + Module: dep.mod, } - direct[dep.mod.Path] = true + // cfg.BuildMod does not allow us to change dep.mod to be a direct + // dependency, so don't mark it as such. + continue } } + + // dep is a package directly imported by a package or test in the main + // module and loaded from some other module (not the standard library). + // Mark its module as a direct dependency. + direct[dep.mod.Path] = true } } - base.ExitIfErrors() // If we didn't scan all of the imports from the main module, or didn't use // imports.AnyTags, then we didn't necessarily load every package that // contributes “direct” imports — so we can't safely mark existing // direct dependencies in ld.requirements as indirect-only. Propagate them as direct. - rs := ld.requirements if !ld.loadedDirect() { for mPath := range rs.direct { direct[mPath] = true diff --git a/src/cmd/go/testdata/script/mod_get_promote_implicit.txt b/src/cmd/go/testdata/script/mod_get_promote_implicit.txt index 10ca6594e4..9eec201321 100644 --- a/src/cmd/go/testdata/script/mod_get_promote_implicit.txt +++ b/src/cmd/go/testdata/script/mod_get_promote_implicit.txt @@ -6,7 +6,7 @@ cp go.mod.orig go.mod go list -m indirect-with-pkg stdout '^indirect-with-pkg v1.0.0 => ./indirect-with-pkg$' ! go list ./use-indirect -stderr '^go: m/use-indirect: package indirect-with-pkg imported from implicitly required module; to add missing requirements, run:\n\tgo get indirect-with-pkg@v1.0.0$' +stderr '^package m/use-indirect imports indirect-with-pkg from implicitly required module; to add missing requirements, run:\n\tgo get indirect-with-pkg@v1.0.0$' # We can promote the implicit requirement by getting the importing package. # NOTE: the hint recommends getting the imported package (tested below) since