Patchwork [V2] rebase: don't rebase obsolete commit whose successor is already rebased

login
register
mail settings
Submitter Laurent Charignon
Date Sept. 23, 2015, 11:59 p.m.
Message ID <fc1ddcad33586c10bcc6.1443052766@lcharignon-mbp.local>
Download mbox | patch
Permalink /patch/10602/
State Accepted
Headers show

Comments

Laurent Charignon - Sept. 23, 2015, 11:59 p.m.
# HG changeset patch
# User Laurent Charignon <lcharignon@fb.com>
# Date 1442277108 25200
#      Mon Sep 14 17:31:48 2015 -0700
# Node ID fc1ddcad33586c10bcc643f28798a96e88992849
# Parent  60558319ce724e8377c56591af3089380753f6de
rebase: don't rebase obsolete commit whose successor is already rebased

This patch avoids unnecessary conflicts to resolve during rebase for the users
of changeset evolution.

This patch modifies rebase to skip obsolete commits if they are being rebased on
their successors.
It introduces a new rebase state 'revobsolete' for these revisions that are
being skipped and a new message to inform the user of what is happening.
This feature is gated behind the config flag experimental.rebaseskipobsolete

When an obsolete commit is skipped, the output is:
not rebasing 14:9ad579b4a5de "I", already in destination as 17:fc37a630c901 "K"
Matt Mackall - Sept. 25, 2015, 2:46 a.m.
On Wed, 2015-09-23 at 16:59 -0700, Laurent Charignon wrote:
> # HG changeset patch
> # User Laurent Charignon <lcharignon@fb.com>
> # Date 1442277108 25200
> #      Mon Sep 14 17:31:48 2015 -0700
> # Node ID fc1ddcad33586c10bcc643f28798a96e88992849
> # Parent  60558319ce724e8377c56591af3089380753f6de
> rebase: don't rebase obsolete commit whose successor is already rebased

Queued for default, thanks.

Patch

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -26,6 +26,7 @@  import os, errno
 revtodo = -1
 nullmerge = -2
 revignored = -3
+revprecursor = -4
 
 cmdtable = {}
 command = cmdutil.command(cmdtable)
@@ -333,7 +334,20 @@  def rebase(ui, repo, **opts):
                       " unrebased descendants"),
                     hint=_('use --keep to keep original changesets'))
 
-            result = buildstate(repo, dest, rebaseset, collapsef)
+            obsoletenotrebased = {}
+            if ui.configbool('experimental', 'rebaseskipobsolete'):
+                rebasesetrevs = set(rebaseset)
+                obsoletenotrebased = _computeobsoletenotrebased(repo,
+                                                                rebasesetrevs,
+                                                                dest)
+
+                # - plain prune (no successor) changesets are rebased
+                # - split changesets are not rebased if at least one of the
+                # changeset resulting from the split is an ancestor of dest
+                rebaseset = rebasesetrevs - set(obsoletenotrebased)
+            result = buildstate(repo, dest, rebaseset, collapsef,
+                                obsoletenotrebased)
+
             if not result:
                 # Empty state built, nothing to rebase
                 ui.status(_('nothing to rebase\n'))
@@ -439,6 +453,12 @@  def rebase(ui, repo, **opts):
                 ui.debug('ignoring null merge rebase of %s\n' % rev)
             elif state[rev] == revignored:
                 ui.status(_('not rebasing ignored %s\n') % desc)
+            elif state[rev] == revprecursor:
+                targetctx = repo[obsoletenotrebased[rev]]
+                desctarget = '%d:%s "%s"' % (targetctx.rev(), targetctx,
+                             targetctx.description().split('\n', 1)[0])
+                msg = _('note: not rebasing %s, already in destination as %s\n')
+                ui.status(msg % (desc, desctarget))
             else:
                 ui.status(_('already rebased %s as %s\n') %
                           (desc, repo[state[rev]]))
@@ -620,7 +640,7 @@  def defineparents(repo, rev, target, sta
     elif p1n in state:
         if state[p1n] == nullmerge:
             p1 = target
-        elif state[p1n] == revignored:
+        elif state[p1n] in (revignored, revprecursor):
             p1 = nearestrebased(repo, p1n, state)
             if p1 is None:
                 p1 = target
@@ -636,7 +656,7 @@  def defineparents(repo, rev, target, sta
         if p2n in state:
             if p1 == target: # p1n in targetancestors or external
                 p1 = state[p2n]
-            elif state[p2n] == revignored:
+            elif state[p2n] in (revignored, revprecursor):
                 p2 = nearestrebased(repo, p2n, state)
                 if p2 is None:
                     # no ancestors rebased yet, detach
@@ -824,7 +844,8 @@  def restorestatus(repo):
                 activebookmark = l
             else:
                 oldrev, newrev = l.split(':')
-                if newrev in (str(nullmerge), str(revignored)):
+                if newrev in (str(nullmerge), str(revignored),
+                              str(revprecursor)):
                     state[repo[oldrev].rev()] = int(newrev)
                 elif newrev == nullid:
                     state[repo[oldrev].rev()] = revtodo
@@ -912,7 +933,7 @@  def abort(repo, originalwd, target, stat
     repo.ui.warn(_('rebase aborted\n'))
     return 0
 
-def buildstate(repo, dest, rebaseset, collapse):
+def buildstate(repo, dest, rebaseset, collapse, obsoletenotrebased):
     '''Define which revisions are going to be rebased and where
 
     repo: repo
@@ -999,6 +1020,8 @@  def buildstate(repo, dest, rebaseset, co
         rebasedomain = set(repo.revs('%ld::%ld', rebaseset, rebaseset))
         for ignored in set(rebasedomain) - set(rebaseset):
             state[ignored] = revignored
+    for r in obsoletenotrebased:
+        state[r] = revprecursor
     return repo['.'].rev(), dest.rev(), state
 
 def clearrebased(ui, repo, state, skipped, collapsedas=None):
@@ -1107,6 +1130,32 @@  def _rebasedvisible(orig, repo):
     blockers.update(getattr(repo, '_rebaseset', ()))
     return blockers
 
+def _computeobsoletenotrebased(repo, rebasesetrevs, dest):
+    """return a mapping obsolete => successor for all obsolete nodes to be
+    rebased that have a successors in the destination"""
+    obsoletenotrebased = {}
+
+    # Build a mapping succesor => obsolete nodes for the obsolete
+    # nodes to be rebased
+    allsuccessors = {}
+    for r in rebasesetrevs:
+        n = repo[r]
+        if n.obsolete():
+            node = repo.changelog.node(r)
+            for s in obsolete.allsuccessors(repo.obsstore, [node]):
+                allsuccessors[repo.changelog.rev(s)] = repo.changelog.rev(node)
+
+    if allsuccessors:
+        # Look for successors of obsolete nodes to be rebased among
+        # the ancestors of dest
+        ancs = repo.changelog.ancestors([repo[dest].rev()],
+                                        stoprev=min(allsuccessors),
+                                        inclusive=True)
+        for s in allsuccessors:
+            if s in ancs:
+                obsoletenotrebased[allsuccessors[s]] = s
+    return obsoletenotrebased
+
 def summaryhook(ui, repo):
     if not os.path.exists(repo.join('rebasestate')):
         return
diff --git a/tests/test-rebase-obsolete.t b/tests/test-rebase-obsolete.t
--- a/tests/test-rebase-obsolete.t
+++ b/tests/test-rebase-obsolete.t
@@ -203,10 +203,9 @@  More complex case were part of the rebas
   |/
   o  0:cd010b8cd998 A
   
-  $ hg rebase --source 'desc(B)' --dest 'tip'
+  $ hg rebase --source 'desc(B)' --dest 'tip' --config experimental.rebaseskipobsolete=True
   rebasing 8:8877864f1edb "B"
-  rebasing 9:08483444fef9 "D"
-  note: rebase of 9:08483444fef9 created no changes to commit
+  note: not rebasing 9:08483444fef9 "D", already in destination as 11:4596109a6a43 "D"
   rebasing 10:5ae4c968c6ac "C"
   $ hg debugobsolete
   42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0 {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'user': 'test'} (glob)
@@ -214,7 +213,6 @@  More complex case were part of the rebas
   32af7686d403cf45b5d95f2d70cebea587ac806a 0 {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'user': 'test'} (glob)
   08483444fef91d6224f6655ee586a65d263ad34c 4596109a6a4328c398bde3a4a3b6737cfade3003 0 (*) {'user': 'test'} (glob)
   8877864f1edb05d0e07dc4ba77b67a80a7b86672 462a34d07e599b87ea08676a449373fe4e2e1347 0 (*) {'user': 'test'} (glob)
-  08483444fef91d6224f6655ee586a65d263ad34c 0 {8877864f1edb05d0e07dc4ba77b67a80a7b86672} (*) {'user': 'test'} (glob)
   5ae4c968c6aca831df823664e706c9d4aa34473d 98f6af4ee9539e14da4465128f894c274900b6e5 0 (*) {'user': 'test'} (glob)
   $ hg log --rev 'divergent()'
   $ hg log -G
@@ -540,3 +538,55 @@  Test hidden changesets in the rebase set
   |/
   o  0:cd010b8cd998 A
   
+  $ hg up 14 -C
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  $ echo "K" > K
+  $ hg add K
+  $ hg commit --amend -m "K"
+  $ echo "L" > L
+  $ hg add L
+  $ hg commit -m "L"
+  $ hg up '.^'
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  $ echo "M" > M
+  $ hg add M
+  $ hg commit --amend -m "M"
+  $ hg log -G
+  @  20:bfaedf8eb73b M
+  |
+  | o  18:97219452e4bd L
+  | |
+  | x  17:fc37a630c901 K
+  |/
+  | o  15:5ae8a643467b J
+  | |
+  | x  14:9ad579b4a5de I
+  |/
+  | o  12:acd174b7ab39 I
+  | |
+  | o  11:6c11a6218c97 H
+  | |
+  o |  10:b5313c85b22e D
+  |/
+  | o    8:53a6a128b2b7 M
+  | |\
+  | | x  7:02de42196ebe H
+  | | |
+  o---+  6:eea13746799a G
+  | | |
+  | | o  5:24b6387c8c8c F
+  | | |
+  o---+  4:9520eea781bc E
+   / /
+  x |  3:32af7686d403 D
+  | |
+  o |  2:5fddd98957c8 C
+  | |
+  o |  1:42ccdea3bb16 B
+  |/
+  o  0:cd010b8cd998 A
+  
+  $ hg rebase -s 14 -d 18 --config experimental.rebaseskipobsolete=True
+  note: not rebasing 14:9ad579b4a5de "I", already in destination as 17:fc37a630c901 "K"
+  rebasing 15:5ae8a643467b "J"
+