This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1782

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

Authored by phillco on Dec 27 2017, 6:34 PM.

Details

Reviewers
quark
Group Reviewers
hg-reviewers
Commits
rHGc5d220a621e7: rebase: don't run IMM if running rebase in a transaction
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.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

phillco created this revision.Dec 27 2017, 6:34 PM
quark accepted this revision.Dec 27 2017, 6:35 PM
quark added inline comments.
hgext/rebase.py
775

The indentation is usually like:

if (opts.get('continue') or opts.get('abort') or
    repo.currenttransaction() is None):
phillco updated this revision to Diff 4645.Dec 27 2017, 6:36 PM
phillco updated this revision to Diff 4647.
phillco updated this revision to Diff 4648.Dec 27 2017, 6:44 PM
Closed by commit rHGc5d220a621e7: rebase: don't run IMM if running rebase in a transaction (authored by phillco). · Explain WhyJan 18 2018, 8:01 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/rebase.py (5 lines)

Diff 4869

hgext/rebase.py

Return Values: Return Values:
Returns 0 on success, 1 if nothing to rebase or there are Returns 0 on success, 1 if nothing to rebase or there are
unresolved conflicts. unresolved conflicts.
""" """
inmemory = ui.configbool('rebase', 'experimental.inmemory') 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 not None):
quarkUnsubmitted
Not Done

The indentation is usually like:

if (opts.get('continue') or opts.get('abort') or
    repo.currenttransaction() is None):
quark: The indentation is usually like: if (opts.get('continue') or opts.get('abort') or…
# in-memory rebase is not compatible with resuming rebases. # 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 inmemory = False
if inmemory: if inmemory:
try: try:
# in-memory merge doesn't support conflicts, so if we hit any, abort # in-memory merge doesn't support conflicts, so if we hit any, abort
# and re-run as an on-disk merge. # and re-run as an on-disk merge.
return _origrebase(ui, repo, inmemory=inmemory, **opts) return _origrebase(ui, repo, inmemory=inmemory, **opts)
except error.InMemoryMergeConflictsError: except error.InMemoryMergeConflictsError: