]> Cypherpunks repositories - gostls13.git/commitdiff
misc/cgo/testcshared: avoid writing to GOROOT in tests
authorBryan C. Mills <bcmills@google.com>
Wed, 20 Nov 2019 19:39:19 +0000 (14:39 -0500)
committerBryan C. Mills <bcmills@google.com>
Fri, 22 Nov 2019 15:34:14 +0000 (15:34 +0000)
The tests in this package invoked 'go install -i -buildmode=c-shared'
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_testcshared_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: I622426a860828020d98f7040636f374e5c766d28
Reviewed-on: https://go-review.googlesource.com/c/go/+/208119
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/testcshared/cshared_test.go

index 194dec96ad762bc3402b9205869702f42bd6e04b..cb95153abf603ee38adff4bea749107b9265ddca 100644 (file)
@@ -130,8 +130,6 @@ func testMain(m *testing.M) int {
        defer os.RemoveAll(GOPATH)
        os.Setenv("GOPATH", GOPATH)
 
-       // Copy testdata into GOPATH/src/testarchive, along with a go.mod file
-       // declaring the same path.
        modRoot := filepath.Join(GOPATH, "src", "testcshared")
        if err := overlayDir(modRoot, "testdata"); err != nil {
                log.Panic(err)
@@ -257,14 +255,38 @@ func runCC(t *testing.T, args ...string) string {
 }
 
 func createHeaders() error {
-       args := []string{"go", "install", "-i", "-buildmode=c-shared",
-               "-installsuffix", "testcshared", "./libgo"}
+       // 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("", "testcshared_obj")
+       if err != nil {
+               return err
+       }
+       defer os.RemoveAll(objDir)
+
+       // Generate a C header file for p, which is a non-main dependency
+       // of main package libgo.
+       //
+       // TODO(golang.org/issue/35715): This should be simpler.
+       args := []string{"go", "tool", "cgo",
+               "-objdir", objDir,
+               "-exportheader", "p.h",
+               filepath.Join(".", "p", "p.go")}
        cmd := exec.Command(args[0], args[1:]...)
        out, err := cmd.CombinedOutput()
        if err != nil {
                return fmt.Errorf("command failed: %v\n%v\n%s\n", args, err, out)
        }
 
+       // Generate a C header file for libgo itself.
+       args = []string{"go", "install", "-buildmode=c-shared",
+               "-installsuffix", "testcshared", "./libgo"}
+       cmd = exec.Command(args[0], args[1:]...)
+       out, err = cmd.CombinedOutput()
+       if err != nil {
+               return fmt.Errorf("command failed: %v\n%v\n%s\n", args, err, out)
+       }
+
        args = []string{"go", "build", "-buildmode=c-shared",
                "-installsuffix", "testcshared",
                "-o", libgoname,
@@ -522,7 +544,7 @@ func TestPIE(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) {
        tmpdir, err := ioutil.TempDir("", "cshared")
        if err != nil {
@@ -536,7 +558,7 @@ func TestCachedInstall(t *testing.T) {
 
        env := append(os.Environ(), "GOPATH="+tmpdir, "GOBIN="+filepath.Join(tmpdir, "bin"))
 
-       buildcmd := []string{"go", "install", "-x", "-i", "-buildmode=c-shared", "-installsuffix", "testcshared", "./libgo"}
+       buildcmd := []string{"go", "install", "-x", "-buildmode=c-shared", "-installsuffix", "testcshared", "./libgo"}
 
        cmd := exec.Command(buildcmd[0], buildcmd[1:]...)
        cmd.Dir = filepath.Join(tmpdir, "src", "testcshared")
@@ -577,16 +599,10 @@ func TestCachedInstall(t *testing.T) {
        if libgoh == "" {
                t.Fatal("libgo.h not installed")
        }
-       if ph == "" {
-               t.Fatal("p.h not installed")
-       }
 
        if err := os.Remove(libgoh); err != nil {
                t.Fatal(err)
        }
-       if err := os.Remove(ph); err != nil {
-               t.Fatal(err)
-       }
 
        cmd = exec.Command(buildcmd[0], buildcmd[1:]...)
        cmd.Dir = filepath.Join(tmpdir, "src", "testcshared")
@@ -601,9 +617,6 @@ func TestCachedInstall(t *testing.T) {
        if _, err := os.Stat(libgoh); err != nil {
                t.Errorf("libgo.h not installed in second run: %v", err)
        }
-       if _, err := os.Stat(ph); err != nil {
-               t.Errorf("p.h not installed in second run: %v", err)
-       }
 }
 
 // copyFile copies src to dst.