Re: [gentoo-portage-dev] [PATCH 3/3] pym/portage/util/locale.py: add a C module to help check locale

2016-05-30 Thread Brian Dolbec 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512

On Mon, 30 May 2016 11:08:03 +0200
Alexander Berntsen  wrote:

> On 27/05/16 16:26, Anthony G. Basile wrote:
> > However, this gets bogged down in implementation details and fails 
> > with musl.  
> Is it possible to elucidate this a bit more in a paragraph? Right now
> it reads a bit like "but this fails because reasons -_o_-", but in the
> future it might be nice to know said reasons.
> 
> Patch 1 and 2 look great. Patch 3 looks good to me, but obviously
> Michał knows more about this so I'll let him be the judge.
> - -- 
> Alexander
> berna...@gentoo.org

I agree that all 3 look good,  But I would say that the other changes
Michal proposed should be their own commit(s).  This being the first,
the others each adding to it.

- -- 
Brian Dolbec 

-BEGIN PGP SIGNATURE-
Version: GnuPG v2.1

iQJ8BAEBCgBmBQJXTDXCXxSAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w
ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXRBNUQ3Qzc0RTA4MUNDNzBEQjRBNEFBRjVG
QkJEMDg3Mjc1ODIwRUQ4AAoJEPu9CHJ1gg7YBWYP/2iFd7DsDKDwZJtTjUJfWAdJ
o4EqXmRtPOom0EIf5etfuiTRIKVAnALo9SeFJtp8nkBkRTcv5sa9IfB8ZKucslgW
VwlZCNBbXQIWm/d7zFYWSWw6rm0JM6Yq5hIrQ9R4eaKXZTZfNZUZla1u9LnGYmcs
VgNXd7EZ1R5zIxugWlLca5FhoEuNHiSrBQn5Vi9mfkngNAeqd5sl+dHBpfrFJlPs
2a7ia1YXXMe6ZVYWD0QW0Z2NcuZ16/7wkI7GH6qqglXkitP31g4tLlQZsVD9MU9k
VggQCcA2wdzyaEmLMBn+qE0Uu0yZ+uhIvFLkkbRVbb33WG9u/P5HC6Eio8m7YGb4
2qywJUiJkmdpOlcpfKdvjUpGa8TKF1UqjlsZWYVB65jmx3g4pCorwd2y3nOJktG3
Qodzm4/b3vaK+qLTmJWXqcQr2nvlj9vaibbrhX6RUhsHtlZJPrujiLv6g7m0wL+W
0rLhWL0J1+rmI+uUjzW62YmE3b6Yc7oAL7PEXLU+/R7vLSMA/Brfy5x/wMgwoMCW
MAq53aagDXKO2jWVtHiZ2Dj4AAcl5CmA4Gw5gtEezVnIQNTj/Lcui/p+li2q8N9O
RN0uG7xffmSqwbivJCwPUesxKBRDsQMRHk0R2L4VI5CQSrEq8d8vZRNUlf8EGisX
ODRF7zi9Uvu7WwjRCk1L
=9G6+
-END PGP SIGNATURE-

Re: [gentoo-portage-dev] [PATCH 3/3] pym/portage/util/locale.py: add a C module to help check locale

2016-05-30 Thread Alexander Berntsen 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512

On 27/05/16 16:26, Anthony G. Basile wrote:
> However, this gets bogged down in implementation details and fails 
> with musl.
Is it possible to elucidate this a bit more in a paragraph? Right now
it reads a bit like "but this fails because reasons -_o_-", but in the
future it might be nice to know said reasons.

Patch 1 and 2 look great. Patch 3 looks good to me, but obviously
Michał knows more about this so I'll let him be the judge.
- -- 
Alexander
berna...@gentoo.org
https://secure.plaimi.net/~alexander
-BEGIN PGP SIGNATURE-
Version: GnuPG v2

iQIcBAEBCgAGBQJXTALyAAoJENQqWdRUGk8BN6IP/0QqY6JdhzBGbNxdE3HCSDBt
kih75Hmjck6VQGlKNl+GLeML/ilTqitsVnkpbxeL3j00P69ZrW7OkZiLxGUQn+5P
9N0ljVR/FhHD5wJE3deS1IzNSERhuhSllIGKeweMIYXO3IT9tx4vDVNMB7v9zi+i
gduaxZr7byu3NSt//y+NJqHBUfxs2iWHA6+/KPDpKvs+E0hg2lycQVfsfCKFhQ/v
W+Jg5uw7Fo+6bTyUe5+b+12KlotFuHOko0unq9pt+PdT9GMek617kiMqDbXvazYf
/cgHiL1jIhSgRGbCmMVxAAo+BMp2AWrG4CyOdMHcS4shSw97jk/2NCI+1sUuuaNo
9go6ZCbLEx3LvL1kdCeR1lOk1kE6tC/lkiH1ZGMWuuYmF85qsCWgPHy1UZ+jXq1R
SkmwDXdJbbjVEzT8uVTm+hD8GyARXw7iP/1ZMLVgSRZfFGD48USIo/sVmyYdLOd5
xb90fupwpQiq49Rh+haeCN0uzSAPyAp4asrr9p409t8hH4i1sfDW/ed3jw+a/Pna
DyvPi/dY0ER9F1FnUxz4LYogm4Qa0IeFsQJUZZOlG8ekx5pBGxGqHOPH/Jx+iRLz
Pt9IvPKRInuU6LZ4gfkDvC5JfrX4DG8+PldsqItLP6GtPZSsp46f8Gdg2HUskmaD
8ESZIFkuxfL5wiKlK8Wi
=UDnS
-END PGP SIGNATURE-

Re: [gentoo-portage-dev] [PATCH 3/3] pym/portage/util/locale.py: add a C module to help check locale

2016-05-29 Thread Michał Górny 
On Sun, 29 May 2016 02:37:50 -0400
"Anthony G. Basile"  wrote:

> On 5/29/16 2:30 AM, Michał Górny wrote:
> > On Fri, 27 May 2016 10:26:44 -0400
> > "Anthony G. Basile"  wrote:
> >   
> >> From: "Anthony G. Basile" 
> >>
> >> The current method to check for a sane system locale is to use python's
> >> ctypes.util.find_library() to construct a full library path to the
> >> system libc.so and pass that path to ctypes.CDLL() so we can call
> >> toupper() and tolower() directly.  However, this gets bogged down in
> >> implementation details and fails with musl.
> >>
> >> We work around this design flaw in ctypes with a small python module
> >> written in C which provides thin wrappers to toupper() and tolower(),
> >> and only fall back on the current ctypes-based check when this module
> >> is not available.
> >>
> >> This has been tested on glibc, uClibc and musl systems.
> >>
> >> X-Gentoo-bug: 571444
> >> X-Gentoo-bug-url: https://bugs.gentoo.org/show_bug.cgi?id=571444
> >>
> >> Signed-off-by: Anthony G. Basile 
> >> ---
> >>  pym/portage/util/locale.py | 16 ++-
> >>  setup.py   |  6 +++-
> >>  src/portage_util_libc.c| 70 
> >> ++
> >>  3 files changed, 84 insertions(+), 8 deletions(-)
> >>  create mode 100644 src/portage_util_libc.c
> >>
> >> diff --git a/pym/portage/util/locale.py b/pym/portage/util/locale.py
> >> index 093eb86..5b09945 100644
> >> --- a/pym/portage/util/locale.py
> >> +++ b/pym/portage/util/locale.py
> >> @@ -34,13 +34,15 @@ def _check_locale(silent):
> >>"""
> >>The inner locale check function.
> >>"""
> >> -
> >> -  libc_fn = find_library("c")
> >> -  if libc_fn is None:
> >> -  return None
> >> -  libc = LoadLibrary(libc_fn)
> >> -  if libc is None:
> >> -  return None
> >> +  try:
> >> +  from portage.util import libc
> >> +  except ImportError:
> >> +  libc_fn = find_library("c")
> >> +  if libc_fn is None:
> >> +  return None
> >> +  libc = LoadLibrary(libc_fn)
> >> +  if libc is None:
> >> +  return None
> >>  
> >>lc = list(range(ord('a'), ord('z')+1))
> >>uc = list(range(ord('A'), ord('Z')+1))
> >> diff --git a/setup.py b/setup.py
> >> index 25429bc..5ca8156 100755
> >> --- a/setup.py
> >> +++ b/setup.py
> >> @@ -47,7 +47,11 @@ x_scripts = {
> >>  # Dictionary custom modules written in C/C++ here.  The structure is
> >>  #   key   = module name
> >>  #   value = list of C/C++ source code, path relative to top source 
> >> directory
> >> -x_c_helpers = {}
> >> +x_c_helpers = {
> >> +  'portage.util.libc' : [
> >> +  'src/portage_util_libc.c',
> >> +  ],
> >> +}
> >>  
> >>  class x_build(build):
> >>""" Build command with extra build_man call. """
> >> diff --git a/src/portage_util_libc.c b/src/portage_util_libc.c
> >> new file mode 100644
> >> index 000..00b09c2
> >> --- /dev/null
> >> +++ b/src/portage_util_libc.c
> >> @@ -0,0 +1,70 @@
> >> +/* Copyright 2005-2016 Gentoo Foundation
> >> + * Distributed under the terms of the GNU General Public License v2
> >> + */
> >> +
> >> +#include 
> >> +#include 
> >> +#include 
> >> +
> >> +static PyObject * _libc_tolower(PyObject *, PyObject *);
> >> +static PyObject * _libc_toupper(PyObject *, PyObject *);
> >> +
> >> +static PyMethodDef LibcMethods[] = {
> >> +  {"tolower", _libc_tolower, METH_VARARGS, "Convert to lower case using 
> >> system locale."},
> >> +  {"toupper", _libc_toupper, METH_VARARGS, "Convert to upper case using 
> >> system locale."},
> >> +  {NULL, NULL, 0, NULL}
> >> +};
> >> +
> >> +#if PY_MAJOR_VERSION >= 3
> >> +static struct PyModuleDef moduledef = {
> >> +  PyModuleDef_HEAD_INIT,
> >> +  "libc", /* 
> >> m_name */
> >> +  "Module for converting case using the system locale",   /* 
> >> m_doc */
> >> +  -1, /* 
> >> m_size */
> >> +  LibcMethods,/* 
> >> m_methods */
> >> +  NULL,   /* 
> >> m_reload */
> >> +  NULL,   /* 
> >> m_traverse */
> >> +  NULL,   /* 
> >> m_clear */
> >> +  NULL,   /* 
> >> m_free */
> >> +};
> >> +#endif
> >> +
> >> +PyMODINIT_FUNC  
> > 
> > Now you could call it nitpicking but since it decorates the function,
> > it would be better to have it inside the #ifdef. Otherwise, people
> > would think it's separate from that.  
> 
> You mean repeat it twice?

Yes. Otherwise it makes people think it outputs a separate block
independent of the functions.

> >> +
> >> +#if PY_MAJOR_VERSION >= 3
> >>

Re: [gentoo-portage-dev] [PATCH 3/3] pym/portage/util/locale.py: add a C module to help check locale

2016-05-29 Thread Anthony G. Basile 
On 5/29/16 2:30 AM, Michał Górny wrote:
> On Fri, 27 May 2016 10:26:44 -0400
> "Anthony G. Basile"  wrote:
> 
>> From: "Anthony G. Basile" 
>>
>> The current method to check for a sane system locale is to use python's
>> ctypes.util.find_library() to construct a full library path to the
>> system libc.so and pass that path to ctypes.CDLL() so we can call
>> toupper() and tolower() directly.  However, this gets bogged down in
>> implementation details and fails with musl.
>>
>> We work around this design flaw in ctypes with a small python module
>> written in C which provides thin wrappers to toupper() and tolower(),
>> and only fall back on the current ctypes-based check when this module
>> is not available.
>>
>> This has been tested on glibc, uClibc and musl systems.
>>
>> X-Gentoo-bug: 571444
>> X-Gentoo-bug-url: https://bugs.gentoo.org/show_bug.cgi?id=571444
>>
>> Signed-off-by: Anthony G. Basile 
>> ---
>>  pym/portage/util/locale.py | 16 ++-
>>  setup.py   |  6 +++-
>>  src/portage_util_libc.c| 70 
>> ++
>>  3 files changed, 84 insertions(+), 8 deletions(-)
>>  create mode 100644 src/portage_util_libc.c
>>
>> diff --git a/pym/portage/util/locale.py b/pym/portage/util/locale.py
>> index 093eb86..5b09945 100644
>> --- a/pym/portage/util/locale.py
>> +++ b/pym/portage/util/locale.py
>> @@ -34,13 +34,15 @@ def _check_locale(silent):
>>  """
>>  The inner locale check function.
>>  """
>> -
>> -libc_fn = find_library("c")
>> -if libc_fn is None:
>> -return None
>> -libc = LoadLibrary(libc_fn)
>> -if libc is None:
>> -return None
>> +try:
>> +from portage.util import libc
>> +except ImportError:
>> +libc_fn = find_library("c")
>> +if libc_fn is None:
>> +return None
>> +libc = LoadLibrary(libc_fn)
>> +if libc is None:
>> +return None
>>  
>>  lc = list(range(ord('a'), ord('z')+1))
>>  uc = list(range(ord('A'), ord('Z')+1))
>> diff --git a/setup.py b/setup.py
>> index 25429bc..5ca8156 100755
>> --- a/setup.py
>> +++ b/setup.py
>> @@ -47,7 +47,11 @@ x_scripts = {
>>  # Dictionary custom modules written in C/C++ here.  The structure is
>>  #   key   = module name
>>  #   value = list of C/C++ source code, path relative to top source directory
>> -x_c_helpers = {}
>> +x_c_helpers = {
>> +'portage.util.libc' : [
>> +'src/portage_util_libc.c',
>> +],
>> +}
>>  
>>  class x_build(build):
>>  """ Build command with extra build_man call. """
>> diff --git a/src/portage_util_libc.c b/src/portage_util_libc.c
>> new file mode 100644
>> index 000..00b09c2
>> --- /dev/null
>> +++ b/src/portage_util_libc.c
>> @@ -0,0 +1,70 @@
>> +/* Copyright 2005-2016 Gentoo Foundation
>> + * Distributed under the terms of the GNU General Public License v2
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +
>> +static PyObject * _libc_tolower(PyObject *, PyObject *);
>> +static PyObject * _libc_toupper(PyObject *, PyObject *);
>> +
>> +static PyMethodDef LibcMethods[] = {
>> +{"tolower", _libc_tolower, METH_VARARGS, "Convert to lower case using 
>> system locale."},
>> +{"toupper", _libc_toupper, METH_VARARGS, "Convert to upper case using 
>> system locale."},
>> +{NULL, NULL, 0, NULL}
>> +};
>> +
>> +#if PY_MAJOR_VERSION >= 3
>> +static struct PyModuleDef moduledef = {
>> +PyModuleDef_HEAD_INIT,
>> +"libc", /* 
>> m_name */
>> +"Module for converting case using the system locale",   /* 
>> m_doc */
>> +-1, /* 
>> m_size */
>> +LibcMethods,/* 
>> m_methods */
>> +NULL,   /* 
>> m_reload */
>> +NULL,   /* 
>> m_traverse */
>> +NULL,   /* 
>> m_clear */
>> +NULL,   /* 
>> m_free */
>> +};
>> +#endif
>> +
>> +PyMODINIT_FUNC
> 
> Now you could call it nitpicking but since it decorates the function,
> it would be better to have it inside the #ifdef. Otherwise, people
> would think it's separate from that.

You mean repeat it twice?

> 
>> +
>> +#if PY_MAJOR_VERSION >= 3
>> +PyInit_libc(void)
>> +{
>> +PyObject *m;
>> +m = PyModule_Create();
>> +return m;
>> +}
>> +#else
>> +initlibc(void)
>> +{
>> +Py_InitModule("libc", LibcMethods);
>> +}
>> +#endif
>> +
>> +
>> +static PyObject *
>> +_libc_tolower(PyObject *self, PyObject *args)
>> +{
>> +int c;
>> +
>> +if (!PyArg_ParseTuple(args, "i", ))
>> +return NULL;
>> +
>> +return