index: let MasterIndex.Save also delete obsolete indexes

internal/index/master_index.go

@@ -9,7 +9,6 @@ import (
 
 	"github.com/restic/restic/internal/debug"
 	"github.com/restic/restic/internal/restic"
-	"github.com/restic/restic/internal/ui/progress"
 	"golang.org/x/sync/errgroup"
 )
 
@@ -267,23 +266,22 @@ func (mi *MasterIndex) MergeFinalIndexes() error {
 
 // Save saves all known indexes to index files, leaving out any
 // packs whose ID is contained in packBlacklist from finalized indexes.
-// The new index contains the IDs of all known indexes in the "supersedes"
-// field. The IDs are also returned in the IDSet obsolete.
-// After calling this function, you should remove the obsolete index files.
-func (mi *MasterIndex) Save(ctx context.Context, repo restic.SaverUnpacked, packBlacklist restic.IDSet, extraObsolete restic.IDs, p *progress.Counter) (obsolete restic.IDSet, err error) {
-	p.SetMax(uint64(len(mi.Packs(packBlacklist))))
+// It also removes the old index files and those listed in extraObsolete.
+func (mi *MasterIndex) Save(ctx context.Context, repo restic.Repository, excludePacks restic.IDSet, extraObsolete restic.IDs, opts restic.MasterIndexSaveOpts) error {
+	p := opts.SaveProgress
+	p.SetMax(uint64(len(mi.Packs(excludePacks))))
 
 	mi.idxMutex.Lock()
 	defer mi.idxMutex.Unlock()
 
-	debug.Log("start rebuilding index of %d indexes, pack blacklist: %v", len(mi.idx), packBlacklist)
+	debug.Log("start rebuilding index of %d indexes, excludePacks: %v", len(mi.idx), excludePacks)
 
 	newIndex := NewIndex()
-	obsolete = restic.NewIDSet()
+	obsolete := restic.NewIDSet()
 
 	// track spawned goroutines using wg, create a new context which is
 	// cancelled as soon as an error occurs.
-	wg, ctx := errgroup.WithContext(ctx)
+	wg, wgCtx := errgroup.WithContext(ctx)
 
 	ch := make(chan *Index)
 
@@ -310,21 +308,21 @@ func (mi *MasterIndex) Save(ctx context.Context, repo restic.SaverUnpacked, pack
 
 			debug.Log("adding index %d", i)
 
-			for pbs := range idx.EachByPack(ctx, packBlacklist) {
+			for pbs := range idx.EachByPack(wgCtx, excludePacks) {
 				newIndex.StorePack(pbs.PackID, pbs.Blobs)
 				p.Add(1)
 				if IndexFull(newIndex, mi.compress) {
 					select {
 					case ch <- newIndex:
-					case <-ctx.Done():
-						return ctx.Err()
+					case <-wgCtx.Done():
+						return wgCtx.Err()
 					}
 					newIndex = NewIndex()
 				}
 			}
 		}
 
-		err = newIndex.AddToSupersedes(extraObsolete...)
+		err := newIndex.AddToSupersedes(extraObsolete...)
 		if err != nil {
 			return err
 		}
@@ -332,7 +330,7 @@ func (mi *MasterIndex) Save(ctx context.Context, repo restic.SaverUnpacked, pack
 
 		select {
 		case ch <- newIndex:
-		case <-ctx.Done():
+		case <-wgCtx.Done():
 		}
 		return nil
 	})
@@ -341,7 +339,7 @@ func (mi *MasterIndex) Save(ctx context.Context, repo restic.SaverUnpacked, pack
 	worker := func() error {
 		for idx := range ch {
 			idx.Finalize()
-			if _, err := SaveIndex(ctx, repo, idx); err != nil {
+			if _, err := SaveIndex(wgCtx, repo, idx); err != nil {
 				return err
 			}
 		}
@@ -354,9 +352,27 @@ func (mi *MasterIndex) Save(ctx context.Context, repo restic.SaverUnpacked, pack
 	for i := 0; i < workerCount; i++ {
 		wg.Go(worker)
 	}
-	err = wg.Wait()
+	err := wg.Wait()
+	p.Done()
+	if err != nil {
+		return err
+	}
 
-	return obsolete, err
+	if opts.SkipDeletion {
+		return nil
+	}
+
+	p = nil
+	if opts.DeleteProgress != nil {
+		p = opts.DeleteProgress()
+	}
+	defer p.Done()
+	return restic.ParallelRemove(ctx, repo, obsolete, restic.IndexFile, func(id restic.ID, err error) error {
+		if opts.DeleteReport != nil {
+			opts.DeleteReport(id, err)
+		}
+		return err
+	}, p)
 }
 
 // SaveIndex saves an index in the repository.

internal/index/master_index_test.go

@@ -8,7 +8,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/restic/restic/internal/backend"
 	"github.com/restic/restic/internal/checker"
 	"github.com/restic/restic/internal/crypto"
 	"github.com/restic/restic/internal/index"
@@ -363,20 +362,11 @@ func testIndexSave(t *testing.T, version uint) {
 		t.Fatal(err)
 	}
 
-	obsoletes, err := repo.Index().Save(context.TODO(), repo, nil, nil, nil)
+	err = repo.Index().Save(context.TODO(), repo, nil, nil, restic.MasterIndexSaveOpts{})
 	if err != nil {
 		t.Fatalf("unable to save new index: %v", err)
 	}
 
-	for id := range obsoletes {
-		t.Logf("remove index %v", id.Str())
-		h := backend.Handle{Type: restic.IndexFile, Name: id.String()}
-		err = repo.Backend().Remove(context.TODO(), h)
-		if err != nil {
-			t.Errorf("error removing index %v: %v", id, err)
-		}
-	}
-
 	checker := checker.New(repo, false)
 	err = checker.LoadSnapshots(context.TODO())
 	if err != nil {

internal/repository/repack_test.go

@@ -173,39 +173,27 @@ func flush(t *testing.T, repo restic.Repository) {
 
 func rebuildIndex(t *testing.T, repo restic.Repository) {
 	err := repo.SetIndex(index.NewMasterIndex())
-	if err != nil {
-		t.Fatal(err)
-	}
+	rtest.OK(t, err)
 
 	packs := make(map[restic.ID]int64)
 	err = repo.List(context.TODO(), restic.PackFile, func(id restic.ID, size int64) error {
 		packs[id] = size
 		return nil
 	})
-	if err != nil {
-		t.Fatal(err)
-	}
+	rtest.OK(t, err)
 
 	_, err = repo.(*repository.Repository).CreateIndexFromPacks(context.TODO(), packs, nil)
-	if err != nil {
-		t.Fatal(err)
-	}
+	rtest.OK(t, err)
 
+	var obsoleteIndexes restic.IDs
 	err = repo.List(context.TODO(), restic.IndexFile, func(id restic.ID, size int64) error {
-		h := backend.Handle{
-			Type: restic.IndexFile,
-			Name: id.String(),
-		}
-		return repo.Backend().Remove(context.TODO(), h)
+		obsoleteIndexes = append(obsoleteIndexes, id)
+		return nil
 	})
-	if err != nil {
-		t.Fatal(err)
-	}
+	rtest.OK(t, err)
 
-	_, err = repo.Index().Save(context.TODO(), repo, restic.NewIDSet(), nil, nil)
-	if err != nil {
-		t.Fatal(err)
-	}
+	err = repo.Index().Save(context.TODO(), repo, restic.NewIDSet(), obsoleteIndexes, restic.MasterIndexSaveOpts{})
+	rtest.OK(t, err)
 }
 
 func reloadIndex(t *testing.T, repo restic.Repository) {

internal/restic/repository.go

@@ -89,6 +89,13 @@ type PackBlobs struct {
 	Blobs  []Blob
 }
 
+type MasterIndexSaveOpts struct {
+	SaveProgress   *progress.Counter
+	DeleteProgress func() *progress.Counter
+	DeleteReport   func(id ID, err error)
+	SkipDeletion   bool
+}
+
 // MasterIndex keeps track of the blobs are stored within files.
 type MasterIndex interface {
 	Has(BlobHandle) bool
@@ -99,7 +106,7 @@ type MasterIndex interface {
 	Each(ctx context.Context, fn func(PackedBlob))
 	ListPacks(ctx context.Context, packs IDSet) <-chan PackBlobs
 
-	Save(ctx context.Context, repo SaverUnpacked, packBlacklist IDSet, extraObsolete IDs, p *progress.Counter) (obsolete IDSet, err error)
+	Save(ctx context.Context, repo Repository, excludePacks IDSet, extraObsolete IDs, opts MasterIndexSaveOpts) error
 }
 
 // Lister allows listing files in a backend.