Patchwork [STABLE] bdiff: (pure) support array.array arrays (issue5130)

login
register
mail settings
Submitter timeless@mozdev.org
Date March 8, 2016, 5:28 p.m.
Message ID <5d69a2ef233c74a13d58.1457458091@waste.org>
Download mbox | patch
Permalink /patch/13683/
State Accepted
Delegated to: Pierre-Yves David
Headers show

Comments

timeless@mozdev.org - March 8, 2016, 5:28 p.m.
# HG changeset patch
# User timeless <timeless@mozdev.org>
# Date 1457457972 0
#      Tue Mar 08 17:26:12 2016 +0000
# Branch stable
# Node ID 5d69a2ef233c74a13d584679bbbb6200a3904441
# Parent  8949d73b2e1f5c0b9c4c6c195bef2fe284349c6e
bdiff: (pure) support array.array arrays (issue5130)
timeless - March 8, 2016, 5:32 p.m.
So, this is less than ideal, as it results in pure paying to box and
unbox "self.text" in manifest.fastdelta.

I'd rather pure not have to do that, but I don't know a way that
manifest.fastdelta could conditionally do the boxing only for
non-pure.

That said, this minimally fixes the crash.

On Tue, Mar 8, 2016 at 12:28 PM, timeless <timeless@mozdev.org> wrote:
> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1457457972 0
> #      Tue Mar 08 17:26:12 2016 +0000
> # Branch stable
> # Node ID 5d69a2ef233c74a13d584679bbbb6200a3904441
> # Parent  8949d73b2e1f5c0b9c4c6c195bef2fe284349c6e
> bdiff: (pure) support array.array arrays (issue5130)
>
> diff --git a/mercurial/pure/bdiff.py b/mercurial/pure/bdiff.py
> --- a/mercurial/pure/bdiff.py
> +++ b/mercurial/pure/bdiff.py
> @@ -7,6 +7,7 @@
>
>  from __future__ import absolute_import
>
> +import array
>  import difflib
>  import re
>  import struct
> @@ -50,9 +51,14 @@
>      r.append(prev)
>      return r
>
> +def _tostring(c):
> +    if type(c) is array.array:
> +        return c.tostring()
> +    return str(c)
> +
>  def bdiff(a, b):
> -    a = str(a).splitlines(True)
> -    b = str(b).splitlines(True)
> +    a = _tostring(a).splitlines(True)
> +    b = _tostring(b).splitlines(True)
>
>      if not a:
>          s = "".join(b)
> diff --git a/tests/test-clone-uncompressed.t b/tests/test-clone-uncompressed.t
> --- a/tests/test-clone-uncompressed.t
> +++ b/tests/test-clone-uncompressed.t
> @@ -1,5 +1,8 @@
>  #require serve
>
> +Initialize repository
> +the status call is to check for issue5130
> +
>    $ hg init server
>    $ cd server
>    $ touch foo
> @@ -8,6 +11,7 @@
>    ...     with open(str(i), 'wb') as fh:
>    ...         fh.write(str(i))
>    $ hg -q commit -A -m 'add a lot of files'
> +  $ hg st
>    $ hg serve -p $HGPORT -d --pid-file=hg.pid
>    $ cat hg.pid >> $DAEMON_PIDS
>    $ cd ..
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Pierre-Yves David - March 9, 2016, 5:38 p.m.
On 03/08/2016 05:32 PM, timeless wrote:
> So, this is less than ideal, as it results in pure paying to box and
> unbox "self.text" in manifest.fastdelta.
>
> I'd rather pure not have to do that, but I don't know a way that
> manifest.fastdelta could conditionally do the boxing only for
> non-pure.
>
> That said, this minimally fixes the crash.

Can you point out this "less than ideal"lity in an inline comment so 
that the next soul looking at it can pay attention to it? probably point 
to the issue in it.

Patch looks fine otherwise, but I would prefer a v2 with such comment.

>
> On Tue, Mar 8, 2016 at 12:28 PM, timeless <timeless@mozdev.org> wrote:
>> # HG changeset patch
>> # User timeless <timeless@mozdev.org>
>> # Date 1457457972 0
>> #      Tue Mar 08 17:26:12 2016 +0000
>> # Branch stable
>> # Node ID 5d69a2ef233c74a13d584679bbbb6200a3904441
>> # Parent  8949d73b2e1f5c0b9c4c6c195bef2fe284349c6e
>> bdiff: (pure) support array.array arrays (issue5130)
>>
>> diff --git a/mercurial/pure/bdiff.py b/mercurial/pure/bdiff.py
>> --- a/mercurial/pure/bdiff.py
>> +++ b/mercurial/pure/bdiff.py
>> @@ -7,6 +7,7 @@
>>
>>   from __future__ import absolute_import
>>
>> +import array
>>   import difflib
>>   import re
>>   import struct
>> @@ -50,9 +51,14 @@
>>       r.append(prev)
>>       return r
>>
>> +def _tostring(c):
>> +    if type(c) is array.array:
>> +        return c.tostring()
>> +    return str(c)
>> +
>>   def bdiff(a, b):
>> -    a = str(a).splitlines(True)
>> -    b = str(b).splitlines(True)
>> +    a = _tostring(a).splitlines(True)
>> +    b = _tostring(b).splitlines(True)
>>
>>       if not a:
>>           s = "".join(b)
>> diff --git a/tests/test-clone-uncompressed.t b/tests/test-clone-uncompressed.t
>> --- a/tests/test-clone-uncompressed.t
>> +++ b/tests/test-clone-uncompressed.t
>> @@ -1,5 +1,8 @@
>>   #require serve
>>
>> +Initialize repository
>> +the status call is to check for issue5130
>> +
>>     $ hg init server
>>     $ cd server
>>     $ touch foo
>> @@ -8,6 +11,7 @@
>>     ...     with open(str(i), 'wb') as fh:
>>     ...         fh.write(str(i))
>>     $ hg -q commit -A -m 'add a lot of files'
>> +  $ hg st
>>     $ hg serve -p $HGPORT -d --pid-file=hg.pid
>>     $ cat hg.pid >> $DAEMON_PIDS
>>     $ cd ..
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Matt Mackall - March 9, 2016, 11:13 p.m.
On Tue, 2016-03-08 at 11:28 -0600, timeless wrote:
> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1457457972 0
> #      Tue Mar 08 17:26:12 2016 +0000
> # Branch stable
> # Node ID 5d69a2ef233c74a13d584679bbbb6200a3904441
> # Parent  8949d73b2e1f5c0b9c4c6c195bef2fe284349c6e
> bdiff: (pure) support array.array arrays (issue5130)

I've queued this for stable with a brief comment about overhead, thanks.

-- 
Mathematics is the supreme nostalgia of our time.

Patch

diff --git a/mercurial/pure/bdiff.py b/mercurial/pure/bdiff.py
--- a/mercurial/pure/bdiff.py
+++ b/mercurial/pure/bdiff.py
@@ -7,6 +7,7 @@ 
 
 from __future__ import absolute_import
 
+import array
 import difflib
 import re
 import struct
@@ -50,9 +51,14 @@ 
     r.append(prev)
     return r
 
+def _tostring(c):
+    if type(c) is array.array:
+        return c.tostring()
+    return str(c)
+
 def bdiff(a, b):
-    a = str(a).splitlines(True)
-    b = str(b).splitlines(True)
+    a = _tostring(a).splitlines(True)
+    b = _tostring(b).splitlines(True)
 
     if not a:
         s = "".join(b)
diff --git a/tests/test-clone-uncompressed.t b/tests/test-clone-uncompressed.t
--- a/tests/test-clone-uncompressed.t
+++ b/tests/test-clone-uncompressed.t
@@ -1,5 +1,8 @@ 
 #require serve
 
+Initialize repository
+the status call is to check for issue5130
+
   $ hg init server
   $ cd server
   $ touch foo
@@ -8,6 +11,7 @@ 
   ...     with open(str(i), 'wb') as fh:
   ...         fh.write(str(i))
   $ hg -q commit -A -m 'add a lot of files'
+  $ hg st
   $ hg serve -p $HGPORT -d --pid-file=hg.pid
   $ cat hg.pid >> $DAEMON_PIDS
   $ cd ..