From 333d2010ec98aaea244b65b7bc4d7d80c71e21b1 Mon Sep 17 00:00:00 2001 From: Dmitri Shuralyov Date: Sun, 1 Nov 2020 06:54:21 +0000 Subject: [PATCH] cmd/go: revert "support cgo files in overlays" This reverts CL 262618 (commit 48be3ed1394d85af5a7e1a6313fa9cab4d1b7cf9). Reason for revert: breaks longtest builders. Change-Id: Iec1e236ba793f24394442d04eb846f8a73ab2e68 Reviewed-on: https://go-review.googlesource.com/c/go/+/267037 Trust: Dmitri Shuralyov Trust: Alberto Donizetti Run-TryBot: Dmitri Shuralyov TryBot-Result: Go Bot Reviewed-by: Alberto Donizetti --- src/cmd/go/internal/work/exec.go | 48 ++--------- src/cmd/go/internal/work/gc.go | 9 +- src/cmd/go/testdata/script/build_overlay.txt | 83 +------------------ .../go/testdata/script/build_trimpath_cgo.txt | 28 ------- 4 files changed, 12 insertions(+), 156 deletions(-) diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go index a1a357e2ac..838b00a00d 100644 --- a/src/cmd/go/internal/work/exec.go +++ b/src/cmd/go/internal/work/exec.go @@ -8,7 +8,6 @@ package work import ( "bytes" - "cmd/go/internal/fsys" "context" "encoding/json" "errors" @@ -2243,6 +2242,8 @@ func (b *Builder) ccompile(a *Action, p *load.Package, outfile string, flags []s // when -trimpath is enabled. if b.gccSupportsFlag(compiler, "-fdebug-prefix-map=a=b") { if cfg.BuildTrimpath { + // TODO(#39958): handle overlays + // Keep in sync with Action.trimpath. // The trimmed paths are a little different, but we need to trim in the // same situations. @@ -2312,8 +2313,7 @@ func (b *Builder) gccld(a *Action, p *load.Package, objdir, outfile string, flag cmdargs := []interface{}{cmd, "-o", outfile, objs, flags} dir := p.Dir - out, err := b.runOut(a, base.Cwd, b.cCompilerEnv(), cmdargs...) - + out, err := b.runOut(a, dir, b.cCompilerEnv(), cmdargs...) if len(out) > 0 { // Filter out useless linker warnings caused by bugs outside Go. // See also cmd/link/internal/ld's hostlink method. @@ -2641,8 +2641,7 @@ func (b *Builder) cgo(a *Action, cgoExe, objdir string, pcCFLAGS, pcLDFLAGS, cgo cgoLDFLAGS = append([]string{"-fsanitize=memory"}, cgoLDFLAGS...) } - // Allows including _cgo_export.h, as well as the user's .h files, - // from .[ch] files in the package. + // Allows including _cgo_export.h from .[ch] files in the package. cgoCPPFLAGS = append(cgoCPPFLAGS, "-I", objdir) // cgo @@ -2655,8 +2654,6 @@ func (b *Builder) cgo(a *Action, cgoExe, objdir string, pcCFLAGS, pcLDFLAGS, cgo cfiles = append(cfiles, f+".cgo2.c") } - hfiles := append([]string{}, p.HFiles...) - // TODO: make cgo not depend on $GOARCH? cgoflags := []string{} @@ -2701,38 +2698,7 @@ func (b *Builder) cgo(a *Action, cgoExe, objdir string, pcCFLAGS, pcLDFLAGS, cgo cgoflags = append(cgoflags, "-exportheader="+objdir+"_cgo_install.h") } - execdir := p.Dir - - // If any of the Cgo, C, or H files are overlaid, copy them all to - // objdir to ensure that they refer to the right header files. - // TODO(#39958): Ideally, we'd always do this, but this could - // subtly break some cgo files that include .h files across directory - // boundaries, even though they shouldn't. - hasOverlay := false - cgoFileLists := [][]string{cgofiles, gccfiles, gxxfiles, mfiles, ffiles, hfiles} -OverlayLoop: - for _, fs := range cgoFileLists { - for _, f := range fs { - if _, ok := fsys.OverlayPath(mkAbs(p.Dir, f)); ok { - hasOverlay = true - break OverlayLoop - } - } - } - if hasOverlay { - execdir = objdir - for _, fs := range cgoFileLists { - for i := range fs { - opath, _ := fsys.OverlayPath(mkAbs(p.Dir, fs[i])) - fs[i] = objdir + filepath.Base(fs[i]) - if err := b.copyFile(fs[i], opath, 0666, false); err != nil { - return nil, nil, err - } - } - } - } - - if err := b.run(a, execdir, p.ImportPath, cgoenv, cfg.BuildToolexec, cgoExe, "-objdir", objdir, "-importpath", p.ImportPath, cgoflags, "--", cgoCPPFLAGS, cgoCFLAGS, cgofiles); err != nil { + if err := b.run(a, p.Dir, p.ImportPath, cgoenv, cfg.BuildToolexec, cgoExe, "-objdir", objdir, "-importpath", p.ImportPath, cgoflags, "--", cgoCPPFLAGS, cgoCFLAGS, cgofiles); err != nil { return nil, nil, err } outGo = append(outGo, gofiles...) @@ -2826,7 +2792,7 @@ func (b *Builder) dynimport(a *Action, p *load.Package, objdir, importGo, cgoExe return err } - linkobj := str.StringList(ofile, outObj, mkAbsFiles(p.Dir, p.SysoFiles)) + linkobj := str.StringList(ofile, outObj, p.SysoFiles) dynobj := objdir + "_cgo_.o" // we need to use -pie for Linux/ARM to get accurate imported sym @@ -2851,7 +2817,7 @@ func (b *Builder) dynimport(a *Action, p *load.Package, objdir, importGo, cgoExe if p.Standard && p.ImportPath == "runtime/cgo" { cgoflags = []string{"-dynlinker"} // record path to dynamic linker } - return b.run(a, base.Cwd, p.ImportPath, b.cCompilerEnv(), cfg.BuildToolexec, cgoExe, "-dynpackage", p.Name, "-dynimport", dynobj, "-dynout", importGo, cgoflags) + return b.run(a, p.Dir, p.ImportPath, b.cCompilerEnv(), cfg.BuildToolexec, cgoExe, "-dynpackage", p.Name, "-dynimport", dynobj, "-dynout", importGo, cgoflags) } // Run SWIG on all SWIG input files. diff --git a/src/cmd/go/internal/work/gc.go b/src/cmd/go/internal/work/gc.go index 4ba9be7829..e79173485d 100644 --- a/src/cmd/go/internal/work/gc.go +++ b/src/cmd/go/internal/work/gc.go @@ -262,7 +262,7 @@ func (a *Action) trimpath() string { if len(objdir) > 1 && objdir[len(objdir)-1] == filepath.Separator { objdir = objdir[:len(objdir)-1] } - rewrite := "" + rewrite := objdir + "=>" rewriteDir := a.Package.Dir if cfg.BuildTrimpath { @@ -271,7 +271,7 @@ func (a *Action) trimpath() string { } else { rewriteDir = a.Package.ImportPath } - rewrite += a.Package.Dir + "=>" + rewriteDir + ";" + rewrite += ";" + a.Package.Dir + "=>" + rewriteDir } // Add rewrites for overlays. The 'from' and 'to' paths in overlays don't need to have @@ -280,14 +280,11 @@ func (a *Action) trimpath() string { if fsys.OverlayFile != "" { for _, filename := range a.Package.AllFiles() { overlayPath, ok := fsys.OverlayPath(filepath.Join(a.Package.Dir, filename)) - rewrite += filepath.Join(objdir, filename) + "=>" + filepath.Join(rewriteDir, filename) + ";" if !ok { continue } - rewrite += overlayPath + "=>" + filepath.Join(rewriteDir, filename) + ";" + rewrite += ";" + overlayPath + "=>" + filepath.Join(rewriteDir, filename) } - } else { - rewrite += objdir + "=>" } return rewrite diff --git a/src/cmd/go/testdata/script/build_overlay.txt b/src/cmd/go/testdata/script/build_overlay.txt index 3af10083af..0602e706e9 100644 --- a/src/cmd/go/testdata/script/build_overlay.txt +++ b/src/cmd/go/testdata/script/build_overlay.txt @@ -1,11 +1,9 @@ [short] skip # Test building in overlays. -# TODO(#39958): add a test case where the destination file in the replace map +# TODO(matloob): add a test case where the destination file in the replace map # isn't a go file. Either completely exclude that case in fs.IsDirWithGoFiles # if the compiler doesn't allow it, or test that it works all the way. -# TODO(#39958): add a test that both gc and gccgo assembly files can include .h -# files. # The main package (m) is contained in an overlay. It imports m/dir2 which has one # file in an overlay and one file outside the overlay, which in turn imports m/dir, @@ -31,18 +29,6 @@ exec ./print_trimpath_two_files$GOEXE stdout $WORK[/\\]gopath[/\\]src[/\\]m[/\\]printpath[/\\]main.go stdout $WORK[/\\]gopath[/\\]src[/\\]m[/\\]printpath[/\\]other.go -go build -overlay overlay.json -o main_cgo_replace$GOEXE ./cgo_hello_replace -exec ./main_cgo_replace$GOEXE -stdout '^hello cgo$' - -go build -overlay overlay.json -o main_cgo_quote$GOEXE ./cgo_hello_quote -exec ./main_cgo_quote$GOEXE -stdout '^hello cgo$' - -go build -overlay overlay.json -o main_cgo_angle$GOEXE ./cgo_hello_angle -exec ./main_cgo_angle$GOEXE -stdout '^hello cgo$' - # Run same tests but with gccgo. env GO111MODULE=off [!exec:gccgo] stop @@ -60,19 +46,6 @@ go build -compiler=gccgo -overlay overlay.json -o print_trimpath_gccgo$GOEXE -tr exec ./print_trimpath_gccgo$GOEXE stdout ^\.[/\\]printpath[/\\]main.go - -go build -compiler=gccgo -overlay overlay.json -o main_cgo_replace_gccgo$GOEXE ./cgo_hello_replace -exec ./main_cgo_replace_gccgo$GOEXE -stdout '^hello cgo$' - -go build -compiler=gccgo -overlay overlay.json -o main_cgo_quote_gccgo$GOEXE ./cgo_hello_quote -exec ./main_cgo_quote_gccgo$GOEXE -stdout '^hello cgo$' - -go build -compiler=gccgo -overlay overlay.json -o main_cgo_angle_gccgo$GOEXE ./cgo_hello_angle -exec ./main_cgo_angle_gccgo$GOEXE -stdout '^hello cgo$' - -- m/go.mod -- // TODO(matloob): how do overlays work with go.mod (especially if mod=readonly) module m @@ -98,32 +71,9 @@ the actual code is in the overlay "dir/g.go": "overlay/dir_g.go", "dir2/i.go": "overlay/dir2_i.go", "printpath/main.go": "overlay/printpath.go", - "printpath/other.go": "overlay2/printpath2.go", - "cgo_hello_replace/cgo_header.h": "overlay/cgo_head.h", - "cgo_hello_quote/cgo_hello.go": "overlay/cgo_hello_quote.go", - "cgo_hello_quote/cgo_header.h": "overlay/cgo_head.h", - "cgo_hello_angle/cgo_hello.go": "overlay/cgo_hello_angle.go", - "cgo_hello_angle/cgo_header.h": "overlay/cgo_head.h" + "printpath/other.go": "overlay2/printpath2.go" } } --- m/cgo_hello_replace/cgo_hello_replace.go -- -package main - -// #include "cgo_header.h" -import "C" - -func main() { - C.say_hello() -} --- m/cgo_hello_replace/cgo_header.h -- - // Test that this header is replaced with one that has the proper declaration. -void say_goodbye(); - --- m/cgo_hello_replace/goodbye.c -- -#include - -void say_hello() { puts("hello cgo\n"); } - -- m/overlay/f.go -- package main @@ -178,32 +128,3 @@ import "m/dir" func printMessage() { dir.PrintMessage() } --- m/overlay/cgo_hello_quote.go -- -package main - -// #include "cgo_header.h" -import "C" - -func main() { - C.say_hello() -} --- m/overlay/cgo_hello_angle.go -- -package main - -// #include -import "C" - -func main() { - C.say_hello() -} --- m/overlay/cgo_head.h -- -void say_hello(); --- m/cgo_hello_quote/hello.c -- -#include - -void say_hello() { puts("hello cgo\n"); } --- m/cgo_hello_angle/hello.c -- -#include - -void say_hello() { puts("hello cgo\n"); } - diff --git a/src/cmd/go/testdata/script/build_trimpath_cgo.txt b/src/cmd/go/testdata/script/build_trimpath_cgo.txt index 3187b4d643..4608d9ac6b 100644 --- a/src/cmd/go/testdata/script/build_trimpath_cgo.txt +++ b/src/cmd/go/testdata/script/build_trimpath_cgo.txt @@ -20,38 +20,10 @@ go build -trimpath -o hello.exe . go run ./list-dwarf hello.exe ! stdout gopath/src - -# Do the above, with the cgo (but not .c) sources in an overlay -# Check that the source path appears when -trimpath is not used. -mkdir $WORK/overlay -cp hello.go $WORK/overlay/hello.go -mkdir hello_overlay -cp hello.c hello_overlay/hello.c -go build -overlay overlay.json -o hello_overlay.exe ./hello_overlay -grep -q gopath[/\\]src hello_overlay.exe -! grep -q $WORK[/\\]overlay hello_overlay.exe -go run ./list-dwarf hello_overlay.exe -stdout gopath[/\\]src -! stdout $WORK[/\\]overlay - -# Check that the source path does not appear when -trimpath is used. -go build -overlay overlay.json -trimpath -o hello_overlay.exe ./hello_overlay -! grep -q gopath[/\\]src hello_overlay.exe -! grep -q $WORK[/\\]overlay hello_overlay.exe -go run ./list-dwarf hello_overlay.exe -! stdout gopath/src -! stdout $WORK[/\\]overlay - -- go.mod -- module m go 1.14 --- overlay.json -- -{ - "Replace": { - "hello_overlay/hello.go": "../../overlay/hello.go" - } -} -- hello.c -- #include -- 2.48.1