Patchwork [1,of,4,V2] histedit: store full node hash in rules

login
register
mail settings
Submitter Mateusz Kwapich
Date Feb. 3, 2015, 12:32 a.m.
Message ID <0c8cf9835334d76f52e7.1422923546@dev1429.prn1.facebook.com>
Download mbox | patch
Permalink /patch/7616/
State Accepted
Commit 96d130697f07e2f755ce4fb9d0e5bf2c49a3ce51
Headers show

Comments

Mateusz Kwapich - Feb. 3, 2015, 12:32 a.m.
# HG changeset patch
# User Mateusz Kwapich <mitrandir@fb.com>
# Date 1422314287 28800
#      Mon Jan 26 15:18:07 2015 -0800
# Node ID 0c8cf9835334d76f52e7cebf9d70b5b0df646871
# Parent  a8dc5a3f4f4c2d7195a1ccf1712ef70e2080dc0e
histedit: store full node hash in rules

Previously histedit only stored the short version of the rule nodes in the
state. This meant that later we couldn't resolve a rule node to its full
form if the commit had been deleted from the repo.

Let's store the full form from the beginning.
Augie Fackler - Feb. 3, 2015, 3:36 p.m.
On Mon, Feb 02, 2015 at 04:32:26PM -0800, Mateusz Kwapich wrote:
> # HG changeset patch
> # User Mateusz Kwapich <mitrandir@fb.com>
> # Date 1422314287 28800
> #      Mon Jan 26 15:18:07 2015 -0800
> # Node ID 0c8cf9835334d76f52e7cebf9d70b5b0df646871
> # Parent  a8dc5a3f4f4c2d7195a1ccf1712ef70e2080dc0e
> histedit: store full node hash in rules
>
> Previously histedit only stored the short version of the rule nodes in the
> state. This meant that later we couldn't resolve a rule node to its full
> form if the commit had been deleted from the repo.
>
> Let's store the full form from the beginning.
>
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -349,7 +349,7 @@
>      repo, ctx = state.repo, state.parentctx
>      oldctx = repo[ha]
>      if oldctx.parents()[0] == ctx:
> -        ui.debug('node %s unchanged\n' % ha)
> +        ui.debug('node %s unchanged\n' % ha[:12])

Here and elsewhere: I'm -0 on encoding the 12 everywhere, since some
of us are starting to think about enormous repositories where 12 bytes
isn't reliably unique. I think I'd rather figure out how to keep using
ctx.short() or something here, so that there's one place for future UI
work to do "12 bytes, or shortest unique prefix" instead of just 12
bytes.

>          return oldctx, []
>      hg.update(repo, ctx.node())
>      stats = applychanges(ui, repo, oldctx, opts)
> @@ -361,7 +361,7 @@
>      n = commit(text=oldctx.description(), user=oldctx.user(),
>                 date=oldctx.date(), extra=oldctx.extra())
>      if n is None:
> -        ui.warn(_('%s: empty changeset\n') % node.hex(ha))
> +        ui.warn(_('%s: empty changeset\n') % ha[:12])
>          return ctx, []
>      new = repo[n]
>      return new, [(oldctx.node(), (n,))]
> @@ -389,10 +389,10 @@
>      if stats and stats[3] > 0:
>          raise error.InterventionRequired(
>              _('Fix up the change and run hg histedit --continue'))
> -    n = repo.commit(text='fold-temp-revision %s' % ha, user=oldctx.user(),
> +    n = repo.commit(text='fold-temp-revision %s' % ha[:12], user=oldctx.user(),
>                      date=oldctx.date(), extra=oldctx.extra())
>      if n is None:
> -        ui.warn(_('%s: empty changeset') % node.hex(ha))
> +        ui.warn(_('%s: empty changeset') % ha[:12])
>          return ctx, []
>      return finishfold(ui, repo, ctx, oldctx, n, opts, [])
>
> @@ -666,7 +666,7 @@
>      while state.rules:
>          state.write()
>          action, ha = state.rules.pop(0)
> -        ui.debug('histedit: processing %s %s\n' % (action, ha))
> +        ui.debug('histedit: processing %s %s\n' % (action, ha[:12]))
>          actfunc = actiontable[action]
>          state.parentctx, replacement_ = actfunc(ui, state, ha, opts)
>          state.replacements.extend(replacement_)
> @@ -736,7 +736,7 @@
>      if s.modified or s.added or s.removed or s.deleted:
>          # prepare the message for the commit to comes
>          if action in ('f', 'fold', 'r', 'roll'):
> -            message = 'fold-temp-revision %s' % currentnode
> +            message = 'fold-temp-revision %s' % currentnode[:12]
>          else:
>              message = ctx.description()
>          editopt = action in ('e', 'edit', 'm', 'mess')
> @@ -822,7 +822,7 @@
>      or a rule on a changeset outside of the user-given range.
>      """
>      parsed = []
> -    expected = set(str(c) for c in ctxs)
> +    expected = set(c.hex() for c in ctxs)

Why not store c.node() here? I'm probably missing something.

>      seen = set()
>      for r in rules:
>          if ' ' not in r:
> @@ -830,22 +830,24 @@
>          action, rest = r.split(' ', 1)
>          ha = rest.strip().split(' ', 1)[0]
>          try:
> -            ha = str(repo[ha])  # ensure its a short hash
> +            ha = repo[ha].hex()
>          except error.RepoError:
> -            raise util.Abort(_('unknown changeset %s listed') % ha)
> +            raise util.Abort(_('unknown changeset %s listed') % ha[:12])
>          if ha not in expected:
>              raise util.Abort(
>                  _('may not use changesets other than the ones listed'))
>          if ha in seen:
> -            raise util.Abort(_('duplicated command for changeset %s') % ha)
> +            raise util.Abort(_('duplicated command for changeset %s') %
> +                    ha[:12])
>          seen.add(ha)
>          if action not in actiontable:
>              raise util.Abort(_('unknown action "%s"') % action)
>          parsed.append([action, ha])
>      missing = sorted(expected - seen)  # sort to stabilize output
>      if missing:
> -        raise util.Abort(_('missing rules for changeset %s') % missing[0],
> -                         hint=_('do you want to use the drop action?'))
> +        raise util.Abort(_('missing rules for changeset %s') %
> +                missing[0][:12],
> +                hint=_('do you want to use the drop action?'))
>      return parsed
>
>  def processreplacement(state):
> diff --git a/tests/test-histedit-drop.t b/tests/test-histedit-drop.t
> --- a/tests/test-histedit-drop.t
> +++ b/tests/test-histedit-drop.t
> @@ -96,7 +96,6 @@
>  Check histedit_source
>
>    $ hg log --debug --rev f518305ce889
> -  invalid branchheads cache (visible): tip differs

What's up with this test output change?

>    changeset:   4:f518305ce889c07cb5bd05522176d75590ef3324
>    tag:         tip
>    phase:       draft
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -349,7 +349,7 @@ 
     repo, ctx = state.repo, state.parentctx
     oldctx = repo[ha]
     if oldctx.parents()[0] == ctx:
-        ui.debug('node %s unchanged\n' % ha)
+        ui.debug('node %s unchanged\n' % ha[:12])
         return oldctx, []
     hg.update(repo, ctx.node())
     stats = applychanges(ui, repo, oldctx, opts)
@@ -361,7 +361,7 @@ 
     n = commit(text=oldctx.description(), user=oldctx.user(),
                date=oldctx.date(), extra=oldctx.extra())
     if n is None:
-        ui.warn(_('%s: empty changeset\n') % node.hex(ha))
+        ui.warn(_('%s: empty changeset\n') % ha[:12])
         return ctx, []
     new = repo[n]
     return new, [(oldctx.node(), (n,))]
@@ -389,10 +389,10 @@ 
     if stats and stats[3] > 0:
         raise error.InterventionRequired(
             _('Fix up the change and run hg histedit --continue'))
-    n = repo.commit(text='fold-temp-revision %s' % ha, user=oldctx.user(),
+    n = repo.commit(text='fold-temp-revision %s' % ha[:12], user=oldctx.user(),
                     date=oldctx.date(), extra=oldctx.extra())
     if n is None:
-        ui.warn(_('%s: empty changeset') % node.hex(ha))
+        ui.warn(_('%s: empty changeset') % ha[:12])
         return ctx, []
     return finishfold(ui, repo, ctx, oldctx, n, opts, [])
 
@@ -666,7 +666,7 @@ 
     while state.rules:
         state.write()
         action, ha = state.rules.pop(0)
-        ui.debug('histedit: processing %s %s\n' % (action, ha))
+        ui.debug('histedit: processing %s %s\n' % (action, ha[:12]))
         actfunc = actiontable[action]
         state.parentctx, replacement_ = actfunc(ui, state, ha, opts)
         state.replacements.extend(replacement_)
@@ -736,7 +736,7 @@ 
     if s.modified or s.added or s.removed or s.deleted:
         # prepare the message for the commit to comes
         if action in ('f', 'fold', 'r', 'roll'):
-            message = 'fold-temp-revision %s' % currentnode
+            message = 'fold-temp-revision %s' % currentnode[:12]
         else:
             message = ctx.description()
         editopt = action in ('e', 'edit', 'm', 'mess')
@@ -822,7 +822,7 @@ 
     or a rule on a changeset outside of the user-given range.
     """
     parsed = []
-    expected = set(str(c) for c in ctxs)
+    expected = set(c.hex() for c in ctxs)
     seen = set()
     for r in rules:
         if ' ' not in r:
@@ -830,22 +830,24 @@ 
         action, rest = r.split(' ', 1)
         ha = rest.strip().split(' ', 1)[0]
         try:
-            ha = str(repo[ha])  # ensure its a short hash
+            ha = repo[ha].hex()
         except error.RepoError:
-            raise util.Abort(_('unknown changeset %s listed') % ha)
+            raise util.Abort(_('unknown changeset %s listed') % ha[:12])
         if ha not in expected:
             raise util.Abort(
                 _('may not use changesets other than the ones listed'))
         if ha in seen:
-            raise util.Abort(_('duplicated command for changeset %s') % ha)
+            raise util.Abort(_('duplicated command for changeset %s') %
+                    ha[:12])
         seen.add(ha)
         if action not in actiontable:
             raise util.Abort(_('unknown action "%s"') % action)
         parsed.append([action, ha])
     missing = sorted(expected - seen)  # sort to stabilize output
     if missing:
-        raise util.Abort(_('missing rules for changeset %s') % missing[0],
-                         hint=_('do you want to use the drop action?'))
+        raise util.Abort(_('missing rules for changeset %s') %
+                missing[0][:12],
+                hint=_('do you want to use the drop action?'))
     return parsed
 
 def processreplacement(state):
diff --git a/tests/test-histedit-drop.t b/tests/test-histedit-drop.t
--- a/tests/test-histedit-drop.t
+++ b/tests/test-histedit-drop.t
@@ -96,7 +96,6 @@ 
 Check histedit_source
 
   $ hg log --debug --rev f518305ce889
-  invalid branchheads cache (visible): tip differs
   changeset:   4:f518305ce889c07cb5bd05522176d75590ef3324
   tag:         tip
   phase:       draft