Patchwork [4,of,4] rebase: update working directory when aborting (issue5084)

login
register
mail settings
Submitter timeless@mozdev.org
Date Feb. 5, 2016, 2:09 a.m.
Message ID <84fe8a7dbcb98a17ac60.1454638185@waste.org>
Download mbox | patch
Permalink /patch/12997/
State Accepted
Headers show

Comments

timeless@mozdev.org - Feb. 5, 2016, 2:09 a.m.
# HG changeset patch
# User timeless <timeless@mozdev.org>
# Date 1454637406 0
#      Fri Feb 05 01:56:46 2016 +0000
# Node ID 84fe8a7dbcb98a17ac600b9936c7480d02298647
# Parent  f523a6414cecdce46685f7d3e67db942869d8803
rebase: update working directory when aborting (issue5084)
timeless - Feb. 5, 2016, 2:13 a.m.
This fixes a crash, it could /probably/ go to stable, but I'm not
particularly familiar w/ sending things to stable. The other changes
aren't necessary...

On Thu, Feb 4, 2016 at 9:09 PM, timeless <timeless@mozdev.org> wrote:
> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1454637406 0
> #      Fri Feb 05 01:56:46 2016 +0000
> # Node ID 84fe8a7dbcb98a17ac600b9936c7480d02298647
> # Parent  f523a6414cecdce46685f7d3e67db942869d8803
> rebase: update working directory when aborting (issue5084)
>
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -980,15 +980,20 @@
>              cleanup = False
>
>          if cleanup:
> +            shouldupdate = False
> +            rebased = filter(lambda x: x >= 0 and x != target, state.values())
> +            if rebased:
> +                strippoints = [
> +                        c.node() for c in repo.set('roots(%ld)', rebased)]
> +                shouldupdate = len([
> +                        c.node() for c in repo.set('. & (%ld)', rebased)]) > 0
> +
>              # Update away from the rebase if necessary
> -            if needupdate(repo, state):
> +            if shouldupdate or needupdate(repo, state):
>                  merge.update(repo, originalwd, False, True)
>
>              # Strip from the first rebased revision
> -            rebased = filter(lambda x: x >= 0 and x != target, state.values())
>              if rebased:
> -                strippoints = [
> -                        c.node()  for c in repo.set('roots(%ld)', rebased)]
>                  # no backup of rebased cset versions needed
>                  repair.strip(repo.ui, repo, strippoints)
>
> diff --git a/tests/failfilemerge.py b/tests/failfilemerge.py
> new file mode 100644
> --- /dev/null
> +++ b/tests/failfilemerge.py
> @@ -0,0 +1,18 @@
> +# extension to emulate interupting filemerge._filemerge
> +
> +from __future__ import absolute_import
> +
> +from mercurial import (
> +    filemerge,
> +    extensions,
> +    error,
> +)
> +
> +def failfilemerge(filemergefn,
> +        premerge, repo, mynode, orig, fcd, fco, fca, labels=None):
> +    raise error.Abort("^C")
> +    return filemergefn(premerge, repo, mynode, orig, fcd, fco, fca, labels)
> +
> +def extsetup(ui):
> +    extensions.wrapfunction(filemerge, '_filemerge',
> +                            failfilemerge)
> 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
> @@ -392,3 +392,75 @@
>    rebase aborted
>    $ cd ..
>
> +test aborting an interrupted series (issue5084)
> +  $ hg init interrupted
> +  $ cd interrupted
> +  $ touch base
> +  $ hg add base
> +  $ hg commit -m base
> +  $ touch a
> +  $ hg add a
> +  $ hg commit -m a
> +  $ echo 1 > a
> +  $ hg commit -m 1
> +  $ touch b
> +  $ hg add b
> +  $ hg commit -m b
> +  $ echo 2 >> a
> +  $ hg commit -m c
> +  $ touch d
> +  $ hg add d
> +  $ hg commit -m d
> +  $ hg co -q 1
> +  $ hg rm a
> +  $ hg commit -m no-a
> +  created new head
> +  $ hg co 0
> +  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  $ hg log -G --template "{rev} {desc} {bookmarks}"
> +  o  6 no-a
> +  |
> +  | o  5 d
> +  | |
> +  | o  4 c
> +  | |
> +  | o  3 b
> +  | |
> +  | o  2 1
> +  |/
> +  o  1 a
> +  |
> +  @  0 base
> +
> +  $ hg --config extensions.n=$TESTDIR/failfilemerge.py rebase -s 3 -d tip
> +  rebasing 3:3a71550954f1 "b"
> +  rebasing 4:e80b69427d80 "c"
> +  abort: ^C
> +  [255]
> +  $ hg rebase --abort
> +  saved backup bundle to $TESTTMP/interrupted/.hg/strip-backup/3d8812cf300d-93041a90-backup.hg (glob)
> +  rebase aborted
> +  $ hg log -G --template "{rev} {desc} {bookmarks}"
> +  o  6 no-a
> +  |
> +  | o  5 d
> +  | |
> +  | o  4 c
> +  | |
> +  | o  3 b
> +  | |
> +  | o  2 1
> +  |/
> +  o  1 a
> +  |
> +  @  0 base
> +
> +  $ hg summary
> +  parent: 0:df4f53cec30a
> +   base
> +  branch: default
> +  commit: (clean)
> +  update: 6 new changesets (update)
> +  phases: 7 draft
> +
> +  $ cd ..
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Augie Fackler - Feb. 5, 2016, 10:14 p.m.
On Thu, Feb 04, 2016 at 08:09:45PM -0600, timeless wrote:
> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1454637406 0
> #      Fri Feb 05 01:56:46 2016 +0000
> # Node ID 84fe8a7dbcb98a17ac600b9936c7480d02298647
> # Parent  f523a6414cecdce46685f7d3e67db942869d8803
> rebase: update working directory when aborting (issue5084)

Went ahead and queued this one for stable, thanks!

>
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -980,15 +980,20 @@
>              cleanup = False
>
>          if cleanup:
> +            shouldupdate = False
> +            rebased = filter(lambda x: x >= 0 and x != target, state.values())
> +            if rebased:
> +                strippoints = [
> +                        c.node() for c in repo.set('roots(%ld)', rebased)]
> +                shouldupdate = len([
> +                        c.node() for c in repo.set('. & (%ld)', rebased)]) > 0
> +
>              # Update away from the rebase if necessary
> -            if needupdate(repo, state):
> +            if shouldupdate or needupdate(repo, state):
>                  merge.update(repo, originalwd, False, True)
>
>              # Strip from the first rebased revision
> -            rebased = filter(lambda x: x >= 0 and x != target, state.values())
>              if rebased:
> -                strippoints = [
> -                        c.node()  for c in repo.set('roots(%ld)', rebased)]
>                  # no backup of rebased cset versions needed
>                  repair.strip(repo.ui, repo, strippoints)
>
> diff --git a/tests/failfilemerge.py b/tests/failfilemerge.py
> new file mode 100644
> --- /dev/null
> +++ b/tests/failfilemerge.py
> @@ -0,0 +1,18 @@
> +# extension to emulate interupting filemerge._filemerge
> +
> +from __future__ import absolute_import
> +
> +from mercurial import (
> +    filemerge,
> +    extensions,
> +    error,
> +)
> +
> +def failfilemerge(filemergefn,
> +        premerge, repo, mynode, orig, fcd, fco, fca, labels=None):
> +    raise error.Abort("^C")
> +    return filemergefn(premerge, repo, mynode, orig, fcd, fco, fca, labels)
> +
> +def extsetup(ui):
> +    extensions.wrapfunction(filemerge, '_filemerge',
> +                            failfilemerge)
> 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
> @@ -392,3 +392,75 @@
>    rebase aborted
>    $ cd ..
>
> +test aborting an interrupted series (issue5084)
> +  $ hg init interrupted
> +  $ cd interrupted
> +  $ touch base
> +  $ hg add base
> +  $ hg commit -m base
> +  $ touch a
> +  $ hg add a
> +  $ hg commit -m a
> +  $ echo 1 > a
> +  $ hg commit -m 1
> +  $ touch b
> +  $ hg add b
> +  $ hg commit -m b
> +  $ echo 2 >> a
> +  $ hg commit -m c
> +  $ touch d
> +  $ hg add d
> +  $ hg commit -m d
> +  $ hg co -q 1
> +  $ hg rm a
> +  $ hg commit -m no-a
> +  created new head
> +  $ hg co 0
> +  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  $ hg log -G --template "{rev} {desc} {bookmarks}"
> +  o  6 no-a
> +  |
> +  | o  5 d
> +  | |
> +  | o  4 c
> +  | |
> +  | o  3 b
> +  | |
> +  | o  2 1
> +  |/
> +  o  1 a
> +  |
> +  @  0 base
> +
> +  $ hg --config extensions.n=$TESTDIR/failfilemerge.py rebase -s 3 -d tip
> +  rebasing 3:3a71550954f1 "b"
> +  rebasing 4:e80b69427d80 "c"
> +  abort: ^C
> +  [255]
> +  $ hg rebase --abort
> +  saved backup bundle to $TESTTMP/interrupted/.hg/strip-backup/3d8812cf300d-93041a90-backup.hg (glob)
> +  rebase aborted
> +  $ hg log -G --template "{rev} {desc} {bookmarks}"
> +  o  6 no-a
> +  |
> +  | o  5 d
> +  | |
> +  | o  4 c
> +  | |
> +  | o  3 b
> +  | |
> +  | o  2 1
> +  |/
> +  o  1 a
> +  |
> +  @  0 base
> +
> +  $ hg summary
> +  parent: 0:df4f53cec30a
> +   base
> +  branch: default
> +  commit: (clean)
> +  update: 6 new changesets (update)
> +  phases: 7 draft
> +
> +  $ cd ..
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Katsunori FUJIWARA - March 2, 2016, 8 a.m.
At Thu, 04 Feb 2016 20:09:45 -0600,
timeless wrote:
> 
> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1454637406 0
> #      Fri Feb 05 01:56:46 2016 +0000
> # Node ID 84fe8a7dbcb98a17ac600b9936c7480d02298647
> # Parent  f523a6414cecdce46685f7d3e67db942869d8803
> rebase: update working directory when aborting (issue5084)
> 
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -980,15 +980,20 @@
>              cleanup = False
>  
>          if cleanup:
> +            shouldupdate = False
> +            rebased = filter(lambda x: x >= 0 and x != target, state.values())
> +            if rebased:
> +                strippoints = [
> +                        c.node() for c in repo.set('roots(%ld)', rebased)]
> +                shouldupdate = len([
> +                        c.node() for c in repo.set('. & (%ld)', rebased)]) > 0

(sorry for much late comment)

In this case, the result of '. & (%ld)' query for shouldupdate is used
only by len(), and node-nization isn't needed for len().

Therefore, "len(repo.revs('. & (%ld)', rebased))" seems enough, and
more efficient than implementation above. The former can omit
instantiation of changectx and invocation of node() on it for each
revisions.

(or, do I overlook any important side effect of node-nization in this case ?)

> +
>              # Update away from the rebase if necessary
> -            if needupdate(repo, state):
> +            if shouldupdate or needupdate(repo, state):
>                  merge.update(repo, originalwd, False, True)
>  
>              # Strip from the first rebased revision
> -            rebased = filter(lambda x: x >= 0 and x != target, state.values())
>              if rebased:
> -                strippoints = [
> -                        c.node()  for c in repo.set('roots(%ld)', rebased)]
>                  # no backup of rebased cset versions needed
>                  repair.strip(repo.ui, repo, strippoints)
>  
> diff --git a/tests/failfilemerge.py b/tests/failfilemerge.py
> new file mode 100644
> --- /dev/null
> +++ b/tests/failfilemerge.py
> @@ -0,0 +1,18 @@
> +# extension to emulate interupting filemerge._filemerge
> +
> +from __future__ import absolute_import
> +
> +from mercurial import (
> +    filemerge,
> +    extensions,
> +    error,
> +)
> +
> +def failfilemerge(filemergefn,
> +        premerge, repo, mynode, orig, fcd, fco, fca, labels=None):
> +    raise error.Abort("^C")
> +    return filemergefn(premerge, repo, mynode, orig, fcd, fco, fca, labels)
> +
> +def extsetup(ui):
> +    extensions.wrapfunction(filemerge, '_filemerge',
> +                            failfilemerge)
> 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
> @@ -392,3 +392,75 @@
>    rebase aborted
>    $ cd ..
>  
> +test aborting an interrupted series (issue5084)
> +  $ hg init interrupted
> +  $ cd interrupted
> +  $ touch base
> +  $ hg add base
> +  $ hg commit -m base
> +  $ touch a
> +  $ hg add a
> +  $ hg commit -m a
> +  $ echo 1 > a
> +  $ hg commit -m 1
> +  $ touch b
> +  $ hg add b
> +  $ hg commit -m b
> +  $ echo 2 >> a
> +  $ hg commit -m c
> +  $ touch d
> +  $ hg add d
> +  $ hg commit -m d
> +  $ hg co -q 1
> +  $ hg rm a
> +  $ hg commit -m no-a
> +  created new head
> +  $ hg co 0
> +  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  $ hg log -G --template "{rev} {desc} {bookmarks}"
> +  o  6 no-a
> +  |
> +  | o  5 d
> +  | |
> +  | o  4 c
> +  | |
> +  | o  3 b
> +  | |
> +  | o  2 1
> +  |/
> +  o  1 a
> +  |
> +  @  0 base
> +  
> +  $ hg --config extensions.n=$TESTDIR/failfilemerge.py rebase -s 3 -d tip
> +  rebasing 3:3a71550954f1 "b"
> +  rebasing 4:e80b69427d80 "c"
> +  abort: ^C
> +  [255]
> +  $ hg rebase --abort
> +  saved backup bundle to $TESTTMP/interrupted/.hg/strip-backup/3d8812cf300d-93041a90-backup.hg (glob)
> +  rebase aborted
> +  $ hg log -G --template "{rev} {desc} {bookmarks}"
> +  o  6 no-a
> +  |
> +  | o  5 d
> +  | |
> +  | o  4 c
> +  | |
> +  | o  3 b
> +  | |
> +  | o  2 1
> +  |/
> +  o  1 a
> +  |
> +  @  0 base
> +  
> +  $ hg summary
> +  parent: 0:df4f53cec30a 
> +   base
> +  branch: default
> +  commit: (clean)
> +  update: 6 new changesets (update)
> +  phases: 7 draft
> +
> +  $ cd ..
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
timeless - March 7, 2016, 5:10 p.m.
FUJIWARA Katsunori <foozy@lares.dti.ne.jp> wrote:
> In this case, the result of '. & (%ld)' query for shouldupdate is used
> only by len(), and node-nization isn't needed for len().
>
> Therefore, "len(repo.revs('. & (%ld)', rebased))" seems enough, and
> more efficient than implementation above. The former can omit
> instantiation of changectx and invocation of node() on it for each
> revisions.
>
> (or, do I overlook any important side effect of node-nization in this case ?)

I don't understand revsets anywhere near enough. If you're confident,
please feel free to submit a followup.

Patch

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -980,15 +980,20 @@ 
             cleanup = False
 
         if cleanup:
+            shouldupdate = False
+            rebased = filter(lambda x: x >= 0 and x != target, state.values())
+            if rebased:
+                strippoints = [
+                        c.node() for c in repo.set('roots(%ld)', rebased)]
+                shouldupdate = len([
+                        c.node() for c in repo.set('. & (%ld)', rebased)]) > 0
+
             # Update away from the rebase if necessary
-            if needupdate(repo, state):
+            if shouldupdate or needupdate(repo, state):
                 merge.update(repo, originalwd, False, True)
 
             # Strip from the first rebased revision
-            rebased = filter(lambda x: x >= 0 and x != target, state.values())
             if rebased:
-                strippoints = [
-                        c.node()  for c in repo.set('roots(%ld)', rebased)]
                 # no backup of rebased cset versions needed
                 repair.strip(repo.ui, repo, strippoints)
 
diff --git a/tests/failfilemerge.py b/tests/failfilemerge.py
new file mode 100644
--- /dev/null
+++ b/tests/failfilemerge.py
@@ -0,0 +1,18 @@ 
+# extension to emulate interupting filemerge._filemerge
+
+from __future__ import absolute_import
+
+from mercurial import (
+    filemerge,
+    extensions,
+    error,
+)
+
+def failfilemerge(filemergefn,
+        premerge, repo, mynode, orig, fcd, fco, fca, labels=None):
+    raise error.Abort("^C")
+    return filemergefn(premerge, repo, mynode, orig, fcd, fco, fca, labels)
+
+def extsetup(ui):
+    extensions.wrapfunction(filemerge, '_filemerge',
+                            failfilemerge)
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
@@ -392,3 +392,75 @@ 
   rebase aborted
   $ cd ..
 
+test aborting an interrupted series (issue5084)
+  $ hg init interrupted
+  $ cd interrupted
+  $ touch base
+  $ hg add base
+  $ hg commit -m base
+  $ touch a
+  $ hg add a
+  $ hg commit -m a
+  $ echo 1 > a
+  $ hg commit -m 1
+  $ touch b
+  $ hg add b
+  $ hg commit -m b
+  $ echo 2 >> a
+  $ hg commit -m c
+  $ touch d
+  $ hg add d
+  $ hg commit -m d
+  $ hg co -q 1
+  $ hg rm a
+  $ hg commit -m no-a
+  created new head
+  $ hg co 0
+  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ hg log -G --template "{rev} {desc} {bookmarks}"
+  o  6 no-a
+  |
+  | o  5 d
+  | |
+  | o  4 c
+  | |
+  | o  3 b
+  | |
+  | o  2 1
+  |/
+  o  1 a
+  |
+  @  0 base
+  
+  $ hg --config extensions.n=$TESTDIR/failfilemerge.py rebase -s 3 -d tip
+  rebasing 3:3a71550954f1 "b"
+  rebasing 4:e80b69427d80 "c"
+  abort: ^C
+  [255]
+  $ hg rebase --abort
+  saved backup bundle to $TESTTMP/interrupted/.hg/strip-backup/3d8812cf300d-93041a90-backup.hg (glob)
+  rebase aborted
+  $ hg log -G --template "{rev} {desc} {bookmarks}"
+  o  6 no-a
+  |
+  | o  5 d
+  | |
+  | o  4 c
+  | |
+  | o  3 b
+  | |
+  | o  2 1
+  |/
+  o  1 a
+  |
+  @  0 base
+  
+  $ hg summary
+  parent: 0:df4f53cec30a 
+   base
+  branch: default
+  commit: (clean)
+  update: 6 new changesets (update)
+  phases: 7 draft
+
+  $ cd ..