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 725446 - Issues with dbus# 0.8
Issues with dbus# 0.8
Status: RESOLVED FIXED
Product: banshee
Classification: Other
Component: general
unspecified
Other Linux
: Normal normal
: 3.0
Assigned To: Banshee Maintainers
Banshee Maintainers
Depends on:
Blocks: 630110
 
 
Reported: 2014-03-01 16:43 UTC by Chow Loong Jin
Modified: 2015-07-17 14:15 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to remove IDBusExportable from inheritance chain of DBus-exported interfaces (6.46 KB, patch)
2014-03-01 16:45 UTC, Chow Loong Jin
none Details | Review
Alternative patch that exports IDBusExportable on DBus. (1.01 KB, patch)
2014-03-01 16:47 UTC, Chow Loong Jin
none Details | Review
Proposed patch from Bertrand (8.41 KB, patch)
2014-06-17 11:47 UTC, Andrés G. Aragoneses (IRC: knocte)
committed Details | Review
dbus-sharp patch (3.75 KB, patch)
2014-06-17 23:35 UTC, Nicholas Little
none Details | Review
dbus-sharp patch v2 (2.26 KB, patch)
2014-06-18 15:12 UTC, Nicholas Little
none Details | Review

Description Chow Loong Jin 2014-03-01 16:43:43 UTC
It looks like IDBusExportable contains a property "Parent" that returns another IDBusExportable. However, IDBusExportable itself isn't exported on DBus, so this property can't be handled properly when an interface that derives from it gets exported on DBus.

It seems that dbus# 0.7 ignores this, but dbus# 0.8 throws an error, as per https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=731978.
Comment 1 Chow Loong Jin 2014-03-01 16:45:18 UTC
Created attachment 270630 [details] [review]
Patch to remove IDBusExportable from inheritance chain of DBus-exported interfaces
Comment 2 Chow Loong Jin 2014-03-01 16:47:11 UTC
Created attachment 270631 [details] [review]
Alternative patch that exports IDBusExportable on DBus.
Comment 3 Andrés G. Aragoneses (IRC: knocte) 2014-03-18 10:13:57 UTC
None of this patches change the required dbus# version, so I guess they need to be applied on top of this other patch? http://anonscm.debian.org/gitweb/?p=pkg-cli-apps/packages/banshee.git;a=blob;f=debian/patches/Use-dbus-2.patch;h=9ce676446f8838ad5427f37b3bd17a5867932490;hb=HEAD
Comment 4 Chow Loong Jin 2014-03-18 14:50:02 UTC
I think the patches can be applied on Banshee pre-dbus#0.8 as well, considering it was a misuse of the dbus# api on Banshee's part.
Comment 5 Andrés G. Aragoneses (IRC: knocte) 2014-03-18 14:57:12 UTC
(In reply to comment #4)
> I think the patches can be applied on Banshee pre-dbus#0.8 as well, considering
> it was a misuse of the dbus# api on Banshee's part.

These patches don't make d-feet work though.
Comment 6 Chow Loong Jin 2014-03-18 15:40:11 UTC
No they don't. But neither do they break Banshee with dbus#0.7.
Comment 7 Andrés G. Aragoneses (IRC: knocte) 2014-03-18 15:55:31 UTC
(In reply to comment #6)
> No they don't. But neither do they break Banshee with dbus#0.7.

Right, but this bug is about migrating to 0.8 :) Did you finish bisecting what you said you were close to?
Comment 8 Chow Loong Jin 2014-03-19 02:39:59 UTC
Not yet. I've bisected it down to commit 3c278975c59cf8dbcfdfdf1184e4cf9c89de7ce3, but it's a pretty big commit, so I'm still cutting that commit down to size.

I've narrowed it down to the MethodCall bits, so I reckon something there wasn't ported properly to MessageContainer.
Comment 9 Chow Loong Jin 2014-03-19 02:40:36 UTC
https://github.com/mono/dbus-sharp/commit/3c278975c59cf8dbcfdfdf1184e4cf9c89de7ce3 is the commit, by the way.
Comment 10 Andrés G. Aragoneses (IRC: knocte) 2014-05-07 13:10:58 UTC
(In reply to comment #9)
> https://github.com/mono/dbus-sharp/commit/3c278975c59cf8dbcfdfdf1184e4cf9c89de7ce3
> is the commit, by the way.

Any progress on that Chow?

By the way, by bisecting that were you trying to find the culprit for the d-feet problem?

(In reply to comment #5)
> These patches don't make d-feet work though.

For the record, the d-feet error is:

- to instruct banshee to start playing, for example, on the PlayerEngine interface:

'GDBus.Error:org.freedesktop.DBus.Error.ServiceUnknown: The name :1.128 was not
provided by any .service files'

And today directhex was bringing this up in #banshee:

14:38 <directhex> it looks a lot like 0.8 blocks dbus service activation from working
14:39 <directhex> ( http://dbus.freedesktop.org/doc/system-activation.txt )
14:58 <knocte> directhex: on purpose?
14:58 <directhex> knocte, mmmmm, more like a side effect of an attempt to prevent a null return
14:59 <knocte> so, needs to be fixed
Comment 11 Chow Loong Jin 2014-05-08 02:51:18 UTC
Uh whoops, I've been a bit preoccupied with other things, so I haven't actually made any progress.
Comment 12 Nicholas Little 2014-06-11 12:46:25 UTC
Any progress on this?

The error message from the debian bug report is very similar to issues I've been having getting dbus-sharp to instantiate types which export events, from the debian log the problem occurs when attempting to implement the IPlayerEngineService proxy class. See my second comment here: https://github.com/mono/dbus-sharp/pull/37

Anyway, I applied the patch from Andrés Comment 3 to build against the new api version dbus-sharp with the two commits from the above pull request and I don't encounter the "cannot create a struct with no fields" issue for types with events in their interfaces.

When attempting to start a new instance of banshee it simply exits after the reply from DBus telling us that someone already has our name.
Comment 13 Chow Loong Jin 2014-06-12 01:54:33 UTC
(In reply to comment #12)
> Any progress on this?
> 
> The error message from the debian bug report is very similar to issues I've
> been having getting dbus-sharp to instantiate types which export events, from
> the debian log the problem occurs when attempting to implement the
> IPlayerEngineService proxy class. See my second comment here:
> https://github.com/mono/dbus-sharp/pull/37
> 
> Anyway, I applied the patch from Andrés Comment 3 to build against the new api
> version dbus-sharp with the two commits from the above pull request and I don't
> encounter the "cannot create a struct with no fields" issue for types with
> events in their interfaces.

The MakeStruct exception comes from attempting to implement an interface that isn't exported. In Banshee's case, it was due to an exported method/property having a signature involving an unexported type. You need to track down which interface it's trying to implement, and make sure it's exported, or remove the methods which reference that interface.

> When attempting to start a new instance of banshee it simply exits after the
> reply from DBus telling us that someone already has our name.

That means you already have another instance of Banshee running.
Comment 14 Nicholas Little 2014-06-12 18:01:47 UTC
(In reply to comment #13)
> (In reply to comment #12)
> > Any progress on this?
> > 
> > The error message from the debian bug report is very similar to issues I've
> > been having getting dbus-sharp to instantiate types which export events, from
> > the debian log the problem occurs when attempting to implement the
> > IPlayerEngineService proxy class. See my second comment here:
> > https://github.com/mono/dbus-sharp/pull/37
> > 
> > Anyway, I applied the patch from Andrés Comment 3 to build against the new api
> > version dbus-sharp with the two commits from the above pull request and I don't
> > encounter the "cannot create a struct with no fields" issue for types with
> > events in their interfaces.
> 
> The MakeStruct exception comes from attempting to implement an interface that
> isn't exported. In Banshee's case, it was due to an exported method/property
> having a signature involving an unexported type. You need to track down which
> interface it's trying to implement, and make sure it's exported, or remove the
> methods which reference that interface.

My bad, it is apparently just F# instantiations that have the problem relating to events.

> > When attempting to start a new instance of banshee it simply exits after the
> > reply from DBus telling us that someone already has our name.
> 
> That means you already have another instance of Banshee running.

I know that :) My point was that from commit cd0a64252618931a8b625aa79f59813c27b82e76, the one immediately following the 0.8 version bump, onwards it exits cleanly without triggering the ArgumentException mentioned in the debian bug report.

So I guess this is more about the issue encountered when attempting to execute methods and get properties via D-feet. FWIW, using our registered service name works fine:

dbus-send --dest=org.bansheeproject.Banshee --type=method_all --print-reply /org/bansheeproject/Banshee/PlayerEngine org.bansheeproject.Banshee.PlayerEngine.Play

Did you notice this message "Warning: Connection was probably hung up (In, Hup)" when running Banshee with DBUS_VERBOSE=true? (Just after the call, when all of banshee's exported objects disappear from the bus)

I mention it because I couldn't find the message anywhere in the dbus-sharp source so finding where that's produced might get us quite close to where the error occurs in dbus-sharp.
Comment 15 Andrés G. Aragoneses (IRC: knocte) 2014-06-17 11:47:52 UTC
Created attachment 278590 [details] [review]
Proposed patch from Bertrand

(In reply to comment #12)
> Anyway, I applied the patch from Andrés Comment 3 to build against the new api

I'm posting here a patch from Bertrand that he posted in bug 630110, which should replace that debian downstream patch, and may render both Chow's 2 patches posted here unnecessary.

> version dbus-sharp with the two commits from the above pull request and I don't
> encounter the "cannot create a struct with no fields" issue for types with
> events in their interfaces.

Great, let's not chase an old bug then, because when we mark this bug as FIXED, it will mean that Banshee has switched to only be compatible with dbus#0.8, so you don't need to worry about this anymore.

What is left to figure out here is d-feet's inability to work with Banshee+Dbus#0.8, and the culprit may be in the big patch that Chow pointed out (3c278975c59cf8dbcfdfdf1184e4cf9c89de7ce3). One should try to bisect that big patch bit by bit in smaller patches, and try to find out what exact change in it causes d-feet to be unusable.
Comment 16 Chow Loong Jin 2014-06-17 18:41:29 UTC
If it helps, it's not just d-feet that has problems -- gdbus introspect triggers the issue as well.
Comment 17 Nicholas Little 2014-06-17 23:35:53 UTC
Created attachment 278636 [details] [review]
dbus-sharp patch

Well, that was unexpected. This patch works corrects the issue in d-feet on my system, though I'm not entirely sure what the difference really is, I'll look again tomorrow...

Chow, do you mind compiling your dbus-sharp with it and seeing if it sorts out gdbus introspect too?

This is with Bertrand's patch applied to Banshee, in case you get a different result.

Cheers!
Comment 18 Andrés G. Aragoneses (IRC: knocte) 2014-06-18 09:52:40 UTC
(In reply to comment #17)
> Created an attachment (id=278636) [details] [review]
> dbus-sharp patch
> 
> Well, that was unexpected. This patch works corrects the issue in d-feet on my
> system, though I'm not entirely sure what the difference really is, I'll look
> again tomorrow...
> 
> Chow, do you mind compiling your dbus-sharp with it and seeing if it sorts out
> gdbus introspect too?
> 
> This is with Bertrand's patch applied to Banshee, in case you get a different
> result.
> 
> Cheers!

Well, if that patch fixes the issue, hats off! (Haven't tested it yet.)

I recommend you to, though:

1. I assume that encapsulating that logic in the Error class is a "refactoring", which doesn't really help in knowing which bit of 3c278975c59cf8dbcfdfdf1184e4cf9c89de7ce3 was wrong. I recommend creating a patch that doesn't introduce this new class yet, but just fixes the issue, and then after this patch is merged, propose a 2nd patch that introduces the Error class as a refactoring that doesn't change behaviour.
2. For the second patch, it seems the Error class would not be used by the consumers of dbus-sharp, so then mark it as internal.
3. Propose the 1st and 2nd patch on github as pull requests, please.
4. Use a better 1st line of the commit message than "Revert: 3c278975c59cf8dbcfdfdf1184e4cf9c89de7ce3", because you're actually not reverting that commit, just reverting one bit of it. So adding the word "partially" could maybe be enough.
Comment 19 Nicholas Little 2014-06-18 15:12:05 UTC
Created attachment 278693 [details] [review]
dbus-sharp patch v2

Try this one instead please, and if it works for you guys too I'll make a pull request with it on GitHub.

Cheers!
Comment 20 Andrés G. Aragoneses (IRC: knocte) 2014-06-18 18:34:51 UTC
Patch works for me! Please propose it upstream.
Comment 21 Andrés G. Aragoneses (IRC: knocte) 2015-07-17 14:15:21 UTC
(In reply to Andrés G. Aragoneses (IRC: knocte) from comment #20)
> Patch works for me! Please propose it upstream.

For the record, that patch went in:
https://github.com/mono/dbus-sharp/commit/4b69a4724b528180493c2474457464b7926c18e0

And version 0.8.1 of dbus-sharp included it. So I've finally raised the dependency in banshee, by committing Bertrand's patch.