From 69ba2980319d576faf0ce81074a02e7d869a21b9 Mon Sep 17 00:00:00 2001 From: Jay Conrod Date: Fri, 11 Oct 2019 16:13:32 -0400 Subject: [PATCH] all: integrate changes made in cmd/go packages This change integrates changes made to cmd/go packages into the corresponding x/mod packages. This is the second step of a bidirectional synchronization. The previous step was CL 200138, which copied changes made in these packages back into cmd/go. With this change, the cmd/go and x/mod packages should be the same except for imports. After this change, we can vendor x/mod into cmd, then remove the duplicate packages in cmd/go. Other important changes: * Updated requirement on golang.org/x/crypto to latest and added golang.org/x/xerrors. crypto is needed for ed25519, which was added to std in go1.13. xerrors is needed for unwrapping, which was also added in 1.13. The x versions of these are compatible with go1.12. * Copied internal/lazyregexp from std, since it's used in sumdb. Updates golang/go#34801 Change-Id: Iafdd0668970f9004e663040535c521241d11a6a8 Reviewed-on: https://go-review.googlesource.com/c/mod/+/200767 Run-TryBot: Jay Conrod TryBot-Result: Gobot Gobot Reviewed-by: Bryan C. Mills --- go.mod | 7 +- go.sum | 6 +- internal/lazyregexp/lazyre.go | 78 ++++++++++++++++++ module/module.go | 147 +++++++++++++++++++++++++++++----- module/module_test.go | 4 +- semver/semver.go | 2 +- sumdb/server.go | 4 +- sumdb/test.go | 12 +-- sumdb/tlog/tlog.go | 4 +- 9 files changed, 225 insertions(+), 39 deletions(-) create mode 100644 internal/lazyregexp/lazyre.go diff --git a/go.mod b/go.mod index a5fc715..9d78a76 100644 --- a/go.mod +++ b/go.mod @@ -1,5 +1,8 @@ module golang.org/x/mod -require golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529 - go 1.12 + +require ( + golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550 + golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898 +) diff --git a/go.sum b/go.sum index 179d072..249ed71 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,9 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= -golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529 h1:iMGN4xG0cnqj3t+zOM8wUB0BiPKHEwSxEZCvzcbZuvk= -golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550 h1:ObdrDkeb4kJdCP557AjRjq69pTHfNouLtWZG7j9rPN8= +golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898 h1:/atklqdjdhuosWIl6AIbOeHJjicWYPqR9bpxqxYG2pA= +golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/internal/lazyregexp/lazyre.go b/internal/lazyregexp/lazyre.go new file mode 100644 index 0000000..2681af3 --- /dev/null +++ b/internal/lazyregexp/lazyre.go @@ -0,0 +1,78 @@ +// Copyright 2018 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package lazyregexp is a thin wrapper over regexp, allowing the use of global +// regexp variables without forcing them to be compiled at init. +package lazyregexp + +import ( + "os" + "regexp" + "strings" + "sync" +) + +// Regexp is a wrapper around regexp.Regexp, where the underlying regexp will be +// compiled the first time it is needed. +type Regexp struct { + str string + once sync.Once + rx *regexp.Regexp +} + +func (r *Regexp) re() *regexp.Regexp { + r.once.Do(r.build) + return r.rx +} + +func (r *Regexp) build() { + r.rx = regexp.MustCompile(r.str) + r.str = "" +} + +func (r *Regexp) FindSubmatch(s []byte) [][]byte { + return r.re().FindSubmatch(s) +} + +func (r *Regexp) FindStringSubmatch(s string) []string { + return r.re().FindStringSubmatch(s) +} + +func (r *Regexp) FindStringSubmatchIndex(s string) []int { + return r.re().FindStringSubmatchIndex(s) +} + +func (r *Regexp) ReplaceAllString(src, repl string) string { + return r.re().ReplaceAllString(src, repl) +} + +func (r *Regexp) FindString(s string) string { + return r.re().FindString(s) +} + +func (r *Regexp) FindAllString(s string, n int) []string { + return r.re().FindAllString(s, n) +} + +func (r *Regexp) MatchString(s string) bool { + return r.re().MatchString(s) +} + +func (r *Regexp) SubexpNames() []string { + return r.re().SubexpNames() +} + +var inTest = len(os.Args) > 0 && strings.HasSuffix(strings.TrimSuffix(os.Args[0], ".exe"), ".test") + +// New creates a new lazy regexp, delaying the compiling work until it is first +// needed. If the code is being run as part of tests, the regexp compiling will +// happen immediately. +func New(str string) *Regexp { + lr := &Regexp{str: str} + if inTest { + // In tests, always compile the regexps early. + lr.re() + } + return lr +} diff --git a/module/module.go b/module/module.go index 3135e4b..abd8149 100644 --- a/module/module.go +++ b/module/module.go @@ -103,6 +103,7 @@ import ( "unicode/utf8" "golang.org/x/mod/semver" + errors "golang.org/x/xerrors" ) // A Version (for clients, a module.Version) is defined by a module path and version pair. @@ -111,9 +112,14 @@ type Version struct { // Path is a module path, like "golang.org/x/text" or "rsc.io/quote/v2". Path string - // Version is a module version. - // By convention, the version string is in canonical semver form, - // but enforcement is left up to individual APIs. + // Version is usually a semantic version in canonical form. + // There are three exceptions to this general rule. + // First, the top-level target of a build has no specific version + // and uses Version = "". + // Second, during MVS calculations the version "none" is used + // to represent the decision to take no version of a given module. + // Third, filesystem paths found in "replace" directives are + // represented by a path with an empty version. Version string `json:",omitempty"` } @@ -122,6 +128,65 @@ func (m Version) String() string { return m.Path + "@" + m.Version } +// A ModuleError indicates an error specific to a module. +type ModuleError struct { + Path string + Version string + Err error +} + +// VersionError returns a ModuleError derived from a Version and error, +// or err itself if it is already such an error. +func VersionError(v Version, err error) error { + var mErr *ModuleError + if errors.As(err, &mErr) && mErr.Path == v.Path && mErr.Version == v.Version { + return err + } + return &ModuleError{ + Path: v.Path, + Version: v.Version, + Err: err, + } +} + +func (e *ModuleError) Error() string { + if v, ok := e.Err.(*InvalidVersionError); ok { + return fmt.Sprintf("%s@%s: invalid %s: %v", e.Path, v.Version, v.noun(), v.Err) + } + if e.Version != "" { + return fmt.Sprintf("%s@%s: %v", e.Path, e.Version, e.Err) + } + return fmt.Sprintf("module %s: %v", e.Path, e.Err) +} + +func (e *ModuleError) Unwrap() error { return e.Err } + +// An InvalidVersionError indicates an error specific to a version, with the +// module path unknown or specified externally. +// +// A ModuleError may wrap an InvalidVersionError, but an InvalidVersionError +// must not wrap a ModuleError. +type InvalidVersionError struct { + Version string + Pseudo bool + Err error +} + +// noun returns either "version" or "pseudo-version", depending on whether +// e.Version is a pseudo-version. +func (e *InvalidVersionError) noun() string { + if e.Pseudo { + return "pseudo-version" + } + return "version" +} + +func (e *InvalidVersionError) Error() string { + return fmt.Sprintf("%s %q invalid: %s", e.noun(), e.Version, e.Err) +} + +func (e *InvalidVersionError) Unwrap() error { return e.Err } + // Check checks that a given module path, version pair is valid. // In addition to the path being a valid module path // and the version being a valid semantic version, @@ -133,17 +198,14 @@ func Check(path, version string) error { return err } if !semver.IsValid(version) { - return fmt.Errorf("malformed semantic version %v", version) + return &ModuleError{ + Path: path, + Err: &InvalidVersionError{Version: version, Err: errors.New("not a semantic version")}, + } } _, pathMajor, _ := SplitPathVersion(path) - if !MatchPathMajor(version, pathMajor) { - if pathMajor == "" { - pathMajor = "v0 or v1" - } - if pathMajor[0] == '.' { // .v1 - pathMajor = pathMajor[1:] - } - return fmt.Errorf("mismatched module path %v and version %v (want %v)", path, version, pathMajor) + if err := CheckPathMajor(version, pathMajor); err != nil { + return &ModuleError{Path: path, Err: err} } return nil } @@ -161,7 +223,7 @@ func firstPathOK(r rune) bool { // Paths can be ASCII letters, ASCII digits, and limited ASCII punctuation: + - . _ and ~. // This matches what "go get" has historically recognized in import paths. // TODO(rsc): We would like to allow Unicode letters, but that requires additional -// care in the safe encoding (see note above). +// care in the safe encoding (see "escaped paths" above). func pathOK(r rune) bool { if r < utf8.RuneSelf { return r == '+' || r == '-' || r == '.' || r == '_' || r == '~' || @@ -176,7 +238,7 @@ func pathOK(r rune) bool { // For now we allow all Unicode letters but otherwise limit to pathOK plus a few more punctuation characters. // If we expand the set of allowed characters here, we have to // work harder at detecting potential case-folding and normalization collisions. -// See note about "safe encoding" above. +// See note about "escaped paths" above. func fileNameOK(r rune) bool { if r < utf8.RuneSelf { // Entire set of ASCII punctuation, from which we remove characters: @@ -276,8 +338,8 @@ func checkPath(path string, fileName bool) error { if path == "" { return fmt.Errorf("empty string") } - if strings.Contains(path, "..") { - return fmt.Errorf("double dot") + if path[0] == '-' { + return fmt.Errorf("leading dash") } if strings.Contains(path, "//") { return fmt.Errorf("double slash") @@ -440,20 +502,62 @@ func splitGopkgIn(path string) (prefix, pathMajor string, ok bool) { // MatchPathMajor reports whether the semantic version v // matches the path major version pathMajor. +// +// MatchPathMajor returns true if and only if CheckPathMajor returns nil. func MatchPathMajor(v, pathMajor string) bool { + return CheckPathMajor(v, pathMajor) == nil +} + +// CheckPathMajor returns a non-nil error if the semantic version v +// does not match the path major version pathMajor. +func CheckPathMajor(v, pathMajor string) error { if strings.HasPrefix(pathMajor, ".v") && strings.HasSuffix(pathMajor, "-unstable") { pathMajor = strings.TrimSuffix(pathMajor, "-unstable") } if strings.HasPrefix(v, "v0.0.0-") && pathMajor == ".v1" { // Allow old bug in pseudo-versions that generated v0.0.0- pseudoversion for gopkg .v1. // For example, gopkg.in/yaml.v2@v2.2.1's go.mod requires gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405. - return true + return nil } m := semver.Major(v) if pathMajor == "" { - return m == "v0" || m == "v1" || semver.Build(v) == "+incompatible" + if m == "v0" || m == "v1" || semver.Build(v) == "+incompatible" { + return nil + } + pathMajor = "v0 or v1" + } else if pathMajor[0] == '/' || pathMajor[0] == '.' { + if m == pathMajor[1:] { + return nil + } + pathMajor = pathMajor[1:] } - return (pathMajor[0] == '/' || pathMajor[0] == '.') && m == pathMajor[1:] + return &InvalidVersionError{ + Version: v, + Err: fmt.Errorf("should be %s, not %s", pathMajor, semver.Major(v)), + } +} + +// PathMajorPrefix returns the major-version tag prefix implied by pathMajor. +// An empty PathMajorPrefix allows either v0 or v1. +// +// Note that MatchPathMajor may accept some versions that do not actually begin +// with this prefix: namely, it accepts a 'v0.0.0-' prefix for a '.v1' +// pathMajor, even though that pathMajor implies 'v1' tagging. +func PathMajorPrefix(pathMajor string) string { + if pathMajor == "" { + return "" + } + if pathMajor[0] != '/' && pathMajor[0] != '.' { + panic("pathMajor suffix " + pathMajor + " passed to PathMajorPrefix lacks separator") + } + if strings.HasPrefix(pathMajor, ".v") && strings.HasSuffix(pathMajor, "-unstable") { + pathMajor = strings.TrimSuffix(pathMajor, "-unstable") + } + m := pathMajor[1:] + if m != semver.Major(m) { + panic("pathMajor suffix " + pathMajor + "passed to PathMajorPrefix is not a valid major version") + } + return m } // CanonicalVersion returns the canonical form of the version string v. @@ -511,7 +615,10 @@ func EscapePath(path string) (escaped string, err error) { // and not contain exclamation marks. func EscapeVersion(v string) (escaped string, err error) { if err := checkElem(v, true); err != nil || strings.Contains(v, "!") { - return "", fmt.Errorf("disallowed version string %q", v) + return "", &InvalidVersionError{ + Version: v, + Err: fmt.Errorf("disallowed version string"), + } } return escapeString(v) } diff --git a/module/module_test.go b/module/module_test.go index 0759bb1..87f8ec8 100644 --- a/module/module_test.go +++ b/module/module_test.go @@ -79,8 +79,8 @@ var checkPathTests = []struct { {"/x.y/z", false, false, false}, {"x./z", false, false, false}, {".x/z", false, false, true}, - {"-x/z", false, true, true}, - {"x..y/z", false, false, false}, + {"-x/z", false, false, false}, + {"x..y/z", true, true, true}, {"x.y/z/../../w", false, false, false}, {"x.y//z", false, false, false}, {"x.y/z//w", false, false, false}, diff --git a/semver/semver.go b/semver/semver.go index 122e612..2988e3c 100644 --- a/semver/semver.go +++ b/semver/semver.go @@ -107,7 +107,7 @@ func Build(v string) string { } // Compare returns an integer comparing two versions according to -// according to semantic version precedence. +// semantic version precedence. // The result will be 0 if v == w, -1 if v < w, or +1 if v > w. // // An invalid semantic version string is considered less than a valid one. diff --git a/sumdb/server.go b/sumdb/server.go index acaf720..a4f8bee 100644 --- a/sumdb/server.go +++ b/sumdb/server.go @@ -9,9 +9,9 @@ import ( "context" "net/http" "os" - "regexp" "strings" + "golang.org/x/mod/internal/lazyregexp" "golang.org/x/mod/module" "golang.org/x/mod/sumdb/tlog" ) @@ -61,7 +61,7 @@ var ServerPaths = []string{ "/tile/", } -var modVerRE = regexp.MustCompile(`^[^@]+@v[0-9]+\.[0-9]+\.[0-9]+(-[^@]*)?(\+incompatible)?$`) +var modVerRE = lazyregexp.New(`^[^@]+@v[0-9]+\.[0-9]+\.[0-9]+(-[^@]*)?(\+incompatible)?$`) func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { ctx := r.Context() diff --git a/sumdb/test.go b/sumdb/test.go index 534ca3e..e4c166d 100644 --- a/sumdb/test.go +++ b/sumdb/test.go @@ -7,9 +7,9 @@ package sumdb import ( "context" "fmt" - "strings" "sync" + "golang.org/x/mod/module" "golang.org/x/mod/sumdb/note" "golang.org/x/mod/sumdb/tlog" ) @@ -75,7 +75,8 @@ func (s *TestServer) ReadRecords(ctx context.Context, id, n int64) ([][]byte, er return list, nil } -func (s *TestServer) Lookup(ctx context.Context, key string) (int64, error) { +func (s *TestServer) Lookup(ctx context.Context, m module.Version) (int64, error) { + key := m.String() s.mu.Lock() id, ok := s.lookup[key] s.mu.Unlock() @@ -84,12 +85,7 @@ func (s *TestServer) Lookup(ctx context.Context, key string) (int64, error) { } // Look up module and compute go.sum lines. - i := strings.Index(key, "@") - if i < 0 { - return 0, fmt.Errorf("invalid lookup key %q", key) - } - path, vers := key[:i], key[i+1:] - data, err := s.gosum(path, vers) + data, err := s.gosum(m.Path, m.Version) if err != nil { return 0, err } diff --git a/sumdb/tlog/tlog.go b/sumdb/tlog/tlog.go index a5b3d03..01d06c4 100644 --- a/sumdb/tlog/tlog.go +++ b/sumdb/tlog/tlog.go @@ -118,7 +118,7 @@ func NodeHash(left, right Hash) Hash { func StoredHashIndex(level int, n int64) int64 { // Level L's n'th hash is written right after level L+1's 2n+1'th hash. // Work our way down to the level 0 ordering. - // We'll add back the orignal level count at the end. + // We'll add back the original level count at the end. for l := level; l > 0; l-- { n = 2*n + 1 } @@ -152,7 +152,7 @@ func SplitStoredHashIndex(index int64) (level int, n int64) { n++ indexN = x } - // The hash we want was commited with record n, + // The hash we want was committed with record n, // meaning it is one of (0, n), (1, n/2), (2, n/4), ... level = int(index - indexN) return level, n >> uint(level)