Skip to content

Commit 3922c00

Browse files
author
Bryan C. Mills
committed
misc/cgo/testcshared: avoid writing to GOROOT in tests
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 <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
1 parent c02f3b8 commit 3922c00

File tree

1 file changed

+28
-15
lines changed

1 file changed

+28
-15
lines changed

misc/cgo/testcshared/cshared_test.go

+28-15
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,6 @@ func testMain(m *testing.M) int {
130130
defer os.RemoveAll(GOPATH)
131131
os.Setenv("GOPATH", GOPATH)
132132

133-
// Copy testdata into GOPATH/src/testarchive, along with a go.mod file
134-
// declaring the same path.
135133
modRoot := filepath.Join(GOPATH, "src", "testcshared")
136134
if err := overlayDir(modRoot, "testdata"); err != nil {
137135
log.Panic(err)
@@ -257,14 +255,38 @@ func runCC(t *testing.T, args ...string) string {
257255
}
258256

259257
func createHeaders() error {
260-
args := []string{"go", "install", "-i", "-buildmode=c-shared",
261-
"-installsuffix", "testcshared", "./libgo"}
258+
// The 'cgo' command generates a number of additional artifacts,
259+
// but we're only interested in the header.
260+
// Shunt the rest of the outputs to a temporary directory.
261+
objDir, err := ioutil.TempDir("", "testcshared_obj")
262+
if err != nil {
263+
return err
264+
}
265+
defer os.RemoveAll(objDir)
266+
267+
// Generate a C header file for p, which is a non-main dependency
268+
// of main package libgo.
269+
//
270+
// TODO(golang.org/issue/35715): This should be simpler.
271+
args := []string{"go", "tool", "cgo",
272+
"-objdir", objDir,
273+
"-exportheader", "p.h",
274+
filepath.Join(".", "p", "p.go")}
262275
cmd := exec.Command(args[0], args[1:]...)
263276
out, err := cmd.CombinedOutput()
264277
if err != nil {
265278
return fmt.Errorf("command failed: %v\n%v\n%s\n", args, err, out)
266279
}
267280

281+
// Generate a C header file for libgo itself.
282+
args = []string{"go", "install", "-buildmode=c-shared",
283+
"-installsuffix", "testcshared", "./libgo"}
284+
cmd = exec.Command(args[0], args[1:]...)
285+
out, err = cmd.CombinedOutput()
286+
if err != nil {
287+
return fmt.Errorf("command failed: %v\n%v\n%s\n", args, err, out)
288+
}
289+
268290
args = []string{"go", "build", "-buildmode=c-shared",
269291
"-installsuffix", "testcshared",
270292
"-o", libgoname,
@@ -522,7 +544,7 @@ func TestPIE(t *testing.T) {
522544
}
523545
}
524546

525-
// Test that installing a second time recreates the header files.
547+
// Test that installing a second time recreates the header file.
526548
func TestCachedInstall(t *testing.T) {
527549
tmpdir, err := ioutil.TempDir("", "cshared")
528550
if err != nil {
@@ -536,7 +558,7 @@ func TestCachedInstall(t *testing.T) {
536558

537559
env := append(os.Environ(), "GOPATH="+tmpdir, "GOBIN="+filepath.Join(tmpdir, "bin"))
538560

539-
buildcmd := []string{"go", "install", "-x", "-i", "-buildmode=c-shared", "-installsuffix", "testcshared", "./libgo"}
561+
buildcmd := []string{"go", "install", "-x", "-buildmode=c-shared", "-installsuffix", "testcshared", "./libgo"}
540562

541563
cmd := exec.Command(buildcmd[0], buildcmd[1:]...)
542564
cmd.Dir = filepath.Join(tmpdir, "src", "testcshared")
@@ -577,16 +599,10 @@ func TestCachedInstall(t *testing.T) {
577599
if libgoh == "" {
578600
t.Fatal("libgo.h not installed")
579601
}
580-
if ph == "" {
581-
t.Fatal("p.h not installed")
582-
}
583602

584603
if err := os.Remove(libgoh); err != nil {
585604
t.Fatal(err)
586605
}
587-
if err := os.Remove(ph); err != nil {
588-
t.Fatal(err)
589-
}
590606

591607
cmd = exec.Command(buildcmd[0], buildcmd[1:]...)
592608
cmd.Dir = filepath.Join(tmpdir, "src", "testcshared")
@@ -601,9 +617,6 @@ func TestCachedInstall(t *testing.T) {
601617
if _, err := os.Stat(libgoh); err != nil {
602618
t.Errorf("libgo.h not installed in second run: %v", err)
603619
}
604-
if _, err := os.Stat(ph); err != nil {
605-
t.Errorf("p.h not installed in second run: %v", err)
606-
}
607620
}
608621

609622
// copyFile copies src to dst.

0 commit comments

Comments
 (0)