Re: [PATCH v2 2/5] pack-objects: reset kept-pack cache for cruft walk

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 22, 2026, 22:31 UTC
Message-ID: <xmqqjyocdijn.fsf@gitster.g>
In-Reply-To: <77aec8941f5d17654f58956c7c643b47dd5a8d93.1789700615.git.gitgitgadget@gmail.com>

"Qin ShiCheng via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 15 quoted lines

> @@ -4301,10 +4302,17 @@ static void enumerate_and_traverse_cruft_objects(struct string_list *fresh_packs
>  	/*
>  	 * Re-mark only the fresh packs as kept so that objects in
>  	 * unknown packs do not halt the reachability traversal early.
> +	 * The kept-pack cache was built while those packs were still
> +	 * marked, so drop it too.
>  	 */
>  	repo_for_each_pack(the_repository, p)
>  		p->pack_keep_in_core = 0;
>  	mark_pack_kept_in_core(fresh_packs, 1);
> +	for (source = the_repository->objects->sources; source;
> +	     source = source->next) {
> +		struct odb_source_files *files = odb_source_files_downcast(source);
> +		packfile_store_invalidate_kept_pack_cache(files->packed);
> +	}

This question is primarily meant for folks who are pushing different ODB backends, but I am not sure this is safe in the long term.

When downcasting finds that 'source' is not from the files backend, we immediately hit BUG(). Is checking the type of 'source' first and calling packfile_store_invalidate_kept_pack_cache() only when it is from the files backend a sensible workaround? That sounds like a blatant layering violation.

One of the recent design decisions, unrelated to this, was to make the concept of "alternate object store" an implementation detail of the files backend, if I recall correctly. Do we need a similar rearchitecting of the code here, pushing details like packfile management down to the files backend layer, before we can properly fix this?

Of course, until an ODB backend other than files materializes, all of the above is merely academic and the proposed change might be sufficient. However, relying on an unchecked downcast feels like laying mines for our future selves.