cmd/go, cmd/cgo: only set TERM=dumb when running the compiler

The clang compiler on some terminals will issue colored error
messages, which can confuse tools like cgo. To avoid this we used to
set TERM=dumb for all programs started by the go tool. However, that
confuses the pprof tool, which doesn't know whether to support fancy
editing and colors itself.

Instead, change the go tool and the cgo tool to set TERM=dumb where it
matters--when invoking the C compiler--rather than in all cases.

Updates #26254

Change-Id: I95174f961ac269a50a83f5f9d268219043cba968
Reviewed-on: https://go-review.googlesource.com/122975
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/cmd/cgo/util.go b/src/cmd/cgo/util.go
index 4f5c48864e997329301bf976b6ddf84102a38123..c9c6f3f58bde44622f8e95db93806b8220fe74df 100644 (file)
--- a/src/cmd/cgo/util.go
+++ b/src/cmd/cgo/util.go
@@ -59,6 +59,8 @@ func run(stdin []byte, argv []string) (stdout, stderr []byte, ok bool) {
        var bout, berr bytes.Buffer
        p.Stdout = &bout
        p.Stderr = &berr
+       // Disable escape codes in clang error messages.
+       p.Env = append(os.Environ(), "TERM=dumb")
        err := p.Run()
        if _, ok := err.(*exec.ExitError); err != nil && !ok {
                fatalf("%s", err)

diff --git a/src/cmd/go/internal/envcmd/env.go b/src/cmd/go/internal/envcmd/env.go
index bd66a98f21fc949ade7d67e896082c7a3199c63d..1c5640f208c08d865532cd868863351a995130ea 100644 (file)
--- a/src/cmd/go/internal/envcmd/env.go
+++ b/src/cmd/go/internal/envcmd/env.go
@@ -62,9 +62,6 @@ func MkEnv() []cfg.EnvVar {
                {Name: "GOROOT", Value: cfg.GOROOT},
                {Name: "GOTMPDIR", Value: os.Getenv("GOTMPDIR")},
                {Name: "GOTOOLDIR", Value: base.ToolDir},
-
-               // disable escape codes in clang errors
-               {Name: "TERM", Value: "dumb"},
        }
 
        if work.GccgoBin != "" {

diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go
index 38425944542c160d9462ad8d368f9bbdff540921..b51cd47e7041ad5cf11e8e6c3184ae5cd0dc39a4 100644 (file)
--- a/src/cmd/go/internal/work/exec.go
+++ b/src/cmd/go/internal/work/exec.go
@@ -990,7 +990,7 @@ func (b *Builder) vet(a *Action) error {
                return err
        }
 
-       var env []string
+       env := b.cCompilerEnv()
        if cfg.BuildToolchainName == "gccgo" {
                env = append(env, "GCCGO="+BuildToolchain.compiler())
        }
@@ -1863,6 +1863,13 @@ func joinUnambiguously(a []string) string {
        return buf.String()
 }
 
+// cCompilerEnv returns environment variables to set when running the
+// C compiler. This is needed to disable escape codes in clang error
+// messages that confuse tools like cgo.
+func (b *Builder) cCompilerEnv() []string {
+       return []string{"TERM=dumb"}
+}
+
 // mkdir makes the named directory.
 func (b *Builder) Mkdir(dir string) error {
        // Make Mkdir(a.Objdir) a no-op instead of an error when a.Objdir == "".
@@ -2010,7 +2017,7 @@ func (b *Builder) ccompile(a *Action, p *load.Package, outfile string, flags []s
        if !filepath.IsAbs(outfile) {
                outfile = filepath.Join(p.Dir, outfile)
        }
-       output, err := b.runOut(filepath.Dir(file), nil, compiler, flags, "-o", outfile, "-c", filepath.Base(file))
+       output, err := b.runOut(filepath.Dir(file), b.cCompilerEnv(), compiler, flags, "-o", outfile, "-c", filepath.Base(file))
        if len(output) > 0 {
                // On FreeBSD 11, when we pass -g to clang 3.8 it
                // invokes its internal assembler with -dwarf-version=2.
@@ -2050,7 +2057,7 @@ func (b *Builder) gccld(p *load.Package, objdir, out string, flags []string, obj
        } else {
                cmd = b.GccCmd(p.Dir, objdir)
        }
-       return b.run(nil, p.Dir, p.ImportPath, nil, cmd, "-o", out, objs, flags)
+       return b.run(nil, p.Dir, p.ImportPath, b.cCompilerEnv(), cmd, "-o", out, objs, flags)
 }
 
 // Grab these before main helpfully overwrites them.
@@ -2345,7 +2352,7 @@ func (b *Builder) cgo(a *Action, cgoExe, objdir string, pcCFLAGS, pcLDFLAGS, cgo
        // along to the host linker. At this point in the code, cgoLDFLAGS
        // consists of the original $CGO_LDFLAGS (unchecked) and all the
        // flags put together from source code (checked).
-       var cgoenv []string
+       cgoenv := b.cCompilerEnv()
        if len(cgoLDFLAGS) > 0 {
                flags := make([]string, len(cgoLDFLAGS))
                for i, f := range cgoLDFLAGS {
@@ -2492,7 +2499,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, p.Dir, p.ImportPath, nil, 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.