Re: [PATCH] revision: mark blobs needed for resolve-undo as reachable

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 13, 2022, 20:11 UTC
Message-ID: <xmqqczfc5nob.fsf@gitster.g>
In-Reply-To: <7cd41846-e6ef-7a24-0426-6031a529360f@github.com>

Derrick Stolee <derrickstolee@github.com> writes:

Show 13 quoted lines

> On 6/9/2022 7:44 PM, Junio C Hamano wrote:
>
>> +	struct string_list *resolve_undo = istate->resolve_undo;
>> +
>> +	if (!resolve_undo)
>> +		return 0;
>> +
>> +	for_each_string_list_item(item, resolve_undo) {
>
> I see this is necessary since for_each_string_list_item() does not
> handle NULL lists.  After attempting to allow it to handle NULL
> lists, I see that the compiler complains about the cases where it
> would _never_ be NULL, so that change appears to be impossible.

Heh, no such deep thought went into this. I just copied what is done in builtin/ls-files.c::show_ru_info() that grabs these blobs and does something interesting and the only difference is this does something else interesting.

We _could_ refactor these to into "take a callback and iterate over resolve-undo information" shell plus two callback functions, one to print them and the other to mark them still reachable, but the iterator being relatively short, I doubt that it is worth it.

> The patch looks good. I liked the comments for the three phases
> of the test.

Thanks.