diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index e302b2080e..b49c558f4a 100644 --- a/src/cmd/go/go_test.go +++ b/src/cmd/go/go_test.go @@ -2646,6 +2646,8 @@ func main() { // "go test -c -test.bench=XXX errors" should not hang. // "go test -c" should also produce reproducible binaries. +// "go test -c" should also appear to write a new binary every time, +// even if it's really just updating the mtime on an existing up-to-date binary. func TestIssue6480(t *testing.T) { tg := testgo(t) defer tg.cleanup() @@ -2662,6 +2664,43 @@ func TestIssue6480(t *testing.T) { if !bytes.Equal(data1, data2) { t.Fatalf("go test -c errors produced different binaries when run twice") } + + start := time.Now() + tg.run("test", "-x", "-c", "-test.bench=XXX", "errors") + tg.grepStderrNot(`[\\/]link|gccgo`, "incorrectly relinked up-to-date test binary") + info, err := os.Stat("errors.test" + exeSuffix) + if err != nil { + t.Fatal(err) + } + start = truncateLike(start, info.ModTime()) + if info.ModTime().Before(start) { + t.Fatalf("mtime of errors.test predates test -c command (%v < %v)", info.ModTime(), start) + } + + start = time.Now() + tg.run("test", "-x", "-c", "-o", "errors2.test", "errors") + tg.grepStderrNot(`[\\/]link|gccgo`, "incorrectly relinked up-to-date test binary") + info, err = os.Stat("errors2.test") + if err != nil { + t.Fatal(err) + } + start = truncateLike(start, info.ModTime()) + if info.ModTime().Before(start) { + t.Fatalf("mtime of errors2.test predates test -c command (%v < %v)", info.ModTime(), start) + } +} + +// truncateLike returns the result of truncating t to the apparent precision of p. +func truncateLike(t, p time.Time) time.Time { + nano := p.UnixNano() + d := 1 * time.Nanosecond + for nano%int64(d) == 0 && d < 1*time.Second { + d *= 10 + } + for nano%int64(d) == 0 && d < 2*time.Second { + d *= 2 + } + return t.Truncate(d) } // cmd/cgo: undefined reference when linking a C-library using gccgo diff --git a/src/cmd/go/internal/work/action.go b/src/cmd/go/internal/work/action.go index 1cca8d9cc0..4a12858170 100644 --- a/src/cmd/go/internal/work/action.go +++ b/src/cmd/go/internal/work/action.go @@ -68,6 +68,8 @@ type Action struct { triggers []*Action // inverse of deps + buggyInstall bool // is this a buggy install (see -linkshared)? + TryCache func(*Builder, *Action) bool // callback for cache bypass // Generated files, directories. @@ -196,6 +198,7 @@ type BuildMode int const ( ModeBuild BuildMode = iota ModeInstall + ModeBuggyInstall ) func (b *Builder) Init() { @@ -309,11 +312,11 @@ func (b *Builder) AutoAction(mode, depMode BuildMode, p *load.Package) *Action { // depMode is the action (build or install) to use when building dependencies. // To turn package main into an executable, call b.Link instead. func (b *Builder) CompileAction(mode, depMode BuildMode, p *load.Package) *Action { - if mode == ModeInstall && p.Internal.Local && p.Target == "" { + if mode != ModeBuild && p.Internal.Local && p.Target == "" { // Imported via local path. No permanent target. mode = ModeBuild } - if mode == ModeInstall && p.Name == "main" { + if mode != ModeBuild && p.Name == "main" { // We never install the .a file for a main package. mode = ModeBuild } @@ -354,8 +357,8 @@ func (b *Builder) CompileAction(mode, depMode BuildMode, p *load.Package) *Actio }) // Construct install action. - if mode == ModeInstall { - a = b.installAction(a) + if mode == ModeInstall || mode == ModeBuggyInstall { + a = b.installAction(a, mode) } return a @@ -446,19 +449,23 @@ func (b *Builder) LinkAction(mode, depMode BuildMode, p *load.Package) *Action { return a }) - if mode == ModeInstall { - a = b.installAction(a) + if mode == ModeInstall || mode == ModeBuggyInstall { + a = b.installAction(a, mode) } return a } // installAction returns the action for installing the result of a1. -func (b *Builder) installAction(a1 *Action) *Action { +func (b *Builder) installAction(a1 *Action, mode BuildMode) *Action { // Because we overwrite the build action with the install action below, // a1 may already be an install action fetched from the "build" cache key, // and the caller just doesn't realize. if strings.HasSuffix(a1.Mode, "-install") { + if a1.buggyInstall && mode == ModeInstall { + // Congratulations! The buggy install is now a proper install. + a1.buggyInstall = false + } return a1 } @@ -497,6 +504,8 @@ func (b *Builder) installAction(a1 *Action) *Action { Deps: []*Action{buildAction}, Target: p.Target, built: p.Target, + + buggyInstall: mode == ModeBuggyInstall, } b.addInstallHeaderAction(a1) @@ -553,7 +562,8 @@ func (b *Builder) addTransitiveLinkDeps(a, a1 *Action, shlib string) { // TODO(rsc): The use of ModeInstall here is suspect, but if we only do ModeBuild, // we'll end up building an overall library or executable that depends at runtime // on other libraries that are out-of-date, which is clearly not good either. - a.Deps = append(a.Deps, b.linkSharedAction(ModeInstall, ModeInstall, p1.Shlib, nil)) + // We call it ModeBuggyInstall to make clear that this is not right. + a.Deps = append(a.Deps, b.linkSharedAction(ModeBuggyInstall, ModeBuggyInstall, p1.Shlib, nil)) } } } @@ -682,7 +692,7 @@ func (b *Builder) linkSharedAction(mode, depMode BuildMode, shlib string, a1 *Ac }) // Install result. - if mode == ModeInstall && a.Func != nil { + if (mode == ModeInstall || mode == ModeBuggyInstall) && a.Func != nil { buildAction := a a = b.cacheAction("install-shlib "+shlib, nil, func() *Action { diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go index 112a5f9cf8..1323394a35 100644 --- a/src/cmd/go/internal/work/exec.go +++ b/src/cmd/go/internal/work/exec.go @@ -21,6 +21,7 @@ import ( "strconv" "strings" "sync" + "time" "cmd/go/internal/base" "cmd/go/internal/cache" @@ -978,6 +979,28 @@ func BuildInstallFunc(b *Builder, a *Action) (err error) { if a1.built == a.Target { a.built = a.Target b.cleanup(a1) + // Whether we're smart enough to avoid a complete rebuild + // depends on exactly what the staleness and rebuild algorithms + // are, as well as potentially the state of the Go build cache. + // We don't really want users to be able to infer (or worse start depending on) + // those details from whether the modification time changes during + // "go install", so do a best-effort update of the file times to make it + // look like we rewrote a.Target even if we did not. Updating the mtime + // may also help other mtime-based systems that depend on our + // previous mtime updates that happened more often. + // This is still not perfect - we ignore the error result, and if the file was + // unwritable for some reason then pretending to have written it is also + // confusing - but it's probably better than not doing the mtime update. + // + // But don't do that for the special case where building an executable + // with -linkshared implicitly installs all its dependent libraries. + // We want to hide that awful detail as much as possible, so don't + // advertise it by touching the mtimes (usually the libraries are up + // to date). + if !a.buggyInstall { + now := time.Now() + os.Chtimes(a.Target, now, now) + } return nil } if b.ComputeStaleOnly {