Ticket #2784 (closed defect: fixed)

Opened 2 years ago

Last modified 23 months ago

D-Bus code is incompatible with forthcoming dbus-python 0.80, and uses non-public API

Reported by: simon.mcvittie@… Owned by: asterix
Priority: normal Milestone: 0.11.1
Component: gajim-remote Version: svn
Severity: normal Keywords:
Cc: OS:

Description

I'm currently working on a rewrite of the dbus-python bindings, replacing the Pyrex module dbus_bindings with C code and changing the API where necessary to make the bindings more Pythonic. In practice, most Python D-Bus apps will continue to work with my rewrite, which is going to be released as version 0.80 (I released 0.80rc2 today).

However, the D-Bus code in Gajim uses some interfaces which were never meant to be part of the public API; these will stop working with dbus-python 0.80. The attached patches (untested) should at least get you started on porting gajim to dbus-python 0.80, in a way that will still work on dbus-python 0.60.

Specific issues:

  • dbus_bindings is not public API, and will go away. You can address most of this by just replacing "dbus.dbus_bindings.DBusException" with "dbus.DBusException" (this works in something like 0.40 and up).
  • In the remote control, you seem to be emitting signals using a much lower-level interface than is intended. The normal way to do this is by decorating a function of the same name as the signal with dbus.service.signal, then calling it (see my patch).
  • There is no such thing as None in the D-Bus world. You seem to represent it as the integer 0, which seems likely to cause confusion in future!
  • The API for Variants has changed - there's no longer a Variant class, to make method arguments and return values consistent and unambiguous. Marking dicts and arrays with a signature that includes 'v' should achieve what you want to do.

Attachments

gajim-01-no-dbus_bindings.patch (3.1 kB) - added by anonymous 2 years ago.
gajim-02-remote.patch (5.1 kB) - added by simon.mcvittie@… 2 years ago.

Change History

Changed 2 years ago by anonymous

Changed 2 years ago by simon.mcvittie@…

  Changed 2 years ago by nk

  • owner changed from asterix to nk
  • milestone set to 0.11

wow this is serious and it's good thing you informed us and helped us. I will try to do my best, but really, after the rewrite is done, consider high quality documentation please :)

  Changed 2 years ago by nk

(In [7672]) make sure we use dbus public api so we work for python dbus 0.80; see #2784

  Changed 2 years ago by asterix

this can't be for 0.11 ! this patch require python-dbus >= 0.80. debian doesn't have it .. and maybe other distro too. gajim works well with 0.71, so why require 0.80 ??

  Changed 2 years ago by nk

  • status changed from new to assigned

Simon,

what do you mean by:

# FIXME: can't specify any introspect signature for any of these 
 	181	        # since they take a variable number of arguments (this is not 
 	182	        # recommended in a D-Bus interface) 

what exactly is signature 'sis'?

what is the problem with 0 being None [okay we may wanna fix it, any suggestions?]

and if I understood correctly you want api docs on your api?

src/gajim-remote.py should help you what everything is (yes okay..)

and Yann, yes it's not out yet, but when it will be AFAICT, we will break, and Simon with those patches in, do we work in python 0.71 ?

  Changed 2 years ago by nk

(In [7673]) another patch so we work in python dbus 0.80; see #2784

  Changed 2 years ago by simon.mcvittie@…

Hi,

While it's 0.80 that would trigger incompatibility, I believe my patches would work in any dbus-python newer than about 0.60 (released over a year ago).

There is some API documentation for 0.80 in http://people.freedesktop.org/~smcv/dbus-python-0.8pre/epydoc/ which you might find useful. I realise this is no substitute for a tutorial, but documenting has been one of my major goals for 0.80. There are some fairly complete examples in the documentation for 'dbus' (client) and 'dbus.service.Object' (service), and if you download the 0.80rc2 tarball, the examples directory contains some more.

If you're not familiar with D-Bus signatures, I'll try to summarize:

D-Bus messages are strongly typed, and the interfaces implemented by D-Bus objects are also meant to be strongly typed. It's technically possible to implement methods that take arbitrary arguments, but users of less dynamic languages (C, C++, Java) are likely to have difficulty calling (or implementing) those methods.

The signature is written in quite a concise form: 'sis' means three arguments, string, int32, string. '(sis)' means a struct containing a string, an int32 and a string. 'a(sis)' would be an array of such structs. See the D-Bus specification for more.

dbus-python allows you to omit the in_signature and out_signature arguments from the @method decorator, or omit the signature from the @signal decorator. This is not really recommended, and perhaps I should make it emit a warning. Anyway, the effects of this are:

  • When a client calls Introspect() on your object, the introspection data will not indicate whether that method/signal has any arguments (since we can't tell)
  • When a dbus-python client calls the method, or you emit the signal, dbus-python will take its best guess at what signature you meant; it may get it wrong if the parameters are in some way ambiguous (the exact rules are in the API docs for the static method dbus.lowlevel.Message.guess_signature if you're interested). Using the special types like dbus.String and dbus.Int32 is always unambiguous.
  • When the method is called with a guessed signature that turns out to be wrong, the client will just get a TypeError? back.
  • Whether other client libraries can call that method or make a useful binding to the signal is up to the author of that client library; notably, signals with variable arguments can't be received in a useful way by dbus-glib.

Formal API docs aren't necessary (although they're always helpful), but the minimum you need in order that others might be able to interoperate with you would be that you specify the signature, and don't change it in future.

As for the None thing... there is no concept of None or NULL in the wire protocol, not even a null type in variants. If you want a special null value you'll have to use something else, usually 0 or negative for numbers, or the empty string for strings. For instance, in the Telepathy specification we define handles to be non-zero, specifically so we can make handle==0 special in various places.

If any Debian users among you want to try out 0.80, 0.80rc2 is available in experimental.

  Changed 2 years ago by asterix

>>> import dbus
>>> dbus.version
(0, 70, 0)
>>> bus = dbus.SessionBus()
>>> hasattr(bus, 'name_has_owner')
False

and in code:

if not hasattr(bus, 'name_has_owner'):
	print 'You need dbus-python >= 0.80' #FIXME: translate me or RM me for .12
	return None

so I will revert those changes if we can't make it work with python-dbus < 0.80 ...

now when I try to use gajim-remote I get that:

$ ./gajim-remote.py toggle_roster_appearance
Introspect error: The name org.gajim.dbus was not provided by any .service files

so for me those patches break dbus.

  Changed 2 years ago by asterix

after [7676] (typo) I get that:

[...]
  File "/home/asterix/gajim/src/remote_control.py", line 94, in raise_signal
    self.signal_object.getattr(signal)(get_dbus_struct(arg))
AttributeError: 'SignalObject' object has no attribute 'getattr'

so we can't release because of that ... is there a solution that work on both <0.8 and >= 0.8 ?

else we'll keep the raise_signal func. doesn't it work with 0.8 ?

  Changed 2 years ago by asterix

(In [7678]) partially revert [7673]. see #2784

  Changed 2 years ago by asterix

(In [7681]) make music_track_listener work with older python-dbus than 0.80. see #2784

  Changed 2 years ago by simon.mcvittie@…

so I will revert those changes if we can't make it work with python-dbus < 0.80 ...

Alternatively, you could use something like:

bus_obj = dbus.SessionBus().get_object('org.freedesktop.DBus', '/org/freedesktop/DBus')
bus_obj.NameHasOwner(dbus_interface='org.freedesktop.DBus')

which is what name_has_owner() ends up as.


[...]
  File "/home/asterix/gajim/src/remote_control.py", line 94, in raise_signal
    self.signal_object.getattr(signal)(get_dbus_struct(arg))
AttributeError: 'SignalObject' object has no attribute 'getattr'

Oops, I meant getattr(self.signal_object, signal)(get_dbus_struct(arg)). I did say "untested".

The normal way to emit D-Bus signals is just to call the signal-emitting method. For instance in examples/example-signal-emitter.py in 0.80rc2:

    @dbus.service.signal('com.example.TestService')
    def HelloSignal(self, message):
        # The signal is emitted when this method exits
        # You can have code here if you wish
        pass

    @dbus.service.method('com.example.TestService')
    def emitHelloSignal(self):
        #you emit signals by calling the signal's skeleton method
        self.HelloSignal('Hello')
        return 'Signal emitted'

Since your other code emits signals by name, though, I altered raise_signal to do this by dispatching to the appropriate method (and got it wrong, but hopefully you get the idea...)

Using decorated methods for signals like this means that it's known what signals are included in your interface, and that the D-Bus introspection XML provided when a D-Bus client calls Introspect() can include those signals. You can (and should) add a signature='...' parameter to the @signal decorator - I didn't in my patch, since it wasn't clear to me what data types were actually used.

Sending Messages directly is a much lower-level API than you need to use: in dbus-python 0.70 it's not even public API (in dbus_bindings). In 0.80 it's made into public API in dbus.lowlevel, but you should only need to use it in obscure situations (a Python clone of dbus-monitor or dbus-send, perhaps).

  Changed 2 years ago by nk

  • milestone changed from 0.11 to 0.11.1

  Changed 2 years ago by asterix

(In [7714]) make gajim-remote work with python-dbus 0.80+. see #2784

follow-up: ↓ 16   Changed 2 years ago by nk

Simon, thanks again for your replies and help, but given the fact that the guy he hacked D-Bus and Gajim is no more around, and the fact that I quickly looked at the api docs you linked me and found no solution, I ask you this:

please put a page where there are some short and clear sample source on doing the three to five most common things one may want to do.

so basically now raise_signal will also work for all versions right? so we work allover.

so what you mostly say is that we need introspect but then again you say:

# FIXME: can't specify any introspect signature for any of these

# since they take a variable number of arguments (this is not # recommended in a D-Bus interface)

but we need optional variables! so we can't do it?

then you say:

"sending message directly is much lower-level api". Yes I was always fun of higher APIs, but what is exactly that we do low, and how should we do it to be up?

  Changed 2 years ago by nk

btw Simon, also see http://trac.gajim.org/ticket/2865

in reply to: ↑ 14   Changed 23 months ago by simon.mcvittie@…

Replying to nk:

please put a page where there are some short and clear sample source on doing the three to five most common things one may want to do.

I'm working on it, but at the moment getting the API into a state where it's documentable is taking priority over documentation! For now try the "examples" directory in dbus-python 0.80rc3 (I've updated them to use the recommended API).

so basically now raise_signal will also work for all versions right? so we work allover.

I believe so.

so what you mostly say is that we need introspect but then again you say: # FIXME: can't specify any introspect signature for any of these # since they take a variable number of arguments (this is not # recommended in a D-Bus interface) but we need optional variables! so we can't do it?

D-Bus is designed to be a strongly-typed, statically-typed protocol (more like Java than Python), so, not really...

The usual approach to the sort of thing you'd do with optional arguments in Python is to either reserve a "null" value, typically 0 or the empty string (in Telepathy we define handles to be nonzero, specifically so we can use 0 to mean "no such handle"), or have one argument be a dict of string => variant (a{sv} in D-Bus signature terms) which you can use for any optional parameters.

Would it be possible to alter the interface gajim-remote uses to talk to gajim? I understand using gajim from one release and gajim-remote from another would be a bit mad anyway?

The "remote" code in gajim seems to have been designed with an approach of "just dump arbitrary Python objects onto the bus and let gajim-remote sort them out", then hacked in assorted ways to work around the fact that D-Bus is not actually a transparent channel for arbitrary Python objects. It looks like it may be due for some redesign... The remote also tries to turn command-line arguments straight into method calls, which seems incredibly error-prone.

If you just want a "quick and dirty" remote-control, you could always have the remote just forward its entire command line to a method with an 'as' (array of string) argument, and have the main Gajim program do the parsing.

then you say: "sending message directly is much lower-level api". Yes I was always fun of higher APIs, but what is exactly that we do low, and how should we do it to be up?

Anything dealing with Message objects or dbus_bindings is generally a bad move for application code. (In 0.80 I've re-exposed the Message-based API in the new dbus.lowlevel module to make it clearer that this is the case...)

Using dbus.service.Object is the way forward. Have a look at the examples directory in dbus-python 0.80rc3, specifically example-signal-emitter.py and example-signal-receiver.py for signals, and example-client.py and example-service.py for methods (I'm about to check in an asynchronous call example to the git version of dbus-python too).

  Changed 23 months ago by asterix

  • owner changed from nk to asterix
  • status changed from assigned to new

ok I read Dbus spec and exemple code and understand things better. So if I understand correctly, we can remove all our dbus.Int32(5) to just 5 and dbus.String('qwe') to 'qwe' if we use signatures. Am I right ? Will that be compatible with previous version of dbus ? (seems it's not according to #2865. So we should keep both dbus.String calls and signatures, right ?)

and I also see the way to do things for optional args. We should send empty string, and remote_control must handle empty strings.

PS: little typo in your example-service.py:

object = SomeObject(session_bus, '/SomeObject')

should be

object = SomeObject(name, '/SomeObject')

the same in example-signal-emitter.py

  Changed 23 months ago by simon.mcvittie@…

Um, OK, there are two sorts of signature here: the ones in the @dbus.service.method and @dbus.service.signal decorators, and the ones in the constructor of dbus.Array and dbus.Dictionary (and also dbus.Struct, as of 0.80).

The ones in the @method and @signal decorators are very useful - you should definitely be using them if you can.

If method and signal signatures are present on the service side (and can be introspected, in the case of the client), you don't *necessarily* need to convert to D-Bus-specific types, although it doesn't hurt. In dbus-python 0.80, if there's introspection info available, the code will basically accept anything that looks sensible and convert it into the right type.

I can't make any guarantees about the behaviour of dbus-python 0.71 or older (that's partly why I rewrote it - it's often unclear what's going on).

The signature attached to a dbus.Array or dbus.Dictionary (or in 0.80, dbus.Struct) is somewhat less useful. If there *isn't* any introspection info available (as in the current Gajim codebase), it's used to guess what the signature should be. It's also used when filling in arguments that appear in the signature as "variant".

PS: not a typo. The dbus.service.Object constructor requires a BusName? in 0.71, but accepts either a BusName? or a Bus in 0.80 - this is mainly so you can export objects in a process that doesn't want or need a well-known bus name. Using a Bus also fits D-Bus' conceptual model better, which is why I used one in the example. There is some slightly hairy code in service.py to deal with this in a compatible way.

  Changed 23 months ago by asterix

  • status changed from new to closed
  • resolution set to fixed

(In [7811]) always give the same number of arguments to dbus methods, use signatures. fixes #2784

simon, could you have a look to this patch ? does it seems correct ?

  Changed 23 months ago by anonymous

Looks good to me - I can't see anything there that I believe will break with either 0.80 or 0.71.

  Changed 23 months ago by simon.mcvittie@…

Sorry, that anonymous approval was from me. -Simon

Add/Change #2784 (D-Bus code is incompatible with forthcoming dbus-python 0.80, and uses non-public API)

Author



Change Properties
<Author field>
Action
as closed
Next status will be 'reopened'
 
Note: See TracTickets for help on using tickets.