Skip to content

dev: sync cache internals with go1.24 #5576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 18, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions internal/go/base/error_notunix.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Copyright 2023 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

//go:build !unix

package base

func IsETXTBSY(err error) bool {
// syscall.ETXTBSY is only meaningful on Unix platforms.
return false
}
16 changes: 16 additions & 0 deletions internal/go/base/error_unix.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright 2023 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

//go:build unix

package base

import (
"errors"
"syscall"
)

func IsETXTBSY(err error) bool {
return errors.Is(err, syscall.ETXTBSY)
}
10 changes: 10 additions & 0 deletions internal/go/base/readme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# quoted

Extracted from `go/src/cmd/go/internal/base/` (related to `cache`).

Only the function `IsETXTBSY` is extracted.

## History

- https://github.com/golangci/golangci-lint/pull/5576
- sync go1.24.1
142 changes: 97 additions & 45 deletions internal/go/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,10 @@ import (
"time"

"github.com/rogpeppe/go-internal/lockedfile"
"github.com/rogpeppe/go-internal/robustio"

"github.com/golangci/golangci-lint/v2/internal/go/base"
"github.com/golangci/golangci-lint/v2/internal/go/mmap"
"github.com/golangci/golangci-lint/v2/internal/go/robustio"
)

// An ActionID is a cache action key, the hash of a complete description of a
Expand All @@ -41,8 +42,8 @@ type Cache interface {
// Get returns the cache entry for the provided ActionID.
// On miss, the error type should be of type *entryNotFoundError.
//
// After a success call to Get, OutputFile(Entry.OutputID) must
// exist on disk for until Close is called (at the end of the process).
// After a successful call to Get, OutputFile(Entry.OutputID) must
// exist on disk until Close is called (at the end of the process).
Get(ActionID) (Entry, error)

// Put adds an item to the cache.
Expand All @@ -53,14 +54,14 @@ type Cache interface {
// As a special case, if the ReadSeeker is of type noVerifyReadSeeker,
// the verification from GODEBUG=goverifycache=1 is skipped.
//
// After a success call to Get, OutputFile(Entry.OutputID) must
// exist on disk for until Close is called (at the end of the process).
// After a successful call to Put, OutputFile(OutputID) must
// exist on disk until Close is called (at the end of the process).
Put(ActionID, io.ReadSeeker) (_ OutputID, size int64, _ error)

// Close is called at the end of the go process. Implementations can do
// cache cleanup work at this phase, or wait for and report any errors from
// background cleanup work started earlier. Any cache trimming should in one
// process should not violate cause the invariants of this interface to be
// background cleanup work started earlier. Any cache trimming in one
// process should not cause the invariants of this interface to be
// violated in another process. Namely, a cache trim from one process should
// not delete an ObjectID from disk that was recently Get or Put from
// another process. As a rule of thumb, don't trim things used in the last
Expand Down Expand Up @@ -105,7 +106,7 @@ func Open(dir string) (*DiskCache, error) {
}
for i := 0; i < 256; i++ {
name := filepath.Join(dir, fmt.Sprintf("%02x", i))
if err := os.MkdirAll(name, 0744); err != nil {
if err := os.MkdirAll(name, 0o777); err != nil {
return nil, err
}
}
Expand Down Expand Up @@ -161,13 +162,13 @@ var errVerifyMode = errors.New("gocacheverify=1")
var DebugTest = false

// func init() { initEnv() }

//
// var (
// gocacheverify = godebug.New("gocacheverify")
// gocachehash = godebug.New("gocachehash")
// gocachetest = godebug.New("gocachetest")
// )

//
// func initEnv() {
// if gocacheverify.Value() == "1" {
// gocacheverify.IncNonDefault()
Expand Down Expand Up @@ -258,10 +259,7 @@ func (c *DiskCache) get(id ActionID) (Entry, error) {
return missing(errors.New("negative timestamp"))
}

err = c.used(c.fileName(id, "a"))
if err != nil {
return Entry{}, fmt.Errorf("failed to mark %s as used: %w", c.fileName(id, "a"), err)
}
c.markUsed(c.fileName(id, "a"))

return Entry{buf, size, time.Unix(0, tm)}, nil
}
Expand Down Expand Up @@ -305,25 +303,35 @@ func GetBytes(c Cache, id ActionID) ([]byte, Entry, error) {
// GetMmap looks up the action ID in the cache and returns
// the corresponding output bytes.
// GetMmap should only be used for data that can be expected to fit in memory.
func GetMmap(c Cache, id ActionID) ([]byte, Entry, error) {
func GetMmap(c Cache, id ActionID) ([]byte, Entry, bool, error) {
entry, err := c.Get(id)
if err != nil {
return nil, entry, err
return nil, entry, false, err
}
md, err := mmap.Mmap(c.OutputFile(entry.OutputID))
md, opened, err := mmap.Mmap(c.OutputFile(entry.OutputID))
if err != nil {
return nil, Entry{}, err
return nil, Entry{}, opened, err
}
if int64(len(md.Data)) != entry.Size {
return nil, Entry{}, &entryNotFoundError{Err: errors.New("file incomplete")}
return nil, Entry{}, true, &entryNotFoundError{Err: errors.New("file incomplete")}
}
return md.Data, entry, nil
return md.Data, entry, true, nil
}

// OutputFile returns the name of the cache file storing output with the given OutputID.
func (c *DiskCache) OutputFile(out OutputID) string {
file := c.fileName(out, "d")
c.used(file)
isDir := c.markUsed(file)
if isDir { // => cached executable
entries, err := os.ReadDir(file)
if err != nil {
return fmt.Sprintf("DO NOT USE - missing binary cache entry: %v", err)
}
if len(entries) != 1 {
return "DO NOT USE - invalid binary cache entry"
}
return filepath.Join(file, entries[0].Name())
}
return file
}

Expand All @@ -345,7 +353,7 @@ const (
trimLimit = 5 * 24 * time.Hour
)

// used makes a best-effort attempt to update mtime on file,
// markUsed makes a best-effort attempt to update mtime on file,
// so that mtime reflects cache access time.
//
// Because the reflection only needs to be approximate,
Expand All @@ -354,25 +362,17 @@ const (
// mtime is more than an hour old. This heuristic eliminates
// nearly all of the mtime updates that would otherwise happen,
// while still keeping the mtimes useful for cache trimming.
func (c *DiskCache) used(file string) error {
//
// markUsed reports whether the file is a directory (an executable cache entry).
func (c *DiskCache) markUsed(file string) (isDir bool) {
info, err := os.Stat(file)
if err == nil && c.now().Sub(info.ModTime()) < mtimeInterval {
return nil
}

if err != nil {
if os.IsNotExist(err) {
return &entryNotFoundError{Err: err}
}
return &entryNotFoundError{Err: fmt.Errorf("failed to stat file %s: %w", file, err)}
return false
}

err = os.Chtimes(file, c.now(), c.now())
if err != nil {
return fmt.Errorf("failed to change time of file %s: %w", file, err)
if now := c.now(); now.Sub(info.ModTime()) >= mtimeInterval {
os.Chtimes(file, now, now)
}

return nil
return info.IsDir()
}

func (c *DiskCache) Close() error { return c.Trim() }
Expand Down Expand Up @@ -410,7 +410,7 @@ func (c *DiskCache) Trim() error {
// cache will appear older than it is, and we'll trim it again next time.
var b bytes.Buffer
fmt.Fprintf(&b, "%d", now.Unix())
if err := lockedfile.Write(filepath.Join(c.dir, "trim.txt"), &b, 0666); err != nil {
if err := lockedfile.Write(filepath.Join(c.dir, "trim.txt"), &b, 0o666); err != nil {
return err
}

Expand Down Expand Up @@ -439,6 +439,10 @@ func (c *DiskCache) trimSubdir(subdir string, cutoff time.Time) {
entry := filepath.Join(subdir, name)
info, err := os.Stat(entry)
if err == nil && info.ModTime().Before(cutoff) {
if info.IsDir() { // executable cache entry
os.RemoveAll(entry)
continue
}
os.Remove(entry)
}
}
Expand Down Expand Up @@ -471,7 +475,7 @@ func (c *DiskCache) putIndexEntry(id ActionID, out OutputID, size int64, allowVe

// Copy file to cache directory.
mode := os.O_WRONLY | os.O_CREATE
f, err := os.OpenFile(file, mode, 0666)
f, err := os.OpenFile(file, mode, 0o666)
if err != nil {
return err
}
Expand Down Expand Up @@ -517,7 +521,21 @@ func (c *DiskCache) Put(id ActionID, file io.ReadSeeker) (OutputID, int64, error
if isNoVerify {
file = wrapper.ReadSeeker
}
return c.put(id, file, !isNoVerify)
return c.put(id, "", file, !isNoVerify)
}

// PutExecutable is used to store the output as the output for the action ID into a
// file with the given base name, with the executable mode bit set.
// It may read file twice. The content of file must not change between the two passes.
func (c *DiskCache) PutExecutable(id ActionID, name string, file io.ReadSeeker) (OutputID, int64, error) {
if name == "" {
panic("PutExecutable called without a name")
}
wrapper, isNoVerify := file.(noVerifyReadSeeker)
if isNoVerify {
file = wrapper.ReadSeeker
}
return c.put(id, name, file, !isNoVerify)
}

// PutNoVerify is like Put but disables the verify check
Expand All @@ -528,7 +546,7 @@ func PutNoVerify(c Cache, id ActionID, file io.ReadSeeker) (OutputID, int64, err
return c.Put(id, noVerifyReadSeeker{file})
}

func (c *DiskCache) put(id ActionID, file io.ReadSeeker, allowVerify bool) (OutputID, int64, error) {
func (c *DiskCache) put(id ActionID, executableName string, file io.ReadSeeker, allowVerify bool) (OutputID, int64, error) {
// Compute output ID.
h := sha256.New()
if _, err := file.Seek(0, 0); err != nil {
Expand All @@ -542,7 +560,11 @@ func (c *DiskCache) put(id ActionID, file io.ReadSeeker, allowVerify bool) (Outp
h.Sum(out[:0])

// Copy to cached output file (if not already present).
if err := c.copyFile(file, out, size); err != nil {
fileMode := fs.FileMode(0o666)
if executableName != "" {
fileMode = 0o777
}
if err := c.copyFile(file, executableName, out, size, fileMode); err != nil {
return out, size, err
}

Expand All @@ -558,9 +580,33 @@ func PutBytes(c Cache, id ActionID, data []byte) error {

// copyFile copies file into the cache, expecting it to have the given
// output ID and size, if that file is not present already.
func (c *DiskCache) copyFile(file io.ReadSeeker, out OutputID, size int64) error {
name := c.fileName(out, "d")
func (c *DiskCache) copyFile(file io.ReadSeeker, executableName string, out OutputID, size int64, perm os.FileMode) error {
name := c.fileName(out, "d") // TODO(matloob): use a different suffix for the executable cache?
info, err := os.Stat(name)
if executableName != "" {
// This is an executable file. The file at name won't hold the output itself, but will
// be a directory that holds the output, named according to executableName. Check to see
// if the directory already exists, and if it does not, create it. Then reset name
// to the name we want the output written to.
if err != nil {
if !os.IsNotExist(err) {
return err
}
if err := os.Mkdir(name, 0o777); err != nil {
return err
}
if info, err = os.Stat(name); err != nil {
return err
}
}
if !info.IsDir() {
return errors.New("internal error: invalid binary cache entry: not a directory")
}

// directory exists. now set name to the inner file
name = filepath.Join(name, executableName)
info, err = os.Stat(name)
}
if err == nil && info.Size() == size {
// Check hash.
if f, err := os.Open(name); err == nil {
Expand All @@ -585,8 +631,14 @@ func (c *DiskCache) copyFile(file io.ReadSeeker, out OutputID, size int64) error
if err == nil && info.Size() > size { // shouldn't happen but fix in case
mode |= os.O_TRUNC
}
f, err := os.OpenFile(name, mode, 0666)
f, err := os.OpenFile(name, mode, perm)
if err != nil {
if base.IsETXTBSY(err) {
// This file is being used by an executable. It must have
// already been written by another go process and then run.
// return without an error.
return nil
}
return err
}
defer f.Close()
Expand Down
33 changes: 6 additions & 27 deletions internal/go/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,14 @@ func init() {
}

func TestBasic(t *testing.T) {
dir, err := os.MkdirTemp("", "cachetest-")
if err != nil {
t.Fatal(err)
}
defer os.RemoveAll(dir)
_, err = Open(filepath.Join(dir, "notexist"))
dir := t.TempDir()
_, err := Open(filepath.Join(dir, "notexist"))
if err == nil {
t.Fatal(`Open("tmp/notexist") succeeded, want failure`)
}

cdir := filepath.Join(dir, "c1")
if err := os.Mkdir(cdir, 0744); err != nil {
if err := os.Mkdir(cdir, 0777); err != nil {
t.Fatal(err)
}

Expand Down Expand Up @@ -66,13 +62,7 @@ func TestBasic(t *testing.T) {
}

func TestGrowth(t *testing.T) {
dir, err := os.MkdirTemp("", "cachetest-")
if err != nil {
t.Fatal(err)
}
defer os.RemoveAll(dir)

c, err := Open(dir)
c, err := Open(t.TempDir())
if err != nil {
t.Fatalf("Open: %v", err)
}
Expand Down Expand Up @@ -119,13 +109,7 @@ func TestGrowth(t *testing.T) {
// t.Fatal("initEnv did not set verify")
// }
//
// dir, err := os.MkdirTemp("", "cachetest-")
// if err != nil {
// t.Fatal(err)
// }
// defer os.RemoveAll(dir)
//
// c, err := Open(dir)
// c, err := Open(t.TempDir())
// if err != nil {
// t.Fatalf("Open: %v", err)
// }
Expand All @@ -152,12 +136,7 @@ func dummyID(x int) [HashSize]byte {
}

func TestCacheTrim(t *testing.T) {
dir, err := os.MkdirTemp("", "cachetest-")
if err != nil {
t.Fatal(err)
}
defer os.RemoveAll(dir)

dir := t.TempDir()
c, err := Open(dir)
if err != nil {
t.Fatalf("Open: %v", err)
Expand Down
Loading
Loading