> quark wrote in rebase.py:1004
> Line 1088 is not `isancestor`. I'm okay with having two functions here. Let me know what do you prefer.
It looks like it effectively is. You can rewrite it using isancestor(). The reason I brought this up at all was that cl.ancestor() returns a single ancestor even if there are multiple (as in criss-cross merges), which made me wonder what would happen if the wrong ancestor was returned. I don't think there can be a "wrong" ancestor for the uses here, but using isancestor() instead makes that clearer.
Also, it seems like isancestor(a,b) can be made faster than ancestors(a,b)==a can because it can stop walking when when the rev is lower than a's rev.
> quark wrote in rebase.py:1010
> Interesting. I didn't know that we treat p1 specially here. If so, users can always construct an asymmetric case that feels wrong. For example, if we use B or C as E's parent in the above graph, p1 may not get chosen in one of the case.
> In this diff, it's Line 1091 trying to be conservative. I'll add tests and a warning like https://phab.mercurial-scm.org/D340 (Diff 770) line 1115.
I'm not sure I understand, but I don't think the user can influence the result (and I'm assuming you mean "as *D*'s parent", otherwise I can't make any sense of it). It should be fine to use either B' or C' as the new p1, as long as base is set to B or C (respectively).
> quark wrote in rebase.py:1019
> Maybe `assert i == 0` is a better assertion here. If `nullrev` is considered part of the DAG, then I think it makes sense for `adjustdest` to return `dest` for p2.
True, I can see the point about [nullrev,nullrev] being valid implying that any [X,X] should be valid.
How about the rest of my comment? I didn't understand what you meant by the first part of you reply.
> removing other
> - note: merging f9daf77ffe76+ and 4c5f12f25ebe using bids from ancestors a60552eb93fb and f59da8fc0fcf
Sorry, I forgot to ask before, but why did this change?