From 9a9780a20d6987d462d97fa191de3c4a66980022 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Mon, 9 Oct 2017 15:19:46 -0400 Subject: [PATCH] cmd/go: separate compile and link steps in action graph To the extent that invoking the compiler and invoking the linker have different dependency requirements, representing both steps by a single action node leads to confusion. If we move to having separate .a and .x (import metadata) files in the standard builds, then the .a is a link dependency but not a compile dependency, and vice versa for .x. Today, in shared library builds, the .a is a compile dependency and a link dependency, while the .so is only a link dependency. Also in this CL: change the gccgo link step to extract _cgo_flags into root.Objdir, which is private to the link step, instead of into b.WorkDir, which is shared by all the link steps that could possibly be running in parallel. And attempt to handle the -n and -x flags when loading _cgo_flags, instead of dying attempting to read an archive that wasn't written. Also in this CL: create a.Objdir before running a.Func, to avoid duplicating the Mkdir(a.Objdir) in every a.Func. A future CL will update the link action's Deps to be accurate. (Right now the link steps search out the true Deps by walking the entire action graph.) Change-Id: I15128ce2bd064887f98abc3a4cf204241f518631 Reviewed-on: https://go-review.googlesource.com/69830 Run-TryBot: Russ Cox TryBot-Result: Gobot Gobot Reviewed-by: David Crawshaw --- src/cmd/go/go_test.go | 4 +- src/cmd/go/internal/test/test.go | 5 -- src/cmd/go/internal/work/build.go | 122 ++++++++++++++++-------------- 3 files changed, 67 insertions(+), 64 deletions(-) diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index 53361bb5d7..ae8fd67df6 100644 --- a/src/cmd/go/go_test.go +++ b/src/cmd/go/go_test.go @@ -729,10 +729,10 @@ func TestBuildComplex(t *testing.T) { defer tg.cleanup() tg.parallel() tg.setenv("GOPATH", filepath.Join(tg.pwd(), "testdata")) - tg.run("build", "-o", os.DevNull, "complex") + tg.run("build", "-x", "-o", os.DevNull, "complex") if _, err := exec.LookPath("gccgo"); err == nil { - tg.run("build", "-o", os.DevNull, "-compiler=gccgo", "complex") + tg.run("build", "-x", "-o", os.DevNull, "-compiler=gccgo", "complex") } } diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go index 0d21194287..719a155fe8 100644 --- a/src/cmd/go/internal/test/test.go +++ b/src/cmd/go/internal/test/test.go @@ -896,11 +896,6 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin load.ComputeStale(pmain) - if ptest != p { - a := b.Action(work.ModeBuild, work.ModeBuild, ptest) - a.Link = false - } - a := b.Action(work.ModeBuild, work.ModeBuild, pmain) a.Target = testDir + testBinary + cfg.ExeSuffix if cfg.Goos == "windows" { diff --git a/src/cmd/go/internal/work/build.go b/src/cmd/go/internal/work/build.go index 13bbabf65b..ead79f381c 100644 --- a/src/cmd/go/internal/work/build.go +++ b/src/cmd/go/internal/work/build.go @@ -691,7 +691,6 @@ type Action struct { triggers []*Action // inverse of deps // Generated files, directories. - Link bool // target is executable, not just package Objdir string // directory for intermediate objects Target string // goal of the action: the created package or executable @@ -749,7 +748,6 @@ func actionGraphJSON(a *Action) string { ID: id, IgnoreFail: a.IgnoreFail, Args: a.Args, - Link: a.Link, Objdir: a.Objdir, Target: a.Target, Failed: a.Failed, @@ -955,7 +953,7 @@ func (b *Builder) action1(mode BuildMode, depMode BuildMode, p *load.Package, lo mode = ModeBuild } a.Objdir = b.NewObjdir() - a.Link = p.Name == "main" && !p.Internal.ForceLibrary + link := p.Name == "main" && !p.Internal.ForceLibrary switch mode { case ModeInstall: @@ -989,7 +987,16 @@ func (b *Builder) action1(mode BuildMode, depMode BuildMode, p *load.Package, lo a.Func = (*Builder).build a.Target = a.Objdir + "_pkg_.a" a.Package.Internal.Pkgfile = a.Target - if a.Link { + + if link { + a = &Action{ + Mode: "link", + Func: (*Builder).link, + Package: a.Package, + Objdir: a.Objdir, + Deps: []*Action{a}, + } + // An executable file. (This is the name of a temporary file.) // Because we run the temporary file in 'go run' and 'go test', // the name will show up in ps listings. If the caller has specified @@ -1018,7 +1025,10 @@ func (b *Builder) action1(mode BuildMode, depMode BuildMode, p *load.Package, lo } func (b *Builder) libaction(libname string, pkgs []*load.Package, mode, depMode BuildMode) *Action { - a := &Action{Mode: "libaction???"} + a := &Action{ + Mode: "libaction", // should be overwritten below + Objdir: b.NewObjdir(), + } switch mode { default: base.Fatalf("unrecognized mode %v", mode) @@ -1201,8 +1211,14 @@ func (b *Builder) Do(root *Action) { // any actions that are runnable as a result. handle := func(a *Action) { var err error + if a.Func != nil && (!a.Failed || a.IgnoreFail) { - err = a.Func(b, a) + if a.Objdir != "" { + err = b.Mkdir(a.Objdir) + } + if err == nil { + err = a.Func(b, a) + } } // The actions run in parallel but all the updates to the @@ -1316,11 +1332,7 @@ func (b *Builder) build(a *Action) (err error) { b.Print(a.Package.ImportPath + "\n") } - // Make build directory. objdir := a.Objdir - if err := b.Mkdir(objdir); err != nil { - return err - } // make target directory dir, _ := filepath.Split(a.Target) @@ -1554,22 +1566,32 @@ func (b *Builder) build(a *Action) (err error) { } } - // Link if needed. - if a.Link { - importcfg := a.Objdir + "importcfg.link" - if err := b.writeLinkImportcfg(a, importcfg); err != nil { - return err - } + return nil +} - // The compiler only cares about direct imports, but the - // linker needs the whole dependency tree. - all := ActionList(a) - all = all[:len(all)-1] // drop a - if err := BuildToolchain.ld(b, a, a.Target, importcfg, all, objpkg, objects); err != nil { +func (b *Builder) link(a *Action) (err error) { + importcfg := a.Objdir + "importcfg.link" + if err := b.writeLinkImportcfg(a, importcfg); err != nil { + return err + } + + // make target directory + dir, _ := filepath.Split(a.Target) + if dir != "" { + if err := b.Mkdir(dir); err != nil { return err } } + // The compiler only cares about direct imports, but the + // linker needs the whole dependency tree. + all := ActionList(a) + all = all[:len(all)-1] // drop a + objpkg := a.Objdir + "_pkg_.a" + if err := BuildToolchain.ld(b, a, a.Target, importcfg, all, objpkg, nil); err != nil { // TODO: ofiles + return err + } + return nil } @@ -1698,7 +1720,7 @@ func BuildInstallFunc(b *Builder, a *Action) (err error) { }() a1 := a.Deps[0] perm := os.FileMode(0666) - if a1.Link { + if a1.Mode == "link" { switch cfg.BuildBuildmode { case "c-archive", "c-shared", "plugin": default: @@ -2859,7 +2881,7 @@ func (gccgoToolchain) pack(b *Builder, a *Action, afile string, ofiles []string) return b.run(p.Dir, p.ImportPath, nil, "ar", "rc", mkAbs(objdir, afile), absOfiles) } -func (tools gccgoToolchain) link(b *Builder, root *Action, out, importcfg string, allactions []*Action, mainpkg string, ofiles []string, buildmode, desc string) error { +func (tools gccgoToolchain) link(b *Builder, root *Action, out, importcfg string, allactions []*Action, buildmode, desc string) error { // gccgo needs explicit linking with all package dependencies, // and all LDFLAGS from cgo dependencies. apackagePathsSeen := make(map[string]bool) @@ -2899,42 +2921,36 @@ func (tools gccgoToolchain) link(b *Builder, root *Action, out, importcfg string return nil } + newID := 0 readAndRemoveCgoFlags := func(archive string) (string, error) { - newa, err := ioutil.TempFile(b.WorkDir, filepath.Base(archive)) - if err != nil { - return "", err - } - olda, err := os.Open(archive) - if err != nil { - return "", err - } - _, err = io.Copy(newa, olda) - if err != nil { + newID++ + newArchive := root.Objdir + fmt.Sprintf("_pkg%d_.a", newID) + if err := b.copyFile(root, newArchive, archive, 0666, false); err != nil { return "", err } - err = olda.Close() - if err != nil { - return "", err - } - err = newa.Close() - if err != nil { - return "", err + if cfg.BuildN || cfg.BuildX { + b.Showcmd("", "ar d %s _cgo_flags", newArchive) + if cfg.BuildN { + // TODO(rsc): We could do better about showing the right _cgo_flags even in -n mode. + // Either the archive is already built and we can read them out, + // or we're printing commands to build the archive and can + // forward the _cgo_flags directly to this step. + return "", nil + } } - - newarchive := newa.Name() - err = b.run(b.WorkDir, desc, nil, "ar", "x", newarchive, "_cgo_flags") + err := b.run(root.Objdir, desc, nil, "ar", "x", newArchive, "_cgo_flags") if err != nil { return "", err } - err = b.run(".", desc, nil, "ar", "d", newarchive, "_cgo_flags") + err = b.run(".", desc, nil, "ar", "d", newArchive, "_cgo_flags") if err != nil { return "", err } - err = readCgoFlags(filepath.Join(b.WorkDir, "_cgo_flags")) + err = readCgoFlags(filepath.Join(root.Objdir, "_cgo_flags")) if err != nil { return "", err } - return newarchive, nil + return newArchive, nil } actionsSeen := make(map[*Action]bool) @@ -3018,14 +3034,6 @@ func (tools gccgoToolchain) link(b *Builder, root *Action, out, importcfg string } } - for i, o := range ofiles { - if filepath.Base(o) == "_cgo_flags" { - readCgoFlags(o) - ofiles = append(ofiles[:i], ofiles[i+1:]...) - break - } - } - ldflags = append(ldflags, "-Wl,--whole-archive") ldflags = append(ldflags, afiles...) ldflags = append(ldflags, "-Wl,--no-whole-archive") @@ -3112,7 +3120,7 @@ func (tools gccgoToolchain) link(b *Builder, root *Action, out, importcfg string } } - if err := b.run(".", desc, nil, tools.linker(), "-o", out, ofiles, ldflags, buildGccgoflags); err != nil { + if err := b.run(".", desc, nil, tools.linker(), "-o", out, ldflags, buildGccgoflags); err != nil { return err } @@ -3126,13 +3134,13 @@ func (tools gccgoToolchain) link(b *Builder, root *Action, out, importcfg string } func (tools gccgoToolchain) ld(b *Builder, root *Action, out, importcfg string, allactions []*Action, mainpkg string, ofiles []string) error { - return tools.link(b, root, out, importcfg, allactions, mainpkg, ofiles, ldBuildmode, root.Package.ImportPath) + return tools.link(b, root, out, importcfg, allactions, ldBuildmode, root.Package.ImportPath) } func (tools gccgoToolchain) ldShared(b *Builder, toplevelactions []*Action, out, importcfg string, allactions []*Action) error { fakeRoot := &Action{Mode: "gccgo ldshared"} fakeRoot.Deps = toplevelactions - return tools.link(b, fakeRoot, out, importcfg, allactions, "", nil, "shared", out) + return tools.link(b, fakeRoot, out, importcfg, allactions, "shared", out) } func (tools gccgoToolchain) cc(b *Builder, a *Action, ofile, cfile string) error { -- 2.48.1