Python 3 compatibility #19

Closed
cschramm opened this Issue Nov 7, 2013 · 34 comments

Comments

Projects
None yet
4 participants
Owner

cschramm commented Nov 7, 2013

Currently the code should be compatible with 2.6 and 2.7.

Goal is to also make it compatible with 3.2+ 3.3+

@ghost ghost referenced this issue Apr 22, 2014

Closed

Segfaults (Gentoo) #49

cschramm added a commit that referenced this issue Jan 17, 2015

Fix some python 3 compatibility issues
* print statements
* except statements
* raise statements
* iteritems
* itervalues
* octal literals

References #19
Owner

cschramm commented Jan 17, 2015

Fixed some easy issues. The most obvious items left are:

  • module __builtin__
  • module cPickle
  • module StringIO
  • method cmp
  • method long
  • method file
  • method xrange
  • type basestring
  • U prefix (3.2 only?)

cschramm added a commit that referenced this issue Jan 17, 2015

Owner

cschramm commented Jan 17, 2015

Replaced xrange, file, long, and cPickle as well.

The iniparse library is a real pain including all StringIO, basestring, and the u prefix.

Concerning the blueman code, we only have some usages of cmp (which is easy to replace), __builtin__, and some u prefixes in blueman-sendto.

Contributor

infirit commented Jan 17, 2015

I have in my python3 branch fixes for __builtin__, sorting (cmp) and StringIO.

The u prefix was added in python 3.3 afaik but, I think we may want to import unicode_literals from the __future__ module instead of adding the u prefix on every string. The import enables the unicode handling from python3 and will be ignored completely in python3.

Owner

cschramm commented Jan 17, 2015

Nice. 😃 Shall we add this to master or do you expect any issues?

Concerning the u prefix, apart from the iniparse lib we only use it in apps/blueman-sendto. It's probably unnecessary as we don't use it anywhere else, but I'm not sure.

Owner

cschramm commented Jan 17, 2015

Oh, I see, your branch is python 3 only, right? Since e.g. io.StringIO does not exist in python 2.7 I think.

Contributor

infirit commented Jan 17, 2015

It should be compatible with 2 and 3, io module was added in 2.6.

I am using this branch right now. However I like to test it a bit more before I send it off to master.

Also Python3 does not allow relative imports any-more and we need to add a dots in front. I am looking into this atm but it should be in the list.

Owner

cschramm commented Jan 17, 2015

You're right, io.StringIO works with 2.7 as well.

Contributor

infirit commented Jan 17, 2015

Send a pull request for testing as things ended up being a bit large.

@cschramm cschramm referenced this issue Jan 18, 2015

Merged

Python3 fixes #168

Owner

cschramm commented Feb 8, 2015

@infirit You mentioned there is still work to do. Any concrete issues?

Contributor

infirit commented Feb 8, 2015

Not looked at this but I am seeing maximum recursion.. Below repeats indefinitely.

_________
Destroy (/usr/lib64/python3.3/site-packages/blueman/main/Device.py:98)
invalidating device None 
Exception RuntimeError: 'maximum recursion depth exceeded while calling a Python object' in <bound method Device.__del__ of <Device object at 0x7fa5d7fe8d20 (blueman+main+Device+Device at 0x22e3ae0)>> ignored
_________
__del__ (/usr/lib64/python3.3/site-packages/blueman/main/Device.py:60)
deleting device None 
_________
Destroy (/usr/lib64/python3.3/site-packages/blueman/main/Device.py:98)
invalidating device None 
Exception RuntimeError: 'maximum recursion depth exceeded while calling a Python object' in <bound method Device.__del__ of <Device object at 0x7fa5d7fe8d20 (blueman+main+Device+Device at 0x22e3ae0)>> ignored
_________
__del__ (/usr/lib64/python3.3/site-packages/blueman/main/Device.py:60)
deleting device None 
_________
Destroy (/usr/lib64/python3.3/site-packages/blueman/main/Device.py:98)
invalidating device None 
Exception RuntimeError: 'maximum recursion depth exceeded while calling a Python object' in <bound method Device.__del__ of <Device object at 0x7fa5d7fe8d20 (blueman+main+Device+Device at 0x22e3ae0)>> ignored
_________
Owner

cschramm commented Feb 10, 2015

Looks like the recusion actually happens on line 120 when Destroy tries to get the Signals property. The getter than tries to get the Device property and calls itself recursively, since it is not set. This is probably the result of an exception in the constructor, since the unicode type is not defined in Python 3. Here are all the appearances, not sure what to do about the type checks (ignore iniparse, dropped in #180):

blueman/plugins/applet/NMPANSupport.py
148:        if type(value) == str or type(value) == unicode:

blueman/iniparse/ini.py
542:            if linecount == 0 and isinstance(line, unicode):

blueman/main/Device.py
23:        if isinstance(instance, str) or isinstance(instance, unicode):

apps/blueman-browse
62:            d.props.secondary_text = "%s\n\n" % unicode(e.message, 'UTF-8') + \

Another exception I got looks like we potentially have a problem with any method call for the Cython module with char * arguments due to str vs. bytes.

Traceback (most recent call last):
  File "/home/cschramm/src/blueman/blueman/plugins/applet/StandardItems.py", line 110, in on_devices
    icon="blueman")
  File "/home/cschramm/src/blueman/blueman/Functions.py", line 128, in startup_notification
    sn.set_name(name)
  File "_blueman.pyx", line 448, in _blueman.sn_launcher.set_name (_blueman.c:4614)
TypeError: expected bytes, str found

We could either look at all usages or change the definitions in some way. The _ustring helper from http://docs.cython.org/src/tutorial/strings.html looks a little like what we would need.

WhyNotHugo referenced this issue Mar 5, 2015

@cschramm cschramm modified the milestones: 2.0, 2.1 Mar 5, 2015

Owner

cschramm commented Mar 5, 2015

Any other known issues than the unicode type and cython str vs. bytes?

Contributor

infirit commented Mar 5, 2015

It is one of those things that when you fix one thing three others show up 😠

I now added all the future imports in python2.7 and am working through all the problems now. I pushed these commits just now to my futurize if you are curious 😀

Contributor

infirit commented Mar 6, 2015

It is alive, #215

@cschramm cschramm closed this Apr 12, 2015

Recursion?

I got these on nearby devices discovery:

Destroy
(/usr/local/lib/python2.7/dist-packages/blueman/main/Device.py:92)
invalidating device None Exception RuntimeError: 'maximum recursion
depth exceeded' in <bound method Device.del of <Device object at
0xa6d8d2d4 (blueman+main+Device+Device at 0x9a50e20)>> ignored
_________ del
(/usr/local/lib/python2.7/dist-packages/blueman/main/Device.py:70)
deleting device None _________

They loop infinitely.

It's there for a while, I later could report where it comes from, but
I'm sure you can reproduce and squash the bug much faster than lame I
can.

And thanks in advance!

Owner

cschramm commented Apr 13, 2015

@Plaque-fcc Probably the result of a previous error as in #19 (comment). Any exceptions in the output before that happens?

Contributor

infirit commented Apr 13, 2015

@Plaque-fcc, I can not reproduce this with python 2.7 here on Gentoo. So you'll need to be more descriptive on the steps to reproduce.

Contributor

infirit commented Apr 13, 2015

@Plaque-fcc, Do try https://github.com/infirit/blueman/tree/string_type (two commits)

@cschramm, the new function string_type is a bit hacky but instead of sprinkling hassattr all over the place I thought this may be a better solution. My naming skills are horrible so if you have a better name for it do let me know 😄

No, it feeds all the screen with this looping. ;D

Actually, I'm back to git bisect on it. No steps to reproduce this,
simply because it's as simple as «Hit Search button and have a loop»
and probably is system/versions-depended.

On Sun, 12 Apr 2015 23:45:35 -0700
Christopher Schramm notifications@github.com wrote:

@Plaque-fcc Probably the result of a previous error as in
#19 (comment).
Any exceptions in the output before that happens?


Reply to this email directly or view it on GitHub:
#19 (comment)

Contributor

infirit commented Apr 13, 2015

@Plaque-fcc Don't run bisect as it is quite useless in this case. Plus I know which one you will end up with, it is 82b84ee.

Try my commits in the string_type branch in my fork and see if that sort the problem out.

Hi!

No, I found that on e486d77 it
starts to get reproduced and from there on.

And no, building from your repo/string_type doesn't prevent it from
recursive looping on discovery in Device.py. Although, thank you for
your suggestion.

On Mon, 13 Apr 2015 05:23:46 -0700
Sander Sweers notifications@github.com wrote:

@Plaque-fcc Don't run bisect as it is quite useless in this case.
Plus I know which one you will end up with, it is
82b84ee.

Try my commits in the string_type branch in my fork and see if that
sort the problem out.


Reply to this email directly or view it on GitHub:
#19 (comment)

Owner

cschramm commented Apr 13, 2015

I found that on e486d77 it starts to get reproduced and from there on.

@infirit I guess that means BluezDevice(instance) fails due to the type of instance?

@Plaque-fcc You could redirect the output to less or a file or whatever to see what happens before the loop.

Contributor

infirit commented Apr 13, 2015

@Plaque-fcc please apply the below debug patch and paste(bin) the output.

@cschramm, yes which is weird because I only ever saw BluezDevice instances on my machine. I can undo the logic reverse and check for string by using hasattr but now I am curious 😄

edit: I would expect some Exception to be thrown so I am confused 😕

diff --git a/blueman/main/Device.py b/blueman/main/Device.py
index dd45740..ad298ef 100644
--- a/blueman/main/Device.py
+++ b/blueman/main/Device.py
@@ -30,6 +30,10 @@ class Device(GObject.GObject):
         self.Fake = True
         self.Temp = False

+        dprint("***DEBUG***")
+        dprint(type(instance))
+        dprint(repr(instance))
+        dprint("***DEBUG END***")
         if isinstance(instance, BluezDevice):
             self.Device = instance
         else:
Eh… http://paste.ubuntu.com/10816669/ — you won't like it.

On top of infirit's branch:

$ git diff
diff --git a/blueman/main/Device.py b/blueman/main/Device.py
index dd45740..ad298ef 100644
--- a/blueman/main/Device.py
+++ b/blueman/main/Device.py
@@ -30,6 +30,10 @@ class Device(GObject.GObject):
self.Fake = True
self.Temp = False

  •    dprint("**_DEBUG**_")
    
  •    dprint(type(instance))
    
  •    dprint(repr(instance))
    
  •    dprint("**_DEBUG END**_")
     if isinstance(instance, BluezDevice):
         self.Device = instance
     else:
    

On Mon, 13 Apr 2015 08:20:47 -0700
Sander Sweers notifications@github.com wrote:

@Plaque-fcc please apply the below debug patch and paste(bin) the
output.

@cschramm, yes which is weird because I only ever saw
BluezDevice instances on my machine. I can undo the logic
reverse and check for string by using hasattr but now I am
curious 😄

diff --git a/blueman/main/Device.py b/blueman/main/Device.py
index dd45740..ad298ef 100644
--- a/blueman/main/Device.py
+++ b/blueman/main/Device.py
@@ -30,6 +30,10 @@ class Device(GObject.GObject):
         self.Fake = True
         self.Temp = False

+        dprint("***DEBUG***")
+        dprint(type(instance))
+        dprint(repr(instance))
+        dprint("***DEBUG END***")
         if isinstance(instance, BluezDevice):
             self.Device = instance
         else:

Reply to this email directly or view it on GitHub:
#19 (comment)

Contributor

infirit commented Apr 13, 2015

you won't like it

I do as it is enlightening 😉.

It is weird that this is a problem for you but not for me. But it is clear things go wrong when we see a FakeDevice. I'll poke at the instance object to see if this is an instance of string or unicode and come up with a fix.

Owner

cschramm commented Apr 13, 2015

Then it's probably BlueZ 4 vs. BlueZ 5. There shouldn't be any FakeDevice instances around when you have BlueZ 5.

Contributor

infirit commented Apr 13, 2015

@Plaque-fcc, please test #253

$ dpkg-query --show --showformat='${package} ${version} ${status} \n' bluez
bluez 4.101-0ubuntu20 install ok installed

Hasn't bluez 4 been deprecated for some time now? I recall the move to bluez5 being over half a year ago.

Contributor

infirit commented Apr 13, 2015

It is but BlueZ 5 breaks lots of things so they have been putting it off. AFAIK 15.04 will come with BlueZ 5.

Owner

cschramm commented Apr 13, 2015

Yes, the Ubuntu guys are working on it, but honestly, it is 15.04 now. I have no idea how that shall make it into the release...

Anyway, blueman currently supports both major versions, so issues with BlueZ 4 have to be dealt with. We'll drop that for 2.1 once 2.0 is finished.

Yaaayy! Works!! ^_^ Than you!

On Mon, 13 Apr 2015 09:27:07 -0700
Sander Sweers notifications@github.com wrote:

@Plaque-fcc, please test
#253


Reply to this email directly or view it on GitHub:
#19 (comment)

Contributor

infirit commented Apr 14, 2015

Perfect, pushed the fix to master. Thanks for the report @Plaque-fcc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment