GNOME Bugzilla – Bug 615872
Port to Python 3
Last modified: 2012-10-24 10:55:55 UTC
Created attachment 158830 [details] [review] Initial attempt at porting I'm attaching a work-in-progress patch that gets pygi to compile against Python 3.1 (and,in theory, against 2.6 as well, with 2.5 being farily trivial to add back in, I hope). Compiles and links, but doesn't yet work properly.
J5 fixed up this work; a more up-to-date version of this is in git.gnome.org, in the "py3k" branch of pygi: http://git.gnome.org/browse/pygi/log/?h=py3k
I've done more work on the py3k branch on top of J5's work. I now have a version of pygi on that branch can be built against both Python 2.6 and Python 3.1, and all tests pass with both. This assumes a build of pygobject's py3k branch, with --enable-pygi. Web UI: http://fedorapeople.org/gitweb?p=dmalcolm/public_git/pygi.git;a=tree Cloning: git clone git://fedorapeople.org/home/fedora/dmalcolm/public_git/pygi.git There may be lingering issues with 2.5, but they ought to be trivial to fix if they occur.
Does this depend on an updated version of pygobject? If so can we push them to have a release so pygi can depend on a specific version # instead of HEAD?
Created attachment 167658 [details] [review] move to using richcompare slot instead of compare
Created attachment 167659 [details] [review] prefix compat macros with PYGLIB * refactor from John Ehresman <jpe@wingware.com> py3k branch * fix up some extranious PyString calls * remove duplicate macros from pyglib.h that are in pyglib-python-compat.h * pygobject.h can't import pyglib-python-compat.h so add codepaths for both Py3k and legacy code instead of using macros
Created attachment 167660 [details] [review] use Bytes instead of Unicode when reading io
Created attachment 167661 [details] [review] for py3k we need to do some more processing to get bytes from a unicode string
Created attachment 167662 [details] [review] make giomodule comile under py3k
The above patches do not fix all issues but they do move us in the right direction while still keeping 2.x compiles working with make check. Can I get the ok to push this so it doesn't bit rot like jsh's patches did? It touches so many files that the longer it takes to get approval the harder it is going to be to merge.
Created attachment 167666 [details] [review] prefix compat macros with PYGLIB * refactor from John Ehresman <jpe@wingware.com> py3k branch * fix up some extranious PyString calls * remove duplicate macros from pyglib.h that are in pyglib-python-compat.h * pygobject.h can't import pyglib-python-compat.h so add codepaths for both Py3k and legacy code instead of using macros
You probably want the macros installed somewhere so they can be used by libs that depend on pygobject. Thanks for merging this!
(In reply to comment #9) > The above patches do not fix all issues but they do move us in the right > direction while still keeping 2.x compiles working with make check. Can I get > the ok to push this so it doesn't bit rot like jsh's patches did? It touches > so many files that the longer it takes to get approval the harder it is going > to be to merge. Which is the code being proposed for merging?
All of the patches above that aren't obsolete needs to be merged. There is an order to them - 1. move to using richcompare slot instead of compare 2. prefix compat macros with PYGLIB 3. use Bytes instead of Unicode when reading io 4. for py3k we need to do some more processing to get bytes from a unicode string 5. make giomodule compile under py3k
(In reply to comment #13) > All of the patches above that aren't obsolete needs to be merged. There is an > order to them - > > 1. move to using richcompare slot instead of compare > 2. prefix compat macros with PYGLIB > 3. use Bytes instead of Unicode when reading io > 4. for py3k we need to do some more processing to get bytes from a unicode > string > 5. make giomodule compile under py3k So only these patches, nothing else from the py3k branch?
That is it for now. I'm doing it in stages so the code doesn't bitrot before it gets merged
The py3k branch is not mergable and is being kept for reference as I refactor it on a local branch.
Review of attachment 167658 [details] [review]: Apart from the few style issues, it looks OK to me. ::: glib/pygiochannel.c @@ +739,3 @@ PyGIOChannel_Type.tp_methods = py_io_channel_methods; PyGIOChannel_Type.tp_hash = (hashfunc)py_io_channel_hash; + PyGIOChannel_Type.tp_richcompare = py_io_channel_richcompare; Please cast to richcmpfunc. ::: glib/pyglib.c @@ +616,3 @@ + Py_INCREF(res); + break; + } Maybe Py_INCREF(res) here instead of in each case. @@ +663,3 @@ + Py_INCREF(res); + break; + } Same remark. ::: gobject/pygobject.c @@ +813,3 @@ offsetof(PyTypeObject, tp_compare), +#endif + offsetof(PyTypeObject, tp_richcompare), Seems like an indentation problem. ::: gobject/pygtype.c @@ +1395,3 @@ + PyErr_Clear(); // Is there anything else to do? + } + else if (res) { Indentation issue.
Review of attachment 167660 [details] [review]: ::: glib/pygiochannel.c @@ +240,3 @@ + PyObject *unicode_obj; + + unicode_obj = PyUnicode_FromString(PyBytes_AS_STRING(ret_obj)); Don't you need assert that the channel's encoding is UTF8, or to convert ret_obj to UTF8 if it is not? Otherwise this may make this call fail.
(In reply to comment #18) > Review of attachment 167660 [details] [review]: > > ::: glib/pygiochannel.c > @@ +240,3 @@ > + PyObject *unicode_obj; > + > + unicode_obj = PyUnicode_FromString(PyBytes_AS_STRING(ret_obj)); > > Don't you need assert that the channel's encoding is UTF8, or to convert > ret_obj to UTF8 if it is not? Otherwise this may make this call fail. Above that code is some code which checks encoding: + if (g_io_channel_get_encoding(self->channel) == NULL) + return ret_obj; That should be != NULL as NULL means UTF8 encoding. So if UTF8 return a unicode string, else return the raw bytes.
Review of attachment 167661 [details] [review]: ::: gi/pygi-argument.c @@ +737,2 @@ /* Don't need to check for errors, since g_strdup is NULL-proof. */ arg.v_string = g_strdup (string); Looks like this assignment is missing in the other case. @@ +739,3 @@ +#else + { + PyObject *utf8_bytes_obj = PyUnicode_AsUTF8String (object); Call this variable py_bytes, or py_string. @@ +762,3 @@ + break; + + string = PyBytes_AsString (utf8_bytes_obj); Same remarks as above.
Review of attachment 167662 [details] [review]: ::: gio/giomodule.c @@ +57,3 @@ + pygobject_mod = pygobject_init(2, 15, 2); + if (pygobject_mod == NULL) + return -1; Don't use a variable only for this.
Review of attachment 167666 [details] [review]: ::: gobject/pygobject.h @@ +336,3 @@ + PyErr_SetString(PyExc_ImportError, + "could not import gobject"); + } Why don't you use PyErr_Format? If %U is not available, I'd prefer if you could get a byte object from py_orig_exc rather than overriding all the error message stuff.
(In reply to comment #22) > Review of attachment 167666 [details] [review]: > > ::: gobject/pygobject.h > @@ +336,3 @@ > + PyErr_SetString(PyExc_ImportError, > + "could not import gobject"); > + } > > Why don't you use PyErr_Format? If %U is not available, I'd prefer if you could > get a byte object from py_orig_exc rather than overriding all the error message > stuff. %U is not available and in the process of converting we can get an OOM error. This is a fallback in that case, or we could just fall through since the error should be set and there is no guarantee that setstring won't run into OOM also.
Comment on attachment 167658 [details] [review] move to using richcompare slot instead of compare committed with fixes suggestions
Created attachment 167825 [details] [review] for py3k we need to do some more processing to get bytes from a unicode string
Created attachment 167826 [details] [review] use Bytes instead of Unicode when reading io
Review of attachment 167826 [details] [review]: Fixed per previous review. Can't commit yet because it relies on the "prefix compat macros with PYGLIB" patch
Created attachment 167827 [details] [review] make giomodule compile under py3k
Created attachment 167828 [details] [review] prefix compat macros with PYGLIB * refactor from John Ehresman <jpe@wingware.com> py3k branch * fix up some extranious PyString calls * remove duplicate macros from pyglib.h that are in pyglib-python-compat.h * pygobject.h can't import pyglib-python-compat.h so add codepaths for both Py3k and legacy code instead of using macros
Review of attachment 167825 [details] [review]: OK. Thanks.
Review of attachment 167826 [details] [review]: Thanks!
Review of attachment 167827 [details] [review]: Good. Thanks!
Review of attachment 167828 [details] [review]: Thanks!
Keeping bug open. This was a good start but we aren't done yet
Created attachment 167974 [details] [review] fix deprecated API in the codegen module and replace with 2.x/3.x compat subset * print >> out replaced with out.write (add \n where appropriate) * replace the use of the string module with str methods * try/except blocks now use the sys.exc_info to get the exception values * replace has_key(key) methods with "key in dict" grammer
Comment on attachment 167974 [details] [review] fix deprecated API in the codegen module and replace with 2.x/3.x compat subset Hmm, the generated code output isn't correct in 2.x. A buffer not being flushed I'm guessing.
Created attachment 167975 [details] [review] fix deprecated API in the codegen module and replace with 2.x/3.x compat subset * print >> out replaced with out.write (add \n where appropriate) * replace the use of the string module with str methods * try/except blocks now use the sys.exc_info to get the exception values * replace has_key(key) methods with "key in dict" grammer
Comment on attachment 167974 [details] [review] fix deprecated API in the codegen module and replace with 2.x/3.x compat subset Fixed a small typo
Created attachment 167986 [details] [review] some more p3k PyString eradication in GI * add the glib dir to the includes list in the build * make sure we include the compat macros
Created attachment 168020 [details] [review] some more p3k PyString and PyInt eradication in GI * add the glib dir to the includes list in the build * make sure we include the compat macros * add GLIB_PyBytes_FromString to compat macros * add GLIB_PyNumber_Long to compat macros * use RichCompare instead of Compare
Created attachment 168021 [details] [review] convert to using PYGLIB_DEFINE_TYPE for module objects
Created attachment 168027 [details] [review] make the gi module compile under 3.x * include the compat macros * use GLIB_MODULE_START/END to define module * add PyInit__gi to the exported symbols
Created attachment 168029 [details] [review] fix up testshelper module so it compiles in python 3.x * include the compat header * fix up PyInts to be PYGLIB_Long * Use PYGLIB_DEFINE_TYPE macros to define module objects * Use PYGLIB_MODULE_START/END to define modules
Created attachment 168030 [details] [review] fix exceptions so they work in python 3.x
Created attachment 168089 [details] [review] fix up testshelper module so it compiles in python 3.x * include the compat header * fix up PyInts to be PYGLIB_Long * Use PYGLIB_DEFINE_TYPE macros to define module objects * Use PYGLIB_MODULE_START/END to define modules
Created attachment 168100 [details] [review] make the gi module compile under 3.x * include the compat macros * use GLIB_MODULE_START/END to define module * add PyInit__gi to the exported symbols
Created attachment 168107 [details] [review] some more p3k PyString and PyInt eradication in GI * add the glib dir to the includes list in the build * make sure we include the compat macros * add GLIB_PyBytes_FromString to compat macros * add GLIB_PyNumber_Long to compat macros * use RichCompare instead of Compare
Created attachment 168129 [details] [review] make cairo module compile in py3k
Created attachment 168412 [details] [review] make vfuncs work in py3k * methods now export __func__ instead of im_func for getting the function out of a method closure * however classes no longer return unbound methods in py3k and instead return the actual function * in python 2 we use im_func when getting the function from the vfunc closure * in py3k we simply assign vfunc to the function
Created attachment 168414 [details] [review] fix up tests so they run in py3k * add a compat helper that should only be used by tests * fix long notation to use the compat helper instead * add parens to print statements * use compatable try/except pattern
Created attachment 168415 [details] [review] working enum/flags/pid subclasses of long
Created attachment 168416 [details] [review] we need to specify tp_hash since we overide tp_richcompare
order of patches from first to last: pick c28d806 fix deprecated API in the codegen module and replace with 2.x/3.x compat subset pick 6b3be7d some more p3k PyString and PyInt eradication in GI pick 9a5d03c convert to using PYGLIB_DEFINE_TYPE for module objects pick a44b886 fix up testshelper module so it compiles in python 3.x pick aeb6008 make the gi module compile under 3.x pick 84aab37 fix exceptions so they work in python 3.x pick 0a4b8d4 make cairo module compile in py3k pick d204a93 make vfuncs work in py3k pick 482c6fb fix up tests so they run in py3k pick 0655a5f working enum/flags/pid subclasses of long pick 717913b we need to specify tp_hash since we overide tp_richcompare
Comment on attachment 168107 [details] [review] some more p3k PyString and PyInt eradication in GI Attachment 168107 [details] pushed as 1efa2b1 - some more p3k PyString and PyInt eradication in GI
Comment on attachment 168021 [details] [review] convert to using PYGLIB_DEFINE_TYPE for module objects Attachment 168021 [details] pushed as 1ff83a2 - convert to using PYGLIB_DEFINE_TYPE for module objects
Comment on attachment 168089 [details] [review] fix up testshelper module so it compiles in python 3.x Attachment 168089 [details] pushed as 1dee5dc - fix up testshelper module so it compiles in python 3.x
Comment on attachment 168100 [details] [review] make the gi module compile under 3.x Attachment 168100 [details] pushed as 427a3c8 - make the gi module compile under 3.x
Comment on attachment 168030 [details] [review] fix exceptions so they work in python 3.x Attachment 168030 [details] pushed as bda58ec - fix exceptions so they work in python 3.x
Comment on attachment 168129 [details] [review] make cairo module compile in py3k Attachment 168129 [details] pushed as 286dcd0 - make cairo module compile in py3k
Comment on attachment 168412 [details] [review] make vfuncs work in py3k Attachment 168412 [details] pushed as 0db676f - make vfuncs work in py3k
Review of attachment 168414 [details] [review]: ::: gobject/propertyhelper.py @@ +39,3 @@ +if sys.version_info >= (3, 0): + basestring = str + long = int This makes me really nervous, I would instead add some functions that use one or the other types depending on the version.
Comment on attachment 168415 [details] [review] working enum/flags/pid subclasses of long Attachment 168415 [details] pushed as c03e6b4 - working enum/flags/pid subclasses of long
Comment on attachment 168416 [details] [review] we need to specify tp_hash since we overide tp_richcompare Attachment 168416 [details] pushed as f6c4d9e - we need to specify tp_hash since we overide tp_richcompare
Created attachment 168823 [details] [review] fix up tests so they run in py3k * add a compat helper that should only be used by tests * fix long notation to use the compat helper instead * add parens to print statements * use compatable try/except pattern
Comment on attachment 168823 [details] [review] fix up tests so they run in py3k Attachment 168823 [details] pushed as 29e6cb8 - fix up tests so they run in py3k
Created attachment 169442 [details] [review] remove reliance on _PyUnicode_AsString * _PyUnicode_AsString dumps the internal buffer of a PyUnicode object which is not garunteed to hold any values (it is a cache) * there is a reason why the method is prefixed with an underscore which marks it as internal * since unicode objects need to be converted to byte strings in order to get their underlying byte arrays we need to handle errors that might happen during the conversion process * because of the conversions process there is no easy way to map Python 2.x string method "AsString" to Python 3.x unicode methods so we must handle it with ifdefs https://bugzilla.gnome.org/show_bug.cgi?id=627878
Created attachment 169443 [details] [review] use PyObject_SetAttrString, not PyDict_SetItemString when setting __gtype__ * When registering a gtype wrapper we used to set tp_dict directly. This works in python 2 but python 3 seems to handle attributes in a slightly different way where the tp_dict and attr get out of sync. By setting the attr directly we avoid this issue. * Note that there are many more places where we set __gtype__ using tp_dict however for objects which are not instantiated yet we have to set tp_dict directly. * Since this one change fixes a lot of failed tests, for now we ignore the other places where we set __gtype__. If we run into more issues dealing with __gtype__ we can take a closer look later. https://bugzilla.gnome.org/show_bug.cgi?id=627878
(In reply to comment #66) > Created an attachment (id=169442) [details] [review] > remove reliance on _PyUnicode_AsString > > * _PyUnicode_AsString dumps the internal buffer of a PyUnicode object > which is not garunteed to hold any values (it is a cache) > * there is a reason why the method is prefixed with an underscore > which marks it as internal > * since unicode objects need to be converted to byte strings in order > to get their underlying byte arrays we need to handle errors that > might happen during the conversion process > * because of the conversions process there is no easy way to map Python 2.x > string method "AsString" to Python 3.x unicode methods so we must handle > it with ifdefs > > https://bugzilla.gnome.org/show_bug.cgi?id=627878 When does _PyUnicode_AsString not return a pointer to a utf8 encoded string? I quickly glanced at the bug referenced and wonder if it's a unicode v bytes confusion. You could use ParseArgs with a 's' format character to get at the utf8 representation of a unicode instance -- it ends up calling down to _PyUnicode_AsString though.
Since Unicode requires much more processing and error checking _PyUnicode_AsString is not a direct match to _PyStiring_AsString and it would be wrong to to use. Part of the issue is we need to clean up after ourselves and we need to strdup because the intermediate byte object needs to be dereffed. Furthermore in the current SVN HEAD it has either been taken out or moved somewhere else because I can't find it. I think Dave had mentioned it was being removed. In any case I could remove the strdups in a couple of places if you are worried about performance. It just necessitates another ifdef to decrement the bytes object where I currently free the string. I thought the current pattern simplified the needed code while adding a couple of extra copys and frees. The other thing I can do is just create a NULL PyObject * that gets XDECREF'ed even though it is never used in Py2. That might be a nicer compromise.
(In reply to comment #69) > Since Unicode requires much more processing and error checking > _PyUnicode_AsString is not a direct match to _PyStiring_AsString and it would > be wrong to to use. Part of the issue is we need to clean up after ourselves > and we need to strdup because the intermediate byte object needs to be > dereffed. See _PyUnicode_AsString in http://svn.python.org/view/python/branches/py3k/Objects/unicodeobject.c?revision=84455&view=markup It creates the utf8 encoded bytes object and stores it in a field of the unicode object so it will live as long as the unicode object. This simplifies the memory management / bookkeeping you'd need to do otherwise. I'm dubious that this functionality will ever be removed as ParseArgs depends on it and is used in lots of places. This functionality has been used with Py2 whenever unicode instances are passed in.
Ok, so trunk isn't the main branch as I had thought so it isn't removed. I'm not sure why they don't make it public API as I hate relying on internals. In any case I'm happy to leave things as is if Dave signs off on it.
Except we need to check for errors since there is a memory allocation involved
Comment on attachment 169443 [details] [review] use PyObject_SetAttrString, not PyDict_SetItemString when setting __gtype__ I'd change all places at once, if the tests fails with these changes the problem is probably in another place.
Comment on attachment 169442 [details] [review] remove reliance on _PyUnicode_AsString We can rely on _PyUnicode_AsString but checks have to be made after calling it to make sure an error has not occurred. This is because the first call to _PyUnicode_AsString allocates a PyBytes cache. This operation may fail due to a number of reasons.
Created attachment 169873 [details] [review] minor fixes in tests for py3k compat * add a _bytes wrapper for API that expects bytes in py3k but str in py2 * fix some more exception handling using sys.exc_info()[:2] * use range instead of xrange, items instead of iteritems since py3k dropped support for the different ways of accessing iterators - this is less efficient in py2 but we plan to target py3k as the primary platform * use list(dict.items()) since py3k only returns iterables which are not indexable * missed some _long wrapping
Created attachment 169874 [details] [review] minor py3k fixups for python modules * add _basestring and _bytes and _callable wrappers * use items instead of iteritems and range instead of xrange fix py3k modules
Created attachment 169875 [details] [review] fix subclassing PyLong by calling __new__ correctly
Created attachment 169876 [details] [review] s/METH_KEYWORDS/METH_VARARGS|METH_KEYWORDS/ when defining object methods * in Py3k the METH_KEYWORDS flag by itself is invalid. A method must be defined with both the METH_VARARGS and METH_KEYWORDS flags.
With the above patches all but one tests pass that pass in Python 2. The test in question has to do with a long property type which is harder to determine due to Py3k not distinguishing between int and long. GI tests break on UTF8 and long tests. It also is giving back wrong structure values when passing back structures. This might also have to do with the long/int issues (though it seems to be setting things correctly in C).
Created attachment 169880 [details] [review] use actual unicode in the tests on py3k, not the byte representation
UTF8 issues were with the tests themselves, not in the codebase. Fixed. Now we just have long/int issues and a couple of more obscure failures.
Created attachment 169899 [details] [review] properly handle ulongs properties in py3k * If this is a PyLong object pull use AsUnsignedLong
Created attachment 169907 [details] [review] fix handling of UINT64 and INT64 arguments in py3k * decode to the right sized C long
Almost there - these 4 tests are a bit harder to track down and there are a few more that I commented out because they were asserting and not giving back tracebacks: ====================================================================== ERROR: test_ghashtable_int_none_in (test_gi.TestGHashTable) ---------------------------------------------------------------------- Traceback (most recent call last):
+ Trace 223619
self.assertRaises(TypeError, GIMarshallingTests.ghashtable_int_none_in, '{-1: 1, 0: 0, 1: -1, 2: -2}')
callableObj(*args, **kwargs)
return info.invoke(*args)
====================================================================== ERROR: test_gvariant (test_everything.TestEverything) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/johnp/devel/gnome-shell/source/pygobject-3/tests/test_everything.py", line 53, in test_gvariant variant = GLib.Variant.new_int32(42); AttributeError: type object 'Variant' has no attribute 'new_int32' ====================================================================== FAIL: test_object_method (test_gi.TestGObject) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/johnp/devel/gnome-shell/source/pygobject-3/tests/test_gi.py", line 1241, in test_object_method self.assertRaises(TypeError, GIMarshallingTests.Object.method, GObject.GObject()) AssertionError: TypeError not raised by method ====================================================================== FAIL: test_sub_object_overwritten_method (test_gi.TestGObject) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/johnp/devel/gnome-shell/source/pygobject-3/tests/test_gi.py", line 1269, in test_sub_object_overwritten_method self.assertRaises(TypeError, GIMarshallingTests.SubObject.overwritten_method, GIMarshallingTests.Object()) AssertionError: TypeError not raised by overwritten_method ---------------------------------------------------------------------- Ran 341 tests in 0.859s FAILED (failures=2, errors=2)
Created attachment 169918 [details] [review] Use PyMapping_Keys to determine if an object is a dict (py3k fix) * in Py3k PyMapping_Check returns true for sequences such as strings and lists. Since we need to get the keys anyway, and it returns NULL if this is not a dict, this is a much better test, even in Py2
Traceback (most recent call last):
+ Trace 223620
suite = loader.loadTestsFromNames(names)
suites = [self.loadTestsFromName(name, module) for name in names]
module = __import__('.'.join(parts_copy))
GIMarshallingTests.SubObject.overwritten_method(obj)
This is the exception we should be getting for two of the tests. Instead they pass right through. I'm not sure where this exception is being thrown since no part of the text can be found and invoke is never reached. One thing stands out here: * 'unbound method' - there is no such thing in Py3k anymore. Instead it is simply a function. Perhaps since it is no longer a method the self check is no longer there and we need to check for ourselves.
More ideas. It seems that py3k just isn't binding the object to the method and just calling it as a function without self. I suspect we need to reorganise how we define methods on an object in gi/types.py. Perhaps we should override __getattr__ and when a gi function is requested we wrap it so it does the right thing.
Created attachment 170275 [details] [review] Check the type of the instance object * in python 2 methods were added to classes as unbound methods and they would check the instance type to make sure it was correct * in python 3 for perfomance reasons methods are added to classes as simple functions which treat the instance as an untyped argument so no checks are made. * this patch adds a type check so that the correct errors are thrown in python 3 (python 2 this just adds another layer of redundancy should something change with type checking in the future) * since GI handles regular args and the instance arg slightly differently we had to split out the interface checks in _pygi_g_type_info_check_object in order to not duplicate code
Done, turns out I was on crack. The only issue was not type checking the instance object which would cause a whole host of other side effects when code was run that shouldn't have been on invalid objects. Anyway, all tests pass except for the GVariant one which is not a py3k issue but just a unfinished GVariant implementation issue. Onward and upwards!!!
*** Bug 566641 has been marked as a duplicate of this bug. ***
Created attachment 170342 [details] [review] include the correct pycairo version
Comment on attachment 169873 [details] [review] minor fixes in tests for py3k compat Attachment 169873 [details] pushed as dec9001 - minor fixes in tests for py3k compat
Comment on attachment 169874 [details] [review] minor py3k fixups for python modules Attachment 169874 [details] pushed as a499b2f - minor py3k fixups for python modules
Comment on attachment 169875 [details] [review] fix subclassing PyLong by calling __new__ correctly Attachment 169875 [details] pushed as b5ee20a - fix subclassing PyLong by calling __new__ correctly
Comment on attachment 169880 [details] [review] use actual unicode in the tests on py3k, not the byte representation Attachment 169880 [details] pushed as a808bda - use actual unicode in the tests on py3k, not the byte representation
Comment on attachment 169899 [details] [review] properly handle ulongs properties in py3k Attachment 169899 [details] pushed as d5666d9 - properly handle ulongs properties in py3k
Comment on attachment 169907 [details] [review] fix handling of UINT64 and INT64 arguments in py3k Attachment 169907 [details] pushed as 0e72e28 - fix handling of UINT64 and INT64 arguments in py3k
Attachment 169918 [details] pushed as b855562 - Use PyMapping_Keys to determine if an object is a dict (py3k fix) Attachment 170342 [details] pushed as 5d79498 - include the correct pycairo version
Comment on attachment 170275 [details] [review] Check the type of the instance object Attachment 170275 [details] pushed as 98f54f9 - Check the type of the instance object
Review of attachment 167975 [details] [review]: So far we are not aiming to support the code generator in Python 3 so I don't think we should make these changes.
Created attachment 170503 [details] [review] add --enable-python3 switch * compile for python 3 * disables gio * runs only pertinant tests
Review of attachment 170503 [details] [review]: The traditional way of specifying the python version to configure is to use an environment variable, eg: PYTHON=/usr/bin/python3 ./configure ... It's a quite useful feature which will break with the patch above. I think that the configure logic should be smarter; 1) detect the python version used, either via PYTHON or /usr/bin/python 2) document how different python versions are going to be used Or, a more radical approach; just drop 2.x support completely and require python 3. 2.x users can continue using pygtk and the code generator 3.x users needs python 3 and use only the introspection bindings
For the second option doesn't that break PyGTK since it sill relies on pygobject in 2.x? As for the first option, I can produce a patch that does that. It isn't as discoverable but if that is the common way to do things then I'm game.
Created attachment 170682 [details] [review] add checks so we can compile under python 3 by setting PYTHON=python3 * compile for python 3 * disables gio if compiling under python 3.x * runs only pertinant tests
I now check the python version of $PYTHON and disable GIO using the BUILD_GIO and BUILD_GIOUNIX conditionals which already existed. To compile under python 3 you now need to do PYTHON=python3 ./autogen.sh
Review of attachment 170682 [details] [review]: Looks great! ::: Makefile.am @@ +2,3 @@ AUTOMAKE_OPTIONS = 1.7 +SUBDIRS = docs codegen glib gobject examples gio why is this needed?
We have released as 2.26.0 so I feel I can close this now as Python 3 is now supported. Since we still might have bugs please file all issues under new bugs. Perhaps we should add a python3 component.
(In reply to comment #19) > (In reply to comment #18) > > Review of attachment 167660 [details] [review] [details]: > > > > ::: glib/pygiochannel.c > > @@ +240,3 @@ > > + PyObject *unicode_obj; > > + > > + unicode_obj = PyUnicode_FromString(PyBytes_AS_STRING(ret_obj)); > > > > Don't you need assert that the channel's encoding is UTF8, or to convert > > ret_obj to UTF8 if it is not? Otherwise this may make this call fail. > > Above that code is some code which checks encoding: > > + if (g_io_channel_get_encoding(self->channel) == NULL) > + return ret_obj; > > > That should be != NULL as NULL means UTF8 encoding. So if UTF8 return a > unicode string, else return the raw bytes. Actually not. NULL means that the stream is using binary data, so for == NULL it should always return binary data. So the original == NULL was right. Also, it seems a bit odd that for the "UTF-8" case it would automagically convert to string, while it returns bytes for anything else. For this inconsistency, and as this has been broken all the time, I propose to just drop the special case, and always return bytes, period. I am currently writing tests for GLib.IOChannel (we didn't have any), and stumbled over this (as well as finding some other bugs). I guess it's mostly moot, since few projects seem to actually use GLib.IOChannel, but I want to drop the static IOChannel bindings from pygobject for good and replace them with GI bindings (and need the tests to ensure full backwards compatibility).
For the record, this is covered by the new tests (http://git.gnome.org/browse/pygobject/commit/?id=a98c37937a4f7cb) and fixed in http://git.gnome.org/browse/pygobject/commit/?id=0e1a6cce