{"thread":{"id":"66086","subject":"[PATCH] builtin/maintenance: accept \"none\" as a maintenance strategy","startedAt":"2026-07-29T19:41:33Z","lastAt":"2026-08-04T12:59:04Z","messageCount":3,"participants":["David Lin","Junio C Hamano","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"549229","messageId":"20260729194006.75317-1-davidlin@stripe.com","threadId":"66086","inReplyTo":null,"subject":"[PATCH] builtin/maintenance: accept \"none\" as a maintenance strategy","fromName":"David Lin","fromEmail":"davidzylin@gmail.com","sentAt":"2026-07-29T19:40:06Z","receivedAt":"2026-07-29T19:41:33Z","isPatch":true,"body":"Commit d465be2327 (builtin/maintenance: don't silently ignore invalid\nstrategy, 2025-10-24) changed scheduled maintenance to error on an\nunknown maintenance strategy instead of silently defaulting to the\n`none` strategy.\n\nHowever, `parse_maintenance_strategy()` does not recognize `none`, so\nGit rejects a valid and documented strategy that can be used to override\nan existing strategy and disable maintenance tasks.\n\nAccept `none` as a valid maintenance strategy and add tests to ensure\nit's accepted.\n\nSigned-off-by: David Lin \n---\n builtin/gc.c | 2 ++\n t/t7900-maintenance.sh | 3 +++\n 2 files changed, 5 insertions(+)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 46999a99ab..3d1e39d46a 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -1922,6 +1922,8 @@ static const struct maintenance_strategy geometric_strategy = {\n \n static struct maintenance_strategy parse_maintenance_strategy(const char *name)\n {\n+\tif (!strcasecmp(name, \"none\"))\n+\t\treturn none_strategy;\n \tif (!strcasecmp(name, \"incremental\"))\n \t\treturn incremental_strategy;\n \tif (!strcasecmp(name, \"gc\"))\ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex a8d691719d..130c971b15 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -1022,6 +1022,9 @@ test_expect_success 'maintenance.strategy is respected' '\n \t\ttest_must_fail git -c maintenance.strategy=unknown maintenance run 2>err &&\n \t\ttest_grep \"unknown maintenance strategy: .unknown.\" err &&\n \n+\t\ttest_strategy none writes:\n\nThe author identity uses an @gmail.com address, but the sign-off\nuses a different address. Assuming the preferred address is the one\nused in the sign-off, you can 'lie' about the author identity by\nstarting the body of the message with an 'in-body From:' line:\n\n From: David Lin \n\nThis must be written without indentation and followed by a blank line\nbefore the true first line of the message body.\n\n> Commit d465be2327 (builtin/maintenance: don't silently ignore invalid\n> strategy, 2025-10-24) changed scheduled maintenance to error on an\n> unknown maintenance strategy instead of silently defaulting to the\n> `none` strategy.\n>\n> However, `parse_maintenance_strategy()` does not recognize `none`, so\n> Git rejects a valid and documented strategy that can be used to override\n> an existing strategy and disable maintenance tasks.\n>\n> Accept `none` as a valid maintenance strategy and add tests to ensure\n> it's accepted.\n>\n> Signed-off-by: David Lin \n> ---\n> builtin/gc.c | 2 ++\n> t/t7900-maintenance.sh | 3 +++\n> 2 files changed, 5 insertions(+)\n>\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index 46999a99ab..3d1e39d46a 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -1922,6 +1922,8 @@ static const struct maintenance_strategy geometric_strategy = {\n> \n> static struct maintenance_strategy parse_maintenance_strategy(const char *name)\n> {\n> +\tif (!strcasecmp(name, \"none\"))\n> +\t\treturn none_strategy;\n> \tif (!strcasecmp(name, \"incremental\"))\n> \t\treturn incremental_strategy;\n> \tif (!strcasecmp(name, \"gc\"))\n> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\n> index a8d691719d..130c971b15 100755\n> --- a/t/t7900-maintenance.sh\n> +++ b/t/t7900-maintenance.sh\n> @@ -1022,6 +1022,9 @@ test_expect_success 'maintenance.strategy is respected' '\n> \t\ttest_must_fail git -c maintenance.strategy=unknown maintenance run 2>err &&\n> \t\ttest_grep \"unknown maintenance strategy: .unknown.\" err &&\n> \n> +\t\ttest_strategy none +\t\ttest_strategy none --schedule=weekly +\n> \t\ttest_strategy incremental <<-\\EOF &&\n> \t\tgit pack-refs --all --prune\n> \t\tgit reflog expire --all\n>\n> base-commit: 13c7afec212fc97ce257d15601659314c6673d6c\n"},{"id":"549565","messageId":"anHiDzJbXUAgPRbO@pks.im","threadId":"66086","inReplyTo":"20260729194006.75317-1-davidlin@stripe.com","subject":"Re: [PATCH] builtin/maintenance: accept \"none\" as a maintenance strategy","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-04T12:58:55Z","receivedAt":"2026-08-04T12:59:04Z","isPatch":true,"body":"On Wed, Jul 29, 2026 at 03:40:06PM -0400, David Lin wrote:\n> Commit d465be2327 (builtin/maintenance: don't silently ignore invalid\n> strategy, 2025-10-24) changed scheduled maintenance to error on an\n> unknown maintenance strategy instead of silently defaulting to the\n> `none` strategy.\n> \n> However, `parse_maintenance_strategy()` does not recognize `none`, so\n> Git rejects a valid and documented strategy that can be used to override\n> an existing strategy and disable maintenance tasks.\n\nOh, indeed.\n\n> Accept `none` as a valid maintenance strategy and add tests to ensure\n> it's accepted.\n\nMakes sense. You can of course achieve the same thing by disabling\nmaintenance altogether, but it's a documented thing and users thus\nrightfully expect the \"none\" strategy to exist.\n\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index 46999a99ab..3d1e39d46a 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -1922,6 +1922,8 @@ static const struct maintenance_strategy geometric_strategy = {\n> \n> static struct maintenance_strategy parse_maintenance_strategy(const char *name)\n> {\n> +\tif (!strcasecmp(name, \"none\"))\n> +\t\treturn none_strategy;\n> \tif (!strcasecmp(name, \"incremental\"))\n> \t\treturn incremental_strategy;\n> \tif (!strcasecmp(name, \"gc\"))\n\nYup, looks obviously correct.\n\n> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\n> index a8d691719d..130c971b15 100755\n> --- a/t/t7900-maintenance.sh\n> +++ b/t/t7900-maintenance.sh\n> @@ -1022,6 +1022,9 @@ test_expect_success 'maintenance.strategy is respected' '\n> \t\ttest_must_fail git -c maintenance.strategy=unknown maintenance run 2>err &&\n> \t\ttest_grep \"unknown maintenance strategy: .unknown.\" err &&\n> \n> +\t\ttest_strategy none +\t\ttest_strategy none --schedule=weekly +\n> \t\ttest_strategy incremental <<-\\EOF &&\n> \t\tgit pack-refs --all --prune\n> \t\tgit reflog expire --all\n\nAnd test looks obviously correct to me, too.\n\nSo other than Junio's remark about the SOB this patch looks good to me.\nThanks!\n\nPatrick\n"}]}