Patchwork [2,of,2,chg-port] osutil: implement pure version of recvfds() for PyPy

login
register
mail settings
Submitter Yuya Nishihara
Date Dec. 19, 2015, 3:17 a.m.
Message ID <9726608c61dd53832c82.1450495057@mimosa>
Download mbox | patch
Permalink /patch/12180/
State Accepted
Headers show

Comments

Yuya Nishihara - Dec. 19, 2015, 3:17 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1450363989 -32400
#      Thu Dec 17 23:53:09 2015 +0900
# Node ID 9726608c61dd53832c821d8bb9ce15b8aa268466
# Parent  076525f412e74c913f72ff52034aa7ea046af74b
osutil: implement pure version of recvfds() for PyPy

This is less portable than the C version, but PyPy can't load CPython
extensions. So for now, this will be used on PyPy.

I've tested it on Linux amd64 and Mac OS X.
Sean Farley - Dec. 19, 2015, 6:37 p.m.
Yuya Nishihara <yuya@tcha.org> writes:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1450363989 -32400
> #      Thu Dec 17 23:53:09 2015 +0900
> # Node ID 9726608c61dd53832c821d8bb9ce15b8aa268466
> # Parent  076525f412e74c913f72ff52034aa7ea046af74b
> osutil: implement pure version of recvfds() for PyPy
>
> This is less portable than the C version, but PyPy can't load CPython
> extensions. So for now, this will be used on PyPy.
>
> I've tested it on Linux amd64 and Mac OS X.
>
> diff --git a/mercurial/pure/osutil.py b/mercurial/pure/osutil.py
> --- a/mercurial/pure/osutil.py
> +++ b/mercurial/pure/osutil.py
> @@ -7,8 +7,12 @@
>  
>  from __future__ import absolute_import
>  
> +import ctypes
> +import ctypes.util
>  import os
> +import socket
>  import stat as statmod
> +import sys
>  
>  def _mode_to_kind(mode):
>      if statmod.S_ISREG(mode):
> @@ -59,8 +63,87 @@ def listdir(path, stat=False, skip=None)
>  
>  if os.name != 'nt':
>      posixfile = open
> +
> +    _SCM_RIGHTS = 0x01
> +    _socklen_t = ctypes.c_uint
> +
> +    if sys.platform == 'linux2':
> +        # socket.h says "the type should be socklen_t but the definition of
> +        # the kernel is incompatible with this."
> +        _cmsg_len_t = ctypes.c_size_t
> +        _msg_controllen_t = ctypes.c_size_t
> +        _msg_iovlen_t = ctypes.c_size_t
> +    else:
> +        _cmsg_len_t = _socklen_t
> +        _msg_controllen_t = _socklen_t
> +        _msg_iovlen_t = ctypes.c_int

I think we usually write the above without the else clause:

    _cmsg_len_t = _socklen_t
    _msg_controllen_t = _socklen_t
    _msg_iovlen_t = ctypes.c_int
    if sys.platform == 'linux2':
        # socket.h says "the type should be socklen_t but the definition of
        # the kernel is incompatible with this."
        _cmsg_len_t = ctypes.c_size_t
        _msg_controllen_t = ctypes.c_size_t
        _msg_iovlen_t = ctypes.c_size_t

But that's a minor nit. Otherwise, both these patches look good to me.
Yuya Nishihara - Dec. 20, 2015, 9:18 a.m.
On Sat, 19 Dec 2015 10:37:11 -0800, Sean Farley wrote:
> 
> Yuya Nishihara <yuya@tcha.org> writes:
> 
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1450363989 -32400
> > #      Thu Dec 17 23:53:09 2015 +0900
> > # Node ID 9726608c61dd53832c821d8bb9ce15b8aa268466
> > # Parent  076525f412e74c913f72ff52034aa7ea046af74b
> > osutil: implement pure version of recvfds() for PyPy
> >
> > This is less portable than the C version, but PyPy can't load CPython
> > extensions. So for now, this will be used on PyPy.
> >
> > I've tested it on Linux amd64 and Mac OS X.
> >
> > diff --git a/mercurial/pure/osutil.py b/mercurial/pure/osutil.py
> > --- a/mercurial/pure/osutil.py
> > +++ b/mercurial/pure/osutil.py
> > @@ -7,8 +7,12 @@
> >  
> >  from __future__ import absolute_import
> >  
> > +import ctypes
> > +import ctypes.util
> >  import os
> > +import socket
> >  import stat as statmod
> > +import sys
> >  
> >  def _mode_to_kind(mode):
> >      if statmod.S_ISREG(mode):
> > @@ -59,8 +63,87 @@ def listdir(path, stat=False, skip=None)
> >  
> >  if os.name != 'nt':
> >      posixfile = open
> > +
> > +    _SCM_RIGHTS = 0x01
> > +    _socklen_t = ctypes.c_uint
> > +
> > +    if sys.platform == 'linux2':
> > +        # socket.h says "the type should be socklen_t but the definition of
> > +        # the kernel is incompatible with this."
> > +        _cmsg_len_t = ctypes.c_size_t
> > +        _msg_controllen_t = ctypes.c_size_t
> > +        _msg_iovlen_t = ctypes.c_size_t
> > +    else:
> > +        _cmsg_len_t = _socklen_t
> > +        _msg_controllen_t = _socklen_t
> > +        _msg_iovlen_t = ctypes.c_int
> 
> I think we usually write the above without the else clause:
> 
>     _cmsg_len_t = _socklen_t
>     _msg_controllen_t = _socklen_t
>     _msg_iovlen_t = ctypes.c_int
>     if sys.platform == 'linux2':
>         # socket.h says "the type should be socklen_t but the definition of
>         # the kernel is incompatible with this."
>         _cmsg_len_t = ctypes.c_size_t
>         _msg_controllen_t = ctypes.c_size_t
>         _msg_iovlen_t = ctypes.c_size_t
> 
> But that's a minor nit. Otherwise, both these patches look good to me.

Thanks. I personally prefer if-else, but that isn't big issue. If I coded it
on BSD and found Linux problem later, I would do like your suggestion.

Please feel free to change it in flight.
Matt Mackall - Dec. 22, 2015, 8:46 p.m.
On Sat, 2015-12-19 at 12:17 +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1450363989 -32400
> #      Thu Dec 17 23:53:09 2015 +0900
> # Node ID 9726608c61dd53832c821d8bb9ce15b8aa268466
> # Parent  076525f412e74c913f72ff52034aa7ea046af74b
> osutil: implement pure version of recvfds() for PyPy

These are queued for default, thanks.

-- 
Mathematics is the supreme nostalgia of our time.
Katsunori FUJIWARA - Dec. 23, 2015, 4:30 p.m.
At Sat, 19 Dec 2015 12:17:37 +0900,
Yuya Nishihara wrote:
> 
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1450363989 -32400
> #      Thu Dec 17 23:53:09 2015 +0900
> # Node ID 9726608c61dd53832c821d8bb9ce15b8aa268466
> # Parent  076525f412e74c913f72ff52034aa7ea046af74b
> osutil: implement pure version of recvfds() for PyPy
> 
> This is less portable than the C version, but PyPy can't load CPython
> extensions. So for now, this will be used on PyPy.
> 
> I've tested it on Linux amd64 and Mac OS X.
> 
> diff --git a/mercurial/pure/osutil.py b/mercurial/pure/osutil.py
> --- a/mercurial/pure/osutil.py
> +++ b/mercurial/pure/osutil.py
> @@ -7,8 +7,12 @@
>  
>  from __future__ import absolute_import
>  
> +import ctypes
> +import ctypes.util
>  import os
> +import socket
>  import stat as statmod
> +import sys
>  
>  def _mode_to_kind(mode):
>      if statmod.S_ISREG(mode):
> @@ -59,8 +63,87 @@ def listdir(path, stat=False, skip=None)
>  
>  if os.name != 'nt':
>      posixfile = open
> +
> +    _SCM_RIGHTS = 0x01
> +    _socklen_t = ctypes.c_uint
> +
> +    if sys.platform == 'linux2':
> +        # socket.h says "the type should be socklen_t but the definition of
> +        # the kernel is incompatible with this."
> +        _cmsg_len_t = ctypes.c_size_t
> +        _msg_controllen_t = ctypes.c_size_t
> +        _msg_iovlen_t = ctypes.c_size_t
> +    else:
> +        _cmsg_len_t = _socklen_t
> +        _msg_controllen_t = _socklen_t
> +        _msg_iovlen_t = ctypes.c_int
> +
> +    class _iovec(ctypes.Structure):
> +        _fields_ = [
> +            ('iov_base', ctypes.c_void_p),
> +            ('iov_len', ctypes.c_size_t),
> +        ]
> +
> +    class _msghdr(ctypes.Structure):
> +        _fields_ = [
> +            ('msg_name', ctypes.c_void_p),
> +            ('msg_namelen', _socklen_t),
> +            ('msg_iov', ctypes.POINTER(_iovec)),
> +            ('msg_iovlen', _msg_iovlen_t),
> +            ('msg_control', ctypes.c_void_p),
> +            ('msg_controllen', _msg_controllen_t),
> +            ('msg_flags', ctypes.c_int),
> +        ]
> +
> +    class _cmsghdr(ctypes.Structure):
> +        _fields_ = [
> +            ('cmsg_len', _cmsg_len_t),
> +            ('cmsg_level', ctypes.c_int),
> +            ('cmsg_type', ctypes.c_int),
> +            ('cmsg_data', ctypes.c_ubyte * 0),
> +        ]
> +
> +    _libc = ctypes.CDLL(ctypes.util.find_library('c'), use_errno=True)
> +    _recvmsg = _libc.recvmsg
> +    _recvmsg.restype = ctypes.c_ssize_t

This change doesn't work with Python 2.6.x, because ctypes.c_ssize_t
was introduced by 2.7. Loading osutil itself is aborted.

Even if Mercurial itself will be built without pure python modules,
"hg id -i" before building *.o in setup.py requires pure version of
osutil module.


> +    _recvmsg.argtypes = (ctypes.c_int, ctypes.POINTER(_msghdr), ctypes.c_int)
> +
> +    def _CMSG_FIRSTHDR(msgh):
> +        if msgh.msg_controllen < ctypes.sizeof(_cmsghdr):
> +            return
> +        cmsgptr = ctypes.cast(msgh.msg_control, ctypes.POINTER(_cmsghdr))
> +        return cmsgptr.contents
> +
> +    # The pure version is less portable than the native version because the
> +    # handling of socket ancillary data heavily depends on C preprocessor.
> +    # Also, some length fields are wrongly typed in Linux kernel.
> +    def recvfds(sockfd):
> +        """receive list of file descriptors via socket"""
> +        dummy = (ctypes.c_ubyte * 1)()
> +        iov = _iovec(ctypes.cast(dummy, ctypes.c_void_p), ctypes.sizeof(dummy))
> +        cbuf = ctypes.create_string_buffer(256)
> +        msgh = _msghdr(None, 0,
> +                       ctypes.pointer(iov), 1,
> +                       ctypes.cast(cbuf, ctypes.c_void_p), ctypes.sizeof(cbuf),
> +                       0)
> +        r = _recvmsg(sockfd, ctypes.byref(msgh), 0)
> +        if r < 0:
> +            e = ctypes.get_errno()
> +            raise OSError(e, os.strerror(e))
> +        # assumes that the first cmsg has fds because it isn't easy to write
> +        # portable CMSG_NXTHDR() with ctypes.
> +        cmsg = _CMSG_FIRSTHDR(msgh)
> +        if not cmsg:
> +            return []
> +        if (cmsg.cmsg_level != socket.SOL_SOCKET or
> +            cmsg.cmsg_type != _SCM_RIGHTS):
> +            return []
> +        rfds = ctypes.cast(cmsg.cmsg_data, ctypes.POINTER(ctypes.c_int))
> +        rfdscount = ((cmsg.cmsg_len - _cmsghdr.cmsg_data.offset) /
> +                     ctypes.sizeof(ctypes.c_int))
> +        return [rfds[i] for i in xrange(rfdscount)]
> +
>  else:
> -    import ctypes
>      import msvcrt
>  
>      _kernel32 = ctypes.windll.kernel32
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
Katsunori FUJIWARA - Dec. 23, 2015, 7 p.m.
At Thu, 24 Dec 2015 01:30:40 +0900,
FUJIWARA Katsunori wrote:
> 
> 
> At Sat, 19 Dec 2015 12:17:37 +0900,
> Yuya Nishihara wrote:
> > 
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1450363989 -32400
> > #      Thu Dec 17 23:53:09 2015 +0900
> > # Node ID 9726608c61dd53832c821d8bb9ce15b8aa268466
> > # Parent  076525f412e74c913f72ff52034aa7ea046af74b
> > osutil: implement pure version of recvfds() for PyPy
> > 
> > This is less portable than the C version, but PyPy can't load CPython
> > extensions. So for now, this will be used on PyPy.
> > 
> > I've tested it on Linux amd64 and Mac OS X.
> > 
> > diff --git a/mercurial/pure/osutil.py b/mercurial/pure/osutil.py
> > --- a/mercurial/pure/osutil.py
> > +++ b/mercurial/pure/osutil.py
> > @@ -7,8 +7,12 @@
> >  
> >  from __future__ import absolute_import
> >  
> > +import ctypes
> > +import ctypes.util
> >  import os
> > +import socket
> >  import stat as statmod
> > +import sys
> >  
> >  def _mode_to_kind(mode):
> >      if statmod.S_ISREG(mode):
> > @@ -59,8 +63,87 @@ def listdir(path, stat=False, skip=None)
> >  
> >  if os.name != 'nt':
> >      posixfile = open
> > +
> > +    _SCM_RIGHTS = 0x01
> > +    _socklen_t = ctypes.c_uint
> > +
> > +    if sys.platform == 'linux2':
> > +        # socket.h says "the type should be socklen_t but the definition of
> > +        # the kernel is incompatible with this."
> > +        _cmsg_len_t = ctypes.c_size_t
> > +        _msg_controllen_t = ctypes.c_size_t
> > +        _msg_iovlen_t = ctypes.c_size_t
> > +    else:
> > +        _cmsg_len_t = _socklen_t
> > +        _msg_controllen_t = _socklen_t
> > +        _msg_iovlen_t = ctypes.c_int
> > +
> > +    class _iovec(ctypes.Structure):
> > +        _fields_ = [
> > +            ('iov_base', ctypes.c_void_p),
> > +            ('iov_len', ctypes.c_size_t),
> > +        ]
> > +
> > +    class _msghdr(ctypes.Structure):
> > +        _fields_ = [
> > +            ('msg_name', ctypes.c_void_p),
> > +            ('msg_namelen', _socklen_t),
> > +            ('msg_iov', ctypes.POINTER(_iovec)),
> > +            ('msg_iovlen', _msg_iovlen_t),
> > +            ('msg_control', ctypes.c_void_p),
> > +            ('msg_controllen', _msg_controllen_t),
> > +            ('msg_flags', ctypes.c_int),
> > +        ]
> > +
> > +    class _cmsghdr(ctypes.Structure):
> > +        _fields_ = [
> > +            ('cmsg_len', _cmsg_len_t),
> > +            ('cmsg_level', ctypes.c_int),
> > +            ('cmsg_type', ctypes.c_int),
> > +            ('cmsg_data', ctypes.c_ubyte * 0),
> > +        ]
> > +
> > +    _libc = ctypes.CDLL(ctypes.util.find_library('c'), use_errno=True)
> > +    _recvmsg = _libc.recvmsg
> > +    _recvmsg.restype = ctypes.c_ssize_t
> 
> This change doesn't work with Python 2.6.x, because ctypes.c_ssize_t
> was introduced by 2.7. Loading osutil itself is aborted.
> 
> Even if Mercurial itself will be built without pure python modules,
> "hg id -i" before building *.o in setup.py requires pure version of
> osutil module.

Oops, sorry, I overlooked Bryan's patch posting.


> 
> > +    _recvmsg.argtypes = (ctypes.c_int, ctypes.POINTER(_msghdr), ctypes.c_int)
> > +
> > +    def _CMSG_FIRSTHDR(msgh):
> > +        if msgh.msg_controllen < ctypes.sizeof(_cmsghdr):
> > +            return
> > +        cmsgptr = ctypes.cast(msgh.msg_control, ctypes.POINTER(_cmsghdr))
> > +        return cmsgptr.contents
> > +
> > +    # The pure version is less portable than the native version because the
> > +    # handling of socket ancillary data heavily depends on C preprocessor.
> > +    # Also, some length fields are wrongly typed in Linux kernel.
> > +    def recvfds(sockfd):
> > +        """receive list of file descriptors via socket"""
> > +        dummy = (ctypes.c_ubyte * 1)()
> > +        iov = _iovec(ctypes.cast(dummy, ctypes.c_void_p), ctypes.sizeof(dummy))
> > +        cbuf = ctypes.create_string_buffer(256)
> > +        msgh = _msghdr(None, 0,
> > +                       ctypes.pointer(iov), 1,
> > +                       ctypes.cast(cbuf, ctypes.c_void_p), ctypes.sizeof(cbuf),
> > +                       0)
> > +        r = _recvmsg(sockfd, ctypes.byref(msgh), 0)
> > +        if r < 0:
> > +            e = ctypes.get_errno()
> > +            raise OSError(e, os.strerror(e))
> > +        # assumes that the first cmsg has fds because it isn't easy to write
> > +        # portable CMSG_NXTHDR() with ctypes.
> > +        cmsg = _CMSG_FIRSTHDR(msgh)
> > +        if not cmsg:
> > +            return []
> > +        if (cmsg.cmsg_level != socket.SOL_SOCKET or
> > +            cmsg.cmsg_type != _SCM_RIGHTS):
> > +            return []
> > +        rfds = ctypes.cast(cmsg.cmsg_data, ctypes.POINTER(ctypes.c_int))
> > +        rfdscount = ((cmsg.cmsg_len - _cmsghdr.cmsg_data.offset) /
> > +                     ctypes.sizeof(ctypes.c_int))
> > +        return [rfds[i] for i in xrange(rfdscount)]
> > +
> >  else:
> > -    import ctypes
> >      import msvcrt
> >  
> >      _kernel32 = ctypes.windll.kernel32
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@selenic.com
> > https://selenic.com/mailman/listinfo/mercurial-devel
> 
> ----------------------------------------------------------------------
> [FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp

Patch

diff --git a/mercurial/pure/osutil.py b/mercurial/pure/osutil.py
--- a/mercurial/pure/osutil.py
+++ b/mercurial/pure/osutil.py
@@ -7,8 +7,12 @@ 
 
 from __future__ import absolute_import
 
+import ctypes
+import ctypes.util
 import os
+import socket
 import stat as statmod
+import sys
 
 def _mode_to_kind(mode):
     if statmod.S_ISREG(mode):
@@ -59,8 +63,87 @@  def listdir(path, stat=False, skip=None)
 
 if os.name != 'nt':
     posixfile = open
+
+    _SCM_RIGHTS = 0x01
+    _socklen_t = ctypes.c_uint
+
+    if sys.platform == 'linux2':
+        # socket.h says "the type should be socklen_t but the definition of
+        # the kernel is incompatible with this."
+        _cmsg_len_t = ctypes.c_size_t
+        _msg_controllen_t = ctypes.c_size_t
+        _msg_iovlen_t = ctypes.c_size_t
+    else:
+        _cmsg_len_t = _socklen_t
+        _msg_controllen_t = _socklen_t
+        _msg_iovlen_t = ctypes.c_int
+
+    class _iovec(ctypes.Structure):
+        _fields_ = [
+            ('iov_base', ctypes.c_void_p),
+            ('iov_len', ctypes.c_size_t),
+        ]
+
+    class _msghdr(ctypes.Structure):
+        _fields_ = [
+            ('msg_name', ctypes.c_void_p),
+            ('msg_namelen', _socklen_t),
+            ('msg_iov', ctypes.POINTER(_iovec)),
+            ('msg_iovlen', _msg_iovlen_t),
+            ('msg_control', ctypes.c_void_p),
+            ('msg_controllen', _msg_controllen_t),
+            ('msg_flags', ctypes.c_int),
+        ]
+
+    class _cmsghdr(ctypes.Structure):
+        _fields_ = [
+            ('cmsg_len', _cmsg_len_t),
+            ('cmsg_level', ctypes.c_int),
+            ('cmsg_type', ctypes.c_int),
+            ('cmsg_data', ctypes.c_ubyte * 0),
+        ]
+
+    _libc = ctypes.CDLL(ctypes.util.find_library('c'), use_errno=True)
+    _recvmsg = _libc.recvmsg
+    _recvmsg.restype = ctypes.c_ssize_t
+    _recvmsg.argtypes = (ctypes.c_int, ctypes.POINTER(_msghdr), ctypes.c_int)
+
+    def _CMSG_FIRSTHDR(msgh):
+        if msgh.msg_controllen < ctypes.sizeof(_cmsghdr):
+            return
+        cmsgptr = ctypes.cast(msgh.msg_control, ctypes.POINTER(_cmsghdr))
+        return cmsgptr.contents
+
+    # The pure version is less portable than the native version because the
+    # handling of socket ancillary data heavily depends on C preprocessor.
+    # Also, some length fields are wrongly typed in Linux kernel.
+    def recvfds(sockfd):
+        """receive list of file descriptors via socket"""
+        dummy = (ctypes.c_ubyte * 1)()
+        iov = _iovec(ctypes.cast(dummy, ctypes.c_void_p), ctypes.sizeof(dummy))
+        cbuf = ctypes.create_string_buffer(256)
+        msgh = _msghdr(None, 0,
+                       ctypes.pointer(iov), 1,
+                       ctypes.cast(cbuf, ctypes.c_void_p), ctypes.sizeof(cbuf),
+                       0)
+        r = _recvmsg(sockfd, ctypes.byref(msgh), 0)
+        if r < 0:
+            e = ctypes.get_errno()
+            raise OSError(e, os.strerror(e))
+        # assumes that the first cmsg has fds because it isn't easy to write
+        # portable CMSG_NXTHDR() with ctypes.
+        cmsg = _CMSG_FIRSTHDR(msgh)
+        if not cmsg:
+            return []
+        if (cmsg.cmsg_level != socket.SOL_SOCKET or
+            cmsg.cmsg_type != _SCM_RIGHTS):
+            return []
+        rfds = ctypes.cast(cmsg.cmsg_data, ctypes.POINTER(ctypes.c_int))
+        rfdscount = ((cmsg.cmsg_len - _cmsghdr.cmsg_data.offset) /
+                     ctypes.sizeof(ctypes.c_int))
+        return [rfds[i] for i in xrange(rfdscount)]
+
 else:
-    import ctypes
     import msvcrt
 
     _kernel32 = ctypes.windll.kernel32