builtin/maintenance: don't silently ignore invalid strategy

When parsing maintenance strategies we completely ignore the
user-configured value in case it is unknown to us. This makes it
basically undiscoverable to the user that scheduled maintenance is
devolving into a no-op.

Change this to instead die when seeing an unknown maintenance strategy.
While at it, pull out the parsing logic into a separate function so that
we can reuse it in a subsequent commit.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Acked-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/gc.c b/builtin/gc.c
index fb1a82e0304163866fb250551d5c621b41d9a733..726d944d3bd1e83f4ec5e4dbd5c7d3e1b2916cb7 100644 (file)
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1855,6 +1855,13 @@ static const struct maintenance_strategy incremental_strategy = {
        },
 };
 
+static struct maintenance_strategy parse_maintenance_strategy(const char *name)
+{
+       if (!strcasecmp(name, "incremental"))
+               return incremental_strategy;
+       die(_("unknown maintenance strategy: '%s'"), name);
+}
+
 static void initialize_task_config(struct maintenance_run_opts *opts,
                                   const struct string_list *selected_tasks)
 {
@@ -1890,12 +1897,10 @@ static void initialize_task_config(struct maintenance_run_opts *opts,
         * override specific aspects of our strategy.
         */
        if (opts->schedule) {
-               strategy = none_strategy;
-
-               if (!repo_config_get_string_tmp(the_repository, "maintenance.strategy", &config_str)) {
-                       if (!strcasecmp(config_str, "incremental"))
-                               strategy = incremental_strategy;
-               }
+               if (!repo_config_get_string_tmp(the_repository, "maintenance.strategy", &config_str))
+                       strategy = parse_maintenance_strategy(config_str);
+               else
+                       strategy = none_strategy;
        } else {
                strategy = default_strategy;
        }

diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index e0352fd1965fdd3646139dab3e17fe03274a7494..0fb917dd7b7e35377e637a874b8ed62691325f16 100755 (executable)
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -1263,6 +1263,11 @@ test_expect_success 'fails when running outside of a repository' '
        nongit test_must_fail git maintenance unregister
 '
 
+test_expect_success 'fails when configured to use an invalid strategy' '
+       test_must_fail git -c maintenance.strategy=invalid maintenance run --schedule=hourly 2>err &&
+       test_grep "unknown maintenance strategy: .invalid." err
+'
+
 test_expect_success 'register and unregister bare repo' '
        test_when_finished "git config --global --unset-all maintenance.repo || :" &&
        test_might_fail git config --global --unset-all maintenance.repo &&