After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 615872 - Port to Python 3
Port to Python 3
Status: RESOLVED FIXED
Product: pygobject
Classification: Bindings
Component: introspection
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: Nobody's working on this now (help wanted and appreciated)
Python bindings maintainers
: 566641 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2010-04-15 18:25 UTC by Dave Malcolm
Modified: 2012-10-24 10:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Initial attempt at porting (26.92 KB, patch)
2010-04-15 18:25 UTC, Dave Malcolm
none Details | Review
move to using richcompare slot instead of compare (18.91 KB, patch)
2010-08-11 20:16 UTC, johnp
committed Details | Review
prefix compat macros with PYGLIB (109.00 KB, patch)
2010-08-11 20:17 UTC, johnp
none Details | Review
use Bytes instead of Unicode when reading io (2.18 KB, patch)
2010-08-11 20:17 UTC, johnp
reviewed Details | Review
for py3k we need to do some more processing to get bytes from a unicode string (1.83 KB, patch)
2010-08-11 20:17 UTC, johnp
needs-work Details | Review
make giomodule comile under py3k (14.09 KB, patch)
2010-08-11 20:17 UTC, johnp
reviewed Details | Review
prefix compat macros with PYGLIB (108.67 KB, patch)
2010-08-11 20:35 UTC, johnp
reviewed Details | Review
for py3k we need to do some more processing to get bytes from a unicode string (2.26 KB, patch)
2010-08-13 16:42 UTC, johnp
committed Details | Review
use Bytes instead of Unicode when reading io (2.24 KB, patch)
2010-08-13 17:00 UTC, johnp
committed Details | Review
make giomodule compile under py3k (14.03 KB, patch)
2010-08-13 17:19 UTC, johnp
committed Details | Review
prefix compat macros with PYGLIB (108.94 KB, patch)
2010-08-13 17:20 UTC, johnp
committed Details | Review
fix deprecated API in the codegen module and replace with 2.x/3.x compat subset (31.16 KB, patch)
2010-08-16 16:27 UTC, johnp
needs-work Details | Review
fix deprecated API in the codegen module and replace with 2.x/3.x compat subset (31.16 KB, patch)
2010-08-16 16:42 UTC, johnp
rejected Details | Review
some more p3k PyString eradication in GI (6.00 KB, patch)
2010-08-16 17:53 UTC, johnp
none Details | Review
some more p3k PyString and PyInt eradication in GI (17.32 KB, patch)
2010-08-17 05:33 UTC, johnp
none Details | Review
convert to using PYGLIB_DEFINE_TYPE for module objects (22.53 KB, patch)
2010-08-17 05:36 UTC, johnp
committed Details | Review
make the gi module compile under 3.x (2.44 KB, patch)
2010-08-17 06:27 UTC, johnp
none Details | Review
fix up testshelper module so it compiles in python 3.x (13.66 KB, patch)
2010-08-17 06:28 UTC, johnp
none Details | Review
fix exceptions so they work in python 3.x (1.56 KB, patch)
2010-08-17 06:41 UTC, johnp
committed Details | Review
fix up testshelper module so it compiles in python 3.x (13.69 KB, patch)
2010-08-17 14:48 UTC, johnp
committed Details | Review
make the gi module compile under 3.x (2.45 KB, patch)
2010-08-17 15:34 UTC, johnp
committed Details | Review
some more p3k PyString and PyInt eradication in GI (17.32 KB, patch)
2010-08-17 16:06 UTC, johnp
committed Details | Review
make cairo module compile in py3k (1.81 KB, patch)
2010-08-17 19:45 UTC, johnp
committed Details | Review
make vfuncs work in py3k (1.77 KB, patch)
2010-08-20 15:22 UTC, johnp
committed Details | Review
fix up tests so they run in py3k (6.69 KB, patch)
2010-08-20 15:24 UTC, johnp
needs-work Details | Review
working enum/flags/pid subclasses of long (9.42 KB, patch)
2010-08-20 15:24 UTC, johnp
committed Details | Review
we need to specify tp_hash since we overide tp_richcompare (1.29 KB, patch)
2010-08-20 15:25 UTC, johnp
committed Details | Review
fix up tests so they run in py3k (8.38 KB, patch)
2010-08-26 17:35 UTC, johnp
committed Details | Review
remove reliance on _PyUnicode_AsString (51.69 KB, patch)
2010-09-03 16:31 UTC, johnp
none Details | Review
use PyObject_SetAttrString, not PyDict_SetItemString when setting __gtype__ (1.60 KB, patch)
2010-09-03 16:31 UTC, johnp
committed Details | Review
minor fixes in tests for py3k compat (5.82 KB, patch)
2010-09-09 16:52 UTC, johnp
committed Details | Review
minor py3k fixups for python modules (5.93 KB, patch)
2010-09-09 16:52 UTC, johnp
committed Details | Review
fix subclassing PyLong by calling __new__ correctly (916 bytes, patch)
2010-09-09 16:52 UTC, johnp
committed Details | Review
s/METH_KEYWORDS/METH_VARARGS|METH_KEYWORDS/ when defining object methods (5.70 KB, patch)
2010-09-09 16:53 UTC, johnp
committed Details | Review
use actual unicode in the tests on py3k, not the byte representation (978 bytes, patch)
2010-09-09 17:27 UTC, johnp
committed Details | Review
properly handle ulongs properties in py3k (1.58 KB, patch)
2010-09-09 21:39 UTC, johnp
committed Details | Review
fix handling of UINT64 and INT64 arguments in py3k (1.60 KB, patch)
2010-09-09 22:47 UTC, johnp
committed Details | Review
Use PyMapping_Keys to determine if an object is a dict (py3k fix) (1.47 KB, patch)
2010-09-10 02:25 UTC, johnp
committed Details | Review
Check the type of the instance object (11.22 KB, patch)
2010-09-14 18:19 UTC, johnp
committed Details | Review
include the correct pycairo version (726 bytes, patch)
2010-09-15 14:27 UTC, johnp
committed Details | Review
add --enable-python3 switch (3.93 KB, patch)
2010-09-17 16:09 UTC, johnp
none Details | Review
add checks so we can compile under python 3 by setting PYTHON=python3 (4.69 KB, patch)
2010-09-20 16:46 UTC, johnp
committed Details | Review

Description Dave Malcolm 2010-04-15 18:25:52 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.
Comment 1 Dave Malcolm 2010-04-19 20:11:15 UTC
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
Comment 2 Dave Malcolm 2010-04-20 21:21:14 UTC
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.
Comment 3 Zach Goldberg 2010-04-21 02:24:35 UTC
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?
Comment 4 johnp 2010-08-11 20:16:43 UTC
Created attachment 167658 [details] [review]
move to using richcompare slot instead of compare
Comment 5 johnp 2010-08-11 20:17:30 UTC
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
Comment 6 johnp 2010-08-11 20:17:37 UTC
Created attachment 167660 [details] [review]
use Bytes instead of Unicode when reading io
Comment 7 johnp 2010-08-11 20:17:43 UTC
Created attachment 167661 [details] [review]
for py3k we need to do some more processing to get bytes from a unicode string
Comment 8 johnp 2010-08-11 20:17:51 UTC
Created attachment 167662 [details] [review]
make giomodule comile under py3k
Comment 9 johnp 2010-08-11 20:20:20 UTC
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.
Comment 10 johnp 2010-08-11 20:35:55 UTC
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
Comment 11 John Ehresman 2010-08-11 20:42:51 UTC
You probably want the macros installed somewhere so they can be used by libs that depend on pygobject.  Thanks for merging this!
Comment 12 Tomeu Vizoso 2010-08-12 10:58:52 UTC
(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?
Comment 13 johnp 2010-08-12 14:41:26 UTC
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
Comment 14 Tomeu Vizoso 2010-08-12 14:45:27 UTC
(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?
Comment 15 johnp 2010-08-12 14:56:33 UTC
That is it for now.  I'm doing it in stages so the code doesn't bitrot before it gets merged
Comment 16 johnp 2010-08-12 14:57:44 UTC
The py3k branch is not mergable and is being kept for reference as I refactor it on a local branch.
Comment 17 Simon van der Linden 2010-08-13 14:39:08 UTC
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.
Comment 18 Simon van der Linden 2010-08-13 14:55:04 UTC
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.
Comment 19 johnp 2010-08-13 15:05:58 UTC
(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.
Comment 20 Simon van der Linden 2010-08-13 15:07:54 UTC
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.
Comment 21 Simon van der Linden 2010-08-13 15:11:12 UTC
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.
Comment 22 Simon van der Linden 2010-08-13 15:21:11 UTC
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.
Comment 23 johnp 2010-08-13 15:31:47 UTC
(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 24 johnp 2010-08-13 16:15:21 UTC
Comment on attachment 167658 [details] [review]
move to using richcompare slot instead of compare

committed with fixes suggestions
Comment 25 johnp 2010-08-13 16:42:47 UTC
Created attachment 167825 [details] [review]
for py3k we need to do some more processing to get bytes from a unicode string
Comment 26 johnp 2010-08-13 17:00:21 UTC
Created attachment 167826 [details] [review]
use Bytes instead of Unicode when reading io
Comment 27 johnp 2010-08-13 17:06:29 UTC
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
Comment 28 johnp 2010-08-13 17:19:58 UTC
Created attachment 167827 [details] [review]
make giomodule compile under py3k
Comment 29 johnp 2010-08-13 17:20:17 UTC
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
Comment 30 Simon van der Linden 2010-08-15 06:29:58 UTC
Review of attachment 167825 [details] [review]:

OK. Thanks.
Comment 31 Simon van der Linden 2010-08-15 06:31:33 UTC
Review of attachment 167826 [details] [review]:

Thanks!
Comment 32 Simon van der Linden 2010-08-15 06:32:15 UTC
Review of attachment 167827 [details] [review]:

Good. Thanks!
Comment 33 Simon van der Linden 2010-08-15 06:34:44 UTC
Review of attachment 167828 [details] [review]:

Thanks!
Comment 34 johnp 2010-08-16 14:47:11 UTC
Keeping bug open.  This was a good start but we aren't done yet
Comment 35 johnp 2010-08-16 16:27:49 UTC
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 36 johnp 2010-08-16 16:30:36 UTC
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.
Comment 37 johnp 2010-08-16 16:42:01 UTC
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 38 johnp 2010-08-16 16:43:03 UTC
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
Comment 39 johnp 2010-08-16 17:53:48 UTC
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
Comment 40 johnp 2010-08-17 05:33:40 UTC
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
Comment 41 johnp 2010-08-17 05:36:27 UTC
Created attachment 168021 [details] [review]
convert to using PYGLIB_DEFINE_TYPE for module objects
Comment 42 johnp 2010-08-17 06:27:20 UTC
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
Comment 43 johnp 2010-08-17 06:28:10 UTC
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
Comment 44 johnp 2010-08-17 06:41:02 UTC
Created attachment 168030 [details] [review]
fix exceptions so they work in python 3.x
Comment 45 johnp 2010-08-17 14:48:07 UTC
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
Comment 46 johnp 2010-08-17 15:34:49 UTC
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
Comment 47 johnp 2010-08-17 16:06:00 UTC
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
Comment 48 johnp 2010-08-17 19:45:57 UTC
Created attachment 168129 [details] [review]
make cairo module compile in py3k
Comment 49 johnp 2010-08-20 15:22:23 UTC
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
Comment 50 johnp 2010-08-20 15:24:30 UTC
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
Comment 51 johnp 2010-08-20 15:24:54 UTC
Created attachment 168415 [details] [review]
working enum/flags/pid subclasses of long
Comment 52 johnp 2010-08-20 15:25:36 UTC
Created attachment 168416 [details] [review]
we need to specify tp_hash since we overide tp_richcompare
Comment 53 johnp 2010-08-20 16:16:51 UTC
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 54 Tomeu Vizoso 2010-08-25 08:56:04 UTC
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 55 Tomeu Vizoso 2010-08-25 09:00:08 UTC
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 56 Tomeu Vizoso 2010-08-25 09:28:41 UTC
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 57 Tomeu Vizoso 2010-08-25 09:32:17 UTC
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 58 Tomeu Vizoso 2010-08-25 09:35:52 UTC
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 59 Tomeu Vizoso 2010-08-25 09:39:49 UTC
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 60 Tomeu Vizoso 2010-08-25 09:41:24 UTC
Comment on attachment 168412 [details] [review]
make vfuncs work in py3k

Attachment 168412 [details] pushed as 0db676f - make vfuncs work in py3k
Comment 61 Tomeu Vizoso 2010-08-25 13:12:54 UTC
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 62 Tomeu Vizoso 2010-08-25 13:28:29 UTC
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 63 Tomeu Vizoso 2010-08-25 13:30:31 UTC
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
Comment 64 johnp 2010-08-26 17:35:42 UTC
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 65 Tomeu Vizoso 2010-08-27 13:15:02 UTC
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
Comment 66 johnp 2010-09-03 16:31:34 UTC
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
Comment 67 johnp 2010-09-03 16:31:51 UTC
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
Comment 68 John Ehresman 2010-09-03 17:15:12 UTC
(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.
Comment 69 johnp 2010-09-03 17:59:06 UTC
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.
Comment 70 John Ehresman 2010-09-03 18:20:14 UTC
(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.
Comment 71 johnp 2010-09-03 19:54:42 UTC
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.
Comment 72 johnp 2010-09-03 19:55:34 UTC
Except we need to check for errors since there is a memory allocation involved
Comment 73 Johan (not receiving bugmail) Dahlin 2010-09-03 20:26:51 UTC
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 74 johnp 2010-09-08 15:31:42 UTC
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.
Comment 75 johnp 2010-09-09 16:52:29 UTC
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
Comment 76 johnp 2010-09-09 16:52:40 UTC
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
Comment 77 johnp 2010-09-09 16:52:49 UTC
Created attachment 169875 [details] [review]
fix subclassing PyLong by calling __new__ correctly
Comment 78 johnp 2010-09-09 16:53:02 UTC
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.
Comment 79 johnp 2010-09-09 17:11:20 UTC
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).
Comment 80 johnp 2010-09-09 17:27:09 UTC
Created attachment 169880 [details] [review]
use actual unicode in the tests on py3k, not the byte representation
Comment 81 johnp 2010-09-09 17:29:02 UTC
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.
Comment 82 johnp 2010-09-09 21:39:19 UTC
Created attachment 169899 [details] [review]
properly handle ulongs properties in py3k

* If this is a PyLong object pull use AsUnsignedLong
Comment 83 johnp 2010-09-09 22:47:17 UTC
Created attachment 169907 [details] [review]
fix handling of UINT64 and INT64 arguments in py3k

* decode to the right sized C long
Comment 84 johnp 2010-09-09 22:50:31 UTC
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):
  • File "/home/johnp/devel/gnome-shell/source/pygobject-3/tests/test_gi.py", line 881 in test_ghashtable_int_none_in
    self.assertRaises(TypeError, GIMarshallingTests.ghashtable_int_none_in, '{-1: 1, 0: 0, 1: -1, 2: -2}')
  • File "/usr/lib64/python3.1/unittest.py", line 588 in assertRaises
    callableObj(*args, **kwargs)
  • File "/home/johnp/devel/gnome-shell/source/pygobject-3/gi/types.py", line 40 in function
    return info.invoke(*args)
AttributeError: keys

======================================================================
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)
Comment 85 johnp 2010-09-10 02:25:55 UTC
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
Comment 86 johnp 2010-09-10 03:40:20 UTC
Traceback (most recent call last):
  • File "./runtests.py", line 18 in <module>
    suite = loader.loadTestsFromNames(names)
  • File "/usr/lib64/python2.6/unittest.py", line 613 in loadTestsFromNames
    suites = [self.loadTestsFromName(name, module) for name in names]
  • File "/usr/lib64/python2.6/unittest.py", line 576 in loadTestsFromName
    module = __import__('.'.join(parts_copy))
  • File "/home/johnp/devel/gnome-shell/source/pygobject/tests/test_gi.py", line 15 in <module>
    GIMarshallingTests.SubObject.overwritten_method(obj)
TypeError: unbound method overwritten_method() must be called with SubObject instance as first argument (got Object instance instead)

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.
Comment 87 johnp 2010-09-10 18:57:59 UTC
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.
Comment 88 johnp 2010-09-14 18:19:35 UTC
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
Comment 89 johnp 2010-09-14 18:21:53 UTC
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!!!
Comment 90 Philippe Normand 2010-09-15 06:38:40 UTC
*** Bug 566641 has been marked as a duplicate of this bug. ***
Comment 91 johnp 2010-09-15 14:27:03 UTC
Created attachment 170342 [details] [review]
include the correct pycairo version
Comment 92 Tomeu Vizoso 2010-09-17 09:57:59 UTC
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 93 Tomeu Vizoso 2010-09-17 10:03:54 UTC
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 94 Tomeu Vizoso 2010-09-17 10:09:42 UTC
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 95 Tomeu Vizoso 2010-09-17 10:15:57 UTC
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 96 Tomeu Vizoso 2010-09-17 10:20:49 UTC
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 97 Tomeu Vizoso 2010-09-17 10:22:32 UTC
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
Comment 98 Tomeu Vizoso 2010-09-17 10:26:05 UTC
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 99 Tomeu Vizoso 2010-09-17 10:50:15 UTC
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
Comment 100 Tomeu Vizoso 2010-09-17 13:20:17 UTC
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.
Comment 101 johnp 2010-09-17 16:09:26 UTC
Created attachment 170503 [details] [review]
add --enable-python3 switch

* compile for python 3
* disables gio
* runs only pertinant tests
Comment 102 Johan (not receiving bugmail) Dahlin 2010-09-20 15:00:29 UTC
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
Comment 103 johnp 2010-09-20 15:44:15 UTC
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.
Comment 104 johnp 2010-09-20 16:46:14 UTC
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
Comment 105 johnp 2010-09-20 16:48:48 UTC
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
Comment 106 Tomeu Vizoso 2010-09-21 16:59:31 UTC
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?
Comment 107 johnp 2010-09-29 19:10:17 UTC
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.
Comment 108 Martin Pitt 2012-10-24 10:18:07 UTC
(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).
Comment 109 Martin Pitt 2012-10-24 10:55:55 UTC
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