Patchwork checkhash: merge subfunctions

login
register
mail settings
Submitter Remi Chaintron
Date Oct. 8, 2016, 1:46 p.m.
Message ID <a7f89275e96e0e8fe594.1475934392@remi-mbp2>
Download mbox | patch
Permalink /patch/16920/
State Deferred
Headers show

Comments

Remi Chaintron - Oct. 8, 2016, 1:46 p.m.
# HG changeset patch
# User Remi Chaintron <remi@fb.com>
# Date 1475932840 25200
#      Sat Oct 08 06:20:40 2016 -0700
# Node ID a7f89275e96e0e8fe594f99120034d3345130dcc
# Parent  3741a8f86e88702595c29f8ed824a28da0cfa961
checkhash: merge subfunctions

This patch factors the behavior of both methods into 'checkhash' and returns the
text in order to allow subclasses of revlog and extensions to extend hash
computation, handle hash mismatches and modify the returned text.
Pierre-Yves David - Oct. 16, 2016, 9:16 a.m.
On 10/08/2016 03:46 PM, Remi Chaintron wrote:
> # HG changeset patch
> # User Remi Chaintron <remi@fb.com>
> # Date 1475932840 25200
> #      Sat Oct 08 06:20:40 2016 -0700
> # Node ID a7f89275e96e0e8fe594f99120034d3345130dcc
> # Parent  3741a8f86e88702595c29f8ed824a28da0cfa961
> checkhash: merge subfunctions

The fact that a function called 'checkhash' was in charge of returning 
the text.

Remi and I chatted about this and other things related to LFS and as a 
result Remi will be poking at an approach based on flags.

Patch

diff --git a/contrib/perf.py b/contrib/perf.py
--- a/contrib/perf.py
+++ b/contrib/perf.py
@@ -705,7 +705,7 @@ 
     def dohash(text):
         if not cache:
             r.clearcaches()
-        r._checkhash(text, node, rev)
+        r.checkhash(text, node, rev=rev)
 
     def dorevision():
         if not cache:
diff --git a/mercurial/bundlerepo.py b/mercurial/bundlerepo.py
--- a/mercurial/bundlerepo.py
+++ b/mercurial/bundlerepo.py
@@ -147,7 +147,7 @@ 
             delta = self._chunk(chain.pop())
             text = mdiff.patches(text, [delta])
 
-        self._checkhash(text, node, rev)
+        text = self.checkhash(text, node, rev=rev)
         self._cache = (node, rev, text)
         return text
 
diff --git a/mercurial/filelog.py b/mercurial/filelog.py
--- a/mercurial/filelog.py
+++ b/mercurial/filelog.py
@@ -104,9 +104,10 @@ 
 
         return True
 
-    def checkhash(self, text, p1, p2, node, rev=None):
+    def checkhash(self, text, node, p1=None, p2=None, rev=None):
         try:
-            super(filelog, self).checkhash(text, p1, p2, node, rev=rev)
+            f = super(filelog, self)
+            return f.checkhash(text, node, p1=p1, p2=p2, rev=rev)
         except error.RevlogError:
             if _censoredtext(text):
                 raise error.CensoredNodeError(self.indexfile, node, text)
diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -1219,9 +1219,7 @@ 
             bins = bins[1:]
 
         text = mdiff.patches(text, bins)
-
-        text = self._checkhash(text, node, rev)
-
+        text = self.checkhash(text, node, rev=rev)
         self._cache = (node, rev, text)
         return text
 
@@ -1233,18 +1231,21 @@ 
         """
         return hash(text, p1, p2)
 
-    def _checkhash(self, text, node, rev):
-        p1, p2 = self.parents(node)
-        self.checkhash(text, p1, p2, node, rev)
-        return text
+    def checkhash(self, text, node, p1=None, p2=None, rev=None):
+        """Check node hash integrity.
 
-    def checkhash(self, text, p1, p2, node, rev=None):
+        Available as a function so that subclasses can extend hash mismatch
+        behaviors as needed, including modifying the returned text.
+        """
+        if p1 is None and p2 is None:
+            p1, p2 = self.parents(node)
         if node != self.hash(text, p1, p2):
             revornode = rev
             if revornode is None:
                 revornode = templatefilters.short(hex(node))
             raise RevlogError(_("integrity check failed on %s:%s")
                 % (self.indexfile, revornode))
+        return text
 
     def checkinlinesize(self, tr, fp=None):
         """Check if the revlog is too big for inline and convert if so.
@@ -1413,7 +1414,7 @@ 
                 basetext = self.revision(self.node(baserev), _df=fh)
                 btext[0] = mdiff.patch(basetext, delta)
             try:
-                self.checkhash(btext[0], p1, p2, node)
+                self.checkhash(btext[0], node, p1=p1, p2=p2)
                 if flags & REVIDX_ISCENSORED:
                     raise RevlogError(_('node %s is not censored') % node)
             except CensoredNodeError: