Patchwork mpatch: unify mpatchError (issue5182)

login
register
mail settings
Submitter timeless@mozdev.org
Date April 5, 2016, 2:06 a.m.
Message ID <3f972305323b084f81b2.1459821989@waste.org>
Download mbox | patch
Permalink /patch/14363/
State Accepted
Headers show

Comments

timeless@mozdev.org - April 5, 2016, 2:06 a.m.
# HG changeset patch
# User timeless <timeless@mozdev.org>
# Date 1459389928 0
#      Thu Mar 31 02:05:28 2016 +0000
# Node ID 3f972305323b084f81b2566f9612db17cf0d92e3
# Parent  ff0d3b6b287f89594bd8d0308fe2810d2a18ea01
mpatch: unify mpatchError (issue5182)

The pure version was mpatch was throwing struct.error or ValueError
for errors, whereas the C version was throwing an "mpatch.mpatchError".

Introducing an mpatch.mpatchError into pure and using it consistently
is fairly easy, but the actual form for it is mercurial.mpatch.mpatchError,
so with this commit, we change the C implementation to match the naming
convention too.
Pierre-Yves David - April 5, 2016, 3:12 a.m.
On 04/04/2016 07:06 PM, timeless wrote:
> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1459389928 0
> #      Thu Mar 31 02:05:28 2016 +0000
> # Node ID 3f972305323b084f81b2566f9612db17cf0d92e3
> # Parent  ff0d3b6b287f89594bd8d0308fe2810d2a18ea01
> mpatch: unify mpatchError (issue5182)
>
> The pure version was mpatch was throwing struct.error or ValueError
> for errors, whereas the C version was throwing an "mpatch.mpatchError".
>
> Introducing an mpatch.mpatchError into pure and using it consistently
> is fairly easy, but the actual form for it is mercurial.mpatch.mpatchError,
> so with this commit, we change the C implementation to match the naming
> convention too.

Thanks a lot for tackling this quickly.
timeless - April 5, 2016, 4:05 a.m.
This was sitting around for a while (I have a bunch of things I need to flush).

On Mon, Apr 4, 2016 at 11:12 PM, Pierre-Yves David
<pierre-yves.david@ens-lyon.org> wrote:
>
>
> On 04/04/2016 07:06 PM, timeless wrote:
>>
>> # HG changeset patch
>> # User timeless <timeless@mozdev.org>
>> # Date 1459389928 0
>> #      Thu Mar 31 02:05:28 2016 +0000
>> # Node ID 3f972305323b084f81b2566f9612db17cf0d92e3
>> # Parent  ff0d3b6b287f89594bd8d0308fe2810d2a18ea01
>> mpatch: unify mpatchError (issue5182)
>>
>> The pure version was mpatch was throwing struct.error or ValueError
>> for errors, whereas the C version was throwing an "mpatch.mpatchError".
>>
>> Introducing an mpatch.mpatchError into pure and using it consistently
>> is fairly easy, but the actual form for it is
>> mercurial.mpatch.mpatchError,
>> so with this commit, we change the C implementation to match the naming
>> convention too.
>
>
> Thanks a lot for tackling this quickly.
>
> --
> Pierre-Yves David
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Augie Fackler - April 5, 2016, 5:30 p.m.
On Mon, Apr 04, 2016 at 09:06:29PM -0500, timeless wrote:
> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1459389928 0
> #      Thu Mar 31 02:05:28 2016 +0000
> # Node ID 3f972305323b084f81b2566f9612db17cf0d92e3
> # Parent  ff0d3b6b287f89594bd8d0308fe2810d2a18ea01
> mpatch: unify mpatchError (issue5182)

I don't think pyd queued this, so I've done so. Thanks!

>
> The pure version was mpatch was throwing struct.error or ValueError
> for errors, whereas the C version was throwing an "mpatch.mpatchError".
>
> Introducing an mpatch.mpatchError into pure and using it consistently
> is fairly easy, but the actual form for it is mercurial.mpatch.mpatchError,
> so with this commit, we change the C implementation to match the naming
> convention too.
>
> diff --git a/mercurial/mpatch.c b/mercurial/mpatch.c
> --- a/mercurial/mpatch.c
> +++ b/mercurial/mpatch.c
> @@ -404,7 +404,8 @@
>       if (m == NULL)
>               return NULL;
>
> -	mpatch_Error = PyErr_NewException("mpatch.mpatchError", NULL, NULL);
> +	mpatch_Error = PyErr_NewException("mercurial.mpatch.mpatchError",
> +                                       NULL, NULL);
>       Py_INCREF(mpatch_Error);
>       PyModule_AddObject(m, "mpatchError", mpatch_Error);
>
> @@ -415,6 +416,7 @@
>  initmpatch(void)
>  {
>       Py_InitModule3("mpatch", methods, mpatch_doc);
> -	mpatch_Error = PyErr_NewException("mpatch.mpatchError", NULL, NULL);
> +	mpatch_Error = PyErr_NewException("mercurial.mpatch.mpatchError",
> +                                       NULL, NULL);
>  }
>  #endif
> diff --git a/mercurial/pure/mpatch.py b/mercurial/pure/mpatch.py
> --- a/mercurial/pure/mpatch.py
> +++ b/mercurial/pure/mpatch.py
> @@ -12,6 +12,10 @@
>
>  StringIO = cStringIO.StringIO
>
> +class mpatchError(Exception):
> +    """error raised when a delta cannot be decoded
> +    """
> +
>  # This attempts to apply a series of patches in time proportional to
>  # the total size of the patches, rather than patches * len(text). This
>  # means rather than shuffling strings around, we shuffle around
> @@ -84,7 +88,10 @@
>          last = 0
>          while pos < end:
>              m.seek(pos)
> -            p1, p2, l = struct.unpack(">lll", m.read(12))
> +            try:
> +                p1, p2, l = struct.unpack(">lll", m.read(12))
> +            except struct.error:
> +                raise mpatchError("patch cannot be decoded")
>              _pull(new, frags, p1 - last) # what didn't change
>              _pull([], frags, p2 - p1)    # what got deleted
>              new.append((l, pos + 12))   # what got added
> @@ -114,7 +121,7 @@
>          outlen += length
>
>      if bin != binend:
> -        raise ValueError("patch cannot be decoded")
> +        raise mpatchError("patch cannot be decoded")
>
>      outlen += orig - last
>      return outlen
> diff --git a/tests/test-revlog.t b/tests/test-revlog.t
> --- a/tests/test-revlog.t
> +++ b/tests/test-revlog.t
> @@ -11,5 +11,5 @@
>       rev    offset  length  delta linkrev nodeid       p1           p2
>         0         0      19     -1       2 99e0332bd498 000000000000 000000000000
>         1        19      12      0       3 6674f57a23d8 99e0332bd498 000000000000
> -  $ hg debugdata a.i 1 2>&1 | grep decoded
> -  mpatch.mpatchError: patch cannot be decoded
> +  $ hg debugdata a.i 1 2>&1 | egrep 'Error:.*decoded'
> +  mercurial.mpatch.mpatchError: patch cannot be decoded
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Pierre-Yves David - April 5, 2016, 5:39 p.m.
On 04/05/2016 10:30 AM, Augie Fackler wrote:
> On Mon, Apr 04, 2016 at 09:06:29PM -0500, timeless wrote:
>> # HG changeset patch
>> # User timeless <timeless@mozdev.org>
>> # Date 1459389928 0
>> #      Thu Mar 31 02:05:28 2016 +0000
>> # Node ID 3f972305323b084f81b2566f9612db17cf0d92e3
>> # Parent  ff0d3b6b287f89594bd8d0308fe2810d2a18ea01
>> mpatch: unify mpatchError (issue5182)
>
> I don't think pyd queued this, so I've done so. Thanks!

This was pushed. (sorry for the unclear message about this).

Patch

diff --git a/mercurial/mpatch.c b/mercurial/mpatch.c
--- a/mercurial/mpatch.c
+++ b/mercurial/mpatch.c
@@ -404,7 +404,8 @@ 
 	if (m == NULL)
 		return NULL;
 
-	mpatch_Error = PyErr_NewException("mpatch.mpatchError", NULL, NULL);
+	mpatch_Error = PyErr_NewException("mercurial.mpatch.mpatchError",
+					  NULL, NULL);
 	Py_INCREF(mpatch_Error);
 	PyModule_AddObject(m, "mpatchError", mpatch_Error);
 
@@ -415,6 +416,7 @@ 
 initmpatch(void)
 {
 	Py_InitModule3("mpatch", methods, mpatch_doc);
-	mpatch_Error = PyErr_NewException("mpatch.mpatchError", NULL, NULL);
+	mpatch_Error = PyErr_NewException("mercurial.mpatch.mpatchError",
+					  NULL, NULL);
 }
 #endif
diff --git a/mercurial/pure/mpatch.py b/mercurial/pure/mpatch.py
--- a/mercurial/pure/mpatch.py
+++ b/mercurial/pure/mpatch.py
@@ -12,6 +12,10 @@ 
 
 StringIO = cStringIO.StringIO
 
+class mpatchError(Exception):
+    """error raised when a delta cannot be decoded
+    """
+
 # This attempts to apply a series of patches in time proportional to
 # the total size of the patches, rather than patches * len(text). This
 # means rather than shuffling strings around, we shuffle around
@@ -84,7 +88,10 @@ 
         last = 0
         while pos < end:
             m.seek(pos)
-            p1, p2, l = struct.unpack(">lll", m.read(12))
+            try:
+                p1, p2, l = struct.unpack(">lll", m.read(12))
+            except struct.error:
+                raise mpatchError("patch cannot be decoded")
             _pull(new, frags, p1 - last) # what didn't change
             _pull([], frags, p2 - p1)    # what got deleted
             new.append((l, pos + 12))   # what got added
@@ -114,7 +121,7 @@ 
         outlen += length
 
     if bin != binend:
-        raise ValueError("patch cannot be decoded")
+        raise mpatchError("patch cannot be decoded")
 
     outlen += orig - last
     return outlen
diff --git a/tests/test-revlog.t b/tests/test-revlog.t
--- a/tests/test-revlog.t
+++ b/tests/test-revlog.t
@@ -11,5 +11,5 @@ 
      rev    offset  length  delta linkrev nodeid       p1           p2
        0         0      19     -1       2 99e0332bd498 000000000000 000000000000
        1        19      12      0       3 6674f57a23d8 99e0332bd498 000000000000
-  $ hg debugdata a.i 1 2>&1 | grep decoded
-  mpatch.mpatchError: patch cannot be decoded
+  $ hg debugdata a.i 1 2>&1 | egrep 'Error:.*decoded'
+  mercurial.mpatch.mpatchError: patch cannot be decoded