Re: [PATCH v2 2/3] for-each-ref: add 'is-base' token

[Derrick Stolee <stolee@gmail.com>]

On 8/12/24 5:05 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

>> +		} else if (atom_type == ATOM_ISBASE) {
>> +			if (ref->is_base && ref->is_base[is_base_atoms]) {
>> +				v->s = xstrfmt("(%s)", ref->is_base[is_base_atoms]);
>> +				free(ref->is_base[is_base_atoms]);
>> +			} else {
>> +				/* Not a commit. */
> 
> This is unexpected.  I thought that most of the branches except at
> most one that gets annotated with "Yeah, this is forked from branch
> B" would take the "else" side.  They are still commits, no?

You are correct. This is leftover from copy-pasting the ahead-behind section.
Will remove.

>> +	for (size_t i = 0; i < array->nr; i++) {
>> +		const char *name = array->items[i]->refname;
>> +		struct commit *c = lookup_commit_reference_by_name(name);
>> +
>> +		CALLOC_ARRAY(array->items[i]->is_base, format->is_base_tips.nr);
>> +
>> +		if (!c)
>> +			continue;
> 
> Hmph, wouldn't we want to leave array->items[i]->is_base NULL if
> "name" looked up to "c" happens to be non-commit (i.e. NULL)?

Your comment initially made me second-guess the logic here, but...

>> +		back_index[bases_nr] = array->items[i];
>> +		bases[bases_nr] = c;
>> +		bases_nr++;

This array of "back_index" is intended to allow the array being passed to
get_branch_base_for_tip() to have no gaps with NULL commits. The indices
are then translated back to the original array items when scanning the
results.

This matches the behavior of the ahead-behind code, giving an existing
behavior. The alternative would be to allow get_branch_base_for_tip() to
be sensitive to NULL commits in the 'bases' array. But since we need to
create an array of commit pointers (different from the array of ref
items that we start with) this is likely the simplest approach.

You did inspire me to double-check that this code works in the presence
of non-commit refs, so I'll update some things and send a v3 with a new
test. It will also include some things to make error messages quieter
for that case.

Thanks,
-Stolee