Re: [PATCH for-next RFC 4/4] pygrub: make it build with python 3

["Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3294 bytes --]

On Tue, Mar 05, 2019 at 05:48:10PM +0000, Wei Liu wrote:
> On Tue, Mar 05, 2019 at 05:42:07PM +0000, Andrew Cooper wrote:
> > On 05/03/2019 16:42, Wei Liu wrote:
> > > With the help of two porting guides and cpython source code:
> > >
> > > 1. Use PyUnicode to replace PyString counterparts.
> > > 2. Use PyVarObject_HEAD_INIT and provide compatibility for 2.5 and
> > >    earlier.
> > > 3. Remove usage of Py_FindMethod.
> > > 4. Use new module initialisation routine.
> > >
> > > For #3, Py_FindMethod was removed, yet an alternative wasn't
> > > documented.  The code is the result of reverse-engineering cpython
> > > commit 6116d4a1d1
> > >
> > > https://docs.python.org/3/howto/cporting.html
> > > http://python3porting.com/cextensions.html
> > >
> > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > 
> > Marek already made the tools/python/* libraries compatible with Py2 and Py3
> > 
> > The following commits are the relevant ones:
> > 
> > * be6b316 - python: handle long type in scripts (2 years ago) <Marek Marczykowski-Górecki>
> > * e16c705 - python: adjust module initalization for Python3 (2 years ago) <Marek Marczykowski-Górecki>
> > * dd986cd - python: use PyLong_* for constructing 'int' type in Python3 (2 years ago) <Marek Marczykowski-Górecki>
> > * 121d9d4 - python: use PyBytes/PyUnicode instead of PyString (2 years ago) <Marek Marczykowski-Górecki>
> > * 0c8981f - python: initialize specific fields of PyTypeObject (2 years ago) <Marek Marczykowski-Górecki>
> > * 7b1e5f7 - python: use Py_TYPE instead of looking directly into PyObject_HEAD (2 years ago) <Marek Marczykowski-Górecki>
> > * 96d1ee6 - python: drop tp_getattr implementation (2 years ago) <Marek Marczykowski-Górecki>
> > * 6b28df3 - python: check return value of PyErr_NewException (2 years ago) <Marek Marczykowski-Górecki>
> 
> I knew.
> 
> > 
> > Which in particular handle strings differently in the Py2 case.
> 
> 
> I am not sure his changes for the string APIs are correct -- they seem
> to deviate from the official porting guide. But hey, I don't use these
> bindings myself, so he probably knows better.

That was intentional, because in py2 str type is the same as bytes
types, and in fact some of those str should really be bytes. It's in the
commit message:

    python: use PyBytes/PyUnicode instead of PyString
    
    In Python2 PyBytes is the same as PyString, but in Python3 PyString is
    gone and 'str' is really PyUnicode in C-API.
    When handling arbitrary data, use PyBytes - which is the right thing to
    do in Python3, and pose no API change in Python2. When handling
    xenstore paths and transaction ids, which have well defined format, use
    PyUnicode - to ease API usage - no need to prefix all xenstore paths
    with 'b' when migrating scripts to Python3.

I'm not sure if the same reasoning applies to pygrub, but I guess it
may. For example fsimage_file_read sounds like handling binary data, not
really UTF-8 strings. Using PyUnicode for arbitrary binary data may lead
to various UnicodeDecodeErrors.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel