Skip to content

Commit

Permalink
cmd/go: implement 'internal' convention
Browse files Browse the repository at this point in the history
See golang.org/s/go14internal for design.

LGTM=r
R=r, adg
CC=golang-codereviews
https://golang.org/cl/120600043
  • Loading branch information
rsc committed Aug 6, 2014
1 parent ea3ac6b commit 1338f32
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 5 deletions.
65 changes: 60 additions & 5 deletions src/cmd/go/pkg.go
Expand Up @@ -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)
}

Expand All @@ -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
}

Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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)
}
Expand Down
16 changes: 16 additions & 0 deletions src/cmd/go/test.bash
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions src/cmd/go/testdata/testinternal/p.go
@@ -0,0 +1,3 @@
package p

import _ "net/http/internal"
3 changes: 3 additions & 0 deletions src/cmd/go/testdata/testinternal2/p.go
@@ -0,0 +1,3 @@
package p

import _ "./x/y/z/internal/w"
1 change: 1 addition & 0 deletions src/cmd/go/testdata/testinternal2/x/y/z/internal/w/w.go
@@ -0,0 +1 @@
package w

0 comments on commit 1338f32

Please sign in to comment.