]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go: correct staleness for packages in modules
authorMichael Matloob <matloob@golang.org>
Fri, 21 Oct 2022 15:25:53 +0000 (11:25 -0400)
committerMichael Matloob <matloob@golang.org>
Fri, 21 Oct 2022 20:57:57 +0000 (20:57 +0000)
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 <matloob@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
src/cmd/go/internal/work/buildid.go
src/cmd/go/testdata/script/mod_get_commit.txt
src/cmd/go/testdata/script/mod_stale.txt [new file with mode: 0644]

index e79eb4c66a1d67133b6bd13a329496ad19d7c976..f0b12e103699765966b616a6c58bc31e01cca359 100644 (file)
@@ -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
 }
 
index f60eaab3a77e09d4aee4c57839ce7098f1443f89..76650f3bd36c4953d442c1e2885515c5444c5e8c 100644 (file)
@@ -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 (file)
index 0000000..c6ab27d
--- /dev/null
@@ -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