cmd/fix: disallow cgo errors in tests

The 'cgo' command invoked by 'go fix' was not valid when built with
-trimpath, but the test was not failing because errors from the
command were being logged and ignored instead of causing tests to
fail. Changing the code and test not to ignore the errors revealed
that a number of existing tests were always, unconditionally
triggering cgo errors which were then ignored.

This change updates those tests to no longer produce cgo errors,
and to check their results when cgo is enabled.

For #51473.
Updates #51461.

Change-Id: Ib9d1ea93f26d30daa824d75ed634eaf530af086d
Reviewed-on: https://go-review.googlesource.com/c/go/+/450714
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>

diff --git a/src/cmd/fix/cftype_test.go b/src/cmd/fix/cftype_test.go
index a18eb25261d609509386cdb6413c7174d46973e3..cde47f28a3bbf11898fdf8153781333c5923ce7c 100644 (file)
--- a/src/cmd/fix/cftype_test.go
+++ b/src/cmd/fix/cftype_test.go
@@ -13,6 +13,7 @@ var cftypeTests = []testCase{
                Name: "cftype.localVariable",
                In: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 func f() {
@@ -23,6 +24,7 @@ func f() {
 `,
                Out: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 func f() {
@@ -36,6 +38,7 @@ func f() {
                Name: "cftype.globalVariable",
                In: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x C.CFTypeRef = nil
@@ -46,6 +49,7 @@ func f() {
 `,
                Out: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x C.CFTypeRef = 0
@@ -59,6 +63,7 @@ func f() {
                Name: "cftype.EqualArgument",
                In: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x C.CFTypeRef
@@ -67,6 +72,7 @@ var z = x != nil
 `,
                Out: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x C.CFTypeRef
@@ -78,6 +84,7 @@ var z = x != 0
                Name: "cftype.StructField",
                In: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 type T struct {
@@ -88,6 +95,7 @@ var t = T{x: nil}
 `,
                Out: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 type T struct {
@@ -101,6 +109,7 @@ var t = T{x: 0}
                Name: "cftype.FunctionArgument",
                In: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 func f(x C.CFTypeRef) {
@@ -112,6 +121,7 @@ func g() {
 `,
                Out: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 func f(x C.CFTypeRef) {
@@ -126,12 +136,14 @@ func g() {
                Name: "cftype.ArrayElement",
                In: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x = [3]C.CFTypeRef{nil, nil, nil}
 `,
                Out: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x = [3]C.CFTypeRef{0, 0, 0}
@@ -141,12 +153,14 @@ var x = [3]C.CFTypeRef{0, 0, 0}
                Name: "cftype.SliceElement",
                In: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x = []C.CFTypeRef{nil, nil, nil}
 `,
                Out: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x = []C.CFTypeRef{0, 0, 0}
@@ -156,12 +170,14 @@ var x = []C.CFTypeRef{0, 0, 0}
                Name: "cftype.MapKey",
                In: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x = map[C.CFTypeRef]int{nil: 0}
 `,
                Out: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x = map[C.CFTypeRef]int{0: 0}
@@ -171,12 +187,14 @@ var x = map[C.CFTypeRef]int{0: 0}
                Name: "cftype.MapValue",
                In: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x = map[int]C.CFTypeRef{0: nil}
 `,
                Out: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x = map[int]C.CFTypeRef{0: 0}
@@ -186,6 +204,7 @@ var x = map[int]C.CFTypeRef{0: 0}
                Name: "cftype.Conversion1",
                In: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x C.CFTypeRef
@@ -193,6 +212,7 @@ var y = (*unsafe.Pointer)(&x)
 `,
                Out: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x C.CFTypeRef
@@ -203,6 +223,7 @@ var y = (*unsafe.Pointer)(unsafe.Pointer(&x))
                Name: "cftype.Conversion2",
                In: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x unsafe.Pointer
@@ -210,6 +231,7 @@ var y = (*C.CFTypeRef)(&x)
 `,
                Out: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x unsafe.Pointer

diff --git a/src/cmd/fix/egltype_test.go b/src/cmd/fix/egltype_test.go
index 9b64a7c20b4676d3f86de14b9a72726165bf0190..c44525c0539e8b9fe699799232f0626bff51ea98 100644 (file)
--- a/src/cmd/fix/egltype_test.go
+++ b/src/cmd/fix/egltype_test.go
@@ -17,6 +17,7 @@ func eglTestsFor(tname string) []testCase {
                        Name: "egl.localVariable",
                        In: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 func f() {
@@ -27,6 +28,7 @@ func f() {
 `,
                        Out: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 func f() {
@@ -40,6 +42,7 @@ func f() {
                        Name: "egl.globalVariable",
                        In: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x C.$EGLTYPE = nil
@@ -50,6 +53,7 @@ func f() {
 `,
                        Out: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x C.$EGLTYPE = 0
@@ -63,6 +67,7 @@ func f() {
                        Name: "egl.EqualArgument",
                        In: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x C.$EGLTYPE
@@ -71,6 +76,7 @@ var z = x != nil
 `,
                        Out: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x C.$EGLTYPE
@@ -82,6 +88,7 @@ var z = x != 0
                        Name: "egl.StructField",
                        In: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 type T struct {
@@ -92,6 +99,7 @@ var t = T{x: nil}
 `,
                        Out: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 type T struct {
@@ -105,6 +113,7 @@ var t = T{x: 0}
                        Name: "egl.FunctionArgument",
                        In: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 func f(x C.$EGLTYPE) {
@@ -116,6 +125,7 @@ func g() {
 `,
                        Out: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 func f(x C.$EGLTYPE) {
@@ -130,12 +140,14 @@ func g() {
                        Name: "egl.ArrayElement",
                        In: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x = [3]C.$EGLTYPE{nil, nil, nil}
 `,
                        Out: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x = [3]C.$EGLTYPE{0, 0, 0}
@@ -145,12 +157,14 @@ var x = [3]C.$EGLTYPE{0, 0, 0}
                        Name: "egl.SliceElement",
                        In: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x = []C.$EGLTYPE{nil, nil, nil}
 `,
                        Out: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x = []C.$EGLTYPE{0, 0, 0}
@@ -160,12 +174,14 @@ var x = []C.$EGLTYPE{0, 0, 0}
                        Name: "egl.MapKey",
                        In: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x = map[C.$EGLTYPE]int{nil: 0}
 `,
                        Out: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x = map[C.$EGLTYPE]int{0: 0}
@@ -175,12 +191,14 @@ var x = map[C.$EGLTYPE]int{0: 0}
                        Name: "egl.MapValue",
                        In: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x = map[int]C.$EGLTYPE{0: nil}
 `,
                        Out: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x = map[int]C.$EGLTYPE{0: 0}

diff --git a/src/cmd/fix/jnitype_test.go b/src/cmd/fix/jnitype_test.go
index a6420f7b11fa03fda03c04365eb411209543ea04..ecf01408c7c84058488602461502ac912d2184b1 100644 (file)
--- a/src/cmd/fix/jnitype_test.go
+++ b/src/cmd/fix/jnitype_test.go
@@ -13,6 +13,7 @@ var jniTests = []testCase{
                Name: "jni.localVariable",
                In: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 func f() {
@@ -23,6 +24,7 @@ func f() {
 `,
                Out: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 func f() {
@@ -36,6 +38,7 @@ func f() {
                Name: "jni.globalVariable",
                In: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x C.jobject = nil
@@ -46,6 +49,7 @@ func f() {
 `,
                Out: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x C.jobject = 0
@@ -59,6 +63,7 @@ func f() {
                Name: "jni.EqualArgument",
                In: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x C.jobject
@@ -67,6 +72,7 @@ var z = x != nil
 `,
                Out: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x C.jobject
@@ -78,6 +84,7 @@ var z = x != 0
                Name: "jni.StructField",
                In: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 type T struct {
@@ -88,6 +95,7 @@ var t = T{x: nil}
 `,
                Out: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 type T struct {
@@ -101,6 +109,7 @@ var t = T{x: 0}
                Name: "jni.FunctionArgument",
                In: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 func f(x C.jobject) {
@@ -112,6 +121,7 @@ func g() {
 `,
                Out: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 func f(x C.jobject) {
@@ -126,12 +136,14 @@ func g() {
                Name: "jni.ArrayElement",
                In: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x = [3]C.jobject{nil, nil, nil}
 `,
                Out: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x = [3]C.jobject{0, 0, 0}
@@ -141,12 +153,14 @@ var x = [3]C.jobject{0, 0, 0}
                Name: "jni.SliceElement",
                In: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x = []C.jobject{nil, nil, nil}
 `,
                Out: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x = []C.jobject{0, 0, 0}
@@ -156,12 +170,14 @@ var x = []C.jobject{0, 0, 0}
                Name: "jni.MapKey",
                In: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x = map[C.jobject]int{nil: 0}
 `,
                Out: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x = map[C.jobject]int{0: 0}
@@ -171,12 +187,14 @@ var x = map[C.jobject]int{0: 0}
                Name: "jni.MapValue",
                In: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x = map[int]C.jobject{0: nil}
 `,
                Out: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x = map[int]C.jobject{0: 0}

diff --git a/src/cmd/fix/main_test.go b/src/cmd/fix/main_test.go
index 755007bc0d353c8f9f1fea8992537c3c30b2e211..837a5d72c7216497086539d4fdb018d743e1e6d6 100644 (file)
--- a/src/cmd/fix/main_test.go
+++ b/src/cmd/fix/main_test.go
@@ -5,13 +5,29 @@
 package main
 
 import (
+       "fmt"
        "go/ast"
        "go/parser"
        "internal/diff"
+       "internal/testenv"
        "strings"
        "testing"
 )
 
+func init() {
+       // If cgo is enabled, enforce that cgo commands invoked by cmd/fix
+       // do not fail during testing.
+       if testenv.HasCGO() {
+               // The reportCgoError hook is global, so we can't set it per-test
+               // if we want to be able to run those tests in parallel.
+               // Instead, simply set it to panic on error: the goroutine dump
+               // from the panic should help us determine which test failed.
+               reportCgoError = func(err error) {
+                       panic(fmt.Sprintf("unexpected cgo error: %v", err))
+               }
+       }
+}
+
 type testCase struct {
        Name    string
        Fn      func(*ast.File) bool
@@ -79,7 +95,13 @@ func TestRewrite(t *testing.T) {
                tt := tt
                t.Run(tt.Name, func(t *testing.T) {
                        if tt.Version == 0 {
-                               t.Parallel()
+                               if testing.Verbose() {
+                                       // Don't run in parallel: cmd/fix sometimes writes directly to stderr,
+                                       // and since -v prints which test is currently running we want that
+                                       // information to accurately correlate with the stderr output.
+                               } else {
+                                       t.Parallel()
+                               }
                        } else {
                                old := goVersion
                                goVersion = tt.Version

diff --git a/src/cmd/fix/typecheck.go b/src/cmd/fix/typecheck.go
index 27042e05a5506046a003caab5eb464c18b4b9af4..b115987390cb5221f482ee8a6e22f1a92aeb6ab1 100644 (file)
--- a/src/cmd/fix/typecheck.go
+++ b/src/cmd/fix/typecheck.go
@@ -170,7 +170,16 @@ func typecheck(cfg *TypeConfig, f *ast.File) (typeof map[any]string, assign map[
                        if err != nil {
                                return err
                        }
-                       cmd := exec.Command(filepath.Join(runtime.GOROOT(), "bin", "go"), "tool", "cgo", "-objdir", dir, "-srcdir", dir, "in.go")
+                       goCmd := "go"
+                       if goroot := runtime.GOROOT(); goroot != "" {
+                               goCmd = filepath.Join(goroot, "bin", "go")
+                       }
+                       cmd := exec.Command(goCmd, "tool", "cgo", "-objdir", dir, "-srcdir", dir, "in.go")
+                       if reportCgoError != nil {
+                               // Since cgo command errors will be reported, also forward the error
+                               // output from the command for debugging.
+                               cmd.Stderr = os.Stderr
+                       }
                        err = cmd.Run()
                        if err != nil {
                                return err
@@ -206,7 +215,11 @@ func typecheck(cfg *TypeConfig, f *ast.File) (typeof map[any]string, assign map[
                        return nil
                }()
                if err != nil {
-                       fmt.Fprintf(os.Stderr, "go fix: warning: no cgo types: %s\n", err)
+                       if reportCgoError == nil {
+                               fmt.Fprintf(os.Stderr, "go fix: warning: no cgo types: %s\n", err)
+                       } else {
+                               reportCgoError(err)
+                       }
                }
        }
 
@@ -285,6 +298,10 @@ func typecheck(cfg *TypeConfig, f *ast.File) (typeof map[any]string, assign map[
        return typeof, assign
 }
 
+// reportCgoError, if non-nil, reports a non-nil error from running the "cgo"
+// tool. (Set to a non-nil hook during testing if cgo is expected to work.)
+var reportCgoError func(err error)
+
 func makeExprList(a []*ast.Ident) []ast.Expr {
        var b []ast.Expr
        for _, x := range a {