Patchwork [05,of,11] manifest: coerce _msearch argument to bytestr at method start

login
register
mail settings
Submitter Augie Fackler
Date March 26, 2017, 10:36 p.m.
Message ID <d5dcfa6b2e20183ba2d6.1490567799@imladris.local>
Download mbox | patch
Permalink /patch/19703/
State Accepted
Headers show

Comments

Augie Fackler - March 26, 2017, 10:36 p.m.
# HG changeset patch
# User Augie Fackler <raf@durin42.com>
# Date 1490567242 14400
#      Sun Mar 26 18:27:22 2017 -0400
# Node ID d5dcfa6b2e20183ba2d6e439a23f5f2f4bf7981e
# Parent  54631fab906cb662e370ce313a0395292f7dfa15
manifest: coerce _msearch argument to bytestr at method start

This potentially incurs some extra copies, but on Python 3 you can't
do comparisons between memoryview and bytes sequences. I have a
sneaking suspicion there are potential bugs in Python 2 lurking that
this might fix (but I have no way of proving this theory), so I'm not
going to sweat the risk of extra overhead here.
Pulkit Goyal - March 27, 2017, 6:19 a.m.
Yuya had a different approach on this
https://patchwork.mercurial-scm.org/patch/19682/

On Mon, Mar 27, 2017 at 4:06 AM, Augie Fackler <raf@durin42.com> wrote:

> # HG changeset patch
> # User Augie Fackler <raf@durin42.com>
> # Date 1490567242 14400
> #      Sun Mar 26 18:27:22 2017 -0400
> # Node ID d5dcfa6b2e20183ba2d6e439a23f5f2f4bf7981e
> # Parent  54631fab906cb662e370ce313a0395292f7dfa15
> manifest: coerce _msearch argument to bytestr at method start
>
> This potentially incurs some extra copies, but on Python 3 you can't
> do comparisons between memoryview and bytes sequences. I have a
> sneaking suspicion there are potential bugs in Python 2 lurking that
> this might fix (but I have no way of proving this theory), so I'm not
> going to sweat the risk of extra overhead here.
>
> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
> --- a/mercurial/manifest.py
> +++ b/mercurial/manifest.py
> @@ -20,6 +20,7 @@ from . import (
>      error,
>      mdiff,
>      parsers,
> +    pycompat,
>      revlog,
>      util,
>  )
> @@ -652,6 +653,7 @@ def _msearch(m, s, lo=0, hi=None):
>
>      m should be a buffer or a string
>      s is a string'''
> +    m = pycompat.bytestr(m)
>      def advance(i, c):
>          while i < lenm and m[i] != c:
>              i += 1
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Yuya Nishihara - March 27, 2017, 1:21 p.m.
On Mon, 27 Mar 2017 11:49:45 +0530, Pulkit Goyal wrote:
> Yuya had a different approach on this
> https://patchwork.mercurial-scm.org/patch/19682/

[...]

> On Mon, Mar 27, 2017 at 4:06 AM, Augie Fackler <raf@durin42.com> wrote:
> > # HG changeset patch
> > # User Augie Fackler <raf@durin42.com>
> > # Date 1490567242 14400
> > #      Sun Mar 26 18:27:22 2017 -0400
> > # Node ID d5dcfa6b2e20183ba2d6e439a23f5f2f4bf7981e
> > # Parent  54631fab906cb662e370ce313a0395292f7dfa15
> > manifest: coerce _msearch argument to bytestr at method start
> >
> > This potentially incurs some extra copies, but on Python 3 you can't
> > do comparisons between memoryview and bytes sequences. I have a
> > sneaking suspicion there are potential bugs in Python 2 lurking that
> > this might fix (but I have no way of proving this theory), so I'm not
> > going to sweat the risk of extra overhead here.

We use buffer() on Python 2, which returns bytes for slicing operation.
That's why 'm[start:end] < s' works on Python 2. OTOH, memoryview() can't
be used in that way.

Patch

diff --git a/mercurial/manifest.py b/mercurial/manifest.py
--- a/mercurial/manifest.py
+++ b/mercurial/manifest.py
@@ -20,6 +20,7 @@  from . import (
     error,
     mdiff,
     parsers,
+    pycompat,
     revlog,
     util,
 )
@@ -652,6 +653,7 @@  def _msearch(m, s, lo=0, hi=None):
 
     m should be a buffer or a string
     s is a string'''
+    m = pycompat.bytestr(m)
     def advance(i, c):
         while i < lenm and m[i] != c:
             i += 1