Re: [PATCH 1/3] commit-reach: deduplicate queue entries in paint_down_to_common

[Derrick Stolee <stolee@gmail.com>]

On 5/24/26 7:40 PM, Junio C Hamano wrote:
> "Kristofer Karlsson via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
> 
>> diff --git a/commit-reach.c b/commit-reach.c
>> index d3a9b3ed6f..c16d4b061c 100644
>> --- a/commit-reach.c
>> +++ b/commit-reach.c
>> @@ -17,8 +17,9 @@
>>   #define PARENT2		(1u<<17)
>>   #define STALE		(1u<<18)
>>   #define RESULT		(1u<<19)
>> +#define ENQUEUED	(1u<<20)
>>   
>> -static const unsigned all_flags = (PARENT1 | PARENT2 | STALE | RESULT);
>> +static const unsigned all_flags = (PARENT1 | PARENT2 | STALE | RESULT | ENQUEUED);
>> ...
>> diff --git a/object.h b/object.h
>> index d814647ebe..05cbf728e9 100644
>> --- a/object.h
>> +++ b/object.h
>> @@ -74,7 +74,7 @@ void object_array_init(struct object_array *array);
>>    * bundle.c:                                        16
>>    * http-push.c:                          11-----14
>>    * commit-graph.c:                                15
>> - * commit-reach.c:                                  16-----19
>> + * commit-reach.c:                                  16-------20
>>    * builtin/last-modified.c:                         1617
>>    * sha1-name.c:                                              20
>>    * list-objects-filter.c:                                      21
> 
> Not directly the fault of this series, but we'd need to audit and
> update this table of bit assignment to match more recent reality.
> 
> For example, there no longer exists sha1-name.c but the table claims
> that bit 20 is in use for its own purpose, and it being stale makes
> it harder to audit and ensure that this new use would not crash with
> these existing uses (note. there are other uses of bit 20 in other
> subsystems).

It would be worth adding an update patch before this patch, that
only makes these adjustments

> FWIW, object-name.c, which was formerly known as sha1-name.c, uses
> the bit 20 as ONELINE_SEEN bit, which is used to turn textual object
> names like :/string (i.e., commit with that string in its message)
> into raw object name, and bit 20 is cleared from all the objects
> involved in the search before the helper function returns.

This appears to me like the only interaction that _could_ have
overlap with paint_down_to_common().

> Presumably, once commit-reach.c starts queueing commits and reuses
> this bit for its own purpose, we will never try to parse a textual
> commit object name to clobber what we thought is ENQUEUED bit,
> breaking the code introduced here, so we are probably safe against
> its use.
> 
> I didn't check all other uses of bit 20, though.

FLAG_LINK in builtin/index-pack.c and FLAG_OPEN in
builtin/unpack-objects.c both seem to be completely independent from
this use in commit-reach.c.

Thanks,
-Stolee