From 1dd71645558157f2d4477f21c6e9f18a99b0bc94 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Tue, 30 Jul 2019 14:53:03 -0400 Subject: [PATCH] cmd/go/internal/work: include detail in errors from cache operations Since the cache has been required since Go 1.12, also remove redundant checks for a nil cache. Updates #29127 Updates #29667 Change-Id: Ibc59b659306a4eef2d4f0e3d0b651986d6cf84ad Reviewed-on: https://go-review.googlesource.com/c/go/+/188021 Run-TryBot: Bryan C. Mills TryBot-Result: Gobot Gobot Reviewed-by: Jay Conrod --- src/cmd/go/internal/cache/cache.go | 69 +++++++++++++++++------- src/cmd/go/internal/work/exec.go | 86 +++++++++++++----------------- 2 files changed, 89 insertions(+), 66 deletions(-) diff --git a/src/cmd/go/internal/cache/cache.go b/src/cmd/go/internal/cache/cache.go index a05a08f75f..8797398765 100644 --- a/src/cmd/go/internal/cache/cache.go +++ b/src/cmd/go/internal/cache/cache.go @@ -74,7 +74,22 @@ func (c *Cache) fileName(id [HashSize]byte, key string) string { return filepath.Join(c.dir, fmt.Sprintf("%02x", id[0]), fmt.Sprintf("%x", id)+"-"+key) } -var errMissing = errors.New("cache entry not found") +// An entryNotFoundError indicates that a cache entry was not found, with an +// optional underlying reason. +type entryNotFoundError struct { + Err error +} + +func (e *entryNotFoundError) Error() string { + if e.Err == nil { + return "cache entry not found" + } + return fmt.Sprintf("cache entry not found: %v", e.Err) +} + +func (e *entryNotFoundError) Unwrap() error { + return e.Err +} const ( // action entry file is "v1 \n" @@ -93,6 +108,8 @@ const ( // GODEBUG=gocacheverify=1. var verify = false +var errVerifyMode = errors.New("gocachverify=1") + // DebugTest is set when GODEBUG=gocachetest=1 is in the environment. var DebugTest = false @@ -121,7 +138,7 @@ func initEnv() { // saved file for that output ID is still available. func (c *Cache) Get(id ActionID) (Entry, error) { if verify { - return Entry{}, errMissing + return Entry{}, &entryNotFoundError{Err: errVerifyMode} } return c.get(id) } @@ -134,47 +151,60 @@ type Entry struct { // get is Get but does not respect verify mode, so that Put can use it. func (c *Cache) get(id ActionID) (Entry, error) { - missing := func() (Entry, error) { - return Entry{}, errMissing + missing := func(reason error) (Entry, error) { + return Entry{}, &entryNotFoundError{Err: reason} } f, err := os.Open(c.fileName(id, "a")) if err != nil { - return missing() + return missing(err) } defer f.Close() entry := make([]byte, entrySize+1) // +1 to detect whether f is too long - if n, err := io.ReadFull(f, entry); n != entrySize || err != io.ErrUnexpectedEOF { - return missing() + if n, err := io.ReadFull(f, entry); n > entrySize { + return missing(errors.New("too long")) + } else if err != io.ErrUnexpectedEOF { + if err == io.EOF { + return missing(errors.New("file is empty")) + } + return missing(err) + } else if n < entrySize { + return missing(errors.New("entry file incomplete")) } if entry[0] != 'v' || entry[1] != '1' || entry[2] != ' ' || entry[3+hexSize] != ' ' || entry[3+hexSize+1+hexSize] != ' ' || entry[3+hexSize+1+hexSize+1+20] != ' ' || entry[entrySize-1] != '\n' { - return missing() + return missing(errors.New("invalid header")) } eid, entry := entry[3:3+hexSize], entry[3+hexSize:] eout, entry := entry[1:1+hexSize], entry[1+hexSize:] esize, entry := entry[1:1+20], entry[1+20:] etime, entry := entry[1:1+20], entry[1+20:] var buf [HashSize]byte - if _, err := hex.Decode(buf[:], eid); err != nil || buf != id { - return missing() + if _, err := hex.Decode(buf[:], eid); err != nil { + return missing(fmt.Errorf("decoding ID: %v", err)) + } else if buf != id { + return missing(errors.New("mismatched ID")) } if _, err := hex.Decode(buf[:], eout); err != nil { - return missing() + return missing(fmt.Errorf("decoding output ID: %v", err)) } i := 0 for i < len(esize) && esize[i] == ' ' { i++ } size, err := strconv.ParseInt(string(esize[i:]), 10, 64) - if err != nil || size < 0 { - return missing() + if err != nil { + return missing(fmt.Errorf("parsing size: %v", err)) + } else if size < 0 { + return missing(errors.New("negative size")) } i = 0 for i < len(etime) && etime[i] == ' ' { i++ } tm, err := strconv.ParseInt(string(etime[i:]), 10, 64) - if err != nil || tm < 0 { - return missing() + if err != nil { + return missing(fmt.Errorf("parsing timestamp: %v", err)) + } else if tm < 0 { + return missing(errors.New("negative timestamp")) } c.used(c.fileName(id, "a")) @@ -191,8 +221,11 @@ func (c *Cache) GetFile(id ActionID) (file string, entry Entry, err error) { } file = c.OutputFile(entry.OutputID) info, err := os.Stat(file) - if err != nil || info.Size() != entry.Size { - return "", Entry{}, errMissing + if err != nil { + return "", Entry{}, &entryNotFoundError{Err: err} + } + if info.Size() != entry.Size { + return "", Entry{}, &entryNotFoundError{Err: errors.New("file incomplete")} } return file, entry, nil } @@ -207,7 +240,7 @@ func (c *Cache) GetBytes(id ActionID) ([]byte, Entry, error) { } data, _ := ioutil.ReadFile(c.OutputFile(entry.OutputID)) if sha256.Sum256(data) != entry.OutputID { - return nil, entry, errMissing + return nil, entry, &entryNotFoundError{Err: errors.New("bad checksum")} } return data, entry, nil } diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go index b68f902853..4564e32e65 100644 --- a/src/cmd/go/internal/work/exec.go +++ b/src/cmd/go/internal/work/exec.go @@ -54,8 +54,9 @@ func actionList(root *Action) []*Action { // do runs the action graph rooted at root. func (b *Builder) Do(root *Action) { - if c := cache.Default(); c != nil && !b.IsCmdList { + if !b.IsCmdList { // If we're doing real work, take time at the end to trim the cache. + c := cache.Default() defer c.Trim() } @@ -406,15 +407,19 @@ func (b *Builder) build(a *Action) (err error) { if b.NeedExport { p.Export = a.built } - if need&needCompiledGoFiles != 0 && b.loadCachedSrcFiles(a) { - need &^= needCompiledGoFiles + if need&needCompiledGoFiles != 0 { + if err := b.loadCachedSrcFiles(a); err == nil { + need &^= needCompiledGoFiles + } } } // Source files might be cached, even if the full action is not // (e.g., go list -compiled -find). - if !cachedBuild && need&needCompiledGoFiles != 0 && b.loadCachedSrcFiles(a) { - need &^= needCompiledGoFiles + if !cachedBuild && need&needCompiledGoFiles != 0 { + if err := b.loadCachedSrcFiles(a); err == nil { + need &^= needCompiledGoFiles + } } if need == 0 { @@ -459,16 +464,20 @@ func (b *Builder) build(a *Action) (err error) { objdir := a.Objdir // Load cached cgo header, but only if we're skipping the main build (cachedBuild==true). - if cachedBuild && need&needCgoHdr != 0 && b.loadCachedCgoHdr(a) { - need &^= needCgoHdr + if cachedBuild && need&needCgoHdr != 0 { + if err := b.loadCachedCgoHdr(a); err == nil { + need &^= needCgoHdr + } } // Load cached vet config, but only if that's all we have left // (need == needVet, not testing just the one bit). // If we are going to do a full build anyway, // we're going to regenerate the files below anyway. - if need == needVet && b.loadCachedVet(a) { - need &^= needVet + if need == needVet { + if err := b.loadCachedVet(a); err == nil { + need &^= needVet + } } if need == 0 { return nil @@ -615,8 +624,8 @@ func (b *Builder) build(a *Action) (err error) { need &^= needVet } if need&needCompiledGoFiles != 0 { - if !b.loadCachedSrcFiles(a) { - return fmt.Errorf("failed to cache compiled Go files") + if err := b.loadCachedSrcFiles(a); err != nil { + return fmt.Errorf("loading compiled Go files from cache: %w", err) } need &^= needCompiledGoFiles } @@ -788,7 +797,7 @@ func (b *Builder) cacheObjdirFile(a *Action, c *cache.Cache, name string) error func (b *Builder) findCachedObjdirFile(a *Action, c *cache.Cache, name string) (string, error) { file, _, err := c.GetFile(cache.Subkey(a.actionID, name)) if err != nil { - return "", err + return "", fmt.Errorf("loading cached file %s: %w", name, err) } return file, nil } @@ -803,26 +812,16 @@ func (b *Builder) loadCachedObjdirFile(a *Action, c *cache.Cache, name string) e func (b *Builder) cacheCgoHdr(a *Action) { c := cache.Default() - if c == nil { - return - } b.cacheObjdirFile(a, c, "_cgo_install.h") } -func (b *Builder) loadCachedCgoHdr(a *Action) bool { +func (b *Builder) loadCachedCgoHdr(a *Action) error { c := cache.Default() - if c == nil { - return false - } - err := b.loadCachedObjdirFile(a, c, "_cgo_install.h") - return err == nil + return b.loadCachedObjdirFile(a, c, "_cgo_install.h") } func (b *Builder) cacheSrcFiles(a *Action, srcfiles []string) { c := cache.Default() - if c == nil { - return - } var buf bytes.Buffer for _, file := range srcfiles { if !strings.HasPrefix(file, a.Objdir) { @@ -842,14 +841,11 @@ func (b *Builder) cacheSrcFiles(a *Action, srcfiles []string) { c.PutBytes(cache.Subkey(a.actionID, "srcfiles"), buf.Bytes()) } -func (b *Builder) loadCachedVet(a *Action) bool { +func (b *Builder) loadCachedVet(a *Action) error { c := cache.Default() - if c == nil { - return false - } list, _, err := c.GetBytes(cache.Subkey(a.actionID, "srcfiles")) if err != nil { - return false + return fmt.Errorf("reading srcfiles list: %w", err) } var srcfiles []string for _, name := range strings.Split(string(list), "\n") { @@ -861,22 +857,19 @@ func (b *Builder) loadCachedVet(a *Action) bool { continue } if err := b.loadCachedObjdirFile(a, c, name); err != nil { - return false + return err } srcfiles = append(srcfiles, a.Objdir+name) } buildVetConfig(a, srcfiles) - return true + return nil } -func (b *Builder) loadCachedSrcFiles(a *Action) bool { +func (b *Builder) loadCachedSrcFiles(a *Action) error { c := cache.Default() - if c == nil { - return false - } list, _, err := c.GetBytes(cache.Subkey(a.actionID, "srcfiles")) if err != nil { - return false + return fmt.Errorf("reading srcfiles list: %w", err) } var files []string for _, name := range strings.Split(string(list), "\n") { @@ -889,12 +882,12 @@ func (b *Builder) loadCachedSrcFiles(a *Action) bool { } file, err := b.findCachedObjdirFile(a, c, name) if err != nil { - return false + return fmt.Errorf("finding %s: %w", name, err) } files = append(files, file) } a.Package.CompiledGoFiles = files - return true + return nil } // vetConfig is the configuration passed to vet describing a single package. @@ -1058,12 +1051,11 @@ func (b *Builder) vet(a *Action) error { } key := cache.ActionID(h.Sum()) - if vcfg.VetxOnly { - if c := cache.Default(); c != nil && !cfg.BuildA { - if file, _, err := c.GetFile(key); err == nil { - a.built = file - return nil - } + if vcfg.VetxOnly && !cfg.BuildA { + c := cache.Default() + if file, _, err := c.GetFile(key); err == nil { + a.built = file + return nil } } @@ -1091,9 +1083,7 @@ func (b *Builder) vet(a *Action) error { // If vet wrote export data, save it for input to future vets. if f, err := os.Open(vcfg.VetxOutput); err == nil { a.built = vcfg.VetxOutput - if c := cache.Default(); c != nil { - c.Put(key, f) - } + cache.Default().Put(key, f) f.Close() } @@ -1650,7 +1640,7 @@ func (b *Builder) copyFile(dst, src string, perm os.FileMode, force bool) error df, err = os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, perm) } if err != nil { - return err + return fmt.Errorf("copying %s: %w", src, err) // err should already refer to dst } _, err = io.Copy(df, sf) -- 2.48.1