Patchwork [STABLE] rebase: prevent invalid rebase --abort from causing data loss

login
register
mail settings
Submitter Durham Goode
Date Aug. 1, 2013, 9:01 p.m.
Message ID <26bad57dcb7da0bb754c.1375390895@dev350.prn1.facebook.com>
Download mbox | patch
Permalink /patch/1980/
State Superseded
Headers show

Comments

Durham Goode - Aug. 1, 2013, 9:01 p.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1375390645 25200
#      Thu Aug 01 13:57:25 2013 -0700
# Branch stable
# Node ID 26bad57dcb7da0bb754c9ecb73eebeecf7e04753
# Parent  e86d8b2eba7a09db8ac87d446befaa439ef5be65
rebase: prevent invalid rebase --abort from causing data loss

If a user has an old .hg/rebasestate file in their repo and upgrades
to mercurial 2.7 they will get a message saying they need to abort the
rebase, even though that rebasestate is from long ago. If the user does
run 'rebase --abort' like the message tells them, it wipes out their
pending changes, causing data loss.

We've had this happen to two users already.

The fix is to just unlink the rebasestate file in that case. The rebase
is completely invalid, so we might as well throw it away.
Durham Goode - Aug. 1, 2013, 10:19 p.m.
Matt is working on a refactor of this that addresses the issue a bit more
specifically.  So ignore this patch.

On 8/1/13 2:01 PM, "Durham Goode" <durham@fb.com> wrote:

># HG changeset patch
># User Durham Goode <durham@fb.com>
># Date 1375390645 25200
>#      Thu Aug 01 13:57:25 2013 -0700
># Branch stable
># Node ID 26bad57dcb7da0bb754c9ecb73eebeecf7e04753
># Parent  e86d8b2eba7a09db8ac87d446befaa439ef5be65
>rebase: prevent invalid rebase --abort from causing data loss
>
>If a user has an old .hg/rebasestate file in their repo and upgrades
>to mercurial 2.7 they will get a message saying they need to abort the
>rebase, even though that rebasestate is from long ago. If the user does
>run 'rebase --abort' like the message tells them, it wipes out their
>pending changes, causing data loss.
>
>We've had this happen to two users already.
>
>The fix is to just unlink the rebasestate file in that case. The rebase
>is completely invalid, so we might as well throw it away.
>
>diff --git a/hgext/rebase.py b/hgext/rebase.py
>--- a/hgext/rebase.py
>+++ b/hgext/rebase.py
>@@ -592,8 +592,26 @@
>             raise
>         raise util.Abort(_('no rebase in progress'))
> 
>+def isvalidstate(repo, originalwd, state):
>+    parents = [p.rev() for p in repo.parents()]
>+    if originalwd in parents:
>+        return True
>+
>+    for newrev in state.itervalues():
>+        if newrev in parents:
>+            return True
>+
>+    return False
>+
> def abort(repo, originalwd, target, state):
>     'Restore the repository to its original state'
>+    if not isvalidstate(repo, originalwd, state):
>+        repo.ui.warn(_('warning: the pending rebase is no longer valid -
>' +
>+                       'it has been aborted but commits and pending
>changes ' +
>+                       'were kept to prevent data loss\n'))
>+        util.unlink(repo.join('rebasestate'))
>+        return 0
>+
>     dstates = [s for s in state.values() if s != nullrev]
>     immutable = [d for d in dstates if not repo[d].mutable()]
>     if immutable:
>diff --git a/tests/test-rebase-interruptions.t
>b/tests/test-rebase-interruptions.t
>--- a/tests/test-rebase-interruptions.t
>+++ b/tests/test-rebase-interruptions.t
>@@ -180,8 +180,7 @@
> Abort the rebasing:
> 
>   $ hg rebase --abort
>-  warning: new changesets detected on target branch, can't abort
>-  [255]
>+  warning: the pending rebase is no longer valid - it has been aborted
>but commits and pending changes were kept to prevent data loss
> 
>   $ hg tglog
>   @  6: 'Extra'
>_______________________________________________
>Mercurial-devel mailing list
>Mercurial-devel@selenic.com
>http://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
@@ -592,8 +592,26 @@ 
             raise
         raise util.Abort(_('no rebase in progress'))
 
+def isvalidstate(repo, originalwd, state):
+    parents = [p.rev() for p in repo.parents()]
+    if originalwd in parents:
+        return True
+
+    for newrev in state.itervalues():
+        if newrev in parents:
+            return True
+
+    return False
+
 def abort(repo, originalwd, target, state):
     'Restore the repository to its original state'
+    if not isvalidstate(repo, originalwd, state):
+        repo.ui.warn(_('warning: the pending rebase is no longer valid - ' +
+                       'it has been aborted but commits and pending changes ' +
+                       'were kept to prevent data loss\n'))
+        util.unlink(repo.join('rebasestate'))
+        return 0
+
     dstates = [s for s in state.values() if s != nullrev]
     immutable = [d for d in dstates if not repo[d].mutable()]
     if immutable:
diff --git a/tests/test-rebase-interruptions.t b/tests/test-rebase-interruptions.t
--- a/tests/test-rebase-interruptions.t
+++ b/tests/test-rebase-interruptions.t
@@ -180,8 +180,7 @@ 
 Abort the rebasing:
 
   $ hg rebase --abort
-  warning: new changesets detected on target branch, can't abort
-  [255]
+  warning: the pending rebase is no longer valid - it has been aborted but commits and pending changes were kept to prevent data loss
 
   $ hg tglog
   @  6: 'Extra'