From cc880de4bebf9b9242a8f5d1d7e9cf01863bb701 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 17 Aug 2018 22:32:22 -0400 Subject: [PATCH] cmd/go: allow 'go run x.go' to use nearby internal imports in module mode In GOPATH mode the rule has always been that 'go run x.go' can import whatever the package in x.go's directory would be able to import. Apply the same rule here. The bad import path was triggering other mysterious errors during 'go run' in other circumstances. Setting it correctly fixes those too. Fixes #26046. Fixes #27022. Change-Id: I0a9b0a154a20f48add5a199da85572e7ffe0cde4 Reviewed-on: https://go-review.googlesource.com/129798 Run-TryBot: Russ Cox TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/cmd/go/internal/load/pkg.go | 56 ++++++++++--------- src/cmd/go/internal/modload/init.go | 1 + src/cmd/go/internal/modload/load.go | 22 ++++++++ .../go/testdata/script/mod_run_internal.txt | 46 +++++++++++++++ src/cmd/go/testdata/script/mod_run_path.txt | 15 +++++ 5 files changed, 114 insertions(+), 26 deletions(-) create mode 100644 src/cmd/go/testdata/script/mod_run_internal.txt create mode 100644 src/cmd/go/testdata/script/mod_run_path.txt diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go index 43887b0008..ec2fa730c6 100644 --- a/src/cmd/go/internal/load/pkg.go +++ b/src/cmd/go/internal/load/pkg.go @@ -39,6 +39,7 @@ var ( ModPackageBuildInfo func(main string, deps []string) string // return module info to embed in binary ModInfoProg func(info string) []byte // wrap module info in .go code for binary ModImportFromFiles func([]string) // update go.mod to add modules for imports in these files + ModDirImportPath func(string) string // return effective import path for directory ) var IgnoreImports bool // control whether we ignore imports in packages @@ -567,11 +568,11 @@ func LoadImport(path, srcDir string, parent *Package, stk *ImportStack, importPo } // Checked on every import because the rules depend on the code doing the importing. - if perr := disallowInternal(srcDir, parentPath, p, stk); perr != p { + if perr := disallowInternal(srcDir, parent, parentPath, p, stk); perr != p { return setErrorPos(perr, importPos) } if mode&ResolveImport != 0 { - if perr := disallowVendor(srcDir, parentPath, origPath, p, stk); perr != p { + if perr := disallowVendor(srcDir, parent, parentPath, origPath, p, stk); perr != p { return setErrorPos(perr, importPos) } } @@ -932,7 +933,7 @@ func reusePackage(p *Package, stk *ImportStack) *Package { // is allowed to import p. // If the import is allowed, disallowInternal returns the original package p. // If not, it returns a new package containing just an appropriate error. -func disallowInternal(srcDir, importerPath string, p *Package, stk *ImportStack) *Package { +func disallowInternal(srcDir string, importer *Package, importerPath string, p *Package, stk *ImportStack) *Package { // golang.org/s/go14internal: // An import of a path containing the element “internal” // is disallowed if the importing code is outside the tree @@ -990,10 +991,16 @@ func disallowInternal(srcDir, importerPath string, p *Package, stk *ImportStack) return p } } else { - // p is in a module, so make it available based on the import path instead + // p is in a module, so make it available based on the importer's import path instead // of the file path (https://golang.org/issue/23970). - parent := p.ImportPath[:i] - if str.HasPathPrefix(importerPath, parent) { + if importerPath == "." { + // The importer is a list of command-line files. + // Pretend that the import path is the import path of the + // directory containing them. + importerPath = ModDirImportPath(importer.Dir) + } + parentOfInternal := p.ImportPath[:i] + if str.HasPathPrefix(importerPath, parentOfInternal) { return p } } @@ -1031,7 +1038,7 @@ func findInternal(path string) (index int, ok bool) { // is allowed to import p as path. // If the import is allowed, disallowVendor returns the original package p. // If not, it returns a new package containing just an appropriate error. -func disallowVendor(srcDir, importerPath, path string, p *Package, stk *ImportStack) *Package { +func disallowVendor(srcDir string, importer *Package, importerPath, path string, p *Package, stk *ImportStack) *Package { // The stack includes p.ImportPath. // If that's the only thing on the stack, we started // with a name given on the command line, not an @@ -1040,26 +1047,18 @@ func disallowVendor(srcDir, importerPath, path string, p *Package, stk *ImportSt return p } - if p.Standard && ModPackageModuleInfo != nil && importerPath != "" { - // Modules must not import vendor packages in the standard library, - // but the usual vendor visibility check will not catch them - // because the module loader presents them with an ImportPath starting - // with "golang_org/" instead of "vendor/". - if mod := ModPackageModuleInfo(importerPath); mod != nil { - dir := p.Dir - if relDir, err := filepath.Rel(p.Root, p.Dir); err == nil { - dir = relDir - } - if _, ok := FindVendor(filepath.ToSlash(dir)); ok { - perr := *p - perr.Error = &PackageError{ - ImportStack: stk.Copy(), - Err: "use of vendored package " + path + " not allowed", - } - perr.Incomplete = true - return &perr - } + // Modules must not import vendor packages in the standard library, + // but the usual vendor visibility check will not catch them + // because the module loader presents them with an ImportPath starting + // with "golang_org/" instead of "vendor/". + if p.Standard && !importer.Standard && strings.HasPrefix(p.ImportPath, "golang_org") { + perr := *p + perr.Error = &PackageError{ + ImportStack: stk.Copy(), + Err: "use of vendored package " + path + " not allowed", } + perr.Incomplete = true + return &perr } if perr := disallowVendorVisibility(srcDir, p, stk); perr != p { @@ -1991,6 +1990,11 @@ func GoFilesPackage(gofiles []string) *Package { } bp, err := ctxt.ImportDir(dir, 0) + if ModDirImportPath != nil { + // Use the effective import path of the directory + // for deciding visibility during pkg.load. + bp.ImportPath = ModDirImportPath(dir) + } pkg := new(Package) pkg.Internal.Local = true pkg.Internal.CmdlineFiles = true diff --git a/src/cmd/go/internal/modload/init.go b/src/cmd/go/internal/modload/init.go index 169bb5fdb6..f995bad13b 100644 --- a/src/cmd/go/internal/modload/init.go +++ b/src/cmd/go/internal/modload/init.go @@ -175,6 +175,7 @@ func Init() { load.ModPackageBuildInfo = PackageBuildInfo load.ModInfoProg = ModInfoProg load.ModImportFromFiles = ImportFromFiles + load.ModDirImportPath = DirImportPath search.SetModRoot(ModRoot) } diff --git a/src/cmd/go/internal/modload/load.go b/src/cmd/go/internal/modload/load.go index 5ca2ed2d10..285daa8f4f 100644 --- a/src/cmd/go/internal/modload/load.go +++ b/src/cmd/go/internal/modload/load.go @@ -248,6 +248,28 @@ func ImportFromFiles(gofiles []string) { WriteGoMod() } +// DirImportPath returns the effective import path for dir, +// provided it is within the main module, or else returns ".". +func DirImportPath(dir string) string { + if !filepath.IsAbs(dir) { + dir = filepath.Join(cwd, dir) + } else { + dir = filepath.Clean(dir) + } + + if dir == ModRoot { + return Target.Path + } + if strings.HasPrefix(dir, ModRoot+string(filepath.Separator)) { + suffix := filepath.ToSlash(dir[len(ModRoot):]) + if strings.HasPrefix(suffix, "/vendor/") { + return strings.TrimPrefix(suffix, "/vendor/") + } + return Target.Path + suffix + } + return "." +} + // LoadBuildList loads and returns the build list from go.mod. // The loading of the build list happens automatically in ImportPaths: // LoadBuildList need only be called if ImportPaths is not diff --git a/src/cmd/go/testdata/script/mod_run_internal.txt b/src/cmd/go/testdata/script/mod_run_internal.txt new file mode 100644 index 0000000000..653ad282be --- /dev/null +++ b/src/cmd/go/testdata/script/mod_run_internal.txt @@ -0,0 +1,46 @@ +env GO111MODULE=on + +go list -e -f '{{.Incomplete}}' runbad1.go +stdout true +! go run runbad1.go +stderr 'use of internal package m/x/internal not allowed' + +go list -e -f '{{.Incomplete}}' runbad2.go +stdout true +! go run runbad2.go +stderr 'use of internal package m/x/internal/y not allowed' + +go list -e -f '{{.Incomplete}}' runok.go +stdout false +go run runok.go + +-- go.mod -- +module m + +-- x/internal/internal.go -- +package internal + +-- x/internal/y/y.go -- +package y + +-- internal/internal.go -- +package internal + +-- internal/z/z.go -- +package z + +-- runbad1.go -- +package main +import _ "m/x/internal" +func main() {} + +-- runbad2.go -- +package main +import _ "m/x/internal/y" +func main() {} + +-- runok.go -- +package main +import _ "m/internal" +import _ "m/internal/z" +func main() {} diff --git a/src/cmd/go/testdata/script/mod_run_path.txt b/src/cmd/go/testdata/script/mod_run_path.txt new file mode 100644 index 0000000000..4369ee4131 --- /dev/null +++ b/src/cmd/go/testdata/script/mod_run_path.txt @@ -0,0 +1,15 @@ +# Test that go run does not get confused by conflict +# between go.mod's module path and what you'd +# expect from GOPATH. golang.org/issue/26046. + +env GO111MODULE=on + +cd $GOPATH/src/example.com/hello +go run main.go + +-- $GOPATH/src/example.com/hello/go.mod -- +module example.com/hello/v2 + +-- $GOPATH/src/example.com/hello/main.go -- +package main +func main() {} -- 2.50.0