]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go: fix the default build output name for versioned binaries
authorDmitri Shuralyov <dmitshur@golang.org>
Thu, 21 Mar 2019 17:31:32 +0000 (13:31 -0400)
committerDmitri Shuralyov <dmitshur@golang.org>
Fri, 22 Mar 2019 19:28:22 +0000 (19:28 +0000)
This change is a re-apply of the reverted CL 140863 with changes to
address issue #30821. Specifically, path.Split continues to be used
to split the '/'-separated import path, rather than filepath.Split.

Document the algorithm for how the default executable name is determined
in DefaultExecName.

Rename a variable returned from os.Stat from bs to fi for consistency.

CL 140863 factored out the logic to determine the default executable
name from the Package.load method into a DefaultExecName function,
and started using it in more places to avoid having to re-implement
the logic everywhere it's needed. Most previous callers already computed
the default executable name based on the import path. The load.Package
method, before CL 140863, was the exception, in that it used the p.Dir
value in GOPATH mode instead. There was a NOTE(rsc) comment that it
should be equivalent to use import path, but it was too late in Go 1.11
cycle to risk implementing that change.

This is part 1, a more conservative change for backporting to Go 1.12.2,
and it keeps the original behavior of splitting on p.Dir in GOPATH mode.
Part 2 will address the NOTE(rsc) comment and modify behavior in
Package.load to always use DefaultExecName which splits the import path
rather than directory. It is intended to be included in Go 1.13.

Fixes #27283 (again)
Updates #26869
Fixes #30821

Change-Id: Ib1ebb95acba7c85c24e3a55c40cdf48405af34f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/167503
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
src/cmd/go/internal/load/pkg.go
src/cmd/go/internal/test/test.go
src/cmd/go/internal/work/build.go
src/cmd/go/testdata/mod/rsc.io_fortune_v2_v2.0.0.txt
src/cmd/go/testdata/script/mod_build_versioned.txt [new file with mode: 0644]

index a0333bd5223bc970a0c3f503aa9fcac51304e217..431dfe318ea3d266314e792f46d01f5a0d2b8a81 100644 (file)
@@ -1186,6 +1186,36 @@ var cgoSyscallExclude = map[string]bool{
 
 var foldPath = make(map[string]string)
 
+// DefaultExecName returns the default executable name
+// 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.
+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) {
+                       _, elem = pathpkg.Split(pathpkg.Dir(importPath))
+               }
+       }
+       return elem
+}
+
 // load populates p using information from bp, err, which should
 // be the result of calling build.Context.Import.
 func (p *Package) load(stk *ImportStack, bp *build.Package, err error) {
@@ -1228,7 +1258,7 @@ func (p *Package) load(stk *ImportStack, bp *build.Package, err error) {
                }
                _, elem := filepath.Split(p.Dir)
                if cfg.ModulesEnabled {
-                       // NOTE(rsc): Using p.ImportPath instead of p.Dir
+                       // 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.
@@ -1237,26 +1267,9 @@ func (p *Package) load(stk *ImportStack, bp *build.Package, err error) {
                        // even for non-module-enabled code,
                        // but I'm not brave enough to change the
                        // non-module behavior this late in the
-                       // release cycle. Maybe for Go 1.12.
+                       // release cycle. Can be done for Go 1.13.
                        // See golang.org/issue/26869.
-                       _, elem = pathpkg.Split(p.ImportPath)
-
-                       // 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) {
-                               _, elem = pathpkg.Split(pathpkg.Dir(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 {
index fe90af3be51f51c487afc58c1340f12f3fbee3b1..b43925d5e508fd0ca38ec7f2dec53d93ce57ab16 100644 (file)
@@ -811,7 +811,7 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
        if p.ImportPath == "command-line-arguments" {
                elem = p.Name
        } else {
-               _, elem = path.Split(p.ImportPath)
+               elem = load.DefaultExecName(p.ImportPath)
        }
        testBinary := elem + ".test"
 
index 21abd5ef5bf1d3d4dccd6335f33a522769763030..eac027e09ee7bcc38e39a6ba4b5833bdef0909c3 100644 (file)
@@ -10,7 +10,6 @@ import (
        "go/build"
        "os"
        "os/exec"
-       "path"
        "path/filepath"
        "runtime"
        "strings"
@@ -285,7 +284,7 @@ func runBuild(cmd *base.Command, args []string) {
        pkgs := load.PackagesForBuild(args)
 
        if len(pkgs) == 1 && pkgs[0].Name == "main" && cfg.BuildO == "" {
-               _, cfg.BuildO = path.Split(pkgs[0].ImportPath)
+               cfg.BuildO = load.DefaultExecName(pkgs[0].ImportPath)
                cfg.BuildO += cfg.ExeSuffix
        }
 
@@ -320,14 +319,13 @@ func runBuild(cmd *base.Command, args []string) {
                // If the -o name exists and is a directory, then
                // write all main packages to that directory.
                // Otherwise require only a single package be built.
-               if bs, err := os.Stat(cfg.BuildO); err == nil && bs.IsDir() {
+               if fi, err := os.Stat(cfg.BuildO); err == nil && fi.IsDir() {
                        a := &Action{Mode: "go build"}
                        for _, p := range pkgs {
                                if p.Name != "main" {
                                        continue
                                }
-                               _, elem := path.Split(p.ImportPath)
-                               p.Target = filepath.Join(cfg.BuildO, elem)
+                               p.Target = filepath.Join(cfg.BuildO, load.DefaultExecName(p.ImportPath))
                                p.Target += cfg.ExeSuffix
                                p.Stale = true
                                p.StaleReason = "build -o flag in use"
@@ -540,7 +538,7 @@ func InstallPackages(patterns []string, pkgs []*load.Package) {
        if len(patterns) == 0 && len(pkgs) == 1 && pkgs[0].Name == "main" {
                // Compute file 'go build' would have created.
                // If it exists and is an executable file, remove it.
-               _, targ := filepath.Split(pkgs[0].ImportPath)
+               targ := load.DefaultExecName(pkgs[0].ImportPath)
                targ += cfg.ExeSuffix
                if filepath.Join(pkgs[0].Dir, targ) != pkgs[0].Target { // maybe $GOBIN is the current directory
                        fi, err := os.Stat(targ)
index cfa91f08a5d84ba07ffd96eef8580a43c455899b..3acd6379311f643194e45a712464b4af4aed5fa6 100644 (file)
@@ -13,3 +13,9 @@ import "rsc.io/quote"
 func main() {
        println(quote.Hello())
 }
+-- fortune_test.go --
+package main
+
+import "testing"
+
+func TestFortuneV2(t *testing.T) {}
diff --git a/src/cmd/go/testdata/script/mod_build_versioned.txt b/src/cmd/go/testdata/script/mod_build_versioned.txt
new file mode 100644 (file)
index 0000000..eb081c9
--- /dev/null
@@ -0,0 +1,16 @@
+env GO111MODULE=on
+
+go get -m rsc.io/fortune/v2
+
+# The default executable name shouldn't be v2$exe
+go build rsc.io/fortune/v2
+! exists v2$exe
+exists fortune$exe
+
+# The default test binary name shouldn't be v2.test$exe
+go test -c rsc.io/fortune/v2
+! exists v2.test$exe
+exists fortune.test$exe
+
+-- go.mod --
+module scratch