[0.7.6] the add_action api changed creating issues for clients

[Sebastien Bacher]

That upstream change seems an api change:
https://git.gnome.org/browse/libnotify/commit/?id=2b4ab4d22f42dd264a4ddfa68b02bba0d6c71b0b

Simple testcase:

"from gi.repository import GLib, Notify

def action_callback():
    print action_id
notification = Notify.Notification.new("", "", "")
notification.add_action("one", "two", action_callback, None, None)"

that code worked before and is complaining about the number of arguments (5 instead of 6) after the update

We reverted the change because it was creating problems in unity8 tests (and likely in code in the archive).

Before dropping the revert we should probably figure out if:
- the upstream change is correct is buggy
- why the number of argument changed (it seems from the commit it shouldn't)
- what in the archive use that api and needs to be ported
- how we can make code support both old/new apis at the same time

[Sebastien Bacher (seb128)]

Hey Martin, can you help to answer to some of the questions we have there?

[Michał Sawicz (saviq)]

lp:~saviq/unity8/fix-new-libnotify "fixes" this for unity8, but it doesn't seem correct...

[Martin Pitt (pitti)]

It looks correct to me in the sense that the free_func is not an argument which should be exposed to Python. pygobject itself needs to manage these to free the passed user_data (and its python wrapper). The bit that looks wrong to me is the dropping of the two (allow-none)s, I asked about that in the upstream bug.

[Sebastien Bacher (seb128)]

> It looks correct to me in the sense that the free_func is not an argument which should be exposed to Python.

so it's normal that the signature changed from 6 arguments to 5 arguments? :/ Do you have a recommendation on how to deal with the transition/make code work with both old and new api?

[Vadim Rutkovsky (roignac)]

This change crashes gnome-music as it expects the function to have 5 arguments in libnotify 0.7.6

[John Stowers (nzjrs)]

And this crashes gnome-tweak-tool for the exact reasons Vadim mentioned.

[Sebastien Bacher (seb128)]

well, that's what you get for changing apis in an incompatible ways, some of the clients use the old api, some the new one...

[Martin Pitt (pitti)]

> Do you have a recommendation on how to deal with the transition/make code work with both old and new api?

Unity could try the new API first, catch the AttributeError (or whichever exception is thrown) and re-call with the old API.

[Martin Pitt (pitti)]

This is the relevant GIR delta, for the record.

[Martin Pitt (pitti)]

So, this madness needs to stop at some point, so let's just fix the reverse dependencies and get back in sync. This should be much easier now that PyGObject makes tailing "allow-none" arguments optional and accepts not specifying them explicitly at all. However, we must pay attention as trailing arguments are now considered as user_data list, and passed to callbacks.

[1] Skipping user_data and free_func:
  python3 -c 'from gi.repository import GLib, Notify; Notify.init("test"); n = Notify.Notification.new("summary", "msg"); n.add_action("my-id", "my_label", (lambda n, id, userdata: print(id, userdata)), None, None); n.show(); GLib.MainLoop().run()'

Wrongly gives "None" as user-data in the callback on Ubuntu, but no user_data callback argument at all with the upstream API.

[2] Specifying user_data, skipping free_func:
  python3 -c 'from gi.repository import GLib, Notify; Notify.init("test"); n = Notify.Notification.new("summary", "msg"); n.add_action("my-id", "my_label", (lambda *u: print(u)), "foo"); n.show(); GLib.MainLoop().run()'

Gives "foo" as user_data in the callback for both upstream and Ubuntu.

[3] Specifying both user_data and free_func:
  python3 -c 'from gi.repository import GLib, Notify; Notify.init("test"); n = Notify.Notification.new("summary", "msg"); n.add_action("my-id", "my_label", (lambda *u: print(u)), "foo", None); n.show(); GLib.MainLoop().run()'

Gives "foo", None as two user_data callback args upstream, but just "foo" on Ubuntu, as the extra None is interpreted as free_func.

I checked all rdepends of gir1.2-notify-0.7. "n/a" means the package doesn't use add_action() at all.

autokey: n/a
blueman: OK; explicit None user_data, callback ignores *args
firewall-applet: n/a
friends-dispatcher: n/a
gnome-music: OK (case [2]), explicit None user_data, callback expects one user_data arg
gnome-tweak-tool: BROKEN on ubuntu (case [3]), supplies and expects two user_data args
landscape-client-ui: n/a
lubuntu-software-center: n/a
mailnag: OK (case [1]), passes and expects 1 user_data
nautilus-pastebin: n/a
pitivi: n/a
solaar: n/a
system-config-printer-gnome: BROKEN on ubuntu (case [1]), supplies and expects no user_data
transmageddon: n/a
unity-mail: n/a
unity8-autopilot: BROKEN with upstream, case [3], supplies two user_data, expects one in callback

So in summary, we need to fix tests/autopilot/unity8/shell/emulators/create_interactive_notification.py in unity8-autopilot, then sync libnotify, then system-config-printer and gnome-tweak-tool will get fixed and we finally are back to one and the same API everywhere.

[Launchpad Janitor (janitor)]

This bug was fixed in the package unity8 - 8.02+15.04.20150109.2-0ubuntu1

---------------
unity8 (8.02+15.04.20150109.2-0ubuntu1) vivid; urgency=low

  [ Michał Sawicz ]
  * We don't need the SIGSTOP in main() any more.
  * Add a test to make sure the shell always starts disabled until it is
    enabled by a complete PAM interaction.

  [ Leo Arias ]
  * Added an autopilot test for the edges demo.

  [ Nick Dedekind ]
  * Unhook Lights interface from indicator widgets (LP: #1385331)

  [ Albert Astals ]
  * Fix going to scopes when the Manage Dash is open (LP: #1403464)
  * QSortFilterProxyModelQML -> UnitySortFilterProxyModelQML
  * Clip the Scopes List header
  * Fix ScopesList not being under finger
  * Make waitForRendering with no item fail instead of crash
  * Disable Dash horizontal scroll while Navigation InverseMouseArea is
    pressed (LP: #1403048)
  * Test: Make sure the mouse area is enabled before clicking into it
  * Test: We actually need to click on the customBackButton and not on
    backButton
  * Test: By default mouseX act on the middle

  [ Michael Terry ]
  * Don't block handling power events on loading the greeter's qml and
    the background image.
  * Show OSK above the wizard. (LP: #1401213)
  * Unify the name of the Greeter DBus test, make it use our standard
    binary test macro (which also nicely exports xml results), and make
    the test a little more robust against timing issues.
  * Add a test to make sure the shell always starts disabled until it is
    enabled by a complete PAM interaction.

  [ Martin Pitt ]
  * tests: Fix Notify.Notification.add_action invocation to work also
    with unpatched libnotify API. (LP: #1223401)

  [ Rodney Dawes ]
  * Depend on :native version of g++ to allow cross-compiling to work.
    (LP: #1353855)
 -- Ubuntu daily release <email address hidden> Fri, 09 Jan 2015 10:43:06 +0000

[Martin Pitt (pitti)]

Synced libnotify with Debian. Will close once it propagated into vivid.