cmd/go: always update mtime during go install / go build -o / go test -c

Even if the go command can see that the target is up-to-date
an mtime-based build system invoking the go command may not
be able to tell. Update the mtime to make clear that the target is
up-to-date, and also to hide exactly how smart the go command
is or is not. This keeps users (and programs) from depending on
the exact details of the go command's staleness determination.

Without this I believe we will get a stream of (completely reasonable)
bug reports that "go install (or go test -c) did not update the binary
after I trivially changed the source code or touched a source file".

Change-Id: I920e4aaed2a57319e3c0c37717f872bc059e484e
Reviewed-on: https://go-review.googlesource.com/76590
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>

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

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 {

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 {