Patchwork D3461: shortest: don't keep checking for longer prefix if node doesn't exist (API)

login
register
mail settings
Submitter phabricator
Date May 7, 2018, 4:56 p.m.
Message ID <differential-rev-PHID-DREV-5tfuvqifp5hxg3sblpwz-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/31312/
State Superseded
Headers show

Comments

phabricator - May 7, 2018, 4:56 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
  If revlog.shortest() is called with an invalid nodeid, we keep
  checking if longer and longer prefixes are valid. We call
  revlog._partialmatch() for each prefix. That function will give us
  None if the node doesn't exist (and a RevlogError if it's ambiguous),
  so there's no need to keep checking.
  
  This patch instead makes revlog.shortest() raise a LookupError is the
  node does not exist, and updates the caller to handle it. Before this
  patch, revlog.shortest() would return the full hexnode for nonexistent
  nodeids. By the same reasoning as in https://phab.mercurial-scm.org/rHG7b29556247776a86ead7eb98fd3a20dafd0c08b4 (scmutil: make
  shortesthexnodeidprefix() take a full binary nodeid, 2018-04-14), it's
  not revlog.shortest() that should decide how to present nonexistent
  nodeids, so that's now moved to the template function.
  
  This should speed up cases where {shortest()} is applied to an invalid
  nodeid, but I couldn't think of a reasonable case where that would
  happen.

REPOSITORY
  rHG Mercurial

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

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

CHANGE DETAILS




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

Patch

diff --git a/mercurial/templatefuncs.py b/mercurial/templatefuncs.py
--- a/mercurial/templatefuncs.py
+++ b/mercurial/templatefuncs.py
@@ -608,7 +608,10 @@ 
             return hexnode
         if not node:
             return hexnode
-    return scmutil.shortesthexnodeidprefix(repo, node, minlength)
+    try:
+        return scmutil.shortesthexnodeidprefix(repo, node, minlength)
+    except error.RepoLookupError:
+        return hexnode
 
 @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
@@ -448,7 +448,10 @@ 
     # _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.
-    return repo.unfiltered().changelog.shortest(node, minlength)
+    try:
+        return repo.unfiltered().changelog.shortest(node, minlength)
+    except error.LookupError:
+        raise error.RepoLookupError()
 
 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
@@ -1516,13 +1516,14 @@ 
 
         def isvalid(prefix):
             try:
-                if self._partialmatch(prefix) is None:
-                    return False
+                node = self._partialmatch(prefix)
             except error.RevlogError:
                 return False
             except error.WdirUnsupported:
                 # single 'ff...' match
                 return True
+            if node is None:
+                raise LookupError(node, self.indexfile, _('no node'))
             return not isrev(prefix)
 
         hexnode = hex(node)