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 <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

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
 			}
 		}

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
+}