feat(llard): reuse workspace artifacts before Kodo

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"),

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()