Patchwork rebase: properly abort when destination is public (issue4896)

login
register
mail settings
Submitter Christian Delahousse
Date Oct. 14, 2015, 8:28 p.m.
Message ID <25b0907d39de22cf628d.1444854522@dev4253.prn1.facebook.com>
Download mbox | patch
Permalink /patch/11075/
State Accepted
Headers show

Comments

Christian Delahousse - Oct. 14, 2015, 8:28 p.m.
# HG changeset patch
# User Christian Delahousse <cdelahousse@fb.com>
# Date 1444770411 25200
#      Tue Oct 13 14:06:51 2015 -0700
# Node ID 25b0907d39de22cf628d610565400b5372539630
# Parent  07db7e95c464537aeb2dd7aba39de0813eaffd04
rebase: properly abort when destination is public (issue4896)

After rebasing a set of changes onto a public changeset and having the first one
be skipped, if you try to abort, the operation fails. This fix adds a check to
disallow the target rev into the dstates list within the abort function. This
list is checked for immutable states before the rest of abort does its thing.
Durham Goode - Oct. 14, 2015, 10:37 p.m.
On 10/14/15, 1:28 PM, "Mercurial-devel on behalf of Christian Delahousse" <mercurial-devel-bounces@selenic.com on behalf of cdelahousse@fb.com> wrote:

># HG changeset patch

># User Christian Delahousse <cdelahousse@fb.com>

># Date 1444770411 25200

>#      Tue Oct 13 14:06:51 2015 -0700

># Node ID 25b0907d39de22cf628d610565400b5372539630

># Parent  07db7e95c464537aeb2dd7aba39de0813eaffd04

>rebase: properly abort when destination is public (issue4896)

>

>After rebasing a set of changes onto a public changeset and having the first one

>be skipped, if you try to abort, the operation fails. This fix adds a check to

>disallow the target rev into the dstates list within the abort function. This

>list is checked for immutable states before the rest of abort does its thing.


To be clear, by "the operation fails" we mean the repository is left in a merge state (without us telling them) and the user has to figure out how to get out of it themselves.
Augie Fackler - Oct. 15, 2015, 5:21 p.m.
On Wed, Oct 14, 2015 at 01:28:42PM -0700, Christian Delahousse wrote:
> # HG changeset patch
> # User Christian Delahousse <cdelahousse@fb.com>
> # Date 1444770411 25200
> #      Tue Oct 13 14:06:51 2015 -0700
> # Node ID 25b0907d39de22cf628d610565400b5372539630
> # Parent  07db7e95c464537aeb2dd7aba39de0813eaffd04
> rebase: properly abort when destination is public (issue4896)

This has been clowncopterized, thanks!

>
> After rebasing a set of changes onto a public changeset and having the first one
> be skipped, if you try to abort, the operation fails. This fix adds a check to
> disallow the target rev into the dstates list within the abort function. This
> list is checked for immutable states before the rest of abort does its thing.
>
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -917,7 +917,11 @@
>
>      activebookmark: the name of the bookmark that should be active after the
>          restore'''
> -    dstates = [s for s in state.values() if s >= 0]
> +
> +    # If the first commits in the rebased set get skipped during the rebase,
> +    # their values within the state mapping will be the target rev id. The
> +    # dstates list must must not contain the target rev (issue4896)
> +    dstates = [s for s in state.values() if s >= 0 and s != target]
>      immutable = [d for d in dstates if not repo[d].mutable()]
>      cleanup = True
>      if immutable:
> diff --git a/tests/test-rebase-abort.t b/tests/test-rebase-abort.t
> --- a/tests/test-rebase-abort.t
> +++ b/tests/test-rebase-abort.t
> @@ -318,3 +318,37 @@
>    commit: (clean)
>    update: 1 new changesets, 2 branch heads (merge)
>    phases: 4 draft
> +
> +test aborting a rebase succeeds after rebasing with skipped commits onto a
> +public changeset (issue4896)
> +
> +  $ hg init succeedonpublic
> +  $ cd succeedonpublic
> +  $ echo 'content' > root
> +  $ hg commit -A -m 'root' -q
> +
> +set up public branch
> +  $ echo 'content' > disappear
> +  $ hg commit -A -m 'disappear public' -q
> +commit will cause merge conflict on rebase
> +  $ echo '' > root
> +  $ hg commit -m 'remove content public' -q
> +  $ hg phase --public
> +
> +setup the draft branch that will be rebased onto public commit
> +  $ hg up -r 0 -q
> +  $ echo 'content' > disappear
> +commit will disappear
> +  $ hg commit -A -m 'disappear draft' -q
> +  $ echo 'addedcontADDEDentadded' > root
> +commit will cause merge conflict on rebase
> +  $ hg commit -m 'add content draft' -q
> +
> +  $ hg rebase -d 'public()' --tool :merge -q
> +  note: rebase of 3:0682fd3dabf5 created no changes to commit
> +  warning: conflicts while merging root! (edit, then use 'hg resolve --mark')
> +  unresolved conflicts (see hg resolve, then hg rebase --continue)
> +  [1]
> +  $ hg rebase --abort
> +  rebase aborted
> +
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -917,7 +917,11 @@ 
 
     activebookmark: the name of the bookmark that should be active after the
         restore'''
-    dstates = [s for s in state.values() if s >= 0]
+
+    # If the first commits in the rebased set get skipped during the rebase,
+    # their values within the state mapping will be the target rev id. The
+    # dstates list must must not contain the target rev (issue4896)
+    dstates = [s for s in state.values() if s >= 0 and s != target]
     immutable = [d for d in dstates if not repo[d].mutable()]
     cleanup = True
     if immutable:
diff --git a/tests/test-rebase-abort.t b/tests/test-rebase-abort.t
--- a/tests/test-rebase-abort.t
+++ b/tests/test-rebase-abort.t
@@ -318,3 +318,37 @@ 
   commit: (clean)
   update: 1 new changesets, 2 branch heads (merge)
   phases: 4 draft
+
+test aborting a rebase succeeds after rebasing with skipped commits onto a
+public changeset (issue4896)
+
+  $ hg init succeedonpublic
+  $ cd succeedonpublic
+  $ echo 'content' > root
+  $ hg commit -A -m 'root' -q
+
+set up public branch
+  $ echo 'content' > disappear
+  $ hg commit -A -m 'disappear public' -q
+commit will cause merge conflict on rebase
+  $ echo '' > root
+  $ hg commit -m 'remove content public' -q
+  $ hg phase --public
+
+setup the draft branch that will be rebased onto public commit
+  $ hg up -r 0 -q
+  $ echo 'content' > disappear
+commit will disappear
+  $ hg commit -A -m 'disappear draft' -q
+  $ echo 'addedcontADDEDentadded' > root
+commit will cause merge conflict on rebase
+  $ hg commit -m 'add content draft' -q
+
+  $ hg rebase -d 'public()' --tool :merge -q
+  note: rebase of 3:0682fd3dabf5 created no changes to commit
+  warning: conflicts while merging root! (edit, then use 'hg resolve --mark')
+  unresolved conflicts (see hg resolve, then hg rebase --continue)
+  [1]
+  $ hg rebase --abort
+  rebase aborted
+