Re: [PATCH v9 1/3] push: add reflog check for "--force-if-includes"

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 2, 2020, 20:14 UTC
Message-ID: <xmqqmu148jxu.fsf@gitster.c.googlers.com>
In-Reply-To: <20201002193906.GA78209@mail.clickyotomy.dev>

Srinidhi Kaushik <shrinidhi.kaushik@gmail.com> writes:

Show 19 quoted lines

> Hi Junio,
>
> On 10/02/2020 09:41, Junio C Hamano wrote:
>> Srinidhi Kaushik <shrinidhi.kaushik@gmail.com> writes:
>> 
>> >> We should not call `in_merge_bases_many()` repeatedly: there is a much
>> >> better API for that: `get_reachable_subset()`.
>> >
>> > Perfect. I wasn't aware of this.
>> 
>> This is possibly a piece of misinformation.  in_merge_bases_many()
>> is designed to be callable more than once.  get_reachable_subset()
>> may be an overkill as we only are interested in a single "is this
>> one an ancestor of any of these?", not "which ones among these are
>> ancestors of the other set?".
>
> Noted; even though "get_reachable_subset()" and "in_merge_bases_many()"
> (after the commit-graph fix) return the same result, I suppose the
> latter was designed for this specific use-case.

Yes, in_merge_bases_many() was invented first in 4c4b27e8 (commit.c: add in_merge_bases_many(), 2013-03-04) for this exact use case. For use cases where callers have multiple "these may be ancestors" candidates, instead of having to iterate over them and calling in_merge_bases_many() multiple times, get_reachable_subset() was added much later at fcb2c076 (commit-reach: implement get_reachable_subset, 2018-11-02).

> OK. Shall I update the next set by reverting the "disable commit-graph"
> change, s/list/array/ and leaving the rest as is -- if we decide to go
> forward with "in_merge_bases_many()", that is?

Yes, that would be the ideal endgame. What I pushed out to 'seen' has the removal of "disable" bit as a SQUASH??? commit at the tip, but not s/list/array renaming.

Thanks.