Re: [PATCH 11/17] builtin/pack-objects.c: --cruft with expiration

From: Derrick Stolee <stolee@gmail.com>
Date: Dec 7, 2021, 15:30 UTC
Message-ID: <865b99dd-0b18-9a07-49c1-3959a777c685@gmail.com>
In-Reply-To: <37fda94785f1e689c7a7c32e69c6ff16fee7da4f.1638224692.git.me@ttaylorr.com>

On 11/29/2021 5:25 PM, Taylor Blau wrote:

> +static void enumerate_and_traverse_cruft_objects(struct string_list *fresh_packs)
> +{

...

Show 7 quoted lines

> +	/*
> +	 * Re-mark only the fresh packs as kept so that objects in
> +	 * unknown packs do not halt the reachability traversal early.
> +	 */
> +	for (p = get_all_packs(the_repository); p; p = p->next)
> +		p->pack_keep_in_core = 0;
> +	mark_pack_kept_in_core(fresh_packs, 1);

Are we ever going to recover this pack_keep_in_core state? Should we be saving it somewhere so we can return without mutating this state permanently?

Show 21 quoted lines

> +	if (prepare_revision_walk(&revs))
> +		die(_("revision walk setup failed"));
> +	if (progress)
> +		progress_state = start_progress(_("Traversing cruft objects"), 0);
> +	nr_seen = 0;
> +	traverse_commit_list(&revs, show_cruft_commit, show_cruft_object, NULL);
> +
> +	stop_progress(&progress_state);
> +}
> +
>  static void read_cruft_objects(void)
>  {
>  	struct strbuf buf = STRBUF_INIT;
> @@ -3515,7 +3597,7 @@ static void read_cruft_objects(void)
>  	mark_pack_kept_in_core(&discard_packs, 0);
>  
>  	if (cruft_expiration)
> -		die("--cruft-expiration not yet implemented");
> +		enumerate_and_traverse_cruft_objects(&fresh_packs);
>  	else
>  		enumerate_cruft_objects();

>  basic_cruft_pack_tests never
> +basic_cruft_pack_tests 2.weeks.ago

I'm surprised these tests didn't require any changes to adapt to the new expiration date. But I suppose none of the mtimes were older than two weeks ago?

I continue to miss something in these tests, because I don't see how things are becoming unreachable.

Thanks, -Stolee