From: Michael Matloob Date: Fri, 21 Oct 2022 15:25:53 +0000 (-0400) Subject: cmd/go: correct staleness for packages in modules X-Git-Tag: go1.20rc1~566 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=5b606a9d2b7649532fe25794fa6b99bd24e7697c;p=gostls13.git cmd/go: correct staleness for packages in modules Packages in modules don't have a Target set for them, so the current logic for determining staleness always reports them as stale. Instead it should be reporting whether "go install" would do anything, and sholud be false after a go install. If a package does not have a Target, instead check to see whether we've cached its build artifact. Change-Id: Ie7bdb234944353f6c2727bd8bf939cc27ddf3f18 Reviewed-on: https://go-review.googlesource.com/c/go/+/444619 Run-TryBot: Michael Matloob Reviewed-by: Bryan Mills TryBot-Result: Gopher Robot Reviewed-by: Michael Matloob --- diff --git a/src/cmd/go/internal/work/buildid.go b/src/cmd/go/internal/work/buildid.go index e79eb4c66a..f0b12e1036 100644 --- a/src/cmd/go/internal/work/buildid.go +++ b/src/cmd/go/internal/work/buildid.go @@ -418,11 +418,19 @@ func (b *Builder) useCache(a *Action, actionHash cache.ActionID, target string, a.buildID = actionID + buildIDSeparator + mainpkg.buildID + buildIDSeparator + contentID } - // Check to see if target exists and matches the expected action ID. - // If so, it's up to date and we can reuse it instead of rebuilding it. - var buildID string - if target != "" && !cfg.BuildA { - buildID, _ = buildid.ReadFile(target) + // If user requested -a, we force a rebuild, so don't use the cache. + if cfg.BuildA { + if p := a.Package; p != nil && !p.Stale { + p.Stale = true + p.StaleReason = "build -a flag in use" + } + // Begin saving output for later writing to cache. + a.output = []byte{} + return false + } + + if target != "" { + buildID, _ := buildid.ReadFile(target) if strings.HasPrefix(buildID, actionID+buildIDSeparator) { a.buildID = buildID if a.json != nil { @@ -433,18 +441,13 @@ func (b *Builder) useCache(a *Action, actionHash cache.ActionID, target string, a.Target = "DO NOT USE - " + a.Mode return true } - } - - // Special case for building a main package: if the only thing we - // want the package for is to link a binary, and the binary is - // already up-to-date, then to avoid a rebuild, report the package - // as up-to-date as well. See "Build IDs" comment above. - // TODO(rsc): Rewrite this code to use a TryCache func on the link action. - if target != "" && !cfg.BuildA && !b.NeedExport && a.Mode == "build" && len(a.triggers) == 1 && a.triggers[0].Mode == "link" { - buildID, err := buildid.ReadFile(target) - if err == nil { - id := strings.Split(buildID, buildIDSeparator) - if len(id) == 4 && id[1] == actionID { + // Special case for building a main package: if the only thing we + // want the package for is to link a binary, and the binary is + // already up-to-date, then to avoid a rebuild, report the package + // as up-to-date as well. See "Build IDs" comment above. + // TODO(rsc): Rewrite this code to use a TryCache func on the link action. + if !b.NeedExport && a.Mode == "build" && len(a.triggers) == 1 && a.triggers[0].Mode == "link" { + if id := strings.Split(buildID, buildIDSeparator); len(id) == 4 && id[1] == actionID { // Temporarily assume a.buildID is the package build ID // stored in the installed binary, and see if that makes // the upcoming link action ID a match. If so, report that @@ -488,7 +491,7 @@ func (b *Builder) useCache(a *Action, actionHash cache.ActionID, target string, // then to avoid the link step, report the link as up-to-date. // We avoid the nested build ID problem in the previous special case // by recording the test results in the cache under the action ID half. - if !cfg.BuildA && len(a.triggers) == 1 && a.triggers[0].TryCache != nil && a.triggers[0].TryCache(b, a.triggers[0]) { + if len(a.triggers) == 1 && a.triggers[0].TryCache != nil && a.triggers[0].TryCache(b, a.triggers[0]) { // Best effort attempt to display output from the compile and link steps. // If it doesn't work, it doesn't work: reusing the test result is more // important than reprinting diagnostic information. @@ -505,63 +508,51 @@ func (b *Builder) useCache(a *Action, actionHash cache.ActionID, target string, return true } - if b.IsCmdList { - // Invoked during go list to compute and record staleness. - if p := a.Package; p != nil && !p.Stale { - p.Stale = true - if cfg.BuildA { - p.StaleReason = "build -a flag in use" - } else { - p.StaleReason = "build ID mismatch" - for _, p1 := range p.Internal.Imports { - if p1.Stale && p1.StaleReason != "" { - if strings.HasPrefix(p1.StaleReason, "stale dependency: ") { - p.StaleReason = p1.StaleReason - break - } - if strings.HasPrefix(p.StaleReason, "build ID mismatch") { - p.StaleReason = "stale dependency: " + p1.ImportPath - } - } + // Check to see if the action output is cached. + if c := cache.Default(); c != nil { + if file, _, err := c.GetFile(actionHash); err == nil { + if buildID, err := buildid.ReadFile(file); err == nil { + if printOutput { + showStdout(b, c, a.actionID, "stdout") + } + a.built = file + a.Target = "DO NOT USE - using cache" + a.buildID = buildID + if a.json != nil { + a.json.BuildID = a.buildID } + if p := a.Package; p != nil && target != "" { + p.Stale = true + // Clearer than explaining that something else is stale. + p.StaleReason = "not installed but available in build cache" + } + return true } } - - // Fall through to update a.buildID from the build artifact cache, - // which will affect the computation of buildIDs for targets - // higher up in the dependency graph. } - // Check the build artifact cache. - // We treat hits in this cache as being "stale" for the purposes of go list - // (in effect, "stale" means whether p.Target is up-to-date), - // but we're still happy to use results from the build artifact cache. - if c := cache.Default(); c != nil { - if !cfg.BuildA { - if file, _, err := c.GetFile(actionHash); err == nil { - if buildID, err := buildid.ReadFile(file); err == nil { - if printOutput { - showStdout(b, c, a.actionID, "stdout") + // If we've reached this point, we can't use the cache for the action. + if p := a.Package; p != nil && !p.Stale { + p.Stale = true + p.StaleReason = "build ID mismatch" + if b.IsCmdList { + // Since we may end up printing StaleReason, include more detail. + for _, p1 := range p.Internal.Imports { + if p1.Stale && p1.StaleReason != "" { + if strings.HasPrefix(p1.StaleReason, "stale dependency: ") { + p.StaleReason = p1.StaleReason + break } - a.built = file - a.Target = "DO NOT USE - using cache" - a.buildID = buildID - if a.json != nil { - a.json.BuildID = a.buildID + if strings.HasPrefix(p.StaleReason, "build ID mismatch") { + p.StaleReason = "stale dependency: " + p1.ImportPath } - if p := a.Package; p != nil { - // Clearer than explaining that something else is stale. - p.StaleReason = "not installed but available in build cache" - } - return true } } } - - // Begin saving output for later writing to cache. - a.output = []byte{} } + // Begin saving output for later writing to cache. + a.output = []byte{} return false } diff --git a/src/cmd/go/testdata/script/mod_get_commit.txt b/src/cmd/go/testdata/script/mod_get_commit.txt index f60eaab3a7..76650f3bd3 100644 --- a/src/cmd/go/testdata/script/mod_get_commit.txt +++ b/src/cmd/go/testdata/script/mod_get_commit.txt @@ -19,10 +19,8 @@ env GOCACHE=$WORK/gocache # Looking for compile commands, so need a clean cache go build -x golang.org/x/text/language stderr 'compile|cp|gccgo .*language\.a$' -# BUG: after the build, the package should not be stale, as 'go install' would -# not do anything further. go list -f '{{.Stale}}' golang.org/x/text/language -stdout ^true +stdout ^false # install after build should not run the compiler again. go install -x golang.org/x/text/language diff --git a/src/cmd/go/testdata/script/mod_stale.txt b/src/cmd/go/testdata/script/mod_stale.txt new file mode 100644 index 0000000000..c6ab27dd7c --- /dev/null +++ b/src/cmd/go/testdata/script/mod_stale.txt @@ -0,0 +1,15 @@ +[short] skip + +env GOCACHE=$WORK/cache +go list -f '{{.Stale}}' . +stdout true +go install . +go list -f '{{.Stale}}' . +stdout false + +-- go.mod -- +module example.com/mod + +go 1.20 +-- m.go -- +package m