From fde0dfd3d390eba062e06607ce81a9b575350a6d Mon Sep 17 00:00:00 2001
From: Mike McKay-Dirden <93532247+KastenMike@users.noreply.github.com>
Date: Wed, 1 Nov 2023 19:00:01 +0100
Subject: [PATCH] fix(cli): Don't return error when parameters unchanged
 (#3411)

* don't return error when permissions unchanged
* remove logging since it only happens for a couple edge cases
---
 cli/command_repository_set_parameters.go      |  3 ++-
 cli/command_repository_set_parameters_test.go | 17 ++++++++++++++---
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/cli/command_repository_set_parameters.go b/cli/command_repository_set_parameters.go
index d9b76ea2e..e7204259b 100644
--- a/cli/command_repository_set_parameters.go
+++ b/cli/command_repository_set_parameters.go
@@ -228,7 +228,8 @@ func (c *commandRepositorySetParameters) run(ctx context.Context, rep repo.Direc
 	requiredFeatures = c.addRemoveUpdateRequiredFeatures(requiredFeatures, &anyChange)
 
 	if !anyChange {
-		return errors.Errorf("no changes")
+		log(ctx).Info("no changes")
+		return nil
 	}
 
 	if blobcfg.IsRetentionEnabled() {
diff --git a/cli/command_repository_set_parameters_test.go b/cli/command_repository_set_parameters_test.go
index fe8cc312f..d5b2c3f84 100644
--- a/cli/command_repository_set_parameters_test.go
+++ b/cli/command_repository_set_parameters_test.go
@@ -35,8 +35,10 @@ func (s *formatSpecificTestSuite) TestRepositorySetParameters(t *testing.T) {
 	require.Contains(t, out, "Max pack length:     21 MB")
 	require.Contains(t, out, fmt.Sprintf("Format version:      %d", s.formatVersion))
 
+	_, out = env.RunAndExpectSuccessWithErrOut(t, "repository", "set-parameters")
+	require.Contains(t, out, "no changes")
+
 	// failure cases
-	env.RunAndExpectFailure(t, "repository", "set-parameters")
 	env.RunAndExpectFailure(t, "repository", "set-parameters", "--index-version=33")
 	env.RunAndExpectFailure(t, "repository", "set-parameters", "--max-pack-size-mb=9")
 	env.RunAndExpectFailure(t, "repository", "set-parameters", "--max-pack-size-mb=121")
@@ -73,6 +75,10 @@ func (s *formatSpecificTestSuite) TestRepositorySetParametersRetention(t *testin
 	_, out = env.RunAndExpectSuccessWithErrOut(t, "repository", "set-parameters", "--retention-mode", "none")
 	require.Contains(t, out, "disabling blob retention")
 
+	// 2nd time also succeeds but disabling is skipped due to already being disabled. !anyChanges returns no error.
+	_, out = env.RunAndExpectSuccessWithErrOut(t, "repository", "set-parameters", "--retention-mode", "none")
+	require.Contains(t, out, "no changes")
+
 	out = env.RunAndExpectSuccess(t, "repository", "status")
 	require.NotContains(t, out, "Blob retention mode")
 	require.NotContains(t, out, "Blob retention period")
@@ -201,21 +207,26 @@ func (s *formatSpecificTestSuite) TestRepositorySetParametersDowngrade(t *testin
 			require.Contains(t, out, "Format version:      1")
 			require.Contains(t, out, "Epoch Manager:       disabled")
 			env.RunAndExpectFailure(t, "index", "epoch", "list")
+			// setting the current version again is ok
+			_, out = env.RunAndExpectSuccessWithErrOut(t, "repository", "set-parameters", "--index-version=1")
+			require.Contains(t, out, "no changes")
 		case format.FormatVersion2:
 			require.Contains(t, out, "Format version:      2")
 			require.Contains(t, out, "Epoch Manager:       enabled")
 			env.RunAndExpectSuccess(t, "index", "epoch", "list")
+			_, out = env.RunAndExpectFailure(t, "repository", "set-parameters", "--index-version=1")
+			require.Contains(t, out, "index format version can only be upgraded")
 		default:
 			require.Contains(t, out, "Format version:      3")
 			require.Contains(t, out, "Epoch Manager:       enabled")
 			env.RunAndExpectSuccess(t, "index", "epoch", "list")
+			_, out = env.RunAndExpectFailure(t, "repository", "set-parameters", "--index-version=1")
+			require.Contains(t, out, "index format version can only be upgraded")
 		}
 	}
 
 	checkStatusForVersion()
 
-	env.RunAndExpectFailure(t, "repository", "set-parameters", "--index-version=1")
-
 	checkStatusForVersion()
 
 	// run basic check to ensure that an upgrade can still be performed as expected