Patchwork D3312: revlog: move shortest() to scmutil.shortesthexnodeidprefix() (API)

login
register
mail settings
Submitter phabricator
Date April 13, 2018, 6 p.m.
Message ID <differential-rev-PHID-DREV-54pbkieox6lthroekkva-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/30861/
State Superseded
Headers show

Comments

phabricator - April 13, 2018, 6 p.m.
martinvonz created this revision.
Herald added a reviewer: indygreg.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  I apparently moved this function from templater.py in https://phab.mercurial-scm.org/rHG448725a2ef7356524bcc9638a5c5eaaf59f263af
  (templater: extract shortest() logic from template function,
  2017-09-15). Now we have scmutil.resolvehexnodeidprefix(), so it makes
  sense to have this method next to it.
  
  Note that the change in show.py also makes it so the conversion from
  revnum to nodeid is done on the filtered repo, but that should be
  inconsequential since the revs are all from the filtered repo anyway.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3312

AFFECTED FILES
  hgext/show.py
  mercurial/revlog.py
  mercurial/scmutil.py
  mercurial/templatefuncs.py

CHANGE DETAILS




To: martinvonz, indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - April 13, 2018, 6:24 p.m.
indygreg added a comment.


  I haven't looked at the patch yet, but I also wrote something like this as part of storage refactors. IMO the shortest node logic has no business being on the generic revlog implementation. I never submitted that patch since removing filelog's inheritance of revlog worked around the problem for me.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3312

To: martinvonz, indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - April 14, 2018, 3:26 a.m.
yuja added subscribers: quark, yuja.
yuja added a comment.


  IIRC, @quark said it's in revlog because the implementation may vary
  depending on storage, such as looking up in radix tree and rewind?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3312

To: martinvonz, indygreg, #hg-reviewers
Cc: yuja, quark, mercurial-devel
phabricator - April 14, 2018, 4:09 a.m.
martinvonz added a comment.


  In https://phab.mercurial-scm.org/D3312#53423, @yuja wrote:
  
  > IIRC, @quark said it's in revlog because the implementation may vary
  >  depending on storage, such as looking up in radix tree and rewind?
  
  
  That's reasonable. Perhaps I'll just make shortesthexnodeidprefix() a thin wrapper then, so all it does (at the end of this series) is to delegate to the unfiltered repo. Does that seem okay with you?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3312

To: martinvonz, indygreg, #hg-reviewers
Cc: yuja, quark, mercurial-devel

Patch

diff --git a/mercurial/templatefuncs.py b/mercurial/templatefuncs.py
--- a/mercurial/templatefuncs.py
+++ b/mercurial/templatefuncs.py
@@ -590,8 +590,8 @@ 
     # _partialmatch() of filtered changelog could take O(len(repo)) time,
     # which would be unacceptably slow. so we look for hash collision in
     # unfiltered space, which means some hashes may be slightly longer.
-    cl = context.resource(mapping, 'ctx')._repo.unfiltered().changelog
-    return cl.shortest(node, minlength)
+    repo = context.resource(mapping, 'ctx')._repo
+    return scmutil.shortesthexnodeidprefix(repo.unfiltered(), node, minlength)
 
 @templatefunc('strip(text[, chars])')
 def strip(context, mapping, args):
diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -443,6 +443,46 @@ 
     repo.changelog.rev(node)  # make sure node isn't filtered
     return node
 
+def shortesthexnodeidprefix(repo, hexnode, minlength=1):
+    """Find the shortest unambiguous prefix that matches hexnode."""
+    cl = repo.changelog
+    def isvalid(test):
+        try:
+            if cl._partialmatch(test) is None:
+                return False
+
+            try:
+                i = int(test)
+                # if we are a pure int, then starting with zero will not be
+                # confused as a rev; or, obviously, if the int is larger
+                # than the value of the tip rev
+                if test[0] == '0' or i > len(cl):
+                    return True
+                return False
+            except ValueError:
+                return True
+        except error.RevlogError:
+            return False
+        except error.WdirUnsupported:
+            # single 'ff...' match
+            return True
+
+    shortest = hexnode
+    startlength = max(6, minlength)
+    length = startlength
+    while True:
+        test = hexnode[:length]
+        if isvalid(test):
+            shortest = test
+            if length == minlength or length > startlength:
+                return shortest
+            length -= 1
+        else:
+            length += 1
+            if len(shortest) <= length:
+                return shortest
+
+
 def isrevsymbol(repo, symbol):
     """Checks if a symbol exists in the repo.
 
diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -1500,44 +1500,6 @@ 
 
         raise LookupError(id, self.indexfile, _('no match found'))
 
-    def shortest(self, hexnode, minlength=1):
-        """Find the shortest unambiguous prefix that matches hexnode."""
-        def isvalid(test):
-            try:
-                if self._partialmatch(test) is None:
-                    return False
-
-                try:
-                    i = int(test)
-                    # if we are a pure int, then starting with zero will not be
-                    # confused as a rev; or, obviously, if the int is larger
-                    # than the value of the tip rev
-                    if test[0] == '0' or i > len(self):
-                        return True
-                    return False
-                except ValueError:
-                    return True
-            except error.RevlogError:
-                return False
-            except error.WdirUnsupported:
-                # single 'ff...' match
-                return True
-
-        shortest = hexnode
-        startlength = max(6, minlength)
-        length = startlength
-        while True:
-            test = hexnode[:length]
-            if isvalid(test):
-                shortest = test
-                if length == minlength or length > startlength:
-                    return shortest
-                length -= 1
-            else:
-                length += 1
-                if len(shortest) <= length:
-                    return shortest
-
     def cmp(self, node, text):
         """compare text with a given file revision
 
diff --git a/hgext/show.py b/hgext/show.py
--- a/hgext/show.py
+++ b/hgext/show.py
@@ -45,6 +45,7 @@ 
     registrar,
     revset,
     revsetlang,
+    scmutil,
 )
 
 # Note for extension authors: ONLY specify testedwith = 'ships-with-hg-core' for
@@ -447,8 +448,10 @@ 
     if not revs:
         return minlen
     # don't use filtered repo because it's slow. see templater.shortest().
-    cl = repo.unfiltered().changelog
-    return max(len(cl.shortest(hex(cl.node(r)), minlen)) for r in revs)
+    cl = repo.changelog
+    return max(len(scmutil.shortesthexnodeidprefix(repo.unfiltered(),
+                                                   hex(cl.node(r)),
+                                                   minlen)) for r in revs)
 
 # Adjust the docstring of the show command so it shows all registered views.
 # This is a bit hacky because it runs at the end of module load. When moved