From c5aa53c8c52ec895d7e7c18a8fab9c5786555c1a Mon Sep 17 00:00:00 2001 From: Aleksandr Demakin Date: Tue, 25 Aug 2015 19:54:57 +0300 Subject: [PATCH] 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 Run-TryBot: Ian Lance Taylor TryBot-Result: Gobot Gobot --- src/cmd/go/build.go | 56 +++++++++++++++++++------ src/cmd/go/main.go | 4 +- src/cmd/go/pkg_test.go | 92 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 137 insertions(+), 15 deletions(-) diff --git a/src/cmd/go/build.go b/src/cmd/go/build.go index c11c062a50..dc1e61284e 100644 --- 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 b66049f0c2..ca0ce82082 100644 --- 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 06b9f0ac6e..23c2e08da1 100644 --- 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) + } + } + } +}