From: Dmitri Shuralyov Date: Thu, 21 Mar 2019 18:00:46 +0000 (-0400) Subject: cmd/go/internal/load: always use DefaultExecName to determine binary name X-Git-Tag: go1.13beta1~896 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=e007916cea969c4d5917da931413fba4eb43e8f6;p=gostls13.git cmd/go/internal/load: always use DefaultExecName to determine binary name It should produce equivalent results to split on the import path of the package rather than its directory, in GOPATH mode. That means the common code in DefaultExecName can be used. We're in the middle of Go 1.12 cycle, so now is a good time to make it happen for Go 1.13. Modify isVersionElement to accept path elements like "v2", "v3", "v10", rather than "/v2", "/v3", "/v10". Then use it in DefaultExecName instead of the ad-hoc isVersion function. There is no change in behavior. Add tests for DefaultExecName and isVersionElement. Updates #26869 Change-Id: Ic6da2c92587459aa2b327385e994b72a6e183092 Reviewed-on: https://go-review.googlesource.com/c/go/+/168678 Run-TryBot: Dmitri Shuralyov TryBot-Result: Gobot Gobot Reviewed-by: Jay Conrod --- diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go index 431dfe318e..3827d3184e 100644 --- a/src/cmd/go/internal/load/pkg.go +++ b/src/cmd/go/internal/load/pkg.go @@ -802,7 +802,7 @@ func findVersionElement(path string) (i, j int) { j = len(path) for i = len(path) - 1; i >= 0; i-- { if path[i] == '/' { - if isVersionElement(path[i:j]) { + if isVersionElement(path[i+1 : j]) { return i, j } j = i @@ -814,10 +814,10 @@ func findVersionElement(path string) (i, j int) { // isVersionElement reports whether s is a well-formed path version element: // v2, v3, v10, etc, but not v0, v05, v1. func isVersionElement(s string) bool { - if len(s) < 3 || s[0] != '/' || s[1] != 'v' || s[2] == '0' || s[2] == '1' && len(s) == 3 { + if len(s) < 2 || s[0] != 'v' || s[1] == '0' || s[1] == '1' && len(s) == 2 { return false } - for i := 2; i < len(s); i++ { + for i := 1; i < len(s); i++ { if s[i] < '0' || '9' < s[i] { return false } @@ -1190,26 +1190,16 @@ var foldPath = make(map[string]string) // for a package with the import path importPath. // // The default executable name is the last element of the import path. -// In module-aware mode, an additional rule is used. If the last element -// is a vN path element specifying the major version, then the second last -// element of the import path is used instead. +// In module-aware mode, an additional rule is used on import paths +// consisting of two or more path elements. If the last element is +// a vN path element specifying the major version, then the +// second last element of the import path is used instead. func DefaultExecName(importPath string) string { _, elem := pathpkg.Split(importPath) if cfg.ModulesEnabled { - // If this is example.com/mycmd/v2, it's more useful to install it as mycmd than as v2. - // See golang.org/issue/24667. - isVersion := func(v string) bool { - if len(v) < 2 || v[0] != 'v' || v[1] < '1' || '9' < v[1] { - return false - } - for i := 2; i < len(v); i++ { - if c := v[i]; c < '0' || '9' < c { - return false - } - } - return true - } - if isVersion(elem) { + // If this is example.com/mycmd/v2, it's more useful to + // install it as mycmd than as v2. See golang.org/issue/24667. + if elem != importPath && isVersionElement(elem) { _, elem = pathpkg.Split(pathpkg.Dir(importPath)) } } @@ -1256,21 +1246,7 @@ func (p *Package) load(stk *ImportStack, bp *build.Package, err error) { p.Error = &PackageError{Err: e} return } - _, elem := filepath.Split(p.Dir) - if cfg.ModulesEnabled { - // NOTE(rsc,dmitshur): Using p.ImportPath instead of p.Dir - // makes sure we install a package in the root of a - // cached module directory as that package name - // not name@v1.2.3. - // Using p.ImportPath instead of p.Dir - // is probably correct all the time, - // even for non-module-enabled code, - // but I'm not brave enough to change the - // non-module behavior this late in the - // release cycle. Can be done for Go 1.13. - // See golang.org/issue/26869. - elem = DefaultExecName(p.ImportPath) - } + elem := DefaultExecName(p.ImportPath) full := cfg.BuildContext.GOOS + "_" + cfg.BuildContext.GOARCH + "/" + elem if cfg.BuildContext.GOOS != base.ToolGOOS || cfg.BuildContext.GOARCH != base.ToolGOARCH { // Install cross-compiled binaries to subdirectories of bin. diff --git a/src/cmd/go/internal/load/pkg_test.go b/src/cmd/go/internal/load/pkg_test.go new file mode 100644 index 0000000000..9ddc20d050 --- /dev/null +++ b/src/cmd/go/internal/load/pkg_test.go @@ -0,0 +1,68 @@ +package load + +import ( + "cmd/go/internal/cfg" + "testing" +) + +func TestDefaultExecName(t *testing.T) { + oldModulesEnabled := cfg.ModulesEnabled + defer func() { cfg.ModulesEnabled = oldModulesEnabled }() + for _, tt := range []struct { + in string + wantMod string + wantGopath string + }{ + {"example.com/mycmd", "mycmd", "mycmd"}, + {"example.com/mycmd/v0", "v0", "v0"}, + {"example.com/mycmd/v1", "v1", "v1"}, + {"example.com/mycmd/v2", "mycmd", "v2"}, // Semantic import versioning, use second last element in module mode. + {"example.com/mycmd/v3", "mycmd", "v3"}, // Semantic import versioning, use second last element in module mode. + {"mycmd", "mycmd", "mycmd"}, + {"mycmd/v0", "v0", "v0"}, + {"mycmd/v1", "v1", "v1"}, + {"mycmd/v2", "mycmd", "v2"}, // Semantic import versioning, use second last element in module mode. + {"v0", "v0", "v0"}, + {"v1", "v1", "v1"}, + {"v2", "v2", "v2"}, + } { + { + cfg.ModulesEnabled = true + gotMod := DefaultExecName(tt.in) + if gotMod != tt.wantMod { + t.Errorf("DefaultExecName(%q) in module mode = %v; want %v", tt.in, gotMod, tt.wantMod) + } + } + { + cfg.ModulesEnabled = false + gotGopath := DefaultExecName(tt.in) + if gotGopath != tt.wantGopath { + t.Errorf("DefaultExecName(%q) in gopath mode = %v; want %v", tt.in, gotGopath, tt.wantGopath) + } + } + } +} + +func TestIsVersionElement(t *testing.T) { + t.Parallel() + for _, tt := range []struct { + in string + want bool + }{ + {"v0", false}, + {"v05", false}, + {"v1", false}, + {"v2", true}, + {"v3", true}, + {"v9", true}, + {"v10", true}, + {"v11", true}, + {"v", false}, + {"vx", false}, + } { + got := isVersionElement(tt.in) + if got != tt.want { + t.Errorf("isVersionElement(%q) = %v; want %v", tt.in, got, tt.want) + } + } +}