prune: Delete repacked files as the very last step

src/cmds/restic/cmd_prune.go

@@ -216,10 +216,11 @@ func pruneRepository(gopts GlobalOptions, repo restic.Repository) error {
 	Verbosef("will delete %d packs and rewrite %d packs, this frees %s\n",
 		len(removePacks), len(rewritePacks), formatBytes(uint64(removeBytes)))
 
+	var repackedBlobs restic.IDSet
 	if len(rewritePacks) != 0 {
 		bar = newProgressMax(!gopts.Quiet, uint64(len(rewritePacks)), "packs rewritten")
 		bar.Start()
-		err = repository.Repack(ctx, repo, rewritePacks, usedBlobs, bar)
+		repackedBlobs, err = repository.Repack(ctx, repo, rewritePacks, usedBlobs, bar)
 		if err != nil {
 			return err
 		}
@@ -230,6 +231,7 @@ func pruneRepository(gopts GlobalOptions, repo restic.Repository) error {
 		return err
 	}
 
+	removePacks.Merge(repackedBlobs)
 	if len(removePacks) != 0 {
 		bar = newProgressMax(!gopts.Quiet, uint64(len(removePacks)), "packs deleted")
 		bar.Start()

src/restic/index/index_test.go

@@ -43,7 +43,7 @@ func TestIndexNew(t *testing.T) {
 	repo, cleanup := createFilledRepo(t, 3, 0)
 	defer cleanup()
 
-	idx, err := New(context.TODO(), repo, nil)
+	idx, err := New(context.TODO(), repo, restic.NewIDSet(), nil)
 	if err != nil {
 		t.Fatalf("New() returned error %v", err)
 	}
@@ -70,7 +70,7 @@ func TestIndexLoad(t *testing.T) {
 
 	validateIndex(t, repo, loadIdx)
 
-	newIdx, err := New(context.TODO(), repo, nil)
+	newIdx, err := New(context.TODO(), repo, restic.NewIDSet(), nil)
 	if err != nil {
 		t.Fatalf("New() returned error %v", err)
 	}
@@ -134,7 +134,7 @@ func BenchmarkIndexNew(b *testing.B) {
 	b.ResetTimer()
 
 	for i := 0; i < b.N; i++ {
-		idx, err := New(context.TODO(), repo, nil)
+		idx, err := New(context.TODO(), repo, restic.NewIDSet(), nil)
 
 		if err != nil {
 			b.Fatalf("New() returned error %v", err)
@@ -151,7 +151,7 @@ func BenchmarkIndexSave(b *testing.B) {
 	repo, cleanup := repository.TestRepository(b)
 	defer cleanup()
 
-	idx, err := New(context.TODO(), repo, nil)
+	idx, err := New(context.TODO(), repo, restic.NewIDSet(), nil)
 	test.OK(b, err)
 
 	for i := 0; i < 8000; i++ {
@@ -184,7 +184,7 @@ func TestIndexDuplicateBlobs(t *testing.T) {
 	repo, cleanup := createFilledRepo(t, 3, 0.01)
 	defer cleanup()
 
-	idx, err := New(context.TODO(), repo, nil)
+	idx, err := New(context.TODO(), repo, restic.NewIDSet(), nil)
 	if err != nil {
 		t.Fatal(err)
 	}

src/restic/repository/repack.go

@@ -16,9 +16,9 @@ import (
 
 // Repack takes a list of packs together with a list of blobs contained in
 // these packs. Each pack is loaded and the blobs listed in keepBlobs is saved
-// into a new pack. Afterwards, the packs are removed. This operation requires
+// into a new pack. Returned is the list of obsolete packs which can then
-// an exclusive lock on the repo.
+// be removed.
-func Repack(ctx context.Context, repo restic.Repository, packs restic.IDSet, keepBlobs restic.BlobSet, p *restic.Progress) (err error) {
+func Repack(ctx context.Context, repo restic.Repository, packs restic.IDSet, keepBlobs restic.BlobSet, p *restic.Progress) (obsoletePacks restic.IDSet, err error) {
 	debug.Log("repacking %d packs while keeping %d blobs", len(packs), len(keepBlobs))
 
 	for packID := range packs {
@@ -27,39 +27,39 @@ func Repack(ctx context.Context, repo restic.Repository, packs restic.IDSet, kee
 
 		tempfile, err := fs.TempFile("", "restic-temp-repack-")
 		if err != nil {
-			return errors.Wrap(err, "TempFile")
+			return nil, errors.Wrap(err, "TempFile")
 		}
 
 		beRd, err := repo.Backend().Load(ctx, h, 0, 0)
 		if err != nil {
-			return err
+			return nil, err
 		}
 
 		hrd := hashing.NewReader(beRd, sha256.New())
 		packLength, err := io.Copy(tempfile, hrd)
 		if err != nil {
-			return errors.Wrap(err, "Copy")
+			return nil, errors.Wrap(err, "Copy")
 		}
 
 		if err = beRd.Close(); err != nil {
-			return errors.Wrap(err, "Close")
+			return nil, errors.Wrap(err, "Close")
 		}
 
 		hash := restic.IDFromHash(hrd.Sum(nil))
 		debug.Log("pack %v loaded (%d bytes), hash %v", packID.Str(), packLength, hash.Str())
 
 		if !packID.Equal(hash) {
-			return errors.Errorf("hash does not match id: want %v, got %v", packID, hash)
+			return nil, errors.Errorf("hash does not match id: want %v, got %v", packID, hash)
 		}
 
 		_, err = tempfile.Seek(0, 0)
 		if err != nil {
-			return errors.Wrap(err, "Seek")
+			return nil, errors.Wrap(err, "Seek")
 		}
 
 		blobs, err := pack.List(repo.Key(), tempfile, packLength)
 		if err != nil {
-			return err
+			return nil, err
 		}
 
 		debug.Log("processing pack %v, blobs: %v", packID.Str(), len(blobs))
@@ -80,30 +80,30 @@ func Repack(ctx context.Context, repo restic.Repository, packs restic.IDSet, kee
 
 			n, err := tempfile.ReadAt(buf, int64(entry.Offset))
 			if err != nil {
-				return errors.Wrap(err, "ReadAt")
+				return nil, errors.Wrap(err, "ReadAt")
 			}
 
 			if n != len(buf) {
-				return errors.Errorf("read blob %v from %v: not enough bytes read, want %v, got %v",
+				return nil, errors.Errorf("read blob %v from %v: not enough bytes read, want %v, got %v",
 					h, tempfile.Name(), len(buf), n)
 			}
 
 			n, err = crypto.Decrypt(repo.Key(), buf, buf)
 			if err != nil {
-				return err
+				return nil, err
 			}
 
 			buf = buf[:n]
 
 			id := restic.Hash(buf)
 			if !id.Equal(entry.ID) {
-				return errors.Errorf("read blob %v from %v: wrong data returned, hash is %v",
+				return nil, errors.Errorf("read blob %v from %v: wrong data returned, hash is %v",
 					h, tempfile.Name(), id)
 			}
 
 			_, err = repo.SaveBlob(ctx, entry.Type, buf, entry.ID)
 			if err != nil {
-				return err
+				return nil, err
 			}
 
 			debug.Log("  saved blob %v", entry.ID.Str())
@@ -112,11 +112,11 @@ func Repack(ctx context.Context, repo restic.Repository, packs restic.IDSet, kee
 		}
 
 		if err = tempfile.Close(); err != nil {
-			return errors.Wrap(err, "Close")
+			return nil, errors.Wrap(err, "Close")
 		}
 
 		if err = fs.RemoveIfExists(tempfile.Name()); err != nil {
-			return errors.Wrap(err, "Remove")
+			return nil, errors.Wrap(err, "Remove")
 		}
 		if p != nil {
 			p.Report(restic.Stat{Blobs: 1})
@@ -124,18 +124,8 @@ func Repack(ctx context.Context, repo restic.Repository, packs restic.IDSet, kee
 	}
 
 	if err := repo.Flush(); err != nil {
-		return err
+		return nil, err
 	}
 
-	for packID := range packs {
+	return packs, nil
-		h := restic.Handle{Type: restic.DataFile, Name: packID.String()}
-		err := repo.Backend().Remove(ctx, h)
-		if err != nil {
-			debug.Log("error removing pack %v: %v", packID.Str(), err)
-			return err
-		}
-		debug.Log("removed pack %v", packID.Str())
-	}
-
-	return nil
 }

src/restic/repository/repack_test.go

@@ -127,10 +127,17 @@ func findPacksForBlobs(t *testing.T, repo restic.Repository, blobs restic.BlobSe
 }
 
 func repack(t *testing.T, repo restic.Repository, packs restic.IDSet, blobs restic.BlobSet) {
-	err := repository.Repack(context.TODO(), repo, packs, blobs, nil)
+	repackedBlobs, err := repository.Repack(context.TODO(), repo, packs, blobs, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
+
+	for id := range repackedBlobs {
+		err = repo.Backend().Remove(context.TODO(), restic.Handle{Type: restic.DataFile, Name: id.String()})
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
 }
 
 func saveIndex(t *testing.T, repo restic.Repository) {
@@ -140,7 +147,7 @@ func saveIndex(t *testing.T, repo restic.Repository) {
 }
 
 func rebuildIndex(t *testing.T, repo restic.Repository) {
-	idx, err := index.New(context.TODO(), repo, nil)
+	idx, err := index.New(context.TODO(), repo, restic.NewIDSet(), nil)
 	if err != nil {
 		t.Fatal(err)
 	}