[PATCH 2 of 2 V2] rebase: fix crash when rebase aborts while rebasing obsolete revisions

[Laurent Charignon]

On 3/28/16, 9:30 PM, "Martin von Zweigbergk" <martinvonz at google.com> wrote:

>On Mon, Mar 28, 2016 at 4:54 PM, Laurent Charignon <lcharignon at fb.com>
>wrote:
>> # HG changeset patch
>> # User Laurent Charignon <lcharignon at fb.com>
>> # Date 1459209113 25200
>> #      Mon Mar 28 16:51:53 2016 -0700
>> # Node ID 74b6115b17d0f76a500c2c678bde53592f9b192e
>> # Parent  179e9f0018b3d61f16a8c14bb26a9edb8e765be9
>> rebase: fix crash when rebase aborts while rebasing obsolete revisions
>>
>> Before this patch, rebase --continue would crash when trying to resume
>>a rebase
>> of obsolete revisions whose successors were in the destination.
>> This patch adds logic to recompute the mapping when rebase is resumed.
>>This
>> patch also adds a test that showcased the crash before the code change.
>>
>> diff --git a/hgext/rebase.py b/hgext/rebase.py
>> --- a/hgext/rebase.py
>> +++ b/hgext/rebase.py
>> @@ -279,6 +279,19 @@ def rebase(ui, repo, **opts):
>>                      msg = _('cannot continue inconsistent rebase')
>>                      hint = _('use "hg rebase --abort" to clear broken
>>state')
>>                      raise error.Abort(msg, hint=hint)
>
>Perhaps define "obsoletenotrebased = {}" here too like you do below so
>it works even with "hg --config experimental.rebaseskipobsolete=False
>rebase --continue" (I think).

Sure

>
>> +            if ui.configbool('experimental', 'rebaseskipobsolete',
>> +                             default=True):
>> +                rebaseobsrevs = set([r for r, status in state.items()
>> +                                     if status == revprecursor])
>> +                rebasesetrevs = set(state.keys())
>> +                obsoletenotrebased = _computeobsoletenotrebased(repo,
>> +               
>>rebaseobsrevs,
>> +                                                                target)
>> +                rebaseobsskipped = set(obsoletenotrebased)
>> +                checkdivergenceandemptyrebase(repo, ui, rebaseobsrevs,
>> +                                              rebasesetrevs,
>> +                                              rebaseobsskipped)
>> +
>>              if abortf:
>>                  return abort(repo, originalwd, target, state,
>>                               activebookmark=activebookmark)
>
>Should the new block be moved after the "if abortf:" block? I'm
>thinking we will want to allow the user to abort even if the rebase
>would now cause divergence.

Good point, I will send a V3
>
>> diff --git a/tests/test-rebase-obsolete.t b/tests/test-rebase-obsolete.t
>> --- a/tests/test-rebase-obsolete.t
>> +++ b/tests/test-rebase-obsolete.t
>> @@ -805,3 +805,59 @@ With experimental.allowdivergence=True,
>>    phases: 8 draft
>>    divergent: 2 changesets
>>
>> +rebase --continue + skipped rev because their successors are in
>>destination
>> +we make a change in trunk and work on conflicting changes to make
>>rebase abort.
>> +
>> +  $ hg log -G -r 17::
>> +  @  17:61bd55f69bc4 bar foo
>> +  |
>> +
>> +Create the two changes in trunk
>> +  $ printf "a" > willconflict
>> +  $ hg add willconflict
>> +  $ hg commit -m "willconflict first version"
>> +
>> +  $ printf "dummy" > C
>> +  $ hg commit -m "dummy change successor"
>> +
>> +Create the changes that we will rebase
>> +  $ hg update -C 17 -q
>> +  $ printf "b" > willconflict
>> +  $ hg add willconflict
>> +  $ hg commit -m "willconflict second version"
>> +  created new head
>> +  $ printf "dummy" > K
>> +  $ hg add K
>> +  $ hg commit -m "dummy change"
>> +  $ printf "dummy" > L
>> +  $ hg add L
>> +  $ hg commit -m "dummy change"
>> +  $ hg debugobsolete `hg log -r ".^" -T '{node}'` `hg log -r 19 -T
>>'{node}'` --config experimental.evolution=all
>> +
>> +  $ hg log -G -r 17::
>> +  @  22:7bdc8a87673d dummy change
>> +  |
>> +  x  21:8b31da3c4919 dummy change
>> +  |
>> +  o  20:b82fb57ea638 willconflict second version
>> +  |
>> +  | o  19:601db7a18f51 dummy change successor
>> +  | |
>> +  | o  18:357ddf1602d5 willconflict first version
>> +  |/
>> +  o  17:61bd55f69bc4 bar foo
>> +  |
>> +  $ hg rebase -r ".^^ + .^ + ." -d 19
>> +  rebasing 20:b82fb57ea638 "willconflict second version"
>> +  merging willconflict
>> +  warning: conflicts while merging willconflict! (edit, then use 'hg
>>resolve --mark')
>> +  unresolved conflicts (see hg resolve, then hg rebase --continue)
>> +  [1]
>> +
>> +  $ hg resolve --mark willconflict
>> +  (no more unresolved files)
>> +  continue: hg rebase --continue
>> +  $ hg rebase --continue
>> +  rebasing 20:b82fb57ea638 "willconflict second version"
>> +  note: not rebasing 21:8b31da3c4919 "dummy change", already in
>>destination as 19:601db7a18f51 "dummy change successor"
>> +  rebasing 22:7bdc8a87673d "dummy change" (tip)
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel at mercurial-scm.org
>> 
>>https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.
>>org_mailman_listinfo_mercurial-2Ddevel&d=CwIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&
>>r=qmwlQ6ljsf0--v3ANP53-V-RM6PPUtJ5zK5Y1fStJGg&m=NQmRkJJ3MGJYd2QpeJ8khExuh
>>l0Dks1OMYBvRdPdNL0&s=MZ7xQaEU3NIbqcmUFzcllmvNclZvXmbaPfIq6KxOyM4&e=