diff --git a/cmd/llard/main.go b/cmd/llard/main.go index 564b8c80..7ca14761 100644 --- a/cmd/llard/main.go +++ b/cmd/llard/main.go @@ -8,6 +8,7 @@ import ( "context" "errors" "fmt" + "io/fs" "log" "net/http" "os" @@ -16,10 +17,12 @@ import ( "syscall" "github.com/goplus/llar/internal/artifact" + "github.com/goplus/llar/internal/build" "github.com/goplus/llar/internal/build/cache" buildhttp "github.com/goplus/llar/internal/build/http" "github.com/goplus/llar/internal/formula/repo" "github.com/goplus/llar/internal/vcs" + "github.com/goplus/llar/mod/module" "github.com/joho/godotenv" ) @@ -73,15 +76,23 @@ func run() error { Bucket: cfg.bucket, Prefix: cfg.prefix, }) - buildCache := cache.NewKodo(cache.KodoConfig{ - AccessKey: cfg.accessKey, - SecretKey: cfg.secretKey, - Bucket: cfg.bucket, - PublicDomain: cfg.publicDomain, - Prefix: cfg.prefix, - WorkspaceDir: workspaceDir, - Artifacts: artifacts, - }) + // Reuse artifacts already present in the workspace before downloading them + // from Kodo. The workspace is evictable, so Kodo remains the source of + // truth and restores anything missing back into the workspace. + buildCache := readThroughCache{ + local: build.NewLocalCache(workspaceDir), + artifacts: artifacts, + workspaceDir: workspaceDir, + remote: cache.NewKodo(cache.KodoConfig{ + AccessKey: cfg.accessKey, + SecretKey: cfg.secretKey, + Bucket: cfg.bucket, + PublicDomain: cfg.publicDomain, + Prefix: cfg.prefix, + WorkspaceDir: workspaceDir, + Artifacts: artifacts, + }), + } handler := buildhttp.New(buildhttp.Options{ FormulaStore: formulaStore, Cache: buildCache, @@ -110,6 +121,88 @@ func run() error { return nil } +// readThroughCache reuses artifacts already present in the local workspace +// before fetching them from the remote store. The local workspace is only a +// best-effort cache: the remote artifact record is the source of truth, so a +// deleted record invalidates any local copy, a local miss falls back to the +// remote store, and a remote hit is persisted back into the local cache for +// later reads. +// +// This is a deliberate copy of the llar client's read-through cache: llard and +// the client evolve separately and must not share an abstraction. It differs by +// gating local hits on the remote artifact record, which the client does not +// need because it never deletes published artifacts. +type readThroughCache struct { + local cache.Cache + remote cache.Cache + artifacts artifact.Store + workspaceDir string +} + +func (c readThroughCache) Get(ctx context.Context, key cache.Key) (cache.Entry, bool, error) { + // The remote artifact record is the source of truth: when it is deleted, + // any local copy is stale and must not be used. This is a metadata-only + // lookup, so a local hit still avoids the artifact download. + if _, err := c.artifacts.Get(ctx, artifact.Key{ + Module: key.Module.Path, + Version: key.Module.Version, + MatrixStr: key.Matrix, + }); err != nil { + if errors.Is(err, artifact.ErrNotFound) { + // The artifact was deleted remotely: drop the local install tree + // so the rebuild cannot mix stale files with the new build. + if err := c.removeInstallDir(key); err != nil { + return cache.Entry{}, false, err + } + return cache.Entry{}, false, nil + } + return cache.Entry{}, false, err + } + entry, ok, err := c.local.Get(ctx, key) + if err != nil || ok { + return entry, ok, err + } + entry, ok, err = c.remote.Get(ctx, key) + if err != nil || !ok { + return entry, ok, err + } + // The remote store already restored the artifact into the shared + // workspace, so the local cache only needs to persist its entry. + entry, err = c.local.Put(ctx, key, nil, entry) + if err != nil { + return cache.Entry{}, false, err + } + return entry, true, nil +} + +func (c readThroughCache) Put(ctx context.Context, key cache.Key, output fs.FS, entry cache.Entry) (cache.Entry, error) { + // Publishing is authoritative: upload and record the artifact remotely + // first. When another llard already published it, this fails and the local + // copy must not be cached; the next Get restores the canonical artifact. + stored, err := c.remote.Put(ctx, key, output, entry) + if err != nil { + return cache.Entry{}, err + } + // Cache the authoritative entry locally. A local write failure only costs + // a future restore, so it must not fail the build. + _, _ = c.local.Put(ctx, key, output, stored) + return stored, nil +} + +// removeInstallDir drops the workspace install tree for key. The remote artifact +// record is gone, so the local copy is stale and a rebuild must start clean. +func (c readThroughCache) removeInstallDir(key cache.Key) error { + if c.workspaceDir == "" { + return nil + } + escaped, err := module.EscapePath(key.Module.Path) + if err != nil { + return err + } + installDir := filepath.Join(c.workspaceDir, fmt.Sprintf("%s@%s-%s", escaped, key.Module.Version, key.Matrix)) + return os.RemoveAll(installDir) +} + func loadConfig() (config, error) { cfg := config{ addr: os.Getenv("LLARD_ADDR"), diff --git a/cmd/llard/main_test.go b/cmd/llard/main_test.go index c56884d0..3e558c4b 100644 --- a/cmd/llard/main_test.go +++ b/cmd/llard/main_test.go @@ -5,9 +5,19 @@ package main import ( + "context" + "errors" + "io/fs" "os" + "path/filepath" + "reflect" "strings" "testing" + + "github.com/goplus/llar/internal/artifact" + "github.com/goplus/llar/internal/build" + "github.com/goplus/llar/internal/build/cache" + "github.com/goplus/llar/mod/module" ) func TestLoadConfig(t *testing.T) { @@ -99,6 +109,173 @@ func TestRunRequiresConfig(t *testing.T) { } } +func TestReadThroughCache_LocalHitSkipsRemote(t *testing.T) { + workspaceDir := t.TempDir() + local := build.NewLocalCache(workspaceDir) + key := cache.Key{Module: module.Version{Path: "madler/zlib", Version: "v1.3.1"}, Matrix: "amd64-linux"} + if _, err := local.Put(context.Background(), key, nil, cache.Entry{Metadata: "-local"}); err != nil { + t.Fatal(err) + } + + remote := &countingCache{} + c := readThroughCache{local: local, remote: remote, artifacts: presentArtifacts(key)} + entry, ok, err := c.Get(context.Background(), key) + if err != nil || !ok || entry.Metadata != "-local" { + t.Fatalf("Get() = %+v, %v, %v; want local hit", entry, ok, err) + } + if remote.gets != 0 { + t.Fatalf("remote Get calls = %d, want 0", remote.gets) + } +} + +func TestReadThroughCache_PersistsRemoteHit(t *testing.T) { + workspaceDir := t.TempDir() + local := build.NewLocalCache(workspaceDir) + key := cache.Key{Module: module.Version{Path: "madler/zlib", Version: "v1.3.1"}, Matrix: "amd64-linux"} + remote := &countingCache{entry: cache.Entry{Metadata: "-remote"}, hit: true} + + c := readThroughCache{local: local, remote: remote, artifacts: presentArtifacts(key)} + for i := 0; i < 2; i++ { + entry, ok, err := c.Get(context.Background(), key) + if err != nil || !ok || entry.Metadata != "-remote" { + t.Fatalf("Get() #%d = %+v, %v, %v", i+1, entry, ok, err) + } + } + if remote.gets != 1 { + t.Fatalf("remote Get calls = %d, want 1", remote.gets) + } +} + +// TestReadThroughCache_RecordMissingInvalidatesLocal verifies that deleting the +// authoritative artifact record invalidates a local entry: the next Get drops +// the install tree, misses (so the build runs again), and does not consult the +// remote store. +func TestReadThroughCache_RecordMissingInvalidatesLocal(t *testing.T) { + workspaceDir := t.TempDir() + local := build.NewLocalCache(workspaceDir) + key := cache.Key{Module: module.Version{Path: "madler/zlib", Version: "v1.3.1"}, Matrix: "amd64-linux"} + if _, err := local.Put(context.Background(), key, nil, cache.Entry{Metadata: "-local"}); err != nil { + t.Fatal(err) + } + installDir := filepath.Join(workspaceDir, "madler", "zlib@v1.3.1-amd64-linux") + if err := os.MkdirAll(installDir, 0o755); err != nil { + t.Fatal(err) + } + + remote := &countingCache{entry: cache.Entry{Metadata: "-remote"}, hit: true} + c := readThroughCache{local: local, remote: remote, artifacts: &fakeArtifacts{}, workspaceDir: workspaceDir} + if _, ok, err := c.Get(context.Background(), key); err != nil || ok { + t.Fatalf("Get() = %v, %v; want miss after record deletion", ok, err) + } + if remote.gets != 0 { + t.Fatalf("remote Get calls = %d, want 0", remote.gets) + } + if _, err := os.Stat(installDir); !errors.Is(err, fs.ErrNotExist) { + t.Fatalf("install dir still present after record deletion: %v", err) + } +} + +// TestReadThroughCache_PutOrdersRemoteThenLocal pins the write order: the +// artifact is published remotely before the local entry is written. +func TestReadThroughCache_PutOrdersRemoteThenLocal(t *testing.T) { + var order []string + c := readThroughCache{ + local: &orderCache{name: "local", order: &order}, + remote: &orderCache{name: "remote", order: &order}, + } + key := cache.Key{Module: module.Version{Path: "madler/zlib", Version: "v1.3.1"}, Matrix: "amd64-linux"} + + if _, err := c.Put(context.Background(), key, nil, cache.Entry{Metadata: "-built"}); err != nil { + t.Fatalf("Put() failed: %v", err) + } + if !reflect.DeepEqual(order, []string{"remote", "local"}) { + t.Fatalf("Put order = %v, want [remote local]", order) + } +} + +// TestReadThroughCache_PutSkipsLocalWhenRemoteFails verifies that a remote +// publish failure does not leave a divergent local entry behind: the next Get +// must restore the canonical artifact from the remote store. +func TestReadThroughCache_PutSkipsLocalWhenRemoteFails(t *testing.T) { + var order []string + remoteErr := errors.New("remote put failed") + c := readThroughCache{ + local: &orderCache{name: "local", order: &order}, + remote: &orderCache{name: "remote", order: &order, err: remoteErr}, + } + key := cache.Key{Module: module.Version{Path: "madler/zlib", Version: "v1.3.1"}, Matrix: "amd64-linux"} + + if _, err := c.Put(context.Background(), key, nil, cache.Entry{Metadata: "-built"}); !errors.Is(err, remoteErr) { + t.Fatalf("Put() error = %v, want %v", err, remoteErr) + } + if len(order) != 1 || order[0] != "remote" { + t.Fatalf("Put order = %v, want [remote] only", order) + } +} + +type orderCache struct { + name string + order *[]string + err error +} + +func (c *orderCache) Get(context.Context, cache.Key) (cache.Entry, bool, error) { + return cache.Entry{}, false, nil +} + +func (c *orderCache) Put(context.Context, cache.Key, fs.FS, cache.Entry) (cache.Entry, error) { + *c.order = append(*c.order, c.name) + return cache.Entry{}, c.err +} + +type countingCache struct { + gets int + puts int + entry cache.Entry + hit bool + err error +} + +func (c *countingCache) Get(context.Context, cache.Key) (cache.Entry, bool, error) { + c.gets++ + return c.entry, c.hit, c.err +} + +func (c *countingCache) Put(context.Context, cache.Key, fs.FS, cache.Entry) (cache.Entry, error) { + c.puts++ + return cache.Entry{}, nil +} + +func artifactRecordKey(key cache.Key) string { + return key.Module.Path + "@" + key.Module.Version + "?" + key.Matrix +} + +// presentArtifacts returns an artifact store holding the record for key. +func presentArtifacts(key cache.Key) *fakeArtifacts { + return &fakeArtifacts{record: map[string]artifact.Artifact{artifactRecordKey(key): {}}} +} + +type fakeArtifacts struct { + record map[string]artifact.Artifact + err error +} + +func (f *fakeArtifacts) Get(_ context.Context, key artifact.Key) (artifact.Artifact, error) { + if f.err != nil { + return artifact.Artifact{}, f.err + } + if a, ok := f.record[key.Module+"@"+key.Version+"?"+key.MatrixStr]; ok { + return a, nil + } + return artifact.Artifact{}, artifact.ErrNotFound +} + +func (f *fakeArtifacts) Put(context.Context, artifact.Key, artifact.Artifact) (artifact.Artifact, error) { + return artifact.Artifact{}, nil +} + +func (f *fakeArtifacts) Delete(context.Context, artifact.Key) error { return nil } + func TestRunRejectsInvalidAddress(t *testing.T) { t.Chdir(t.TempDir()) cacheDir := t.TempDir()