Patchwork [6,of,6] rebase: remove revprecursor and revpruned states

login
register
mail settings
Submitter Jun Wu
Date July 9, 2017, 7:14 p.m.
Message ID <22d54f1e4c09d8fb68ea.1499627693@x1c>
Download mbox | patch
Permalink /patch/22181/
State Superseded
Headers show

Comments

Jun Wu - July 9, 2017, 7:14 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1499571514 25200
#      Sat Jul 08 20:38:34 2017 -0700
# Node ID 22d54f1e4c09d8fb68ea897afcdd0b705db94b5a
# Parent  49d1672e7567f2a16c5ce9afa64babe08891718f
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r 22d54f1e4c09
rebase: remove revprecursor and revpruned states

They are no longer necessary to make rebase behavior correct. Therefore
remove them to make the code cleaner and easier to reason about.
Jun Wu - July 10, 2017, 12:46 a.m.
This should be retitled to "remove nullmerge and revignored states".

(it seems Phabricator could be helpful in this situation - the title could
be changed without a resend)

Excerpts from Jun Wu's message of 2017-07-09 12:14:53 -0700:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1499571514 25200
> #      Sat Jul 08 20:38:34 2017 -0700
> # Node ID 22d54f1e4c09d8fb68ea897afcdd0b705db94b5a
> # Parent  49d1672e7567f2a16c5ce9afa64babe08891718f
> # Available At https://bitbucket.org/quark-zju/hg-draft 
> #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r 22d54f1e4c09
> rebase: remove revprecursor and revpruned states
> 
> They are no longer necessary to make rebase behavior correct. Therefore
> remove them to make the code cleaner and easier to reason about.
> 
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -61,10 +61,8 @@ templateopts = cmdutil.templateopts
>  # Indicates that a revision needs to be rebased
>  revtodo = -1
> -nullmerge = -2
> -revignored = -3
>  
>  # legacy revstates no longer needed in current code
> -# -4: revprecursor, -5: revpruned
> -legacystates = {'-4', '-5'}
> +# -2: nullmerge, -3: revignored, -4: revprecursor, -5: revpruned
> +legacystates = {'-2', '-3', '-4', '-5'}
>  
>  cmdtable = {}
> @@ -234,6 +232,4 @@ class rebaseruntime(object):
>                      if newrev in legacystates:
>                          continue
> -                    if newrev in (str(nullmerge), str(revignored)):
> -                        state[repo[oldrev].rev()] = int(newrev)
>                      elif newrev == nullid:
>                          state[repo[oldrev].rev()] = revtodo
> @@ -440,8 +436,4 @@ class rebaseruntime(object):
>                      self.state[rev] = p1
>                      ui.debug('next revision set to %s\n' % p1)
> -            elif self.state[rev] == nullmerge:
> -                pass
> -            elif self.state[rev] == revignored:
> -                pass
>              else:
>                  ui.status(_('already rebased %s as %s\n') %
> @@ -464,5 +456,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
> @@ -480,5 +472,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
>  
> @@ -1231,5 +1223,4 @@ def buildstate(repo, dest, rebaseset, co
>      roots.sort()
>      state = dict.fromkeys(rebaseset, revtodo)
> -    detachset = set()
>      emptyrebase = True
>      for root in roots:
> @@ -1253,45 +1244,4 @@ def buildstate(repo, dest, rebaseset, co
>          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>
> -            detachset.update(repo.changelog.findmissingrevs([commonbase.rev()],
> -                                                            [root.rev()]))
>      if emptyrebase:
>          return None
> @@ -1301,16 +1251,4 @@ def buildstate(repo, dest, rebaseset, co
>          if parents and all((state.get(p) == p for p in parents)):
>              state[rev] = rev
> -    for r in detachset:
> -        if r not in state:
> -            state[r] = nullmerge
> -    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
>      unfi = repo.unfiltered()
>      for r in obsoletenotrebased:
Jun Wu - July 10, 2017, 8:26 p.m.
Excerpts from Jun Wu's message of 2017-07-09 17:46:32 -0700:
> This should be retitled to "remove nullmerge and revignored states".
> 
> (it seems Phabricator could be helpful in this situation - the title could
> be changed without a resend)

This series has been sent to Phabricator as D21 to D26 with the tittle
fixed. I'll drop it from patchwork.

Patch

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -61,10 +61,8 @@  templateopts = cmdutil.templateopts
 # Indicates that a revision needs to be rebased
 revtodo = -1
-nullmerge = -2
-revignored = -3
 
 # legacy revstates no longer needed in current code
-# -4: revprecursor, -5: revpruned
-legacystates = {'-4', '-5'}
+# -2: nullmerge, -3: revignored, -4: revprecursor, -5: revpruned
+legacystates = {'-2', '-3', '-4', '-5'}
 
 cmdtable = {}
@@ -234,6 +232,4 @@  class rebaseruntime(object):
                     if newrev in legacystates:
                         continue
-                    if newrev in (str(nullmerge), str(revignored)):
-                        state[repo[oldrev].rev()] = int(newrev)
                     elif newrev == nullid:
                         state[repo[oldrev].rev()] = revtodo
@@ -440,8 +436,4 @@  class rebaseruntime(object):
                     self.state[rev] = p1
                     ui.debug('next revision set to %s\n' % p1)
-            elif self.state[rev] == nullmerge:
-                pass
-            elif self.state[rev] == revignored:
-                pass
             else:
                 ui.status(_('already rebased %s as %s\n') %
@@ -464,5 +456,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
@@ -480,5 +472,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
 
@@ -1231,5 +1223,4 @@  def buildstate(repo, dest, rebaseset, co
     roots.sort()
     state = dict.fromkeys(rebaseset, revtodo)
-    detachset = set()
     emptyrebase = True
     for root in roots:
@@ -1253,45 +1244,4 @@  def buildstate(repo, dest, rebaseset, co
         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>
-            detachset.update(repo.changelog.findmissingrevs([commonbase.rev()],
-                                                            [root.rev()]))
     if emptyrebase:
         return None
@@ -1301,16 +1251,4 @@  def buildstate(repo, dest, rebaseset, co
         if parents and all((state.get(p) == p for p in parents)):
             state[rev] = rev
-    for r in detachset:
-        if r not in state:
-            state[r] = nullmerge
-    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
     unfi = repo.unfiltered()
     for r in obsoletenotrebased: