From c02f3b86b482c9ae794694d46bc797f2728df578 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Wed, 20 Nov 2019 10:19:43 -0500 Subject: [PATCH] misc/cgo/testcarchive: avoid writing to GOROOT in tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 TryBot-Result: Gobot Gobot Reviewed-by: Ian Lance Taylor --- misc/cgo/testcarchive/carchive_test.go | 239 ++++++++++++++++--------- 1 file changed, 157 insertions(+), 82 deletions(-) diff --git a/misc/cgo/testcarchive/carchive_test.go b/misc/cgo/testcarchive/carchive_test.go index cf2c6264dd1..82a1a5a54ce 100644 --- a/misc/cgo/testcarchive/carchive_test.go +++ b/misc/cgo/testcarchive/carchive_test.go @@ -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")) + }() + } - cmd := exec.Command("go", "install", "-i", "-buildmode=c-archive", "./libgo") + // 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", "-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 {