Hi Junio, On 10/02/2020 09:41, Junio C Hamano wrote: > Srinidhi Kaushik <shrinidhi.kaushik@xxxxxxxxx> 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. > > OK. The tests are passing with or without "GIT_TEST_COMMIT_GRAPH" > > by switching to "get_reachable_subset()" we don't have to toggle > > te feature during the check. > > Correct. Once the "see if this one is reachable from any of these" > is fixed (either by correcting the broken in_merge_bases_many() or > using get_reachable_subset()), we can get rid of this hack. 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? > > Again, thank you so much working on this! If you'd like, I can go ahead > > and apply these patches and rename "reflog_commit_list" to "commit_array" > > in the next series (v10). > > I like the s/list/array/ change, but I do not think switching to > get_reachable_subset() and having to receive a commit list only to > free the list is warranted. > > Derrick sent a fix to in_merge_bases_many() in the near-by thread. Nice! Will take a look. Thanks. -- Srinidhi Kaushik
