cmd/go: fix bad shared lib name with buildmode=shared

Use import paths of packages to build a shared lib name.
Use arguments for meta-packages 'std', 'cmd', and 'all'.

Fixes #12236

Change-Id: If274d63301686ef34e198287eb012f9062541ea0
Reviewed-on: https://go-review.googlesource.com/13921
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/cmd/go/build.go b/src/cmd/go/build.go
index c11c062a5065e20045efb7f0344da08eae98d862..dc1e61284e19f70b1454e5c8c155503df3155fd0 100644 (file)
--- a/src/cmd/go/build.go
+++ b/src/cmd/go/build.go
@@ -480,7 +480,12 @@ func runBuild(cmd *Command, args []string) {
 
        var a *action
        if buildBuildmode == "shared" {
-               a = b.libaction(libname(args), pkgsFilter(packages(args)), modeBuild, depMode)
+               pkgs := pkgsFilter(packages(args))
+               if libName, err := libname(args, pkgs); err != nil {
+                       fatalf("%s", err.Error())
+               } else {
+                       a = b.libaction(libName, pkgs, modeBuild, depMode)
+               }
        } else {
                a = &action{}
                for _, p := range pkgsFilter(packages(args)) {
@@ -504,28 +509,49 @@ See also: go build, go get, go clean.
        `,
 }
 
+// isMetaPackage checks if name is a reserved package name that expands to multiple packages
+func isMetaPackage(name string) bool {
+       return name == "std" || name == "cmd" || name == "all"
+}
+
 // libname returns the filename to use for the shared library when using
 // -buildmode=shared.  The rules we use are:
-//  1) Drop any trailing "/..."s if present
-//  2) Change / to -
-//  3) Join arguments with ,
-// So std -> libstd.so
-//    a b/... -> liba,b.so
-//    gopkg.in/tomb.v2 -> libgopkg.in-tomb.v2.so
-func libname(args []string) string {
+// Use arguments for special 'meta' packages:
+//     std --> libstd.so
+//     std cmd --> libstd,cmd.so
+// Use import paths for other cases, changing '/' to '-':
+//     somelib --> libsubdir-somelib.so
+//     ./ or ../ --> libsubdir-somelib.so
+//     gopkg.in/tomb.v2 -> libgopkg.in-tomb.v2.so
+//     ./... ---> libpkg1,pkg2.so - subset of all import paths
+// Name parts are joined with ','.
+func libname(args []string, pkgs []*Package) (string, error) {
        var libname string
-       for _, arg := range args {
-               arg = strings.TrimSuffix(arg, "/...")
-               arg = strings.Replace(arg, "/", "-", -1)
+       appendName := func(arg string) {
                if libname == "" {
                        libname = arg
                } else {
                        libname += "," + arg
                }
        }
+       var haveNonMeta bool
+       for _, arg := range args {
+               if isMetaPackage(arg) {
+                       appendName(arg)
+               } else {
+                       haveNonMeta = true
+               }
+       }
+       if len(libname) == 0 { // non-meta packages only. use import paths
+               for _, pkg := range pkgs {
+                       appendName(strings.Replace(pkg.ImportPath, "/", "-", -1))
+               }
+       } else if haveNonMeta { // have both meta package and a non-meta one
+               return "", errors.New("mixing of meta and non-meta packages is not allowed")
+       }
        // TODO(mwhudson): Needs to change for platforms that use different naming
        // conventions...
-       return "lib" + libname + ".so"
+       return "lib" + libname + ".so", nil
 }
 
 func runInstall(cmd *Command, args []string) {
@@ -558,7 +584,11 @@ func runInstall(cmd *Command, args []string) {
        b.init()
        var a *action
        if buildBuildmode == "shared" {
-               a = b.libaction(libname(args), pkgs, modeInstall, modeInstall)
+               if libName, err := libname(args, pkgs); err != nil {
+                       fatalf("%s", err.Error())
+               } else {
+                       a = b.libaction(libName, pkgs, modeInstall, modeInstall)
+               }
        } else {
                a = &action{}
                var tools []*action

diff --git a/src/cmd/go/main.go b/src/cmd/go/main.go
index b66049f0c28d1b3091a20a1e85ecaa35b4bf923a..ca0ce82082c5a2a49ff69fb19f38c3c8d3c1df0b 100644 (file)
--- a/src/cmd/go/main.go
+++ b/src/cmd/go/main.go
@@ -353,7 +353,7 @@ func importPathsNoDotExpansion(args []string) []string {
                } else {
                        a = path.Clean(a)
                }
-               if a == "all" || a == "std" || a == "cmd" {
+               if isMetaPackage(a) {
                        out = append(out, allPackages(a)...)
                        continue
                }
@@ -554,7 +554,7 @@ func allPackages(pattern string) []string {
 func matchPackages(pattern string) []string {
        match := func(string) bool { return true }
        treeCanMatch := func(string) bool { return true }
-       if pattern != "all" && pattern != "std" && pattern != "cmd" {
+       if !isMetaPackage(pattern) {
                match = matchPattern(pattern)
                treeCanMatch = treeCanMatchPattern(pattern)
        }

diff --git a/src/cmd/go/pkg_test.go b/src/cmd/go/pkg_test.go
index 06b9f0ac6ebcb0fe1802756c8355edc86d07b72d..23c2e08da19a0fde535be88ab5b393491f5973a2 100644 (file)
--- a/src/cmd/go/pkg_test.go
+++ b/src/cmd/go/pkg_test.go
@@ -71,3 +71,95 @@ func TestParseMetaGoImports(t *testing.T) {
                }
        }
 }
+
+func TestSharedLibName(t *testing.T) {
+       // TODO(avdva) - make these values platform-specific
+       prefix := "lib"
+       suffix := ".so"
+       testData := []struct {
+               args      []string
+               pkgs      []*Package
+               expected  string
+               expectErr bool
+       }{
+               {
+                       []string{"std"},
+                       []*Package{},
+                       "std",
+                       false,
+               },
+               {
+                       []string{"std", "cmd"},
+                       []*Package{},
+                       "std,cmd",
+                       false,
+               },
+               {
+                       []string{},
+                       []*Package{&Package{ImportPath: "gopkg.in/somelib"}},
+                       "gopkg.in-somelib",
+                       false,
+               },
+               {
+                       []string{"./..."},
+                       []*Package{&Package{ImportPath: "somelib"}},
+                       "somelib",
+                       false,
+               },
+               {
+                       []string{"../somelib", "../somelib"},
+                       []*Package{&Package{ImportPath: "somelib"}},
+                       "somelib",
+                       false,
+               },
+               {
+                       []string{"../lib1", "../lib2"},
+                       []*Package{&Package{ImportPath: "gopkg.in/lib1"}, &Package{ImportPath: "gopkg.in/lib2"}},
+                       "gopkg.in-lib1,gopkg.in-lib2",
+                       false,
+               },
+               {
+                       []string{"./..."},
+                       []*Package{
+                               &Package{ImportPath: "gopkg.in/dir/lib1"},
+                               &Package{ImportPath: "gopkg.in/lib2"},
+                               &Package{ImportPath: "gopkg.in/lib3"},
+                       },
+                       "gopkg.in-dir-lib1,gopkg.in-lib2,gopkg.in-lib3",
+                       false,
+               },
+               {
+                       []string{"std", "../lib2"},
+                       []*Package{},
+                       "",
+                       true,
+               },
+               {
+                       []string{"all", "./"},
+                       []*Package{},
+                       "",
+                       true,
+               },
+               {
+                       []string{"cmd", "fmt"},
+                       []*Package{},
+                       "",
+                       true,
+               },
+       }
+       for _, data := range testData {
+               computed, err := libname(data.args, data.pkgs)
+               if err != nil {
+                       if !data.expectErr {
+                               t.Errorf("libname returned an error %q, expected a name", err.Error())
+                       }
+               } else if data.expectErr {
+                       t.Errorf("libname returned %q, expected an error", computed)
+               } else {
+                       expected := prefix + data.expected + suffix
+                       if expected != computed {
+                               t.Errorf("libname returned %q, expected %q", computed, expected)
+                       }
+               }
+       }
+}