Patchwork [1,of,6] revlog: merge hash checking subfunctions

login
register
mail settings
Submitter Remi Chaintron
Date Oct. 27, 2016, 3:03 p.m.
Message ID <7423a5fcbc05f69c84a9.1477580639@remi-mbp2.dhcp.thefacebook.com>
Download mbox | patch
Permalink /patch/17202/
State Deferred
Headers show

Comments

Remi Chaintron - Oct. 27, 2016, 3:03 p.m.
# HG changeset patch
# User Remi Chaintron <remi@fb.com>
# Date 1477579974 -3600
#      Thu Oct 27 15:52:54 2016 +0100
# Branch stable
# Node ID 7423a5fcbc05f69c84a9e0eb4d4b4a94444d83f0
# Parent  b9f7b0c10027764cee77f9c6d61877fcffea837f
revlog: merge hash checking 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 and handle hash mismatches.
Gregory Szorc - Oct. 27, 2016, 3:07 p.m.
We're currently in the 4.0 feature freeze and this series isn't appropriate for stable. Please rebase on default and resend after the 4.0 release ~November 1.

> On Oct 27, 2016, at 08:03, Remi Chaintron <remi@fb.com> wrote:
> 
> # HG changeset patch
> # User Remi Chaintron <remi@fb.com>
> # Date 1477579974 -3600
> #      Thu Oct 27 15:52:54 2016 +0100
> # Branch stable
> # Node ID 7423a5fcbc05f69c84a9e0eb4d4b4a94444d83f0
> # Parent  b9f7b0c10027764cee77f9c6d61877fcffea837f
> revlog: merge hash checking 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 and handle hash mismatches.
> 
> diff --git a/contrib/perf.py b/contrib/perf.py
> --- a/contrib/perf.py
> +++ b/contrib/perf.py
> @@ -861,7 +861,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)
> +        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,9 @@
> 
>         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)
> +            super(filelog, self).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
> @@ -1221,9 +1221,7 @@
>             bins = bins[1:]
> 
>         text = mdiff.patches(text, bins)
> -
> -        text = self._checkhash(text, node, rev)
> -
> +        self.checkhash(text, node, rev=rev)
>         self._cache = (node, rev, text)
>         return text
> 
> @@ -1235,12 +1233,14 @@
>         """
>         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.
> +        """
> +        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:
> @@ -1415,7 +1415,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:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Remi Chaintron - Oct. 27, 2016, 3:22 p.m.
I will be off the grid until November 12, which is why I’m pre-emptively sending these changes in case someone has the time to review them between November 1 and November 12.

-- Rémi


On 10/27/16, 4:07 PM, "Gregory Szorc" <gregory.szorc@gmail.com> wrote:

We're currently in the 4.0 feature freeze and this series isn't appropriate for stable. Please rebase on default and resend after the 4.0 release ~November 1.

> On Oct 27, 2016, at 08:03, Remi Chaintron <remi@fb.com> wrote:

> 

> # HG changeset patch

> # User Remi Chaintron <remi@fb.com>

> # Date 1477579974 -3600

> #      Thu Oct 27 15:52:54 2016 +0100

> # Branch stable

> # Node ID 7423a5fcbc05f69c84a9e0eb4d4b4a94444d83f0

> # Parent  b9f7b0c10027764cee77f9c6d61877fcffea837f

> revlog: merge hash checking 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 and handle hash mismatches.

> 

> diff --git a/contrib/perf.py b/contrib/perf.py

> --- a/contrib/perf.py

> +++ b/contrib/perf.py

> @@ -861,7 +861,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)

> +        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,9 @@

> 

>         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)

> +            super(filelog, self).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

> @@ -1221,9 +1221,7 @@

>             bins = bins[1:]

> 

>         text = mdiff.patches(text, bins)

> -

> -        text = self._checkhash(text, node, rev)

> -

> +        self.checkhash(text, node, rev=rev)

>         self._cache = (node, rev, text)

>         return text

> 

> @@ -1235,12 +1233,14 @@

>         """

>         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.

> +        """

> +        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:

> @@ -1415,7 +1415,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:

> _______________________________________________

> Mercurial-devel mailing list

> Mercurial-devel@mercurial-scm.org

> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=DQIFAg&c=5VD0RTtNlTh3ycd41b3MUw&r=RjAECLzUoKvzn49UTozsUg&m=5OMZWLgAascGucZnwpWDJVOSlfzNhPw6znjilBoXmbM&s=kD4AUs9FhSWAqJZL8uUDG4TIHJ9_v8NuIRbqEisj-GE&e=

Patch

diff --git a/contrib/perf.py b/contrib/perf.py
--- a/contrib/perf.py
+++ b/contrib/perf.py
@@ -861,7 +861,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)
+        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,9 @@ 
 
         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)
+            super(filelog, self).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
@@ -1221,9 +1221,7 @@ 
             bins = bins[1:]
 
         text = mdiff.patches(text, bins)
-
-        text = self._checkhash(text, node, rev)
-
+        self.checkhash(text, node, rev=rev)
         self._cache = (node, rev, text)
         return text
 
@@ -1235,12 +1233,14 @@ 
         """
         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.
+        """
+        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:
@@ -1415,7 +1415,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: