From 1338f327b2deb81ba81107b5033d1b10e97e3abe Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 6 Aug 2014 16:45:06 -0400 Subject: [PATCH] cmd/go: implement 'internal' convention See golang.org/s/go14internal for design. LGTM=r R=r, adg CC=golang-codereviews https://golang.org/cl/120600043 --- src/cmd/go/pkg.go | 65 +++++++++++++++++-- src/cmd/go/test.bash | 16 +++++ src/cmd/go/testdata/testinternal/p.go | 3 + src/cmd/go/testdata/testinternal2/p.go | 3 + .../testinternal2/x/y/z/internal/w/w.go | 1 + 5 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 src/cmd/go/testdata/testinternal/p.go create mode 100644 src/cmd/go/testdata/testinternal2/p.go create mode 100644 src/cmd/go/testdata/testinternal2/x/y/z/internal/w/w.go diff --git a/src/cmd/go/pkg.go b/src/cmd/go/pkg.go index d45df265b96ed..d0dbefed01c34 100644 --- a/src/cmd/go/pkg.go +++ b/src/cmd/go/pkg.go @@ -244,6 +244,9 @@ func loadImport(path string, srcDir string, stk *importStack, importPos []token. importPath = dirToImportPath(filepath.Join(srcDir, path)) } if p := packageCache[importPath]; p != nil { + if perr := disallowInternal(srcDir, p, stk); perr != p { + return perr + } return reusePackage(p, stk) } @@ -270,6 +273,10 @@ func loadImport(path string, srcDir string, stk *importStack, importPos []token. p.Error.Pos = pos.String() } + if perr := disallowInternal(srcDir, p, stk); perr != p { + return perr + } + return p } @@ -298,6 +305,54 @@ func reusePackage(p *Package, stk *importStack) *Package { return p } +// disallowInternal checks that srcDir 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 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 + // rooted at the parent of the “internal” directory. + // + // ... For Go 1.4, we will implement the rule first for $GOROOT, but not $GOPATH. + + // Only applies to $GOROOT. + if !p.Standard { + return p + } + + // 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 + // import. Anything listed on the command line is fine. + if len(*stk) == 1 { + return p + } + + // Check for "internal" element: four cases depending on begin of string and/or end of string. + if p.ImportPath != "internal" && + !strings.HasPrefix(p.ImportPath, "internal/") && + !strings.HasSuffix(p.ImportPath, "/internal") && + !strings.Contains(p.ImportPath, "/internal/") { + return p + } + + // Internal is present. Check directory. + parent := filepath.Dir(p.Dir) + if hasPathPrefix(srcDir, parent) { + return p + } + + // Internal is present, and srcDir is outside parent's tree. Not allowed. + perr := *p + perr.Error = &PackageError{ + ImportStack: stk.copy(), + Err: "use of internal package not allowed", + } + perr.Incomplete = true + return &perr +} + type targetDir int const ( @@ -482,7 +537,7 @@ func (p *Package) load(stk *importStack, bp *build.Package, err error) *Package // Build list of imported packages and full dependency list. imports := make([]*Package, 0, len(p.Imports)) - deps := make(map[string]bool) + deps := make(map[string]*Package) for i, path := range importPaths { if path == "C" { continue @@ -502,10 +557,10 @@ func (p *Package) load(stk *importStack, bp *build.Package, err error) *Package path = p1.ImportPath importPaths[i] = path } - deps[path] = true + deps[path] = p1 imports = append(imports, p1) - for _, dep := range p1.Deps { - deps[dep] = true + for _, dep := range p1.deps { + deps[dep.ImportPath] = dep } if p1.Incomplete { p.Incomplete = true @@ -519,7 +574,7 @@ func (p *Package) load(stk *importStack, bp *build.Package, err error) *Package } sort.Strings(p.Deps) for _, dep := range p.Deps { - p1 := packageCache[dep] + p1 := deps[dep] if p1 == nil { panic("impossible: missing entry in package cache for " + dep + " imported by " + p.ImportPath) } diff --git a/src/cmd/go/test.bash b/src/cmd/go/test.bash index c62f629405fd2..e5ba12b1df783 100755 --- a/src/cmd/go/test.bash +++ b/src/cmd/go/test.bash @@ -105,6 +105,22 @@ cp -R testdata/local "testdata/$bad" testlocal "$bad" 'with bad characters in path' rm -rf "testdata/$bad" +TEST 'internal packages in $GOROOT are respected' +if ./testgo build -v ./testdata/testinternal >testdata/std.out 2>&1; then + echo "go build ./testdata/testinternal succeeded incorrectly" + ok=false +elif ! grep 'use of internal package not allowed' testdata/std.out >/dev/null; then + echo "wrong error message for testdata/testinternal" + cat std.out + ok=false +fi + +TEST 'internal packages outside $GOROOT are not respected' +if ! ./testgo build -v ./testdata/testinternal2; then + echo "go build ./testdata/testinternal2 failed" + ok=false +fi + TEST error message for syntax error in test go file says FAIL export GOPATH=$(pwd)/testdata if ./testgo test syntaxerror 2>testdata/err; then diff --git a/src/cmd/go/testdata/testinternal/p.go b/src/cmd/go/testdata/testinternal/p.go new file mode 100644 index 0000000000000..e3558a53b2442 --- /dev/null +++ b/src/cmd/go/testdata/testinternal/p.go @@ -0,0 +1,3 @@ +package p + +import _ "net/http/internal" diff --git a/src/cmd/go/testdata/testinternal2/p.go b/src/cmd/go/testdata/testinternal2/p.go new file mode 100644 index 0000000000000..c594f5c5e9eff --- /dev/null +++ b/src/cmd/go/testdata/testinternal2/p.go @@ -0,0 +1,3 @@ +package p + +import _ "./x/y/z/internal/w" diff --git a/src/cmd/go/testdata/testinternal2/x/y/z/internal/w/w.go b/src/cmd/go/testdata/testinternal2/x/y/z/internal/w/w.go new file mode 100644 index 0000000000000..a796c0b5f4b14 --- /dev/null +++ b/src/cmd/go/testdata/testinternal2/x/y/z/internal/w/w.go @@ -0,0 +1 @@ +package w