Patchwork [4,of,5,V2] rebase: allow aborting if last-message.txt is missing

login
register
mail settings
Submitter Durham Goode
Date March 8, 2017, 12:37 a.m.
Message ID <177c391caab3912e403e.1488933456@dev111.prn1.facebook.com>
Download mbox | patch
Permalink /patch/18973/
State Accepted
Headers show

Comments

Durham Goode - March 8, 2017, 12:37 a.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1488933031 28800
#      Tue Mar 07 16:30:31 2017 -0800
# Node ID 177c391caab3912e403e0eec469ea45a18c001f2
# Parent  8451bfc2e1d6260a0ce9dac505dd9f34fb3b19aa
rebase: allow aborting if last-message.txt is missing

Previously, if .hg/rebasestate existed but .hg/last-message.txt was missing, 'hg
rebase --abort' would say there's no rebase in progress but 'hg checkout foo'
would say 'abort: rebase in progress'. It turns out loading the collapse message
will throw a "no rebase in progress" error if the file doesn't exist, even
though .hg/rebasestate obviously indicates a rebase is in progress.

The fix is to only throw an exception if we're trying to --continue, and to just
eat the issues if we're doing --abort.

This issue is exposed by us writing the rebase state earlier in the process.
This will be used by later patches to ensure the user can appropriately 'hg
rebase --abort' if there's a crash before the first the first commit has
finished rebasing. Tests cover all of this. The only negative affect is we now
require a hg rebase --abort in a very specific exception case, as shown in the
test.

Patch

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -283,7 +283,7 @@  class rebaseruntime(object):
     def _prepareabortorcontinue(self, isabort):
         try:
             self.restorestatus()
-            self.collapsemsg = restorecollapsemsg(self.repo)
+            self.collapsemsg = restorecollapsemsg(self.repo, isabort)
         except error.RepoLookupError:
             if isabort:
                 clearstatus(self.repo)
@@ -369,6 +369,10 @@  class rebaseruntime(object):
         if self.activebookmark:
             bookmarks.deactivate(repo)
 
+        # Store the state before we begin so users can run 'hg rebase --abort'
+        # if we fail before the transaction closes.
+        self.storestatus()
+
         sortedrevs = repo.revs('sort(%ld, -topo)', self.state)
         cands = [k for k, v in self.state.iteritems() if v == revtodo]
         total = len(cands)
@@ -1092,7 +1096,7 @@  def clearcollapsemsg(repo):
     'Remove collapse message file'
     util.unlinkpath(repo.join("last-message.txt"), ignoremissing=True)
 
-def restorecollapsemsg(repo):
+def restorecollapsemsg(repo, isabort):
     'Restore previously stored collapse message'
     try:
         f = repo.vfs("last-message.txt")
@@ -1101,7 +1105,11 @@  def restorecollapsemsg(repo):
     except IOError as err:
         if err.errno != errno.ENOENT:
             raise
-        raise error.Abort(_('no rebase in progress'))
+        if isabort:
+            # Oh well, just abort like normal
+            collapsemsg = ''
+        else:
+            raise error.Abort(_('missing .hg/last-message.txt for rebase'))
     return collapsemsg
 
 def clearstatus(repo):
diff --git a/tests/test-rebase-conflicts.t b/tests/test-rebase-conflicts.t
--- a/tests/test-rebase-conflicts.t
+++ b/tests/test-rebase-conflicts.t
@@ -218,6 +218,7 @@  Check that the right ancestors is used w
   
   $ hg rebase -s9 -d2 --debug # use debug to really check merge base used
   rebase onto 4bc80088dc6b starting from e31216eec445
+  rebase status stored
   ignoring null merge rebase of 3
   ignoring null merge rebase of 4
   ignoring null merge rebase of 6
diff --git a/tests/test-rebase-scenario-global.t b/tests/test-rebase-scenario-global.t
--- a/tests/test-rebase-scenario-global.t
+++ b/tests/test-rebase-scenario-global.t
@@ -272,7 +272,8 @@  G onto B - merge revision with both pare
   rebasing 6:eea13746799a "G"
   abort: cannot use revision 6 as base, result would have 3 parents
   [255]
-
+  $ hg rebase --abort
+  rebase aborted
 
 These will abort gracefully (using --base):