Remove unneeded Finalize method from ImmutableRef.

Finalize was only used outside the cache package in one place, which
called it with the commit arg set to false. The code path followed
when commit==false turned out to essentially be a no-op because
it set "retain cache" to true if it was already set to true.

It was thus safe to remove the only external call to it and remove it
from the interface. This should be helpful for future efforts to
simplify the equal{Mutable,Immutable} fields in cacheRecord, which exist
due to the "lazy commit" feature that Finalize is tied into.

Signed-off-by: Erik Sipsma <[email protected]>

cache/blobs.go

@@ -31,7 +31,7 @@ func (sr *immutableRef) computeBlobChain(ctx context.Context, createIfNeeded boo
 		return errors.Errorf("missing lease requirement for computeBlobChain")
 	}
 
-	if err := sr.Finalize(ctx, true); err != nil {
+	if err := sr.finalizeLocked(ctx); err != nil {
 		return err
 	}
 
@@ -174,7 +174,7 @@ func (sr *immutableRef) setBlob(ctx context.Context, desc ocispec.Descriptor) er
 		return nil
 	}
 
-	if err := sr.finalize(ctx, true); err != nil {
+	if err := sr.finalize(ctx); err != nil {
 		return err
 	}
 

cache/manager.go

@@ -128,11 +128,11 @@ func (cm *cacheManager) GetByBlob(ctx context.Context, desc ocispec.Descriptor,
 		if err != nil {
 			return nil, err
 		}
-		if err := p2.Finalize(ctx, true); err != nil {
+		p = p2.(*immutableRef)
+		if err := p.finalizeLocked(ctx); err != nil {
 			return nil, err
 		}
-		parentID = p2.ID()
-		p = p2.(*immutableRef)
+		parentID = p.ID()
 	}
 
 	releaseParent := false
@@ -475,7 +475,7 @@ func (cm *cacheManager) New(ctx context.Context, s ImmutableRef, sess session.Gr
 			}
 			parent = p.(*immutableRef)
 		}
-		if err := parent.Finalize(ctx, true); err != nil {
+		if err := parent.finalizeLocked(ctx); err != nil {
 			return nil, err
 		}
 		if err := parent.Extract(ctx, sess); err != nil {

cache/manager_test.go

@@ -209,7 +209,7 @@ func TestManager(t *testing.T) {
 
 	checkDiskUsage(ctx, t, cm, 1, 0)
 
-	err = snap.Finalize(ctx, true)
+	err = snap.(*immutableRef).finalizeLocked(ctx)
 	require.NoError(t, err)
 
 	err = snap.Release(ctx)
@@ -867,7 +867,7 @@ func TestLazyCommit(t *testing.T) {
 	require.NoError(t, err)
 
 	// this time finalize commit
-	err = snap.Finalize(ctx, true)
+	err = snap.(*immutableRef).finalizeLocked(ctx)
 	require.NoError(t, err)
 
 	err = snap.Release(ctx)
@@ -941,7 +941,7 @@ func TestLazyCommit(t *testing.T) {
 	snap2, err = cm.Get(ctx, snap.ID())
 	require.NoError(t, err)
 
-	err = snap2.Finalize(ctx, true)
+	err = snap2.(*immutableRef).finalizeLocked(ctx)
 	require.NoError(t, err)
 
 	err = snap2.Release(ctx)

cache/refs.go

@@ -41,7 +41,6 @@ type Ref interface {
 type ImmutableRef interface {
 	Ref
 	Parent() ImmutableRef
-	Finalize(ctx context.Context, commit bool) error // Make sure reference is flushed to driver
 	Clone() ImmutableRef
 
 	Info() RefInfo
@@ -255,7 +254,7 @@ func (cr *cacheRecord) mount(ctx context.Context, readonly bool) (snapshot.Mount
 		return setReadonly(m), nil
 	}
 
-	if err := cr.finalize(ctx, true); err != nil {
+	if err := cr.finalize(ctx); err != nil {
 		return nil, err
 	}
 	if cr.viewMount == nil { // TODO: handle this better
@@ -717,29 +716,22 @@ func (sr *immutableRef) release(ctx context.Context) error {
 	return nil
 }
 
-func (sr *immutableRef) Finalize(ctx context.Context, b bool) error {
+func (sr *immutableRef) finalizeLocked(ctx context.Context) error {
 	sr.mu.Lock()
 	defer sr.mu.Unlock()
-
-	return sr.finalize(ctx, b)
+	return sr.finalize(ctx)
 }
 
 func (cr *cacheRecord) Metadata() *metadata.StorageItem {
 	return cr.md
 }
 
-func (cr *cacheRecord) finalize(ctx context.Context, commit bool) error {
+// caller must hold cacheRecord.mu
+func (cr *cacheRecord) finalize(ctx context.Context) error {
 	mutable := cr.equalMutable
 	if mutable == nil {
 		return nil
 	}
-	if !commit {
-		if HasCachePolicyRetain(mutable) {
-			CachePolicyRetain(mutable)
-			return mutable.Metadata().Commit()
-		}
-		return nil
-	}
 
 	_, err := cr.cm.ManagerOpt.LeaseManager.Create(ctx, func(l *leases.Lease) error {
 		l.ID = cr.ID()

solver/llbsolver/bridge.go

@@ -105,16 +105,7 @@ func (b *llbBridge) loadResult(ctx context.Context, def *pb.Definition, cacheImp
 	if err != nil {
 		return nil, err
 	}
-	wr, ok := res.Sys().(*worker.WorkerRef)
-	if !ok {
-		return nil, errors.Errorf("invalid reference for exporting: %T", res.Sys())
-	}
-	if wr.ImmutableRef != nil {
-		if err := wr.ImmutableRef.Finalize(ctx, false); err != nil {
-			return nil, err
-		}
-	}
-	return res, err
+	return res, nil
 }
 
 func (b *llbBridge) Solve(ctx context.Context, req frontend.SolveRequest, sid string) (res *frontend.Result, err error) {