Re: [PATCH 1/2] rerere: extract logic to determine whether entries are stale

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

When garbage collecting rerere entries we need to figure out whether any
given entry is stale before pruning it. In a subsequent commit we're
about to introduce a second caller that wants to determine staleness,
but the logic is not currently reusable.

Extract the logic to compute staleness by introducing two new helper
functions `rerere_gc_cutoffs()` and `rerere_id_is_stale()`.

Thanks for doing these extractions. It reduces complexity in the top-
level logic.

-static void prune_one(struct rerere_id *id,
-		      timestamp_t cutoff_resolve, timestamp_t cutoff_noresolve)

...> +static bool rerere_id_is_stale(struct rerere_id *id,

+			       timestamp_t cutoff_resolve,
+			       timestamp_t cutoff_noresolve)

This modification of prune_one() to a staleness check is good to
have split, but...

 		for (id.variant = 0, id.collection = rr_dir;
 		     id.variant < id.collection->status_nr;
 		     id.variant++) {
-			prune_one(&id, cutoff_resolve, cutoff_noresolve);
+			if (rerere_id_is_stale(&id, cutoff_resolve, cutoff_noresolve))
+				unlink_rr_item(&id);
 			if (id.collection->status[id.variant])
 				now_empty = 0;
 		}

...this loop gets slightly more complicated. This is not worth
a change, but I'm thinking out loud that I would have updated
prune_one to be this simple:

static void prune_one(struct rerere_id *id,
		      timestamp_t cutoff_resolve, timestamp_t cutoff_noresolve)
{
	if (rerere_id_is_stale(&id, cutoff_resolve, cutoff_noresolve))
		unlink_rr_item(&id);
} 
and left the loop alone. This is only a preference, as your
implementation is also quite clean.

I did look to patch 2 to see if this choice of splitting the
prune_one() method had an impact there, and it doesn't appear
to matter.

The rerere_gc_cutoffs() and rerere_id_is_stale() methods are
needed in patch 2, so this adjustment to prune_one() is
important.

Thanks,
-Stolee