fix(make): prefer existing local artifacts

cmd/llar/internal/make.go

@@ -43,19 +43,31 @@ var newRemoteStore = func() (repo.Store, error) {
 // publicKodoDomain serves published build artifacts without credentials.
 var publicKodoDomain = "https://llarpackages.xgo.dev"
 
-// readThroughCache reads published artifacts from remote and falls back to the
-// local workspace cache, which also stores locally built artifacts.
+// readThroughCache reuses artifacts in the local workspace before fetching
+// published artifacts from remote. Remote hits are indexed in the workspace
+// cache so later commands can reuse the restored artifact without downloading
+// it again. Locally built artifacts are written only to the workspace cache.
 type readThroughCache struct {
 	remote buildcache.Cache
 	local  buildcache.Cache
 }
 
 func (c readThroughCache) Get(ctx context.Context, key buildcache.Key) (buildcache.Entry, bool, error) {
-	entry, ok, err := c.remote.Get(ctx, key)
+	entry, ok, err := c.local.Get(ctx, key)
 	if err != nil || ok {
 		return entry, ok, err
 	}
-	return c.local.Get(ctx, key)
+	entry, ok, err = c.remote.Get(ctx, key)
+	if err != nil || !ok {
+		return entry, ok, err
+	}
+	// The remote cache 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 buildcache.Entry{}, false, err
+	}
+	return entry, true, nil
 }
 
 func (c readThroughCache) Put(ctx context.Context, key buildcache.Key, output fs.FS, entry buildcache.Entry) (buildcache.Entry, error) {

cmd/llar/internal/make_test.go

@@ -14,13 +14,15 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"reflect"
 	"runtime"
 	"strings"
 	"testing"
 	"time"
 
 	"github.com/goplus/llar/formula"
 	"github.com/goplus/llar/internal/artifact/archiver"
+	"github.com/goplus/llar/internal/build"
 	buildcache "github.com/goplus/llar/internal/build/cache"
 	"github.com/goplus/llar/internal/execbroker"
 	"github.com/goplus/llar/internal/formula/repo"
@@ -444,55 +446,141 @@ func TestMakeLocal_RealDemoWithRemoteZlibDep(t *testing.T) {
 }
 
 type fakeCache struct {
-	entry buildcache.Entry
-	hit   bool
-	err   error
-	puts  int
+	entry  buildcache.Entry
+	hit    bool
+	err    error
+	gets   int
+	putErr error
+	puts   int
 }
 
 func (c *fakeCache) Get(context.Context, buildcache.Key) (buildcache.Entry, bool, error) {
+	c.gets++
 	return c.entry, c.hit, c.err
 }
 
 func (c *fakeCache) Put(_ context.Context, _ buildcache.Key, _ fs.FS, entry buildcache.Entry) (buildcache.Entry, error) {
 	c.puts++
+	if c.putErr != nil {
+		return buildcache.Entry{}, c.putErr
+	}
 	return entry, nil
 }
 
 func TestReadThroughCache(t *testing.T) {
-	remote := &fakeCache{}
-	local := &fakeCache{}
-	c := readThroughCache{remote: remote, local: local}
 	key := buildcache.Key{Module: module.Version{Path: "test/liba", Version: "1.0.0"}, Matrix: "amd64-linux"}
 	ctx := context.Background()
-
-	remote.entry = buildcache.Entry{Metadata: "-remote"}
-	remote.hit = true
-	if entry, ok, err := c.Get(ctx, key); err != nil || !ok || entry.Metadata != "-remote" {
-		t.Fatalf("remote hit Get = %+v, %v, %v", entry, ok, err)
+	localErr := errors.New("local failed")
+	localPutErr := errors.New("local put failed")
+	remoteErr := errors.New("remote failed")
+	tests := []struct {
+		name       string
+		local      *fakeCache
+		remote     *fakeCache
+		want       buildcache.Entry
+		wantOK     bool
+		wantErr    error
+		localPuts  int
+		remoteGets int
+	}{
+		{
+			name:       "local hit skips remote",
+			local:      &fakeCache{entry: buildcache.Entry{Metadata: "-local"}, hit: true},
+			remote:     &fakeCache{entry: buildcache.Entry{Metadata: "-remote"}, hit: true},
+			want:       buildcache.Entry{Metadata: "-local"},
+			wantOK:     true,
+			remoteGets: 0,
+		},
+		{
+			name:       "local error skips remote",
+			local:      &fakeCache{err: localErr},
+			remote:     &fakeCache{entry: buildcache.Entry{Metadata: "-remote"}, hit: true},
+			wantErr:    localErr,
+			remoteGets: 0,
+		},
+		{
+			name:       "remote hit after local miss",
+			local:      &fakeCache{},
+			remote:     &fakeCache{entry: buildcache.Entry{Metadata: "-remote"}, hit: true},
+			want:       buildcache.Entry{Metadata: "-remote"},
+			wantOK:     true,
+			localPuts:  1,
+			remoteGets: 1,
+		},
+		{
+			name:       "local write error after remote hit",
+			local:      &fakeCache{putErr: localPutErr},
+			remote:     &fakeCache{entry: buildcache.Entry{Metadata: "-remote"}, hit: true},
+			wantErr:    localPutErr,
+			localPuts:  1,
+			remoteGets: 1,
+		},
+		{
+			name:       "remote error after local miss",
+			local:      &fakeCache{},
+			remote:     &fakeCache{err: remoteErr},
+			wantErr:    remoteErr,
+			remoteGets: 1,
+		},
+		{
+			name:       "miss",
+			local:      &fakeCache{},
+			remote:     &fakeCache{},
+			remoteGets: 1,
+		},
 	}
-
-	remote.err = errors.New("remote failed")
-	if _, _, err := c.Get(ctx, key); !errors.Is(err, remote.err) {
-		t.Fatalf("remote error = %v, want %v", err, remote.err)
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := readThroughCache{remote: tt.remote, local: tt.local}
+			entry, ok, err := c.Get(ctx, key)
+			if !errors.Is(err, tt.wantErr) || ok != tt.wantOK || !reflect.DeepEqual(entry, tt.want) {
+				t.Fatalf("Get = %+v, %v, %v; want %+v, %v, %v", entry, ok, err, tt.want, tt.wantOK, tt.wantErr)
+			}
+			if tt.local.gets != 1 || tt.remote.gets != tt.remoteGets {
+				t.Fatalf("Get calls = local:%d remote:%d, want local:1 remote:%d", tt.local.gets, tt.remote.gets, tt.remoteGets)
+			}
+			if tt.local.puts != tt.localPuts {
+				t.Fatalf("local Put calls = %d, want %d", tt.local.puts, tt.localPuts)
+			}
+		})
 	}
-	remote.err = nil
 
-	remote.hit = false
-	local.entry = buildcache.Entry{Metadata: "-local"}
-	local.hit = true
-	if entry, ok, err := c.Get(ctx, key); err != nil || !ok || entry.Metadata != "-local" {
-		t.Fatalf("local hit Get = %+v, %v, %v", entry, ok, err)
+	t.Run("put writes only local", func(t *testing.T) {
+		remote := &fakeCache{}
+		local := &fakeCache{}
+		c := readThroughCache{remote: remote, local: local}
+		entry, err := c.Put(ctx, key, nil, buildcache.Entry{Metadata: "-built"})
+		if err != nil || entry.Metadata != "-built" || local.puts != 1 || remote.puts != 0 {
+			t.Fatalf("Put = %+v, %v; puts = local:%d remote:%d", entry, err, local.puts, remote.puts)
+		}
+	})
+}
+
+func TestReadThroughCachePersistsRemoteHit(t *testing.T) {
+	key := buildcache.Key{Module: module.Version{Path: "test/liba", Version: "1.0.0"}, Matrix: "amd64-linux"}
+	remote := &fakeCache{entry: buildcache.Entry{Metadata: "-remote"}, hit: true}
+	workspaceDir := t.TempDir()
+	c := readThroughCache{
+		remote: remote,
+		local:  build.NewLocalCache(workspaceDir),
 	}
 
-	local.hit = false
-	if _, ok, err := c.Get(ctx, key); err != nil || ok {
-		t.Fatalf("miss Get = ok:%v err:%v", ok, err)
+	entry, ok, err := c.Get(context.Background(), key)
+	if err != nil || !ok || entry.Metadata != "-remote" {
+		t.Fatalf("first Get = %+v, %v, %v", entry, ok, err)
+	}
+	if _, err := os.Stat(filepath.Join(workspaceDir, "test", "liba", ".cache.json")); err != nil {
+		t.Fatalf("local cache entry was not persisted: %v", err)
 	}
 
-	entry, err := c.Put(ctx, key, nil, buildcache.Entry{Metadata: "-built"})
-	if err != nil || entry.Metadata != "-built" || local.puts != 1 {
-		t.Fatalf("Put = %+v, %v; local puts = %d", entry, err, local.puts)
+	for i := 0; i < 2; i++ {
+		entry, ok, err := c.Get(context.Background(), key)
+		if err != nil || !ok || entry.Metadata != "-remote" {
+			t.Fatalf("cached Get %d = %+v, %v, %v", i+1, entry, ok, err)
+		}
+	}
+	if remote.gets != 1 {
+		t.Fatalf("remote Get calls = %d, want 1", remote.gets)
 	}
 }