]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go/internal/work: include detail in errors from cache operations
authorBryan C. Mills <bcmills@google.com>
Tue, 30 Jul 2019 18:53:03 +0000 (14:53 -0400)
committerBryan C. Mills <bcmills@google.com>
Thu, 12 Sep 2019 18:33:05 +0000 (18:33 +0000)
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 <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
src/cmd/go/internal/cache/cache.go
src/cmd/go/internal/work/exec.go

index a05a08f75fcc3f0627f6a3357b929a30700a13f4..879739876503b87caf00733c33fa2addb9447d00 100644 (file)
@@ -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 <hex id> <hex out> <decimal size space-padded to 20 bytes> <unixnano space-padded to 20 bytes>\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
 }
index b68f9028533ac34086778757ca5b3592bbca16c4..4564e32e65e18031b64f3cdff2326974e3d24403 100644 (file)
@@ -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)