]> Cypherpunks repositories - gostls13.git/commitdiff
misc/cgo/testcarchive: avoid writing to GOROOT in tests
authorBryan C. Mills <bcmills@google.com>
Wed, 20 Nov 2019 15:19:43 +0000 (10:19 -0500)
committerBryan C. Mills <bcmills@google.com>
Fri, 22 Nov 2019 15:32:23 +0000 (15:32 +0000)
Also add a -testwork flag to facilitate debugging the test itself.

Three of the tests of this package invoked 'go install -i
-buildmode=c-archive' in order to generate an archive as well as
multiple C header files.

Unfortunately, the behavior of the '-i' flag is inappropriately broad
for this use-case: it not only generates the library and header files
(as desired), but also attempts to install a number of (unnecessary)
archive files for transitive dependencies to
GOROOT/pkg/$GOOS_$GOARCH_shared, which may not be writable — for
example, if GOROOT is owned by the root user but the test is being run
by a non-root user.

Instead, for now we generate the header files for transitive dependencies
separately by running 'go tool cgo -exportheader'.

In the future, we should consider how to improve the ergonomics for
generating transitive header files without coupling that to
unnecessary library installation.

Updates #28387
Updates #30316
Updates #35715

Change-Id: I3d483f84e22058561efe740aa4885fc3f26137b5
Reviewed-on: https://go-review.googlesource.com/c/go/+/208117
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
misc/cgo/testcarchive/carchive_test.go

index cf2c6264dd179f97f5645550cee3548bbd2a48e5..82a1a5a54ce63ccd22f9577810f30db1e22c55af 100644 (file)
@@ -36,7 +36,10 @@ var exeSuffix string
 var GOOS, GOARCH, GOPATH string
 var libgodir string
 
+var testWork bool // If true, preserve temporary directories.
+
 func TestMain(m *testing.M) {
+       flag.BoolVar(&testWork, "testwork", false, "if true, log and preserve the test's temporary working directory")
        flag.Parse()
        if testing.Short() && os.Getenv("GO_BUILDER_NAME") == "" {
                fmt.Printf("SKIP - short mode and $GO_BUILDER_NAME not set\n")
@@ -54,7 +57,11 @@ func testMain(m *testing.M) int {
        if err != nil {
                log.Panic(err)
        }
-       defer os.RemoveAll(GOPATH)
+       if testWork {
+               log.Println(GOPATH)
+       } else {
+               defer os.RemoveAll(GOPATH)
+       }
        os.Setenv("GOPATH", GOPATH)
 
        // Copy testdata into GOPATH/src/testarchive, along with a go.mod file
@@ -164,6 +171,38 @@ func cmdToRun(name string) []string {
        return []string{executor, name}
 }
 
+// genHeader writes a C header file for the C-exported declarations found in .go
+// source files in dir.
+//
+// TODO(golang.org/issue/35715): This should be simpler.
+func genHeader(t *testing.T, header, dir string) {
+       t.Helper()
+
+       // The 'cgo' command generates a number of additional artifacts,
+       // but we're only interested in the header.
+       // Shunt the rest of the outputs to a temporary directory.
+       objDir, err := ioutil.TempDir(GOPATH, "_obj")
+       if err != nil {
+               t.Fatal(err)
+       }
+       defer os.RemoveAll(objDir)
+
+       files, err := filepath.Glob(filepath.Join(dir, "*.go"))
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       cmd := exec.Command("go", "tool", "cgo",
+               "-objdir", objDir,
+               "-exportheader", header)
+       cmd.Args = append(cmd.Args, files...)
+       t.Log(cmd.Args)
+       if out, err := cmd.CombinedOutput(); err != nil {
+               t.Logf("%s", out)
+               t.Fatal(err)
+       }
+}
+
 func testInstall(t *testing.T, exe, libgoa, libgoh string, buildcmd ...string) {
        t.Helper()
        cmd := exec.Command(buildcmd[0], buildcmd[1:]...)
@@ -172,10 +211,12 @@ func testInstall(t *testing.T, exe, libgoa, libgoh string, buildcmd ...string) {
                t.Logf("%s", out)
                t.Fatal(err)
        }
-       defer func() {
-               os.Remove(libgoa)
-               os.Remove(libgoh)
-       }()
+       if !testWork {
+               defer func() {
+                       os.Remove(libgoa)
+                       os.Remove(libgoh)
+               }()
+       }
 
        ccArgs := append(cc, "-o", exe, "main.c")
        if GOOS == "windows" {
@@ -191,7 +232,9 @@ func testInstall(t *testing.T, exe, libgoa, libgoh string, buildcmd ...string) {
                t.Logf("%s", out)
                t.Fatal(err)
        }
-       defer os.Remove(exe)
+       if !testWork {
+               defer os.Remove(exe)
+       }
 
        binArgs := append(cmdToRun(exe), "arg1", "arg2")
        cmd = exec.Command(binArgs[0], binArgs[1:]...)
@@ -227,17 +270,27 @@ func checkLineComments(t *testing.T, hdrname string) {
 }
 
 func TestInstall(t *testing.T) {
-       defer os.RemoveAll(filepath.Join(GOPATH, "pkg"))
+       if !testWork {
+               defer os.RemoveAll(filepath.Join(GOPATH, "pkg"))
+       }
 
        libgoa := "libgo.a"
        if runtime.Compiler == "gccgo" {
                libgoa = "liblibgo.a"
        }
 
+       // Generate the p.h header file.
+       //
+       // 'go install -i -buildmode=c-archive ./libgo' would do that too, but that
+       // would also attempt to install transitive standard-library dependencies to
+       // GOROOT, and we cannot assume that GOROOT is writable. (A non-root user may
+       // be running this test in a GOROOT owned by root.)
+       genHeader(t, "p.h", "./p")
+
        testInstall(t, "./testp1"+exeSuffix,
                filepath.Join(libgodir, libgoa),
                filepath.Join(libgodir, "libgo.h"),
-               "go", "install", "-i", "-buildmode=c-archive", "./libgo")
+               "go", "install", "-buildmode=c-archive", "./libgo")
 
        // Test building libgo other than installing it.
        // Header files are now present.
@@ -259,12 +312,14 @@ func TestEarlySignalHandler(t *testing.T) {
                t.Skip("skipping signal test on Windows")
        }
 
-       defer func() {
-               os.Remove("libgo2.a")
-               os.Remove("libgo2.h")
-               os.Remove("testp")
-               os.RemoveAll(filepath.Join(GOPATH, "pkg"))
-       }()
+       if !testWork {
+               defer func() {
+                       os.Remove("libgo2.a")
+                       os.Remove("libgo2.h")
+                       os.Remove("testp")
+                       os.RemoveAll(filepath.Join(GOPATH, "pkg"))
+               }()
+       }
 
        cmd := exec.Command("go", "build", "-buildmode=c-archive", "-o", "libgo2.a", "./libgo2")
        if out, err := cmd.CombinedOutput(); err != nil {
@@ -297,12 +352,14 @@ func TestEarlySignalHandler(t *testing.T) {
 func TestSignalForwarding(t *testing.T) {
        checkSignalForwardingTest(t)
 
-       defer func() {
-               os.Remove("libgo2.a")
-               os.Remove("libgo2.h")
-               os.Remove("testp")
-               os.RemoveAll(filepath.Join(GOPATH, "pkg"))
-       }()
+       if !testWork {
+               defer func() {
+                       os.Remove("libgo2.a")
+                       os.Remove("libgo2.h")
+                       os.Remove("testp")
+                       os.RemoveAll(filepath.Join(GOPATH, "pkg"))
+               }()
+       }
 
        cmd := exec.Command("go", "build", "-buildmode=c-archive", "-o", "libgo2.a", "./libgo2")
        if out, err := cmd.CombinedOutput(); err != nil {
@@ -345,12 +402,14 @@ func TestSignalForwardingExternal(t *testing.T) {
        }
        checkSignalForwardingTest(t)
 
-       defer func() {
-               os.Remove("libgo2.a")
-               os.Remove("libgo2.h")
-               os.Remove("testp")
-               os.RemoveAll(filepath.Join(GOPATH, "pkg"))
-       }()
+       if !testWork {
+               defer func() {
+                       os.Remove("libgo2.a")
+                       os.Remove("libgo2.h")
+                       os.Remove("testp")
+                       os.RemoveAll(filepath.Join(GOPATH, "pkg"))
+               }()
+       }
 
        cmd := exec.Command("go", "build", "-buildmode=c-archive", "-o", "libgo2.a", "./libgo2")
        if out, err := cmd.CombinedOutput(); err != nil {
@@ -460,12 +519,14 @@ func TestOsSignal(t *testing.T) {
                t.Skip("skipping signal test on Windows")
        }
 
-       defer func() {
-               os.Remove("libgo3.a")
-               os.Remove("libgo3.h")
-               os.Remove("testp")
-               os.RemoveAll(filepath.Join(GOPATH, "pkg"))
-       }()
+       if !testWork {
+               defer func() {
+                       os.Remove("libgo3.a")
+                       os.Remove("libgo3.h")
+                       os.Remove("testp")
+                       os.RemoveAll(filepath.Join(GOPATH, "pkg"))
+               }()
+       }
 
        cmd := exec.Command("go", "build", "-buildmode=c-archive", "-o", "libgo3.a", "./libgo3")
        if out, err := cmd.CombinedOutput(); err != nil {
@@ -495,12 +556,14 @@ func TestSigaltstack(t *testing.T) {
                t.Skip("skipping signal test on Windows")
        }
 
-       defer func() {
-               os.Remove("libgo4.a")
-               os.Remove("libgo4.h")
-               os.Remove("testp")
-               os.RemoveAll(filepath.Join(GOPATH, "pkg"))
-       }()
+       if !testWork {
+               defer func() {
+                       os.Remove("libgo4.a")
+                       os.Remove("libgo4.h")
+                       os.Remove("testp")
+                       os.RemoveAll(filepath.Join(GOPATH, "pkg"))
+               }()
+       }
 
        cmd := exec.Command("go", "build", "-buildmode=c-archive", "-o", "libgo4.a", "./libgo4")
        if out, err := cmd.CombinedOutput(); err != nil {
@@ -544,13 +607,15 @@ func TestExtar(t *testing.T) {
                t.Skip("shell scripts are not executable on iOS hosts")
        }
 
-       defer func() {
-               os.Remove("libgo4.a")
-               os.Remove("libgo4.h")
-               os.Remove("testar")
-               os.Remove("testar.ran")
-               os.RemoveAll(filepath.Join(GOPATH, "pkg"))
-       }()
+       if !testWork {
+               defer func() {
+                       os.Remove("libgo4.a")
+                       os.Remove("libgo4.h")
+                       os.Remove("testar")
+                       os.Remove("testar.ran")
+                       os.RemoveAll(filepath.Join(GOPATH, "pkg"))
+               }()
+       }
 
        os.Remove("testar")
        dir, err := os.Getwd()
@@ -584,12 +649,22 @@ func TestPIE(t *testing.T) {
                t.Skipf("skipping PIE test on %s", GOOS)
        }
 
-       defer func() {
-               os.Remove("testp" + exeSuffix)
-               os.RemoveAll(filepath.Join(GOPATH, "pkg"))
-       }()
+       if !testWork {
+               defer func() {
+                       os.Remove("testp" + exeSuffix)
+                       os.RemoveAll(filepath.Join(GOPATH, "pkg"))
+               }()
+       }
+
+       // Generate the p.h header file.
+       //
+       // 'go install -i -buildmode=c-archive ./libgo' would do that too, but that
+       // would also attempt to install transitive standard-library dependencies to
+       // GOROOT, and we cannot assume that GOROOT is writable. (A non-root user may
+       // be running this test in a GOROOT owned by root.)
+       genHeader(t, "p.h", "./p")
 
-       cmd := exec.Command("go", "install", "-i", "-buildmode=c-archive", "./libgo")
+       cmd := exec.Command("go", "install", "-buildmode=c-archive", "./libgo")
        if out, err := cmd.CombinedOutput(); err != nil {
                t.Logf("%s", out)
                t.Fatal(err)
@@ -669,11 +744,13 @@ func TestSIGPROF(t *testing.T) {
 
        t.Parallel()
 
-       defer func() {
-               os.Remove("testp6" + exeSuffix)
-               os.Remove("libgo6.a")
-               os.Remove("libgo6.h")
-       }()
+       if !testWork {
+               defer func() {
+                       os.Remove("testp6" + exeSuffix)
+                       os.Remove("libgo6.a")
+                       os.Remove("libgo6.h")
+               }()
+       }
 
        cmd := exec.Command("go", "build", "-buildmode=c-archive", "-o", "libgo6.a", "./libgo6")
        if out, err := cmd.CombinedOutput(); err != nil {
@@ -709,10 +786,12 @@ func TestCompileWithoutShared(t *testing.T) {
        // For simplicity, reuse the signal forwarding test.
        checkSignalForwardingTest(t)
 
-       defer func() {
-               os.Remove("libgo2.a")
-               os.Remove("libgo2.h")
-       }()
+       if !testWork {
+               defer func() {
+                       os.Remove("libgo2.a")
+                       os.Remove("libgo2.h")
+               }()
+       }
 
        cmd := exec.Command("go", "build", "-buildmode=c-archive", "-gcflags=-shared=false", "-o", "libgo2.a", "./libgo2")
        t.Log(cmd.Args)
@@ -751,7 +830,9 @@ func TestCompileWithoutShared(t *testing.T) {
        if err != nil {
                t.Fatal(err)
        }
-       defer os.Remove(exe)
+       if !testWork {
+               defer os.Remove(exe)
+       }
 
        binArgs := append(cmdToRun(exe), "1")
        t.Log(binArgs)
@@ -769,14 +850,15 @@ func TestCompileWithoutShared(t *testing.T) {
        }
 }
 
-// Test that installing a second time recreates the header files.
+// Test that installing a second time recreates the header file.
 func TestCachedInstall(t *testing.T) {
-       defer os.RemoveAll(filepath.Join(GOPATH, "pkg"))
+       if !testWork {
+               defer os.RemoveAll(filepath.Join(GOPATH, "pkg"))
+       }
 
-       h1 := filepath.Join(libgodir, "libgo.h")
-       h2 := filepath.Join(libgodir, "p.h")
+       h := filepath.Join(libgodir, "libgo.h")
 
-       buildcmd := []string{"go", "install", "-i", "-buildmode=c-archive", "./libgo"}
+       buildcmd := []string{"go", "install", "-buildmode=c-archive", "./libgo"}
 
        cmd := exec.Command(buildcmd[0], buildcmd[1:]...)
        t.Log(buildcmd)
@@ -785,17 +867,11 @@ func TestCachedInstall(t *testing.T) {
                t.Fatal(err)
        }
 
-       if _, err := os.Stat(h1); err != nil {
+       if _, err := os.Stat(h); err != nil {
                t.Errorf("libgo.h not installed: %v", err)
        }
-       if _, err := os.Stat(h2); err != nil {
-               t.Errorf("p.h not installed: %v", err)
-       }
 
-       if err := os.Remove(h1); err != nil {
-               t.Fatal(err)
-       }
-       if err := os.Remove(h2); err != nil {
+       if err := os.Remove(h); err != nil {
                t.Fatal(err)
        }
 
@@ -806,23 +882,22 @@ func TestCachedInstall(t *testing.T) {
                t.Fatal(err)
        }
 
-       if _, err := os.Stat(h1); err != nil {
+       if _, err := os.Stat(h); err != nil {
                t.Errorf("libgo.h not installed in second run: %v", err)
        }
-       if _, err := os.Stat(h2); err != nil {
-               t.Errorf("p.h not installed in second run: %v", err)
-       }
 }
 
 // Issue 35294.
 func TestManyCalls(t *testing.T) {
        t.Parallel()
 
-       defer func() {
-               os.Remove("testp7" + exeSuffix)
-               os.Remove("libgo7.a")
-               os.Remove("libgo7.h")
-       }()
+       if !testWork {
+               defer func() {
+                       os.Remove("testp7" + exeSuffix)
+                       os.Remove("libgo7.a")
+                       os.Remove("libgo7.h")
+               }()
+       }
 
        cmd := exec.Command("go", "build", "-buildmode=c-archive", "-o", "libgo7.a", "./libgo7")
        if out, err := cmd.CombinedOutput(); err != nil {