Skip to content

Commit

Permalink
vendor/golang_org/x: move to internal/x
Browse files Browse the repository at this point in the history
Packages in vendor/ directories have a "vendor/" path prefix in GOPATH
mode, but intentionally do not in module mode. Since the import path
is embedded in the compiled output, changing that path invalidates
cache entries and causes cmd/go to try to rebuild (and reinstall) the
vendored libraries, which will fail if the directory containing those
libraries is read-only.

If I understood correctly, this is the approach Russ suggested as an
alternative to https://golang.org/cl/136138.

Fixes #27285
Fixes #26988

Change-Id: I8a2507fa892b84cde0a803aaa79e460723da572b
Reviewed-on: https://go-review.googlesource.com/c/147443
Run-TryBot: Bryan C. Mills <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Russ Cox <[email protected]>
  • Loading branch information
Bryan C. Mills committed Nov 29, 2018
1 parent 70a684c commit 2012227
Show file tree
Hide file tree
Showing 196 changed files with 148 additions and 152 deletions.
7 changes: 1 addition & 6 deletions src/cmd/api/goapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -442,13 +442,8 @@ func (w *Walker) Import(name string) (*types.Package, error) {
}
w.imported[name] = &importing

root := w.root
if strings.HasPrefix(name, "golang_org/x/") {
root = filepath.Join(root, "vendor")
}

// Determine package files.
dir := filepath.Join(root, filepath.FromSlash(name))
dir := filepath.Join(w.root, filepath.FromSlash(name))
if fi, err := os.Stat(dir); err != nil || !fi.IsDir() {
log.Fatalf("no source in tree for import %q: %v", name, err)
}
Expand Down
16 changes: 0 additions & 16 deletions src/cmd/go/go_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1207,22 +1207,6 @@ func TestImportCycle(t *testing.T) {
tg.run("list", "-e", "-json", "selfimport")
}

func TestListImportMap(t *testing.T) {
skipIfGccgo(t, "gccgo does not have standard packages")
tg := testgo(t)
defer tg.cleanup()
tg.parallel()
tg.run("list", "-f", "{{.ImportPath}}: {{.ImportMap}}", "net", "fmt")
tg.grepStdout(`^net: map\[(.* )?golang_org/x/net/dns/dnsmessage:vendor/golang_org/x/net/dns/dnsmessage.*\]`, "net/http should have rewritten dnsmessage import")
tg.grepStdout(`^fmt: map\[\]`, "fmt should have no rewritten imports")
tg.run("list", "-deps", "-test", "-f", "{{.ImportPath}} MAP: {{.ImportMap}}\n{{.ImportPath}} IMPORT: {{.Imports}}", "fmt")
tg.grepStdout(`^flag \[fmt\.test\] MAP: map\[fmt:fmt \[fmt\.test\]\]`, "flag [fmt.test] should import fmt [fmt.test] as fmt")
tg.grepStdout(`^fmt\.test MAP: map\[(.* )?testing:testing \[fmt\.test\]`, "fmt.test should import testing [fmt.test] as testing")
tg.grepStdout(`^fmt\.test MAP: map\[(.* )?testing:testing \[fmt\.test\]`, "fmt.test should import testing [fmt.test] as testing")
tg.grepStdoutNot(`^fmt\.test MAP: map\[(.* )?os:`, "fmt.test should not import a modified os")
tg.grepStdout(`^fmt\.test IMPORT: \[fmt \[fmt\.test\] fmt_test \[fmt\.test\] os testing \[fmt\.test\] testing/internal/testdeps \[fmt\.test\]\]`, "wrong imports for fmt.test")
}

// cmd/go: custom import path checking should not apply to Go packages without import comment.
func TestIssue10952(t *testing.T) {
testenv.MustHaveExternalNetwork(t)
Expand Down
14 changes: 0 additions & 14 deletions src/cmd/go/internal/load/pkg.go
Original file line number Diff line number Diff line change
Expand Up @@ -1051,20 +1051,6 @@ func disallowVendor(srcDir string, importer *Package, importerPath, path string,
return p
}

// 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 {
return perr
}
Expand Down
3 changes: 0 additions & 3 deletions src/cmd/go/internal/modload/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,6 @@ func Import(path string) (m module.Version, dir string, err error) {

// Is the package in the standard library?
if search.IsStandardImportPath(path) {
if strings.HasPrefix(path, "golang_org/") {
return module.Version{}, filepath.Join(cfg.GOROOT, "src/vendor", path), nil
}
if goroot.IsStandardPackage(cfg.GOROOT, cfg.BuildContext.Compiler, path) {
dir := filepath.Join(cfg.GOROOT, "src", path)
return module.Version{}, dir, nil
Expand Down
25 changes: 25 additions & 0 deletions src/cmd/go/testdata/script/list_importmap.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# gccgo does not have standard packages.
[gccgo] skip

# fmt should have no rewritten imports.
# The import from a/b should map c/d to a's vendor directory.
go list -f '{{.ImportPath}}: {{.ImportMap}}' fmt a/b
stdout 'fmt: map\[\]'
stdout 'a/b: map\[c/d:a/vendor/c/d\]'

# flag [fmt.test] should import fmt [fmt.test] as fmt
# fmt.test should import testing [fmt.test] as testing
# fmt.test should not import a modified os
go list -deps -test -f '{{.ImportPath}} MAP: {{.ImportMap}}{{"\n"}}{{.ImportPath}} IMPORT: {{.Imports}}' fmt
stdout '^flag \[fmt\.test\] MAP: map\[fmt:fmt \[fmt\.test\]\]'
stdout '^fmt\.test MAP: map\[(.* )?testing:testing \[fmt\.test\]'
! stdout '^fmt\.test MAP: map\[(.* )?os:'
stdout '^fmt\.test IMPORT: \[fmt \[fmt\.test\] fmt_test \[fmt\.test\] os testing \[fmt\.test\] testing/internal/testdeps \[fmt\.test\]\]'


-- a/b/b.go --
package b

import _ "c/d"
-- a/vendor/c/d/d.go --
package d
2 changes: 1 addition & 1 deletion src/cmd/go/testdata/script/list_std.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ go list -f '{{if not .Standard}}{{.ImportPath}}{{end}}' ./...

# our vendored packages should be reported as standard
go list std cmd
stdout golang_org/x/net/http2/hpack
stdout internal/x/net/http2/hpack
stdout cmd/vendor/golang\.org/x/arch/x86/x86asm
13 changes: 0 additions & 13 deletions src/cmd/go/testdata/script/mod_internal.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,6 @@ stderr 'use of internal package golang.org/x/.* not allowed'
! go build ./fromstd
stderr 'use of internal package internal/testenv not allowed'

# Packages found via standard-library vendoring should not leak.
! go build ./fromstdvendor
stderr 'use of vendored package golang_org/x/net/http/httpguts not allowed'

env GO111MODULE=off
! go build ./fromstdvendor
stderr 'cannot find package "golang_org/x/net/http/httpguts" in any of:'
env GO111MODULE=on

# Dependencies should be able to use their own internal modules...
rm go.mod
go mod init golang.org/notx
Expand Down Expand Up @@ -83,10 +74,6 @@ import _ "golang.org/notx/useinternal"
package fromstd
import _ "internal/testenv"

-- fromstdvendor/useinternal.go --
package fromstdvendor
import _ "golang_org/x/net/http/httpguts"

-- replace/golang.org/notx/internal/go.mod --
module golang.org/x/internal

Expand Down
2 changes: 1 addition & 1 deletion src/cmd/go/testdata/script/mod_std_vendor.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ go list -f '{{.TestImports}}'
stdout net/http # from .TestImports

go list -test -f '{{.Deps}}'
stdout golang_org/x/crypto # dep of .TestImports
stdout internal/x/crypto # dep of .TestImports

-- go.mod --
module m
Expand Down
2 changes: 1 addition & 1 deletion src/crypto/tls/cipher_suites.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import (
"crypto/sha1"
"crypto/sha256"
"crypto/x509"
"golang_org/x/crypto/chacha20poly1305"
"hash"
"internal/x/crypto/chacha20poly1305"
)

// a keyAgreement implements the client and server side of a TLS key agreement
Expand Down
2 changes: 1 addition & 1 deletion src/crypto/tls/handshake_messages.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ package tls

import (
"fmt"
"golang_org/x/crypto/cryptobyte"
"internal/x/crypto/cryptobyte"
"strings"
)

Expand Down
6 changes: 3 additions & 3 deletions src/crypto/tls/key_schedule.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ import (
"crypto/elliptic"
"crypto/hmac"
"errors"
"golang_org/x/crypto/cryptobyte"
"golang_org/x/crypto/curve25519"
"golang_org/x/crypto/hkdf"
"hash"
"internal/x/crypto/cryptobyte"
"internal/x/crypto/curve25519"
"internal/x/crypto/hkdf"
"io"
"math/big"
)
Expand Down
2 changes: 1 addition & 1 deletion src/crypto/tls/ticket.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"crypto/sha256"
"crypto/subtle"
"errors"
"golang_org/x/crypto/cryptobyte"
"internal/x/crypto/cryptobyte"
"io"
)

Expand Down
4 changes: 2 additions & 2 deletions src/crypto/x509/x509.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ import (
"encoding/pem"
"errors"
"fmt"
"golang_org/x/crypto/cryptobyte"
cryptobyte_asn1 "golang_org/x/crypto/cryptobyte/asn1"
"internal/x/crypto/cryptobyte"
cryptobyte_asn1 "internal/x/crypto/cryptobyte/asn1"
"io"
"math/big"
"net"
Expand Down
30 changes: 21 additions & 9 deletions src/go/build/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -351,12 +351,16 @@ func TestImportDirNotExist(t *testing.T) {
func TestImportVendor(t *testing.T) {
testenv.MustHaveGoBuild(t) // really must just have source
ctxt := Default
ctxt.GOPATH = ""
p, err := ctxt.Import("golang_org/x/net/http2/hpack", filepath.Join(ctxt.GOROOT, "src/net/http"), 0)
wd, err := os.Getwd()
if err != nil {
t.Fatal(err)
}
ctxt.GOPATH = filepath.Join(wd, "testdata/withvendor")
p, err := ctxt.Import("c/d", filepath.Join(ctxt.GOPATH, "src/a/b"), 0)
if err != nil {
t.Fatalf("cannot find vendored golang_org/x/net/http2/hpack from net/http directory: %v", err)
t.Fatalf("cannot find vendored c/d from testdata src/a/b directory: %v", err)
}
want := "vendor/golang_org/x/net/http2/hpack"
want := "a/vendor/c/d"
if p.ImportPath != want {
t.Fatalf("Import succeeded but found %q, want %q", p.ImportPath, want)
}
Expand All @@ -365,8 +369,12 @@ func TestImportVendor(t *testing.T) {
func TestImportVendorFailure(t *testing.T) {
testenv.MustHaveGoBuild(t) // really must just have source
ctxt := Default
ctxt.GOPATH = ""
p, err := ctxt.Import("x.com/y/z", filepath.Join(ctxt.GOROOT, "src/net/http"), 0)
wd, err := os.Getwd()
if err != nil {
t.Fatal(err)
}
ctxt.GOPATH = filepath.Join(wd, "testdata/withvendor")
p, err := ctxt.Import("x.com/y/z", filepath.Join(ctxt.GOPATH, "src/a/b"), 0)
if err == nil {
t.Fatalf("found made-up package x.com/y/z in %s", p.Dir)
}
Expand All @@ -380,9 +388,13 @@ func TestImportVendorFailure(t *testing.T) {
func TestImportVendorParentFailure(t *testing.T) {
testenv.MustHaveGoBuild(t) // really must just have source
ctxt := Default
ctxt.GOPATH = ""
// This import should fail because the vendor/golang.org/x/net/http2 directory has no source code.
p, err := ctxt.Import("golang_org/x/net/http2", filepath.Join(ctxt.GOROOT, "src/net/http"), 0)
wd, err := os.Getwd()
if err != nil {
t.Fatal(err)
}
ctxt.GOPATH = filepath.Join(wd, "testdata/withvendor")
// This import should fail because the vendor/c directory has no source code.
p, err := ctxt.Import("c", filepath.Join(ctxt.GOPATH, "src/a/b"), 0)
if err == nil {
t.Fatalf("found empty parent in %s", p.Dir)
}
Expand Down
30 changes: 15 additions & 15 deletions src/go/build/deps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ var pkgDeps = map[string][]string{
"context", "math/rand", "os", "reflect", "sort", "syscall", "time",
"internal/nettrace", "internal/poll", "internal/syscall/unix",
"internal/syscall/windows", "internal/singleflight", "internal/race",
"golang_org/x/net/dns/dnsmessage", "golang_org/x/net/lif", "golang_org/x/net/route",
"internal/x/net/dns/dnsmessage", "internal/x/net/lif", "internal/x/net/route",
},

// NET enables use of basic network-related packages.
Expand Down Expand Up @@ -357,9 +357,9 @@ var pkgDeps = map[string][]string{
"crypto/sha1",
"crypto/sha256",
"crypto/sha512",
"golang_org/x/crypto/chacha20poly1305",
"golang_org/x/crypto/curve25519",
"golang_org/x/crypto/poly1305",
"internal/x/crypto/chacha20poly1305",
"internal/x/crypto/curve25519",
"internal/x/crypto/poly1305",
},

// Random byte, number generation.
Expand Down Expand Up @@ -387,13 +387,13 @@ var pkgDeps = map[string][]string{

// SSL/TLS.
"crypto/tls": {
"L4", "CRYPTO-MATH", "OS", "golang_org/x/crypto/cryptobyte", "golang_org/x/crypto/hkdf",
"L4", "CRYPTO-MATH", "OS", "internal/x/crypto/cryptobyte", "internal/x/crypto/hkdf",
"container/list", "crypto/x509", "encoding/pem", "net", "syscall",
},
"crypto/x509": {
"L4", "CRYPTO-MATH", "OS", "CGO",
"crypto/x509/pkix", "encoding/pem", "encoding/hex", "net", "os/user", "syscall", "net/url",
"golang_org/x/crypto/cryptobyte", "golang_org/x/crypto/cryptobyte/asn1",
"internal/x/crypto/cryptobyte", "internal/x/crypto/cryptobyte/asn1",
},
"crypto/x509/pkix": {"L4", "CRYPTO-MATH", "encoding/hex"},

Expand All @@ -409,12 +409,12 @@ var pkgDeps = map[string][]string{
"context",
"crypto/rand",
"crypto/tls",
"golang_org/x/net/http/httpguts",
"golang_org/x/net/http/httpproxy",
"golang_org/x/net/http2/hpack",
"golang_org/x/net/idna",
"golang_org/x/text/unicode/norm",
"golang_org/x/text/width",
"internal/x/net/http/httpguts",
"internal/x/net/http/httpproxy",
"internal/x/net/http2/hpack",
"internal/x/net/idna",
"internal/x/text/unicode/norm",
"internal/x/text/width",
"internal/nettrace",
"mime/multipart",
"net/http/httptrace",
Expand All @@ -432,9 +432,9 @@ var pkgDeps = map[string][]string{
"net/http/fcgi": {"L4", "NET", "OS", "context", "net/http", "net/http/cgi"},
"net/http/httptest": {
"L4", "NET", "OS", "crypto/tls", "flag", "net/http", "net/http/internal", "crypto/x509",
"golang_org/x/net/http/httpguts",
"internal/x/net/http/httpguts",
},
"net/http/httputil": {"L4", "NET", "OS", "context", "net/http", "net/http/internal", "golang_org/x/net/http/httpguts"},
"net/http/httputil": {"L4", "NET", "OS", "context", "net/http", "net/http/internal", "internal/x/net/http/httpguts"},
"net/http/pprof": {"L4", "OS", "html/template", "net/http", "runtime/pprof", "runtime/trace"},
"net/rpc": {"L4", "NET", "encoding/gob", "html/template", "net/http"},
"net/rpc/jsonrpc": {"L4", "NET", "encoding/json", "net/rpc"},
Expand Down Expand Up @@ -485,7 +485,7 @@ func listStdPkgs(goroot string) ([]string, error) {
}

name := filepath.ToSlash(path[len(src):])
if name == "builtin" || name == "cmd" || strings.Contains(name, "golang_org") {
if name == "builtin" || name == "cmd" || strings.Contains(name, "internal/x/") {
return filepath.SkipDir
}

Expand Down
3 changes: 3 additions & 0 deletions src/go/build/testdata/withvendor/src/a/b/b.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package b

import _ "c/d"
1 change: 1 addition & 0 deletions src/go/build/testdata/withvendor/src/a/vendor/c/d/d.go
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package d
2 changes: 1 addition & 1 deletion src/go/internal/srcimporter/srcimporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ var importedObjectTests = []struct {
{"math.Pi", "const Pi untyped float"},
{"math.Sin", "func Sin(x float64) float64"},
{"math/big.Int", "type Int struct{neg bool; abs nat}"},
{"golang_org/x/text/unicode/norm.MaxSegmentSize", "const MaxSegmentSize untyped int"},
{"internal/x/text/unicode/norm.MaxSegmentSize", "const MaxSegmentSize untyped int"},
}

func TestImportedTypes(t *testing.T) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// license that can be found in the LICENSE file.

// Package chacha20poly1305 implements the ChaCha20-Poly1305 AEAD as specified in RFC 7539.
package chacha20poly1305 // import "golang.org/x/crypto/chacha20poly1305"
package chacha20poly1305

import (
"crypto/cipher"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ package chacha20poly1305
import (
"encoding/binary"

"golang_org/x/crypto/internal/chacha20"
"golang_org/x/crypto/poly1305"
"internal/x/crypto/internal/chacha20"
"internal/x/crypto/poly1305"
)

func roundTo16(n int) int {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"reflect"
"time"

"golang_org/x/crypto/cryptobyte/asn1"
"internal/x/crypto/cryptobyte/asn1"
)

// This file contains ASN.1-related methods for String and Builder.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

// Package asn1 contains supporting types for parsing and building ASN.1
// messages with the cryptobyte package.
package asn1 // import "golang.org/x/crypto/cryptobyte/asn1"
package asn1

// Tag represents an ASN.1 identifier octet, consisting of a tag number
// (indicating a type) and class (such as context-specific or constructed).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"testing"
"time"

"golang_org/x/crypto/cryptobyte/asn1"
"internal/x/crypto/cryptobyte/asn1"
)

type readASN1Test struct {
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (
"errors"
"fmt"

"golang_org/x/crypto/cryptobyte"
"golang_org/x/crypto/cryptobyte/asn1"
"internal/x/crypto/cryptobyte"
"internal/x/crypto/cryptobyte/asn1"
)

func ExampleString_lengthPrefixed() {
Expand Down
Loading

0 comments on commit 2012227

Please sign in to comment.