Re: [PATCH 2/2] builtin/maintenance: improve heuristic for "rerere gc"

On Thu, Sep 03, 2026 at 10:19:33AM -0400, Derrick Stolee wrote:

On 9/3/2026 5:04 AM, Patrick Steinhardt wrote:

quoted

diff --git a/builtin/gc.c b/builtin/gc.c
index de2f9e7fed..9147418a61 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c

@@ -396,31 +396,13 @@ static int maintenance_task_rerere_gc(struct maintenance_run_opts *opts UNUSED,
 
 static int rerere_gc_condition(struct gc_config *cfg UNUSED)
 {
-	struct strbuf path = STRBUF_INIT;
-	int should_gc = 0, limit = 1;
-	DIR *dir = NULL;
+	int limit = 512;
 
 	repo_config_get_int(the_repository, "maintenance.rerere-gc.auto", &limit);
+	if (limit <= 0)
+		return limit < 0;

This is cute, but works. It's logically equivalent to

	if (!limit)
		return 0;
	if (limit < 0)
		return 1;

which would map more directly to the two documented cases. It takes
the slightest amount of mental processing to connect the docs to
the format you have.

This also existed in the preimage, but I agree it's harder to read than
really necessary. Will improve while at it.

quoted

+	return rerere_gc_estimate(the_repository, limit) >= (size_t)limit;
 }

I do like that this method is simpler in the builtin code in favor
of a method that has access to rerere internals.

I do wonder if rerere_gc_estimate() should be
rerere_stale_above_limit() instead, as we are not using any callers
that care about the resulting number other than "is it at least limit?"

Agreed, the current name isn't great. I'm somehow hestitant to use
`rerere_stale_above_limit()` too, though. I'll adapt it to
`rerere_gc_needed()` instead.

Thanks for your review!

Patrick