From 1acbda18f83f31ccb55c63674ed8b76089f3b912 Mon Sep 17 00:00:00 2001
From: Miles Liu <miles@bung.cc>
Date: Sun, 18 Sep 2022 18:40:19 +0800
Subject: [PATCH] Only display the message if there were locks to be removed

`restic unlock` now only shows `successfully removed locks` if there were locks to be removed.
In addition, it also reports the number of the removed lock files.
---
 changelog/unreleased/issue-3929 |  7 +++++++
 cmd/restic/cmd_unlock.go        |  6 ++++--
 internal/restic/lock.go         | 24 ++++++++++++++++++------
 internal/restic/lock_test.go    | 12 ++++++++++--
 4 files changed, 39 insertions(+), 10 deletions(-)
 create mode 100644 changelog/unreleased/issue-3929

diff --git a/changelog/unreleased/issue-3929 b/changelog/unreleased/issue-3929
new file mode 100644
index 000000000..85a7fed8b
--- /dev/null
+++ b/changelog/unreleased/issue-3929
@@ -0,0 +1,7 @@
+Enhancement: Only display the message if there were locks to be removed
+
+`restic unlock` now only shows `successfully removed locks` if there were locks to be removed.
+In addition, it also reports the number of the removed lock files.
+
+https://github.com/restic/restic/issues/3929
+https://github.com/restic/restic/pull/3935
diff --git a/cmd/restic/cmd_unlock.go b/cmd/restic/cmd_unlock.go
index 7f5d44ada..16a5da579 100644
--- a/cmd/restic/cmd_unlock.go
+++ b/cmd/restic/cmd_unlock.go
@@ -46,11 +46,13 @@ func runUnlock(opts UnlockOptions, gopts GlobalOptions) error {
 		fn = restic.RemoveAllLocks
 	}
 
-	err = fn(gopts.ctx, repo)
+	processed, err := fn(gopts.ctx, repo)
 	if err != nil {
 		return err
 	}
 
-	Verbosef("successfully removed locks\n")
+	if processed > 0 {
+		Verbosef("successfully removed %d locks\n", processed)
+	}
 	return nil
 }
diff --git a/internal/restic/lock.go b/internal/restic/lock.go
index 3f233483f..031e8755c 100644
--- a/internal/restic/lock.go
+++ b/internal/restic/lock.go
@@ -264,8 +264,9 @@ func LoadLock(ctx context.Context, repo Repository, id ID) (*Lock, error) {
 }
 
 // RemoveStaleLocks deletes all locks detected as stale from the repository.
-func RemoveStaleLocks(ctx context.Context, repo Repository) error {
-	return ForAllLocks(ctx, repo, nil, func(id ID, lock *Lock, err error) error {
+func RemoveStaleLocks(ctx context.Context, repo Repository) (uint, error) {
+	var processed uint
+	err := ForAllLocks(ctx, repo, nil, func(id ID, lock *Lock, err error) error {
 		if err != nil {
 			// ignore locks that cannot be loaded
 			debug.Log("ignore lock %v: %v", id, err)
@@ -273,18 +274,29 @@ func RemoveStaleLocks(ctx context.Context, repo Repository) error {
 		}
 
 		if lock.Stale() {
-			return repo.Backend().Remove(ctx, Handle{Type: LockFile, Name: id.String()})
+			err = repo.Backend().Remove(ctx, Handle{Type: LockFile, Name: id.String()})
+			if err == nil {
+				processed++
+			}
+			return err
 		}
 
 		return nil
 	})
+	return processed, err
 }
 
 // RemoveAllLocks removes all locks forcefully.
-func RemoveAllLocks(ctx context.Context, repo Repository) error {
-	return repo.List(ctx, LockFile, func(id ID, size int64) error {
-		return repo.Backend().Remove(ctx, Handle{Type: LockFile, Name: id.String()})
+func RemoveAllLocks(ctx context.Context, repo Repository) (uint, error) {
+	var processed uint
+	err := repo.List(ctx, LockFile, func(id ID, size int64) error {
+		err := repo.Backend().Remove(ctx, Handle{Type: LockFile, Name: id.String()})
+		if err == nil {
+			processed++
+		}
+		return err
 	})
+	return processed, err
 }
 
 // ForAllLocks reads all locks in parallel and calls the given callback.
diff --git a/internal/restic/lock_test.go b/internal/restic/lock_test.go
index b92eace70..8a0622020 100644
--- a/internal/restic/lock_test.go
+++ b/internal/restic/lock_test.go
@@ -184,7 +184,8 @@ func TestLockWithStaleLock(t *testing.T) {
 	id3, err := createFakeLock(repo, time.Now().Add(-time.Minute), os.Getpid()+500000)
 	rtest.OK(t, err)
 
-	rtest.OK(t, restic.RemoveStaleLocks(context.TODO(), repo))
+	processed, err := restic.RemoveStaleLocks(context.TODO(), repo)
+	rtest.OK(t, err)
 
 	rtest.Assert(t, lockExists(repo, t, id1) == false,
 		"stale lock still exists after RemoveStaleLocks was called")
@@ -192,6 +193,9 @@ func TestLockWithStaleLock(t *testing.T) {
 		"non-stale lock was removed by RemoveStaleLocks")
 	rtest.Assert(t, lockExists(repo, t, id3) == false,
 		"stale lock still exists after RemoveStaleLocks was called")
+	rtest.Assert(t, processed == 2,
+		"number of locks removed does not match: expected %d, got %d",
+		2, processed)
 
 	rtest.OK(t, removeLock(repo, id2))
 }
@@ -209,7 +213,8 @@ func TestRemoveAllLocks(t *testing.T) {
 	id3, err := createFakeLock(repo, time.Now().Add(-time.Minute), os.Getpid()+500000)
 	rtest.OK(t, err)
 
-	rtest.OK(t, restic.RemoveAllLocks(context.TODO(), repo))
+	processed, err := restic.RemoveAllLocks(context.TODO(), repo)
+	rtest.OK(t, err)
 
 	rtest.Assert(t, lockExists(repo, t, id1) == false,
 		"lock still exists after RemoveAllLocks was called")
@@ -217,6 +222,9 @@ func TestRemoveAllLocks(t *testing.T) {
 		"lock still exists after RemoveAllLocks was called")
 	rtest.Assert(t, lockExists(repo, t, id3) == false,
 		"lock still exists after RemoveAllLocks was called")
+	rtest.Assert(t, processed == 3,
+		"number of locks removed does not match: expected %d, got %d",
+		3, processed)
 }
 
 func TestLockRefresh(t *testing.T) {