[PATCH STABLE V3] rebase: move bookmark to destination for commits becoming empty (issue5627)

Martin von Zweigbergk martinvonz at google.com
Wed Jul 26 16:37:45 EDT 2017


On Wed, Jul 26, 2017 at 1:31 PM, Jun Wu <quark at fb.com> wrote:
> Excerpts from Martin von Zweigbergk's message of 2017-07-26 11:23:33 -0700:
>> > The `adjustdest` function calculates the destination of bookmark movement.
>> > It was back-ported from https://phab.mercurial-scm.org/D21 . It might be
>> > slightly more powerful than the minimal requirement to solve this issue.
>> > For example, it's impossible for a merge changeset to become empty while any
>> > of its ancestors does not become empty, but the code could handle that case.
>>
>> IIUC, that will be true even with D21. So you're saying that it just
>> happens to handle that hypothetical situation, not that that it was
>> needed for D21 and you didn't bother removing that feature while
>> back-porting it, right?
>
> The "minimal requirement" I was referring to is an alternative algorithm,
> which is very different from "adjustdest" but simpler. It's hard to remove
> features from "adjustdest" (so it's not me not bothering doing it).
>
> When considering the end result (with multi-dest) of rebase.py, I think it's
> overall simpler to not introduce that alternative algorithm.
>
>> But do note (as you probably already have) that the current rebase
>> code (pre-D21) will behave differently between p1 and p2 in this case:
>> [...]
>> If you instead revert E's contents to C and up D in the upstream, you
>> will not get the merge commit E after rebase. I'm not sure which is
>> better, but they should clearly not be different.
>
> Seems another bug of the old code. I didn't spend much to investigate the
> old code since I considered it as "not maintainable". But since I also found
> some cases manually, I might try to find more using some automation.
>
>> > +    rev is what being rebased. Return a list of two revs, which are the
>>
>> I'm adding the missing "rev is what *is* being" here.
>
> Sorry for this and issues below. I thought I had said "I will send a patch
> today" but I didn't. So it was completed in a hurry.

No problem, those were minor things that anyone might miss even if
they spent time. As I said, fixed in flight. Now also queued. Thanks!


More information about the Mercurial-devel mailing list