From 8b4dd70013c34c5d91a7ca6f84d9b840ddb5d0fb Mon Sep 17 00:00:00 2001
From: Michael Eischer <michael.eischer@fau.de>
Date: Sat, 3 Sep 2022 11:49:31 +0200
Subject: [PATCH] migrate: Report why an migration cannot be applied

Just returning that `Migration upgrade cannot be applied: check failed`
is not too useful when running `migrate upgrade_repo_v2`.
---
 cmd/restic/cmd_migrate.go                   | 9 ++++++---
 internal/migrations/interface.go            | 4 ++--
 internal/migrations/s3_layout.go            | 8 ++++----
 internal/migrations/upgrade_repo_v2.go      | 8 ++++++--
 internal/migrations/upgrade_repo_v2_test.go | 4 ++--
 5 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/cmd/restic/cmd_migrate.go b/cmd/restic/cmd_migrate.go
index a9c92b258..b0a5319ea 100644
--- a/cmd/restic/cmd_migrate.go
+++ b/cmd/restic/cmd_migrate.go
@@ -45,7 +45,7 @@ func checkMigrations(opts MigrateOptions, gopts GlobalOptions, repo restic.Repos
 	found := false
 
 	for _, m := range migrations.All {
-		ok, err := m.Check(ctx, repo)
+		ok, _, err := m.Check(ctx, repo)
 		if err != nil {
 			return err
 		}
@@ -70,14 +70,17 @@ func applyMigrations(opts MigrateOptions, gopts GlobalOptions, repo restic.Repos
 	for _, name := range args {
 		for _, m := range migrations.All {
 			if m.Name() == name {
-				ok, err := m.Check(ctx, repo)
+				ok, reason, err := m.Check(ctx, repo)
 				if err != nil {
 					return err
 				}
 
 				if !ok {
 					if !opts.Force {
-						Warnf("migration %v cannot be applied: check failed\nIf you want to apply this migration anyway, re-run with option --force\n", m.Name())
+						if reason == "" {
+							reason = "check failed"
+						}
+						Warnf("migration %v cannot be applied: %v\nIf you want to apply this migration anyway, re-run with option --force\n", m.Name(), reason)
 						continue
 					}
 
diff --git a/internal/migrations/interface.go b/internal/migrations/interface.go
index 99100bce3..63682b46b 100644
--- a/internal/migrations/interface.go
+++ b/internal/migrations/interface.go
@@ -8,8 +8,8 @@ import (
 
 // Migration implements a data migration.
 type Migration interface {
-	// Check returns true if the migration can be applied to a repo.
-	Check(context.Context, restic.Repository) (bool, error)
+	// Check returns true if the migration can be applied to a repo. If the option is not applicable it can return a specific reason.
+	Check(context.Context, restic.Repository) (bool, string, error)
 
 	RepoCheck() bool
 
diff --git a/internal/migrations/s3_layout.go b/internal/migrations/s3_layout.go
index afb14b848..be11733db 100644
--- a/internal/migrations/s3_layout.go
+++ b/internal/migrations/s3_layout.go
@@ -38,19 +38,19 @@ func toS3Backend(repo restic.Repository) *s3.Backend {
 }
 
 // Check tests whether the migration can be applied.
-func (m *S3Layout) Check(ctx context.Context, repo restic.Repository) (bool, error) {
+func (m *S3Layout) Check(ctx context.Context, repo restic.Repository) (bool, string, error) {
 	be := toS3Backend(repo)
 	if be == nil {
 		debug.Log("backend is not s3")
-		return false, nil
+		return false, "backend is not s3", nil
 	}
 
 	if be.Layout.Name() != "s3legacy" {
 		debug.Log("layout is not s3legacy")
-		return false, nil
+		return false, "not using the legacy s3 layout", nil
 	}
 
-	return true, nil
+	return true, "", nil
 }
 
 func (m *S3Layout) RepoCheck() bool {
diff --git a/internal/migrations/upgrade_repo_v2.go b/internal/migrations/upgrade_repo_v2.go
index b29fbcdcc..cc4972701 100644
--- a/internal/migrations/upgrade_repo_v2.go
+++ b/internal/migrations/upgrade_repo_v2.go
@@ -45,9 +45,13 @@ func (*UpgradeRepoV2) Desc() string {
 	return "upgrade a repository to version 2"
 }
 
-func (*UpgradeRepoV2) Check(ctx context.Context, repo restic.Repository) (bool, error) {
+func (*UpgradeRepoV2) Check(ctx context.Context, repo restic.Repository) (bool, string, error) {
 	isV1 := repo.Config().Version == 1
-	return isV1, nil
+	reason := ""
+	if !isV1 {
+		reason = fmt.Sprintf("repository is already upgraded to version %v", repo.Config().Version)
+	}
+	return isV1, reason, nil
 }
 
 func (*UpgradeRepoV2) RepoCheck() bool {
diff --git a/internal/migrations/upgrade_repo_v2_test.go b/internal/migrations/upgrade_repo_v2_test.go
index 0d86d265c..ecded80b6 100644
--- a/internal/migrations/upgrade_repo_v2_test.go
+++ b/internal/migrations/upgrade_repo_v2_test.go
@@ -23,7 +23,7 @@ func TestUpgradeRepoV2(t *testing.T) {
 
 	m := &UpgradeRepoV2{}
 
-	ok, err := m.Check(context.Background(), repo)
+	ok, _, err := m.Check(context.Background(), repo)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -81,7 +81,7 @@ func TestUpgradeRepoV2Failure(t *testing.T) {
 
 	m := &UpgradeRepoV2{}
 
-	ok, err := m.Check(context.Background(), repo)
+	ok, _, err := m.Check(context.Background(), repo)
 	if err != nil {
 		t.Fatal(err)
 	}