From 449c356b79e5abeb12c1239b1837c9ca7677acc3 Mon Sep 17 00:00:00 2001 From: Michael Matloob Date: Thu, 30 Jan 2020 13:34:11 -0500 Subject: [PATCH] go/packages: work around pkg-config errors in go list go list -e exits with a non-zero exit status if pkg-config fails. Add a work-around to that to our long list of go list -e work-arounds. Also add a moderately complicated test case that installs a fake pkg-config. Fixes golang/go#36770 Change-Id: I1187357e567eefde528fe30b27abde4ae346170f Reviewed-on: https://go-review.googlesource.com/c/tools/+/217078 Run-TryBot: Michael Matloob TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- go/packages/golist.go | 7 ++- go/packages/packagescgo_test.go | 85 +++++++++++++++++++++++++++++++-- 2 files changed, 87 insertions(+), 5 deletions(-) diff --git a/go/packages/golist.go b/go/packages/golist.go index bad71b3e1b..639639fedf 100644 --- a/go/packages/golist.go +++ b/go/packages/golist.go @@ -761,7 +761,12 @@ func (state *golistState) invokeGo(verb string, args ...string) (*bytes.Buffer, !strings.ContainsRune("!\"#$%&'()*,:;<=>?[\\]^`{|}\uFFFD", r) } if len(stderr.String()) > 0 && strings.HasPrefix(stderr.String(), "# ") { - if strings.HasPrefix(strings.TrimLeftFunc(stderr.String()[len("# "):], isPkgPathRune), "\n") { + msg := stderr.String()[len("# "):] + if strings.HasPrefix(strings.TrimLeftFunc(msg, isPkgPathRune), "\n") { + return stdout, nil + } + // Treat pkg-config errors as a special case (golang.org/issue/36770). + if strings.HasPrefix(msg, "pkg-config") { return stdout, nil } } diff --git a/go/packages/packagescgo_test.go b/go/packages/packagescgo_test.go index ed51522e3e..a149309c00 100644 --- a/go/packages/packagescgo_test.go +++ b/go/packages/packagescgo_test.go @@ -8,7 +8,10 @@ package packages_test import ( "fmt" + "io/ioutil" "os" + "os/exec" + "path/filepath" "runtime" "strings" "testing" @@ -66,10 +69,6 @@ func TestLoadImportsC(t *testing.T) { } } -func TestCgoNoSyntax(t *testing.T) { - packagestest.TestAll(t, testCgoNoSyntax) -} - // Stolen from internal/testenv package in core. // hasGoBuild reports whether the current system can build programs with ``go build'' // and then run them with os.StartProcess or exec.Command. @@ -92,6 +91,9 @@ func hasGoBuild() bool { return true } +func TestCgoNoSyntax(t *testing.T) { + packagestest.TestAll(t, testCgoNoSyntax) +} func testCgoNoSyntax(t *testing.T, exporter packagestest.Exporter) { // The android builders have a complex setup which causes this test to fail. See discussion on // golang.org/cl/214943 for more details. @@ -133,3 +135,78 @@ func testCgoNoSyntax(t *testing.T, exporter packagestest.Exporter) { }) } } + +func TestCgoBadPkgConfig(t *testing.T) { + packagestest.TestAll(t, testCgoBadPkgConfig) +} +func testCgoBadPkgConfig(t *testing.T, exporter packagestest.Exporter) { + if !hasGoBuild() { + t.Skip("this test can't run on platforms without go build. See discussion on golang.org/cl/214943 for more details.") + } + + exported := packagestest.Export(t, exporter, []packagestest.Module{{ + Name: "golang.org/fake", + Files: map[string]interface{}{ + "c/c.go": `package c + +// #cgo pkg-config: --cflags -- foo +import "C"`, + }, + }}) + + dir := buildFakePkgconfig(t, exported.Config.Env) + defer os.RemoveAll(dir) + env := exported.Config.Env + for i, v := range env { + if strings.HasPrefix(v, "PATH=") { + env[i] = "PATH=" + dir + string(os.PathListSeparator) + v[len("PATH="):] + } + } + + exported.Config.Env = append(exported.Config.Env, "CGO_ENABLED=1") + + exported.Config.Mode = packages.NeedName | packages.NeedCompiledGoFiles + pkgs, err := packages.Load(exported.Config, "golang.org/fake/c") + if err != nil { + t.Fatal(err) + } + if len(pkgs) != 1 { + t.Fatalf("Expected 1 package, got %v", pkgs) + } + if pkgs[0].Name != "c" { + t.Fatalf("Expected package to have name \"c\", got %q", pkgs[0].Name) + } +} + +func buildFakePkgconfig(t *testing.T, env []string) string { + tmpdir, err := ioutil.TempDir("", "fakepkgconfig") + if err != nil { + t.Fatal(err) + } + err = ioutil.WriteFile(filepath.Join(tmpdir, "pkg-config.go"), []byte(` +package main + +import "fmt" +import "os" + +func main() { + fmt.Fprintln(os.Stderr, "bad") + os.Exit(2) +} +`), 0644) + if err != nil { + os.RemoveAll(tmpdir) + t.Fatal(err) + } + cmd := exec.Command("go", "build", "-o", "pkg-config", "pkg-config.go") + cmd.Dir = tmpdir + cmd.Env = env + + if b, err := cmd.CombinedOutput(); err != nil { + os.RemoveAll(tmpdir) + fmt.Println(os.Environ()) + t.Log(string(b)) + t.Fatal(err) + } + return tmpdir +}