patchbuiltin/maintenance: accept "none" as a maintenance strategy
3 messages between Jul 29, 2026 and Aug 4, 2026, from David Lin, Junio C Hamano, Patrick Steinhardt.
Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.
Commit d465be2327 (builtin/maintenance: don't silently ignore invalid strategy, 2025-10-24) changed scheduled maintenance to error on an unknown maintenance strategy instead of silently defaulting to the `none` strategy.
However, `parse_maintenance_strategy()` does not recognize `none`, so Git rejects a valid and documented strategy that can be used to override an existing strategy and disable maintenance tasks.
Accept `none` as a valid maintenance strategy and add tests to ensure it's accepted.
Signed-off-by: David Lin <davidlin@stripe.com>
---
builtin/gc.c | 2 ++
t/t7900-maintenance.sh | 3 +++
2 files changed, 5 insertions(+)
Show changes to 2 files +5 −0
builtin/gc.c, t/t7900-maintenance.sh
diff --git a/builtin/gc.c b/builtin/gc.c
index 46999a99ab..3d1e39d46a 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1922,6 +1922,8 @@ static const struct maintenance_strategy geometric_strategy = {
static struct maintenance_strategy parse_maintenance_strategy(const char *name)
{
+ if (!strcasecmp(name, "none"))
+ return none_strategy;
if (!strcasecmp(name, "incremental"))
return incremental_strategy;
if (!strcasecmp(name, "gc"))
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index a8d691719d..130c971b15 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -1022,6 +1022,9 @@ test_expect_success 'maintenance.strategy is respected' '
test_must_fail git -c maintenance.strategy=unknown maintenance run 2>err &&
test_grep "unknown maintenance strategy: .unknown." err &&
+ test_strategy none </dev/null &&
+ test_strategy none --schedule=weekly </dev/null &&
+
test_strategy incremental <<-\EOF &&
git pack-refs --all --prune
git reflog expire --all
base-commit: 13c7afec212fc97ce257d15601659314c6673d6c
--
2.54.0
Re: [PATCH] builtin/maintenance: accept "none" as a maintenance strategy
David Lin <davidzylin@gmail.com> writes:
The author identity uses an @gmail.com address, but the sign-off uses a different address. Assuming the preferred address is the one used in the sign-off, you can 'lie' about the author identity by starting the body of the message with an 'in-body From:' line:
From: David Lin <davidlin@stripe.com>
This must be written without indentation and followed by a blank line before the true first line of the message body.
Show 47 quoted lines
> Commit d465be2327 (builtin/maintenance: don't silently ignore invalid
> strategy, 2025-10-24) changed scheduled maintenance to error on an
> unknown maintenance strategy instead of silently defaulting to the
> `none` strategy.
>
> However, `parse_maintenance_strategy()` does not recognize `none`, so
> Git rejects a valid and documented strategy that can be used to override
> an existing strategy and disable maintenance tasks.
>
> Accept `none` as a valid maintenance strategy and add tests to ensure
> it's accepted.
>
> Signed-off-by: David Lin <davidlin@stripe.com>
> ---
> builtin/gc.c | 2 ++
> t/t7900-maintenance.sh | 3 +++
> 2 files changed, 5 insertions(+)
>
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 46999a99ab..3d1e39d46a 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -1922,6 +1922,8 @@ static const struct maintenance_strategy geometric_strategy = {
>
> static struct maintenance_strategy parse_maintenance_strategy(const char *name)
> {
> + if (!strcasecmp(name, "none"))
> + return none_strategy;
> if (!strcasecmp(name, "incremental"))
> return incremental_strategy;
> if (!strcasecmp(name, "gc"))
> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> index a8d691719d..130c971b15 100755
> --- a/t/t7900-maintenance.sh
> +++ b/t/t7900-maintenance.sh
> @@ -1022,6 +1022,9 @@ test_expect_success 'maintenance.strategy is respected' '
> test_must_fail git -c maintenance.strategy=unknown maintenance run 2>err &&
> test_grep "unknown maintenance strategy: .unknown." err &&
>
> + test_strategy none </dev/null &&
> + test_strategy none --schedule=weekly </dev/null &&
> +
> test_strategy incremental <<-\EOF &&
> git pack-refs --all --prune
> git reflog expire --all
>
> base-commit: 13c7afec212fc97ce257d15601659314c6673d6cRe: [PATCH] builtin/maintenance: accept "none" as a maintenance strategy
On Wed, Jul 29, 2026 at 03:40:06PM -0400, David Lin wrote:
Show 8 quoted lines
> Commit d465be2327 (builtin/maintenance: don't silently ignore invalid
> strategy, 2025-10-24) changed scheduled maintenance to error on an
> unknown maintenance strategy instead of silently defaulting to the
> `none` strategy.
>
> However, `parse_maintenance_strategy()` does not recognize `none`, so
> Git rejects a valid and documented strategy that can be used to override
> an existing strategy and disable maintenance tasks.
> Accept `none` as a valid maintenance strategy and add tests to ensure
> it's accepted.
Makes sense. You can of course achieve the same thing by disabling maintenance altogether, but it's a documented thing and users thus rightfully expect the "none" strategy to exist.
Show 13 quoted lines
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 46999a99ab..3d1e39d46a 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -1922,6 +1922,8 @@ static const struct maintenance_strategy geometric_strategy = {
>
> static struct maintenance_strategy parse_maintenance_strategy(const char *name)
> {
> + if (!strcasecmp(name, "none"))
> + return none_strategy;
> if (!strcasecmp(name, "incremental"))
> return incremental_strategy;
> if (!strcasecmp(name, "gc"))Yup, looks obviously correct.
Show 14 quoted lines
> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> index a8d691719d..130c971b15 100755
> --- a/t/t7900-maintenance.sh
> +++ b/t/t7900-maintenance.sh
> @@ -1022,6 +1022,9 @@ test_expect_success 'maintenance.strategy is respected' '
> test_must_fail git -c maintenance.strategy=unknown maintenance run 2>err &&
> test_grep "unknown maintenance strategy: .unknown." err &&
>
> + test_strategy none </dev/null &&
> + test_strategy none --schedule=weekly </dev/null &&
> +
> test_strategy incremental <<-\EOF &&
> git pack-refs --all --prune
> git reflog expire --all
And test looks obviously correct to me, too.
So other than Junio's remark about the SOB this patch looks good to me. Thanks!
Patrick