Patchwork [3,of,4] rebase: remove "nullmerge" and "revignored" state (issue5606)

login
register
mail settings
Submitter Jun Wu
Date June 24, 2017, 5:04 a.m.
Message ID <707de31cfae074a76751.1498280682@x1c>
Download mbox | patch
Permalink /patch/21659/
State Changes Requested
Headers show

Comments

Jun Wu - June 24, 2017, 5:04 a.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1498280489 25200
#      Fri Jun 23 22:01:29 2017 -0700
# Node ID 707de31cfae074a767511bd51a197afc7c599323
# Parent  79f7ce082ec2fc97d5bd67e3fcffb66daf6e7d4e
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r 707de31cfae0
rebase: remove "nullmerge" and "revignored" state (issue5606)

Previously, in order to support "hole in rebaseset", or "src^ is not an
ancestor of dest", we fill rebase state with placeholder revs and set their
state to "nullmerge" or "revignored".

The new "defineparents" implementation handles those cases cleanly without
requiring other workarounds like "nullmerge" or "revignored". Therefore
remove them.

This means we no longer pollute rebase state with irrelevant revs. Therefore
a side effect is rebase will not strip innocent revs, solves issue5606.

The "Rebase tries to turn <dest> into a parent of" comment block is in a
wrong place and is outdated (the "abort" condition is inaccurate, and
obsolete situation is not mentioned). Comments in the new "defineparents"
covers this area, so the old comment block gets deleted.

Patch

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -60,11 +60,11 @@  templateopts = cmdutil.templateopts
 # Indicates that a revision needs to be rebased
 revtodo = -1
-nullmerge = -2
-revignored = -3
 # successor in rebase destination
 revprecursor = -4
 # plain prune (no successor)
 revpruned = -5
-revskipped = (revignored, revprecursor, revpruned)
+revskipped = (revprecursor, revpruned)
+# states used by legacy clients
+legacyrevskipped = (-2, -3) # -2: nullmerge, -3: revignored
 
 cmdtable = {}
@@ -231,7 +231,9 @@  class rebaseruntime(object):
                     else:
                         destnode = legacydest
-                    destmap[repo[oldrev].rev()] = repo[destnode].rev()
                     if newrev.startswith('-'):
-                        state[repo[oldrev].rev()] = int(newrev)
+                        revstate = int(newrev)
+                        if revstate in legacyrevskipped:
+                            continue
+                        state[repo[oldrev].rev()] = revstate
                     elif newrev == nullid:
                         state[repo[oldrev].rev()] = revtodo
@@ -239,4 +241,5 @@  class rebaseruntime(object):
                     else:
                         state[repo[oldrev].rev()] = repo[newrev].rev()
+                    destmap[repo[oldrev].rev()] = repo[destnode].rev()
 
         except IOError as err:
@@ -448,8 +451,4 @@  class rebaseruntime(object):
                     self.state[rev] = p1
                     ui.debug('next revision set to %s\n' % p1)
-            elif self.state[rev] == nullmerge:
-                ui.debug('ignoring null merge rebase of %s\n' % rev)
-            elif self.state[rev] == revignored:
-                ui.status(_('not rebasing ignored %s\n') % desc)
             elif self.state[rev] == revprecursor:
                 destctx = repo[self.obsoletenotrebased[rev]]
@@ -481,5 +480,5 @@  class rebaseruntime(object):
                 for rebased in self.state:
                     if rebased not in self.skipped and\
-                       self.state[rebased] > nullmerge:
+                       self.state[rebased] >= revtodo:
                         commitmsg += '\n* %s' % repo[rebased].description()
                 editopt = True
@@ -509,5 +508,5 @@  class rebaseruntime(object):
                 newrev = repo[newnode].rev()
             for oldrev in self.state.iterkeys():
-                if self.state[oldrev] > nullmerge:
+                if self.state[oldrev] >= revtodo:
                     self.state[oldrev] = newrev
 
@@ -519,5 +518,5 @@  class rebaseruntime(object):
             nstate = {}
             for k, v in self.state.iteritems():
-                if v > nullmerge and v != k:
+                if v >= revtodo and v != k:
                     nstate[repo[k].node()] = repo[v].node()
                 elif v == revprecursor:
@@ -1327,68 +1326,11 @@  def buildstate(repo, destmap, rebaseset,
         emptyrebase = False
         repo.ui.debug('rebase onto %s starting from %s\n' % (dest, root))
-        # Rebase tries to turn <dest> into a parent of <root> while
-        # preserving the number of parents of rebased changesets:
-        #
-        # - A changeset with a single parent will always be rebased as a
-        #   changeset with a single parent.
-        #
-        # - A merge will be rebased as merge unless its parents are both
-        #   ancestors of <dest> or are themselves in the rebased set and
-        #   pruned while rebased.
-        #
-        # If one parent of <root> is an ancestor of <dest>, the rebased
-        # version of this parent will be <dest>. This is always true with
-        # --base option.
-        #
-        # Otherwise, we need to *replace* the original parents with
-        # <dest>. This "detaches" the rebased set from its former location
-        # and rebases it onto <dest>. Changes introduced by ancestors of
-        # <root> not common with <dest> (the detachset, marked as
-        # nullmerge) are "removed" from the rebased changesets.
-        #
-        # - If <root> has a single parent, set it to <dest>.
-        #
-        # - If <root> is a merge, we cannot decide which parent to
-        #   replace, the rebase operation is not clearly defined.
-        #
-        # The table below sums up this behavior:
-        #
-        # +------------------+----------------------+-------------------------+
-        # |                  |     one parent       |  merge                  |
-        # +------------------+----------------------+-------------------------+
-        # | parent in        | new parent is <dest> | parents in ::<dest> are |
-        # | ::<dest>         |                      | remapped to <dest>      |
-        # +------------------+----------------------+-------------------------+
-        # | unrelated source | new parent is <dest> | ambiguous, abort        |
-        # +------------------+----------------------+-------------------------+
-        #
-        # The actual abort is handled by `defineparents`
-        if len(root.parents()) <= 1:
-            # ancestors of <root> not ancestors of <dest>
-            destrev = destmap[root.rev()]
-            detachset = repo.changelog.findmissingrevs([commonbase.rev()],
-                                                       [root.rev()])
-            for r in detachset:
-                if r not in state:
-                    state[r] = nullmerge
-                    destmap[r] = destrev
     if emptyrebase:
         return None
     for rev in sorted(state):
-        if state[rev] == nullmerge:
-            continue
         parents = [p for p in repo.changelog.parentrevs(rev) if p != nullrev]
         # if all parents of this revision are done, then so is this revision
         if parents and all((state.get(p) == p for p in parents)):
             state[rev] = rev
-    if len(roots) > 1:
-        # If we have multiple roots, we may have "hole" in the rebase set.
-        # Rebase roots that descend from those "hole" should not be detached as
-        # other root are. We use the special `revignored` to inform rebase that
-        # the revision should be ignored but that `defineparents` should search
-        # a rebase destination that make sense regarding rebased topology.
-        rebasedomain = set(repo.revs('%ld::%ld', rebaseset, rebaseset))
-        for ignored in set(rebasedomain) - set(rebaseset):
-            state[ignored] = revignored
     for r in obsoletenotrebased:
         if obsoletenotrebased[r] is None:
@@ -1419,5 +1361,5 @@  def clearrebased(ui, repo, state, skippe
     else:
         rebased = [rev for rev in state
-                   if state[rev] > nullmerge and state[rev] != rev]
+                   if state[rev] >= revtodo and state[rev] != rev]
         if rebased:
             stripped = []
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
@@ -220,8 +220,4 @@  Check that the right ancestors is used w
   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
-  ignoring null merge rebase of 8
   rebasing 9:e31216eec445 "more changes to f1"
    future parents are 2 and -1
diff --git a/tests/test-rebase-legacy.t b/tests/test-rebase-legacy.t
--- a/tests/test-rebase-legacy.t
+++ b/tests/test-rebase-legacy.t
@@ -43,25 +43,33 @@  rebasestate generated by a legacy client
   $ hg rebase --continue
   rebasing 4:c1e6b162678d "B" (B)
-  not rebasing ignored 6:bd5548558fcf "C" (C)
   rebasing 8:6f7a236de685 "D" (D)
   rebasing 2:de008c61a447 "E" (E)
-  not rebasing ignored 5:aeba276fcb7d "F" (F)
   rebasing 7:d2fa1c02b240 "G" (G)
   rebasing 9:6582e6951a9c "H" (H tip)
-  saved backup bundle to $TESTTMP/.hg/strip-backup/6f7a236de685-f3834b91-backup.hg (glob)
+  warning: new changesets detected on source branch, not stripping
+  warning: new changesets detected on source branch, not stripping
+  saved backup bundle to $TESTTMP/.hg/strip-backup/6f7a236de685-9880a3dc-backup.hg (glob)
 
   $ hg log -G -T '{rev}:{node|short} {desc}\n'
-  o  7:721b8da0a708 H
+  o  11:721b8da0a708 H
   |
-  o  6:9d65695ec3c2 G
+  o  10:9d65695ec3c2 G
   |
-  o  5:21c8397a5d68 E
+  o  9:21c8397a5d68 E
   |
-  | o  4:fc52970345e8 D
+  | o  8:fc52970345e8 D
+  | |
+  | o  7:eac96551b107 B
+  |/
+  | o  6:bd5548558fcf C
   | |
-  | o  3:eac96551b107 B
-  |/
-  o  2:f424eb6a8c01 Z
-  |
+  | | o  5:aeba276fcb7d F
+  | | |
+  | o |  4:c1e6b162678d B
+  | | |
+  o | |  3:f424eb6a8c01 Z
+  | | |
+  +---o  2:de008c61a447 E
+  | |
   | o  1:21a6c4502885 A
   |/
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
@@ -450,5 +450,4 @@  Test multiple root handling
   rebasing 9:cf44d2f5a9f4 "D"
   rebasing 7:02de42196ebe "H"
-  not rebasing ignored 10:7c6027df6a99 "B"
   rebasing 11:0d8f238b634c "C" (tip)
   $ hg log -G
@@ -520,5 +519,4 @@  test on rebase dropping a merge
   rebasing 3:32af7686d403 "D"
   rebasing 7:02de42196ebe "H"
-  not rebasing ignored 8:53a6a128b2b7 "M"
   rebasing 9:4bde274eefcf "I" (tip)
   $ hg log -G
diff --git a/tests/test-rebase-partial.t b/tests/test-rebase-partial.t
--- a/tests/test-rebase-partial.t
+++ b/tests/test-rebase-partial.t
@@ -82,5 +82,4 @@  the hole (B below), not on top of the de
   > EOF
   already rebased 1:112478962961 "B" (B)
-  not rebasing ignored 2:26805aba1e60 "C" (C)
   rebasing 3:f585351a92f8 "D" (D tip)
   o  4: D