Fix calls to repo/backend.List() everywhere

internal/repository/key.go

@@ -113,42 +113,48 @@ func OpenKey(ctx context.Context, s *Repository, name string, password string) (
 // given password. If none could be found, ErrNoKeyFound is returned. When
 // maxKeys is reached, ErrMaxKeysReached is returned. When setting maxKeys to
 // zero, all keys in the repo are checked.
-func SearchKey(ctx context.Context, s *Repository, password string, maxKeys int) (*Key, error) {
+func SearchKey(ctx context.Context, s *Repository, password string, maxKeys int) (k *Key, err error) {
 	checked := 0
 
 	// try at most maxKeysForSearch keys in repo
-	for name := range s.Backend().List(ctx, restic.KeyFile) {
+	err = s.Backend().List(ctx, restic.KeyFile, func(fi restic.FileInfo) error {
 		if maxKeys > 0 && checked > maxKeys {
-			return nil, ErrMaxKeysReached
+			return ErrMaxKeysReached
 		}
 
-		_, err := restic.ParseID(name)
+		_, err := restic.ParseID(fi.Name)
 		if err != nil {
-			debug.Log("rejecting key with invalid name: %v", name)
-			continue
+			debug.Log("rejecting key with invalid name: %v", fi.Name)
+			return nil
 		}
 
-		debug.Log("trying key %q", name)
-		key, err := OpenKey(ctx, s, name, password)
+		debug.Log("trying key %q", fi.Name)
+		key, err := OpenKey(ctx, s, fi.Name, password)
 		if err != nil {
-			debug.Log("key %v returned error %v", name, err)
+			debug.Log("key %v returned error %v", fi.Name, err)
 
 			// ErrUnauthenticated means the password is wrong, try the next key
 			if errors.Cause(err) == crypto.ErrUnauthenticated {
-				continue
+				return nil
 			}
 
-			if err != nil {
-				debug.Log("unable to open key %v: %v\n", err)
-				continue
-			}
+			return err
 		}
 
-		debug.Log("successfully opened key %v", name)
-		return key, nil
+		debug.Log("successfully opened key %v", fi.Name)
+		k = key
+		return nil
+	})
+
+	if err != nil {
+		return nil, err
 	}
 
-	return nil, ErrNoKeyFound
+	if k == nil {
+		return nil, ErrNoKeyFound
+	}
+
+	return k, nil
 }
 
 // LoadKey loads a key from the backend.

internal/repository/parallel.go

@@ -2,10 +2,10 @@ package repository
 
 import (
 	"context"
-	"sync"
 
 	"github.com/restic/restic/internal/debug"
 	"github.com/restic/restic/internal/restic"
+	"golang.org/x/sync/errgroup"
 )
 
 // ParallelWorkFunc gets one file ID to work on. If an error is returned,
@@ -17,47 +17,36 @@ type ParallelWorkFunc func(ctx context.Context, id string) error
 type ParallelIDWorkFunc func(ctx context.Context, id restic.ID) error
 
 // FilesInParallel runs n workers of f in parallel, on the IDs that
-// repo.List(t) yield. If f returns an error, the process is aborted and the
+// repo.List(t) yields. If f returns an error, the process is aborted and the
 // first error is returned.
-func FilesInParallel(ctx context.Context, repo restic.Lister, t restic.FileType, n uint, f ParallelWorkFunc) error {
-	wg := &sync.WaitGroup{}
-	ch := repo.List(ctx, t)
-	errors := make(chan error, n)
+func FilesInParallel(ctx context.Context, repo restic.Lister, t restic.FileType, n int, f ParallelWorkFunc) error {
+	g, ctx := errgroup.WithContext(ctx)
 
-	for i := 0; uint(i) < n; i++ {
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
+	ch := make(chan string, n)
+	g.Go(func() error {
+		defer close(ch)
+		return repo.List(ctx, t, func(fi restic.FileInfo) error {
+			select {
+			case <-ctx.Done():
+			case ch <- fi.Name:
+			}
+			return nil
+		})
+	})
 
-			for {
-				select {
-				case id, ok := <-ch:
-					if !ok {
-						return
-					}
-
-					err := f(ctx, id)
-					if err != nil {
-						errors <- err
-						return
-					}
-				case <-ctx.Done():
-					return
+	for i := 0; i < n; i++ {
+		g.Go(func() error {
+			for name := range ch {
+				err := f(ctx, name)
+				if err != nil {
+					return err
 				}
 			}
-		}()
+			return nil
+		})
 	}
 
-	wg.Wait()
-
-	select {
-	case err := <-errors:
-		return err
-	default:
-		break
-	}
-
-	return nil
+	return g.Wait()
 }
 
 // ParallelWorkFuncParseID converts a function that takes a restic.ID to a

internal/repository/parallel_test.go

@@ -74,24 +74,25 @@ var lister = testIDs{
 	"34dd044c228727f2226a0c9c06a3e5ceb5e30e31cb7854f8fa1cde846b395a58",
 }
 
-func (tests testIDs) List(ctx context.Context, t restic.FileType) <-chan string {
-	ch := make(chan string)
+func (tests testIDs) List(ctx context.Context, t restic.FileType, fn func(restic.FileInfo) error) error {
+	for i := 0; i < 500; i++ {
+		for _, id := range tests {
+			if ctx.Err() != nil {
+				return ctx.Err()
+			}
 
-	go func() {
-		defer close(ch)
+			fi := restic.FileInfo{
+				Name: id,
+			}
 
-		for i := 0; i < 500; i++ {
-			for _, id := range tests {
-				select {
-				case ch <- id:
-				case <-ctx.Done():
-					return
-				}
+			err := fn(fi)
+			if err != nil {
+				return err
 			}
 		}
-	}()
+	}
 
-	return ch
+	return nil
 }
 
 func TestFilesInParallel(t *testing.T) {
@@ -100,7 +101,7 @@ func TestFilesInParallel(t *testing.T) {
 		return nil
 	}
 
-	for n := uint(1); n < 5; n++ {
+	for n := 1; n < 5; n++ {
 		err := repository.FilesInParallel(context.TODO(), lister, restic.DataFile, n*100, f)
 		rtest.OK(t, err)
 	}
@@ -109,7 +110,6 @@ func TestFilesInParallel(t *testing.T) {
 var errTest = errors.New("test error")
 
 func TestFilesInParallelWithError(t *testing.T) {
-
 	f := func(ctx context.Context, id string) error {
 		time.Sleep(1 * time.Millisecond)
 
@@ -120,8 +120,10 @@ func TestFilesInParallelWithError(t *testing.T) {
 		return nil
 	}
 
-	for n := uint(1); n < 5; n++ {
+	for n := 1; n < 5; n++ {
 		err := repository.FilesInParallel(context.TODO(), lister, restic.DataFile, n*100, f)
-		rtest.Equals(t, errTest, err)
+		if err != errTest {
+			t.Fatalf("wrong error returned, want %q, got %v", errTest, err)
+		}
 	}
 }

internal/repository/repack_test.go

@@ -74,7 +74,7 @@ func selectBlobs(t *testing.T, repo restic.Repository, p float32) (list1, list2
 
 	blobs := restic.NewBlobSet()
 
-	for id := range repo.List(context.TODO(), restic.DataFile) {
+	err := repo.List(context.TODO(), restic.DataFile, func(id restic.ID, size int64) error {
 		entries, _, err := repo.ListPack(context.TODO(), id)
 		if err != nil {
 			t.Fatalf("error listing pack %v: %v", id, err)
@@ -84,7 +84,7 @@ func selectBlobs(t *testing.T, repo restic.Repository, p float32) (list1, list2
 			h := restic.BlobHandle{ID: entry.ID, Type: entry.Type}
 			if blobs.Has(h) {
 				t.Errorf("ignoring duplicate blob %v", h)
-				continue
+				return nil
 			}
 			blobs.Insert(h)
 
@@ -93,8 +93,11 @@ func selectBlobs(t *testing.T, repo restic.Repository, p float32) (list1, list2
 			} else {
 				list2.Insert(restic.BlobHandle{ID: entry.ID, Type: entry.Type})
 			}
-
 		}
+		return nil
+	})
+	if err != nil {
+		t.Fatal(err)
 	}
 
 	return list1, list2
@@ -102,8 +105,13 @@ func selectBlobs(t *testing.T, repo restic.Repository, p float32) (list1, list2
 
 func listPacks(t *testing.T, repo restic.Repository) restic.IDSet {
 	list := restic.NewIDSet()
-	for id := range repo.List(context.TODO(), restic.DataFile) {
+	err := repo.List(context.TODO(), restic.DataFile, func(id restic.ID, size int64) error {
 		list.Insert(id)
+		return nil
+	})
+
+	if err != nil {
+		t.Fatal(err)
 	}
 
 	return list
@@ -153,15 +161,15 @@ func rebuildIndex(t *testing.T, repo restic.Repository) {
 		t.Fatal(err)
 	}
 
-	for id := range repo.List(context.TODO(), restic.IndexFile) {
+	err = repo.List(context.TODO(), restic.IndexFile, func(id restic.ID, size int64) error {
 		h := restic.Handle{
 			Type: restic.IndexFile,
 			Name: id.String(),
 		}
-		err = repo.Backend().Remove(context.TODO(), h)
-		if err != nil {
-			t.Fatal(err)
-		}
+		return repo.Backend().Remove(context.TODO(), h)
+	})
+	if err != nil {
+		t.Fatal(err)
 	}
 
 	_, err = idx.Save(context.TODO(), repo, nil)

internal/repository/repository.go

@@ -536,22 +536,15 @@ func (r *Repository) KeyName() string {
 	return r.keyName
 }
 
-// List returns a channel that yields all IDs of type t in the backend.
-func (r *Repository) List(ctx context.Context, t restic.FileType) <-chan restic.ID {
-	out := make(chan restic.ID)
-	go func() {
-		defer close(out)
-		for strID := range r.be.List(ctx, t) {
-			if id, err := restic.ParseID(strID); err == nil {
-				select {
-				case out <- id:
-				case <-ctx.Done():
-					return
-				}
-			}
+// List runs fn for all files of type t in the repo.
+func (r *Repository) List(ctx context.Context, t restic.FileType, fn func(restic.ID, int64) error) error {
+	return r.be.List(ctx, t, func(fi restic.FileInfo) error {
+		id, err := restic.ParseID(fi.Name)
+		if err != nil {
+			debug.Log("unable to parse %v as an ID", fi.Name)
 		}
-	}()
-	return out
+		return fn(id, fi.Size)
+	})
 }
 
 // ListPack returns the list of blobs saved in the pack id and the length of

internal/repository/repository_test.go

@@ -369,7 +369,7 @@ func TestRepositoryIncrementalIndex(t *testing.T) {
 
 	packEntries := make(map[restic.ID]map[restic.ID]struct{})
 
-	for id := range repo.List(context.TODO(), restic.IndexFile) {
+	err := repo.List(context.TODO(), restic.IndexFile, func(id restic.ID, size int64) error {
 		idx, err := repository.LoadIndex(context.TODO(), repo, id)
 		rtest.OK(t, err)
 
@@ -380,6 +380,10 @@ func TestRepositoryIncrementalIndex(t *testing.T) {
 
 			packEntries[pb.PackID][id] = struct{}{}
 		}
+		return nil
+	})
+	if err != nil {
+		t.Fatal(err)
 	}
 
 	for packID, ids := range packEntries {