ubuntu-phone team mailing list archive

[Barry Warsaw <barry@xxxxxxxxxx>]

On Dec 13, 2013, at 11:05 PM, Dimitri John Ledkov wrote:

>dbus apis are hard, hence a few dbus apis are provided as shared-libraries
>(essentially generate client code and compile it into shared library).  When
>looking at dbus api, as if it's a compiled generated shared library the
>ABI/API break is obvious and one typically renames the library package as per
>Debian policy.
>
>Looking at other well-behaved dbus exported daemons, they encode
>version name in the well-known name (e.g. PolicyKit1, RealtimeKit1,
>UDisks2, login1) or the interface names (e.g. com.ubuntu.Upstart0_6 ,
>althought that might be simply a cool looking emoticon 0_6 )

Yep, and we have a wishlist item open for a C wrapper around the D-Bus API.
(LP: #1202299).  It hasn't been high enough priority... yet. ;)

>Keeping ApplyUpdate() as it was, and introducing e.g.  ApplyAsyncUpdate()
>would have been better.

Sure, but as mentioned previously, this would mean that the re-download bug
would be not be fixed from the user's perspective, because it is only possible
with the async version.  And of course, keeping multiple versions of an API
around does complicate the code base, but such is life.  Fortunately, with
just one client, it wouldn't be a long term backward compatibility hack.

>Or e.g. using nih-dbus-tool from the start which explicitly supports
>both sync and async dbus method calls.

Interesting.  I didn't know about nih-dbus-tool.  Is there a reason this
little gem is hidden away in the libnih package? :)

>coordinated promotion is easy, simply make sure the package will get
>stuck in the -proposed pocket and not promoted into release pocket.
>e.g. system-image-update declaring breaks: system-settings (<< X.Y)
>would have been sufficient. But i'm not sure it could have been used
>effectively in this case, thanks to daily-release version numbers are
>not predictable and one cannot know ahead of time which version number
>of fixed system-settings will be.

Right.  The Breaks process only works if you have a definitive version number
at which it *won't* break.

>.... autogenerate tests against all dbus api calls used and validate that
>they do what one expect, make it runable on stock i386/amd64 and make sure
>the reverse depends call them out ....  very heavy weight, but it could have
>worked. also quite pointless, as changing dbus api will be hard (even if
>coordinated). Imho it's best handled by policy, e.g. unit-test and validate
>your dbus api calls to not change (keep api/abi stability) I'm not sure if
>there is a tool like that already, e.g. i do know that libnih-dbus has
>test-suite generators (althought it still does have a lot of boiler plate).

Really the issue here is that we need more integration tests.  Unit tests or
component tests alone wouldn't have caught this because from s-i's perspective
everything was fine.  It was only when integrated with the current version of
system-settings that the problem was evident (even if barely, given how
quickly the message disappears).

>Did the ApplyUpdates() dbus signature change? or did it really changed
>behaviour i.e. instead of returning string and empty string got
>returned?

The return signature did change, unnecessarily so I suppose in hindsight,
since Didier's patch restores the return value.  It's useless though because
the asynchronous version cannot provide anything more than the empty string.
Thus if there are errors in the ApplyUpdate() action, the ui won't see it,
until it's prepared to handle the Rebooting signal.  Going forward, there's a
new bug opened to clean this up, with bug tasks in both s-i and s-s, so that
change will at least be better coordinated.

Cheers,
-Barry

Attachment: signature.asc
Description: PGP signature