Merge ~khurshid-alam/ubuntu/+source/rhythmbox:master into ~ubuntu-desktop/ubuntu/+source/rhythmbox:ubuntu/master

Proposed by Khurshid Alam
Status: Needs review
Proposed branch: ~khurshid-alam/ubuntu/+source/rhythmbox:master
Merge into: ~ubuntu-desktop/ubuntu/+source/rhythmbox:ubuntu/master
Diff against target: 238 lines (+198/-0)
4 files modified
debian/control (+3/-0)
debian/control.in (+3/-0)
debian/patches/series (+1/-0)
debian/patches/zeitgeist-Use-zeitgeist-via-Gobject-introspection.patch (+191/-0)
Reviewer Review Type Date Requested Status
Jeremy Bícha Needs Fixing
Review via email: mp+362491@code.launchpad.net

Commit message

Port zeitgeist plugin to Python3

Zeitgeist already provides python bindings. Convert them to python3 and use them in rbzeitgeist plugin. That way we can avoid using gir bindings and various manual mappings from zeitgeist interpretation to string. Fixes LP: #1813813

To post a comment you must log in.
Revision history for this message
fossfreedom (fossfreedom) wrote :

I know I am not a reviewer ...

From a quick look it seems like a good conversion.

Given that no one uses this plugin other than Unity wouldn't this be a good opportunity to split the plugin into its own package? This would align more to the current Debian rhythmbox package that no longer ships zeitgeist and reduce the overhead of ubuntu specific patches.

Revision history for this message
Khurshid Alam (khurshid-alam) wrote :

@fossfreedom

Perhaps so. But that means finding a repo, work on makefile etc which will take more time. I would appreciate some help regarding this.

Meanwhile this can go into disco and perhaps @jbicha can merge it back on debian.

Revision history for this message
Khurshid Alam (khurshid-alam) wrote :

@fossfreedom Also synapse used to use it. Though I don't know if they still uses but their zeitgeist plugin should still work as the there hasn't been any API change for zeitgeist in recent years.

Revision history for this message
Jeremy Bícha (jbicha) wrote :

Debian might accept our rhythmbox-plugin-zeitgeist package once we get the plugin working.

I would normally expect something like this to be forwarded to https://gitlab.gnome.org/GNOME/rhythmbox/merge_requests

Are you aware of this comment from the zeitgeist Debian changelog?
https://tracker.debian.org/news/939461/accepted-zeitgeist-101-02-source-amd64-all-into-unstable/

  "Drop python3-zeitgeist package as the binding is not compatible with
  python3, for python3, GIR binding should be used instead. Reintroduce the
  python2 binding for now as we still have one rdependency"

Maybe those extra files you added should be shipped by the zeitgeist package instead?

Maybe Ricotz has some time to help review what you're trying to do here.

Revision history for this message
Khurshid Alam (khurshid-alam) wrote :

@jbicha

Ah. Right. It has already been ported. See https://gitlab.gnome.org/GNOME/rhythmbox/merge_requests/18/

Perhaps you could cherry pick the merge into debian and then it can re-sync into Ubuntu ?

Force pushed with author's commit.

And it's working. I checked.

Revision history for this message
Jeremy Bícha (jbicha) wrote :

Khurshid, have you ever used git-buildpackage's patch-queue feature (gbp pq)?

https://wiki.ubuntu.com/DesktopTeam/git#Pick_some_upstream_commits
https://manpages.debian.org/unstable/gbp-pq

Specifically, we don't edit upstream directly, but we use debian/patches/ so gbp pq helps create and modify those patches.

review: Needs Fixing
Revision history for this message
Khurshid Alam (khurshid-alam) wrote :

Never used gbp-pq, but trying now.

cceffdc... by Khurshid Alam

 Zeigeist-plugin: Cherry-pick fix for upstream merge

Revision history for this message
Khurshid Alam (khurshid-alam) wrote :

I simply added the patch with gbp pq and modify debian/control. I left updating changelog, because I never do that for other upstream merge.

Unmerged commits

cceffdc... by Khurshid Alam

 Zeigeist-plugin: Cherry-pick fix for upstream merge

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/debian/control b/debian/control
index 5de2fda..8e6fd7f 100644
--- a/debian/control
+++ b/debian/control
@@ -187,8 +187,11 @@ Depends: ${misc:Depends},
187 ${shlibs:Depends},187 ${shlibs:Depends},
188 rhythmbox (>= ${gnome:Version}),188 rhythmbox (>= ${gnome:Version}),
189 rhythmbox (<< ${gnome:NextVersion}),189 rhythmbox (<< ${gnome:NextVersion}),
190 gir1.2-rb-3.0 (= ${binary:Version}),
190 gir1.2-glib-2.0,191 gir1.2-glib-2.0,
191 gir1.2-peas-1.0,192 gir1.2-peas-1.0,
193 gir1.2-zeitgeist-2.0,
194 python3-gi,
192 zeitgeist-core195 zeitgeist-core
193Description: zeitgeist plugin for rhythmbox music player196Description: zeitgeist plugin for rhythmbox music player
194 Rhythmbox is a very easy to use music playing and management program197 Rhythmbox is a very easy to use music playing and management program
diff --git a/debian/control.in b/debian/control.in
index 049e836..e66c5e4 100644
--- a/debian/control.in
+++ b/debian/control.in
@@ -183,8 +183,11 @@ Depends: ${misc:Depends},
183 ${shlibs:Depends},183 ${shlibs:Depends},
184 rhythmbox (>= ${gnome:Version}),184 rhythmbox (>= ${gnome:Version}),
185 rhythmbox (<< ${gnome:NextVersion}),185 rhythmbox (<< ${gnome:NextVersion}),
186 gir1.2-rb-3.0 (= ${binary:Version}),
186 gir1.2-glib-2.0,187 gir1.2-glib-2.0,
187 gir1.2-peas-1.0,188 gir1.2-peas-1.0,
189 gir1.2-zeitgeist-2.0,
190 python3-gi,
188 zeitgeist-core191 zeitgeist-core
189Description: zeitgeist plugin for rhythmbox music player192Description: zeitgeist plugin for rhythmbox music player
190 Rhythmbox is a very easy to use music playing and management program193 Rhythmbox is a very easy to use music playing and management program
diff --git a/debian/patches/series b/debian/patches/series
index febc85f..f95255e 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -8,3 +8,4 @@ multiarch_fallback.patch
8make-shuffle-repeat-proper-toggle-actions.patch8make-shuffle-repeat-proper-toggle-actions.patch
9restore-traditional-menubar.patch9restore-traditional-menubar.patch
100002-grilo-container-max-tracks.patch100002-grilo-container-max-tracks.patch
11zeitgeist-Use-zeitgeist-via-Gobject-introspection.patch
diff --git a/debian/patches/zeitgeist-Use-zeitgeist-via-Gobject-introspection.patch b/debian/patches/zeitgeist-Use-zeitgeist-via-Gobject-introspection.patch
11new file mode 10064412new file mode 100644
index 0000000..f7b2dc6
--- /dev/null
+++ b/debian/patches/zeitgeist-Use-zeitgeist-via-Gobject-introspection.patch
@@ -0,0 +1,191 @@
1From: joerg eichhorn <je@sys-mail.net>
2Date: Sat, 10 Nov 2018 16:42:58 +0100
3Subject: zeitgeist: Use zeitgeist via Gobject introspection
4
5Fixes #1340
6Fixes https://bugs.launchpad.net/ubuntu/+source/rhythmbox/+bug/1313914
7Fixes https://bugzilla.gnome.org/show_bug.cgi?id=707527
8
9From 33bd55de358325ff775cf903371949ca26198d41 Mon Sep 17 00:00:00 2001
10From: vrishab <gnome.vrb@gmail.com>
11Date: Sun, 27 Nov 2016 18:29:33 +0530
12Subject: [PATCH] zeitgeist: Use zeitgeist via Gobject introspection
13
14(cherry picked from commit efcb4653a13a723b5b99fb5e387f60d5110ae840)
15---
16 plugins/rbzeitgeist/rbzeitgeist.plugin.in | 2 +-
17 plugins/rbzeitgeist/rbzeitgeist.py | 95 ++++++++++++++++++++-----------
18 2 files changed, 63 insertions(+), 34 deletions(-)
19
20diff --git a/plugins/rbzeitgeist/rbzeitgeist.plugin.in b/plugins/rbzeitgeist/rbzeitgeist.plugin.in
21index b699ee7..3d362bb 100644
22--- a/plugins/rbzeitgeist/rbzeitgeist.plugin.in
23+++ b/plugins/rbzeitgeist/rbzeitgeist.plugin.in
24@@ -7,4 +7,4 @@ _Name=Zeitgeist
25 _Description=Inform Zeitgeist about your activity
26 Authors=Markus Korn <thekorn@gmx.de>
27 Copyright=Copyright © 2009 Markus Korn
28-Website=http://www.zeitgeist-project.com
29+Website=http://www.rhythmbox.org/
30diff --git a/plugins/rbzeitgeist/rbzeitgeist.py b/plugins/rbzeitgeist/rbzeitgeist.py
31index b04a082..6bba195 100644
32--- a/plugins/rbzeitgeist/rbzeitgeist.py
33+++ b/plugins/rbzeitgeist/rbzeitgeist.py
34@@ -26,20 +26,23 @@
35 # along with this program; if not, write to the Free Software
36 # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
37
38-import time
39+import gi
40 import rb
41+import time
42
43-from gi.repository import GObject, Gio, GLib, Peas
44-from gi.repository import RB
45+gi.require_version('Peas', '1.0')
46+gi.require_version('RB', '3.0')
47+gi.require_version('Zeitgeist', '2.0')
48
49-from zeitgeist.client import ZeitgeistClient
50-from zeitgeist.datamodel import Event, Subject, Interpretation, Manifestation
51+from gi.repository import GObject, Gio, GLib, Peas, Zeitgeist
52+from gi.repository import RB
53
54 try:
55- IFACE = ZeitgeistClient()
56+ logger = Zeitgeist.Log.new()
57+
58 except RuntimeError as e:
59 print("Unable to connect to Zeitgeist, won't send events. Reason: '%s'" % e)
60- IFACE = None
61+ logger = None
62
63 class ZeitgeistPlugin(GObject.Object, Peas.Activatable):
64 __gtype_name__ = 'ZeitgeistPlugin'
65@@ -50,7 +53,8 @@ class ZeitgeistPlugin(GObject.Object, Peas.Activatable):
66
67 def do_activate(self):
68 print("Loading Zeitgeist plugin...")
69- if IFACE is not None:
70+
71+ if logger is not None:
72 shell = self.object
73 shell_player = shell.props.shell_player
74 self.__psc_id = shell_player.connect("playing-song-changed", self.playing_song_changed)
75@@ -61,9 +65,28 @@ class ZeitgeistPlugin(GObject.Object, Peas.Activatable):
76 self.__manual_switch = True
77 self.__current_song = None
78
79- if IFACE.get_version() >= [0, 3, 2, 999]:
80- IFACE.register_data_source("org.gnome.Rhythmbox,dataprovider", "Rhythmbox", "Play and organize your music collection",
81- [Event.new_for_values(actor="application://rhythmbox.desktop")])
82+ event = Zeitgeist.Event.new()
83+ event.set_property("interpretation", "Source Registration")
84+ event.set_property("manifestation", Zeitgeist.USER_ACTIVITY)
85+ event.set_property("actor", "application://rhythmbox.desktop")
86+
87+ datasource = Zeitgeist.DataSource.new()
88+ datasource.set_unique_id("org.gnome.Rhythmbox3,dataprovider")
89+ datasource.set_name("Rhythmbox")
90+ datasource.set_description("Play and organize your music collection")
91+ datasource.set_event_templates([event,])
92+ datasource.set_enabled(True)
93+ datasource.set_timestamp(int(time.time()*1000))
94+
95+ # Register Rhythmbox as a data source with Zeitgeist
96+ # engine.
97+ registry = Zeitgeist.DataSourceRegistry.new()
98+ registry.register_data_source(datasource)
99+
100+ # Save references as register_data_source is async.
101+ self.event = event
102+ self.datasource = datasource
103+ self.registry = registry
104
105 @staticmethod
106 def get_song_info(db, entry):
107@@ -85,10 +108,10 @@ class ZeitgeistPlugin(GObject.Object, Peas.Activatable):
108
109 def playing_song_changed(self, shell, entry):
110 if self.__current_song is not None:
111- self.send_to_zeitgeist_async(self.__current_song, Interpretation.LEAVE_EVENT)
112+ self.send_to_zeitgeist_async(self.__current_song, Zeitgeist.LEAVE_EVENT)
113
114 if entry is not None:
115- self.send_to_zeitgeist_async(entry, Interpretation.ACCESS_EVENT)
116+ self.send_to_zeitgeist_async(entry, Zeitgeist.ACCESS_EVENT)
117
118 self.__current_song = entry
119 GLib.idle_add(self.reset_manual_switch)
120@@ -111,15 +134,16 @@ class ZeitgeistPlugin(GObject.Object, Peas.Activatable):
121 """
122 shell = self.object
123 db = shell.props.db
124+
125 GLib.idle_add(self.send_to_zeitgeist, db, entry, event_type)
126
127 def send_to_zeitgeist(self, db, entry, event_type):
128 song = self.get_song_info(db, entry)
129
130 if self.__manual_switch:
131- manifest = Manifestation.USER_ACTIVITY
132+ manifest = Zeitgeist.USER_ACTIVITY
133 else:
134- manifest = Manifestation.SCHEDULED_ACTIVITY
135+ manifest = Zeitgeist.SCHEDULED_ACTIVITY
136
137 def file_info_complete(obj, res, user_data):
138 try:
139@@ -129,29 +153,34 @@ class ZeitgeistPlugin(GObject.Object, Peas.Activatable):
140
141 uri_mimetype = fi.get_content_type()
142
143- subject = Subject.new_for_values(
144- uri=song["location"],
145- interpretation=unicode(Interpretation.AUDIO),
146- manifestation=unicode(Manifestation.FILE_DATA_OBJECT),
147- origin=song["location"].rpartition("/")[0],
148- mimetype=uri_mimetype,
149- text=" - ".join([song["title"], song["artist"], song["album"]])
150- )
151- event = Event.new_for_values(
152- timestamp=int(time.time()*1000),
153- interpretation=unicode(event_type),
154- manifestation=unicode(manifest),
155- actor="application://rhythmbox.desktop",
156- subjects=[subject,]
157- )
158- IFACE.insert_event(event)
159+ subject = Zeitgeist.Subject.new()
160+ subject.set_property("uri", song["location"])
161+ subject.set_property("interpretation", str(Zeitgeist.AUDIO))
162+ subject.set_property("manifestation", str(Zeitgeist.FILE_DATA_OBJECT))
163+ subject.set_property("origin", song["location"].rpartition("/")[0])
164+ subject.set_property("mimetype", uri_mimetype)
165+ subject.set_property("text", " - ".join([song["title"], song["artist"], song["album"]]))
166+
167+ event = Zeitgeist.Event.new()
168+ event.set_property("timestamp", int(time.time()*1000))
169+ event.set_property("interpretation", str(event_type))
170+ event.set_property("manifestation", str(manifest))
171+ event.set_property("actor", "application://rhythmbox.desktop")
172+ event.add_subject(subject)
173+
174+ logger.insert_event(event)
175
176 f = Gio.file_new_for_uri(song["location"])
177- f.query_info_async(Gio.FILE_ATTRIBUTE_STANDARD_CONTENT_TYPE, Gio.FileQueryInfoFlags.NONE, GLib.PRIORITY_DEFAULT, None, file_info_complete, None)
178+ f.query_info_async(Gio.FILE_ATTRIBUTE_STANDARD_CONTENT_TYPE,
179+ Gio.FileQueryInfoFlags.NONE,
180+ GLib.PRIORITY_DEFAULT,
181+ None,
182+ file_info_complete,
183+ None)
184
185 def do_deactivate(self):
186 print("Deactivating Zeitgeist plugin...")
187- if IFACE is not None:
188+ if logger is not None:
189 shell = self.object
190 shell_player = shell.props.shell_player
191 shell_player.disconnect(self.__psc_id)

Subscribers

People subscribed via source and target branches

to all changes: