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

diff --git a/src/cmd/go/pkg.go b/src/cmd/go/pkg.go
index d45df265b96ed3371ba8db6f6fa03aef8ee731ef..d0dbefed01c3401316a940f9054dfc0ee59fc791 100644 (file)
--- 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 c62f629405fd27f50e1d99420975e4962af2aebc..e5ba12b1df7834b09769a7ace27a75d4d7f7c9ab 100755 (executable)
--- 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 (file)
index 0000000..e3558a5
--- /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 (file)
index 0000000..c594f5c
--- /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 (file)
index 0000000..a796c0b
--- /dev/null
+++ b/src/cmd/go/testdata/testinternal2/x/y/z/internal/w/w.go
@@ -0,0 +1 @@
+package w