From 555c16d8cbd8372c1f57beb059a23b56bf1e909f Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Thu, 5 Oct 2017 10:07:19 -0400 Subject: [PATCH] cmd/dist, cmd/go: treat cmd/cgo like other build tools The primary build tools cmd/asm, cmd/compile, and cmd/link are built during cmd/dist bootstrap and then assumed by cmd/go to be available for any future builds. The only tool invoked by cmd/go during a build and not in this list is cmd/cgo; instead of being built during cmd/dist and assumed by cmd/go, cmd/go arranges to build cmd/cgo if needed as part of the regular build. We got here because at the time cmd/go was written, cmd/cgo was the only build tool written in Go (the others were in C), and so it made some sense to put cmd/dist in charge of building the C tools and to have custom code in cmd/go to build cmd/cgo just in time for it to be used by a particular build. This custom code has historically been quite subtle, though, because the build of cmd/cgo inherits whatever build flags apply to the build that wants to use cmd/cgo. If you're not careful, "go install -race strings" might under the wrong circumstances also install a race-enabled cmd/cgo binary, which is unexpected at the least. The custom code is only going to get more problematic as we move toward more precise analysis of whether dependencies are up-to-date. In that case, "go build -race strings" will check to see if there is not just a cmd/cgo already but a race-enabled cmd/cgo, which makes no sense. Instead of perpetuating the special case, treat cgo like all the other build tools: build it first in cmd/dist, and then assume it is present. This simplifies cmd/go. Building cmd/cgo during bootstrap also allows the default build of cmd/cgo to be built using cgo, which may be necessary on future essentially-cgo-only systems. Change-Id: I414e22c10c9920f4e98f97fa35ff22058c0f143d Reviewed-on: https://go-review.googlesource.com/68338 Run-TryBot: Russ Cox Reviewed-by: David Crawshaw --- src/cmd/dist/build.go | 2 ++ src/cmd/dist/buildgo.go | 27 ++++++++++++------------ src/cmd/dist/buildtool.go | 9 ++++++++ src/cmd/go/internal/load/pkg.go | 9 ++++++++ src/cmd/go/internal/test/test.go | 4 ---- src/cmd/go/internal/work/build.go | 34 ++++++------------------------- 6 files changed, 39 insertions(+), 46 deletions(-) diff --git a/src/cmd/dist/build.go b/src/cmd/dist/build.go index 89474d7678..c395205c55 100644 --- a/src/cmd/dist/build.go +++ b/src/cmd/dist/build.go @@ -461,6 +461,8 @@ func setup() { // deptab lists changes to the default dependencies for a given prefix. // deps ending in /* read the whole directory; deps beginning with - // exclude files with that prefix. +// Note that this table applies only to the build of cmd/go, +// after the main compiler bootstrap. var deptab = []struct { prefix string // prefix of target dep []string // dependency tweaks for targets with that prefix diff --git a/src/cmd/dist/buildgo.go b/src/cmd/dist/buildgo.go index 60cb24b6b5..19384a1a53 100644 --- a/src/cmd/dist/buildgo.go +++ b/src/cmd/dist/buildgo.go @@ -27,18 +27,20 @@ import ( // It is invoked to write cmd/go/internal/cfg/zdefaultcc.go // but we also write cmd/cgo/zdefaultcc.go func mkzdefaultcc(dir, file string) { + if strings.Contains(file, filepath.FromSlash("go/internal/cfg")) { + var buf bytes.Buffer + fmt.Fprintf(&buf, "// Code generated by go tool dist; DO NOT EDIT.\n") + fmt.Fprintln(&buf) + fmt.Fprintf(&buf, "package cfg\n") + fmt.Fprintln(&buf) + fmt.Fprintf(&buf, "const DefaultCC = `%s`\n", defaultcctarget) + fmt.Fprintf(&buf, "const DefaultCXX = `%s`\n", defaultcxxtarget) + fmt.Fprintf(&buf, "const DefaultPkgConfig = `%s`\n", defaultpkgconfigtarget) + writefile(buf.String(), file, writeSkipSame) + return + } + var buf bytes.Buffer - fmt.Fprintf(&buf, "// Code generated by go tool dist; DO NOT EDIT.\n") - fmt.Fprintln(&buf) - fmt.Fprintf(&buf, "package cfg\n") - fmt.Fprintln(&buf) - fmt.Fprintf(&buf, "const DefaultCC = `%s`\n", defaultcctarget) - fmt.Fprintf(&buf, "const DefaultCXX = `%s`\n", defaultcxxtarget) - fmt.Fprintf(&buf, "const DefaultPkgConfig = `%s`\n", defaultpkgconfigtarget) - - writefile(buf.String(), file, writeSkipSame) - buf.Reset() - fmt.Fprintf(&buf, "// Code generated by go tool dist; DO NOT EDIT.\n") fmt.Fprintln(&buf) fmt.Fprintf(&buf, "package main\n") @@ -46,9 +48,6 @@ func mkzdefaultcc(dir, file string) { fmt.Fprintf(&buf, "const defaultCC = `%s`\n", defaultcctarget) fmt.Fprintf(&buf, "const defaultCXX = `%s`\n", defaultcxxtarget) fmt.Fprintf(&buf, "const defaultPkgConfig = `%s`\n", defaultpkgconfigtarget) - - // Convert file name. - file = strings.Replace(file, filepath.FromSlash("go/internal/cfg"), "cgo", 1) writefile(buf.String(), file, writeSkipSame) } diff --git a/src/cmd/dist/buildtool.go b/src/cmd/dist/buildtool.go index 421ab46a27..c90354c8ff 100644 --- a/src/cmd/dist/buildtool.go +++ b/src/cmd/dist/buildtool.go @@ -35,6 +35,7 @@ var bootstrapDirs = []string{ "cmd/asm/internal/asm", "cmd/asm/internal/flags", "cmd/asm/internal/lex", + "cmd/cgo", "cmd/compile", "cmd/compile/internal/amd64", "cmd/compile/internal/arm", @@ -72,6 +73,9 @@ var bootstrapDirs = []string{ "cmd/link/internal/s390x", "cmd/link/internal/sym", "cmd/link/internal/x86", + "debug/dwarf", + "debug/elf", + "debug/macho", "debug/pe", "math/big", "math/bits", @@ -115,6 +119,11 @@ func bootstrapBuildTools() { src := pathf("%s/src/%s", goroot, dir) dst := pathf("%s/%s", base, dir) xmkdirall(dst) + if dir == "cmd/cgo" { + // Write to src because we need the file both for bootstrap + // and for later in the main build. + mkzdefaultcc("", pathf("%s/zdefaultcc.go", src)) + } Dir: for _, name := range xreaddirfiles(src) { for _, pre := range ignorePrefixes { diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go index ae73c92e94..b902e29c50 100644 --- a/src/cmd/go/internal/load/pkg.go +++ b/src/cmd/go/internal/load/pkg.go @@ -1124,6 +1124,15 @@ func LinkerDeps(p *Package) []string { var deps []string // External linking mode forces an import of runtime/cgo. + // TODO(rsc): The GOROOT exception here is mainly to avoid a circular + // dependency when building cmd/cgo, which the build of + // runtime/cgo needs, but as of CL 68338 we now build + // cmd/cgo during cmd/dist, so that exception is no longer + // needed. At some point it may be worthwhile to remove the + // GOROOT exception here. + // Note that the condition here should also match the condition + // in ../work/build.go's gcToolchain.ld that controls disabling + // external linking during the link step. if cfg.ExternalLinkingForced(p.Goroot) { deps = append(deps, "runtime/cgo") } diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go index 96dced830c..90fa1b1ce1 100644 --- a/src/cmd/go/internal/test/test.go +++ b/src/cmd/go/internal/test/test.go @@ -18,7 +18,6 @@ import ( "path" "path/filepath" "regexp" - "runtime" "sort" "strings" "text/template" @@ -511,9 +510,6 @@ func runTest(cmd *base.Command, args []string) { if deps["C"] { delete(deps, "C") deps["runtime/cgo"] = true - if cfg.Goos == runtime.GOOS && cfg.Goarch == runtime.GOARCH && !cfg.BuildRace && !cfg.BuildMSan { - deps["cmd/cgo"] = true - } } // Ignore pseudo-packages. delete(deps, "unsafe") diff --git a/src/cmd/go/internal/work/build.go b/src/cmd/go/internal/work/build.go index c62735d0aa..d28be759dc 100644 --- a/src/cmd/go/internal/work/build.go +++ b/src/cmd/go/internal/work/build.go @@ -600,10 +600,8 @@ func InstallPackages(args []string, forGet bool) { // If p is a tool, delay the installation until the end of the build. // This avoids installing assemblers/compilers that are being executed // by other steps in the build. - // cmd/cgo is handled specially in b.Action, so that we can - // both build and use it in the same 'go install'. Action := b.Action(ModeInstall, ModeInstall, p) - if load.GoTools[p.ImportPath] == load.ToTool && p.ImportPath != "cmd/cgo" { + if load.GoTools[p.ImportPath] == load.ToTool { a.Deps = append(a.Deps, Action.Deps...) Action.Deps = append(Action.Deps, a) tools = append(tools, Action) @@ -682,7 +680,6 @@ type Action struct { Args []string // additional args for runProgram triggers []*Action // inverse of deps - cgo *Action // action for cgo binary if needed // Generated files, directories. Link bool // target is executable, not just package @@ -853,23 +850,6 @@ func (b *Builder) action1(mode BuildMode, depMode BuildMode, p *load.Package, lo } } - // If we are not doing a cross-build, then record the binary we'll - // generate for cgo as a dependency of the build of any package - // using cgo, to make sure we do not overwrite the binary while - // a package is using it. If this is a cross-build, then the cgo we - // are writing is not the cgo we need to use. - if cfg.Goos == runtime.GOOS && cfg.Goarch == runtime.GOARCH && !cfg.BuildRace && !cfg.BuildMSan { - if (len(p.CgoFiles) > 0 || p.Standard && p.ImportPath == "runtime/cgo") && !cfg.BuildLinkshared && cfg.BuildBuildmode != "shared" { - var stk load.ImportStack - p1 := load.LoadPackage("cmd/cgo", &stk) - if p1.Error != nil { - base.Fatalf("load cmd/cgo: %v", p1.Error) - } - a.cgo = b.Action(depMode, depMode, p1) - a.Deps = append(a.Deps, a.cgo) - } - } - if p.Standard { switch p.ImportPath { case "builtin", "unsafe": @@ -978,6 +958,7 @@ func (b *Builder) libaction(libname string, pkgs []*load.Package, mode, depMode // external linking mode forces an import of runtime/cgo (and // math on arm). So if it was not passed on the command line and // it is not present in another shared library, add it here. + // TODO(rsc): This should probably be changed to use load.LinkerDeps(p). gccgo := cfg.BuildToolchainName == "gccgo" if !gccgo { seencgo := false @@ -1324,13 +1305,7 @@ func (b *Builder) build(a *Action) (err error) { sfiles = nil } - var cgoExe string - if a.cgo != nil && a.cgo.Target != "" { - cgoExe = a.cgo.Target - } else { - cgoExe = base.Tool("cgo") - } - outGo, outObj, err := b.cgo(a, cgoExe, objdir, pcCFLAGS, pcLDFLAGS, cgofiles, objdirCgofiles, gccfiles, cxxfiles, a.Package.MFiles, a.Package.FFiles) + outGo, outObj, err := b.cgo(a, base.Tool("cgo"), objdir, pcCFLAGS, pcLDFLAGS, cgofiles, objdirCgofiles, gccfiles, cxxfiles, a.Package.MFiles, a.Package.FFiles) if err != nil { return err } @@ -2575,6 +2550,9 @@ func (gcToolchain) ld(b *Builder, root *Action, out, importcfg string, allaction // but the whole-GOROOT prohibition matches the similar // logic in ../load/pkg.go that decides whether to add an // implicit runtime/cgo dependency. + // TODO(rsc): We may be able to remove this exception + // now that CL 68338 has made cmd/cgo not a special case. + // See the longer comment in ../load/pkg.go. ldflags = removeLinkmodeExternal(ldflags) } ldflags = setextld(ldflags, compiler)