Patchwork D1782: rebase: don't run IMM if running rebase in a transaction

login
register
mail settings
Submitter phabricator
Date Dec. 27, 2017, 11:34 p.m.
Message ID <differential-rev-PHID-DREV-7xgsbgyamjgnlqfgksat-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/26475/
State Superseded
Headers show

Comments

phabricator - Dec. 27, 2017, 11:34 p.m.
phillco created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Some callers to rebase.rebase(), like `_moverelative` in `fbamend/movement.py`,
  wrap the entire rebase call in a transaction. This raises havoc when IMM tries
  to retry the rebase when it hits merge conflicts, because the abort will fail
  the whole transaction, not the subset. It also fails at the end, losing any
  conflict resolution, as @sid0 noticed.
  
  The right long-term fix that @quark and I have discussed is to change the
  restarting logic such that it doesn't abort at all, but simply switches between
  IMM and non-IMM fluidly for each commit, which has other nice properties. In
  the meantime this will do for now.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1782

AFFECTED FILES
  hgext/rebase.py

CHANGE DETAILS




To: phillco, #hg-reviewers
Cc: quark, mercurial-devel, sid0
phabricator - Dec. 27, 2017, 11:35 p.m.
quark accepted this revision.
quark added inline comments.

INLINE COMMENTS

> rebase.py:783
> +    if (opts.get('continue') or opts.get('abort') or
> +            repo.currenttransaction() is None):
>          # in-memory rebase is not compatible with resuming rebases.

The indentation is usually like:

  if (opts.get('continue') or opts.get('abort') or
      repo.currenttransaction() is None):

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1782

To: phillco, #hg-reviewers, quark
Cc: quark, mercurial-devel, sid0

Patch

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -779,8 +779,11 @@ 
 
     """
     inmemory = ui.configbool('rebase', 'experimental.inmemory')
-    if opts.get('continue') or opts.get('abort'):
+    if (opts.get('continue') or opts.get('abort') or
+            repo.currenttransaction() is None):
         # in-memory rebase is not compatible with resuming rebases.
+        # (Or if it is run within a transaction, since the restart logic can
+        # fail the entire transaction.)
         inmemory = False
 
     if inmemory: