From fa80fe7b1cc770d37b2263eb5d81c87db75ceb80 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 26 Apr 2023 10:45:01 -0700 Subject: [PATCH] cmd/go: simplify code that still assumed the build cache could be nil cache.Default always returns a non-nil value since Go 1.12; the docs were updated in https://go.dev/cl/465555. This updates all the callers of cache.Default that were checking whether the result was nil so the code isn't misleading/confusing to readers. Change-Id: Ia63567dd70affef6041c744259f65cea79a2752e Reviewed-on: https://go-review.googlesource.com/c/go/+/489355 Auto-Submit: Brad Fitzpatrick Reviewed-by: Bryan Mills Run-TryBot: Brad Fitzpatrick TryBot-Result: Gopher Robot Reviewed-by: Dmitri Shuralyov --- src/cmd/go/internal/list/list.go | 14 ----- src/cmd/go/internal/test/test.go | 8 --- src/cmd/go/internal/work/buildid.go | 80 ++++++++++++++--------------- 3 files changed, 38 insertions(+), 64 deletions(-) diff --git a/src/cmd/go/internal/list/list.go b/src/cmd/go/internal/list/list.go index dd3e5cd06f..be2dd60dff 100644 --- a/src/cmd/go/internal/list/list.go +++ b/src/cmd/go/internal/list/list.go @@ -624,20 +624,6 @@ func runList(ctx context.Context, cmd *base.Command, args []string) { base.ExitIfErrors() } - if cache.Default() == nil { - // These flags return file names pointing into the build cache, - // so the build cache must exist. - if *listCompiled { - base.Fatalf("go list -compiled requires build cache") - } - if *listExport { - base.Fatalf("go list -export requires build cache") - } - if *listTest { - base.Fatalf("go list -test requires build cache") - } - } - if *listTest { c := cache.Default() // Add test binaries to packages to be listed. diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go index a986718abf..e82ea72094 100644 --- a/src/cmd/go/internal/test/test.go +++ b/src/cmd/go/internal/test/test.go @@ -1537,14 +1537,6 @@ func (c *runCache) tryCacheWithID(b *work.Builder, a *work.Action, id string) bo } } - if cache.Default() == nil { - if cache.DebugTest { - fmt.Fprintf(os.Stderr, "testcache: GOCACHE=off\n") - } - c.disableCache = true - return false - } - // The test cache result fetch is a two-level lookup. // // First, we use the content hash of the test binary diff --git a/src/cmd/go/internal/work/buildid.go b/src/cmd/go/internal/work/buildid.go index db56714788..ea3240412c 100644 --- a/src/cmd/go/internal/work/buildid.go +++ b/src/cmd/go/internal/work/buildid.go @@ -437,6 +437,8 @@ func (b *Builder) useCache(a *Action, actionHash cache.ActionID, target string, return false } + c := cache.Default() + if target != "" { buildID, _ := buildid.ReadFile(target) if strings.HasPrefix(buildID, actionID+buildIDSeparator) { @@ -474,10 +476,8 @@ func (b *Builder) useCache(a *Action, actionHash cache.ActionID, target string, // If it doesn't work, it doesn't work: reusing the cached binary is more // important than reprinting diagnostic information. if printOutput { - if c := cache.Default(); c != nil { - showStdout(b, c, a.actionID, "stdout") // compile output - showStdout(b, c, a.actionID, "link-stdout") // link output - } + showStdout(b, c, a.actionID, "stdout") // compile output + showStdout(b, c, a.actionID, "link-stdout") // link output } // Poison a.Target to catch uses later in the build. @@ -504,10 +504,8 @@ func (b *Builder) useCache(a *Action, actionHash cache.ActionID, target string, // If it doesn't work, it doesn't work: reusing the test result is more // important than reprinting diagnostic information. if printOutput { - if c := cache.Default(); c != nil { - showStdout(b, c, a.Deps[0].actionID, "stdout") // compile output - showStdout(b, c, a.Deps[0].actionID, "link-stdout") // link output - } + showStdout(b, c, a.Deps[0].actionID, "stdout") // compile output + showStdout(b, c, a.Deps[0].actionID, "link-stdout") // link output } // Poison a.Target to catch uses later in the build. @@ -517,25 +515,23 @@ func (b *Builder) useCache(a *Action, actionHash cache.ActionID, target string, } // 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 + 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 } } @@ -609,22 +605,22 @@ func (b *Builder) updateBuildID(a *Action, target string, rewrite bool) error { } } + c := cache.Default() + // Cache output from compile/link, even if we don't do the rest. - if c := cache.Default(); c != nil { - switch a.Mode { - case "build": - c.PutBytes(cache.Subkey(a.actionID, "stdout"), a.output) - case "link": - // Even though we don't cache the binary, cache the linker text output. - // We might notice that an installed binary is up-to-date but still - // want to pretend to have run the linker. - // Store it under the main package's action ID - // to make it easier to find when that's all we have. - for _, a1 := range a.Deps { - if p1 := a1.Package; p1 != nil && p1.Name == "main" { - c.PutBytes(cache.Subkey(a1.actionID, "link-stdout"), a.output) - break - } + switch a.Mode { + case "build": + c.PutBytes(cache.Subkey(a.actionID, "stdout"), a.output) + case "link": + // Even though we don't cache the binary, cache the linker text output. + // We might notice that an installed binary is up-to-date but still + // want to pretend to have run the linker. + // Store it under the main package's action ID + // to make it easier to find when that's all we have. + for _, a1 := range a.Deps { + if p1 := a1.Package; p1 != nil && p1.Name == "main" { + c.PutBytes(cache.Subkey(a1.actionID, "link-stdout"), a.output) + break } } } @@ -682,7 +678,7 @@ func (b *Builder) updateBuildID(a *Action, target string, rewrite bool) error { // that will mean the go process is itself writing a binary // and then executing it, so we will need to defend against // ETXTBSY problems as discussed in exec.go and golang.org/issue/22220. - if c := cache.Default(); c != nil && a.Mode == "build" { + if a.Mode == "build" { r, err := os.Open(target) if err == nil { if a.output == nil { -- 2.50.0