Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Three D-Bus services #538

Merged
merged 4 commits into from
Feb 5, 2020
Merged

Three D-Bus services #538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f9d2a84
separate GtkApp and WindowServer
ederag Feb 1, 2020
0cffe9c
remove unused edit method
ederag Feb 3, 2020
a0b7f3b
rename Hamster.GtkApp => Hamster.GUI
ederag Feb 5, 2020
b6dbfaa
rename prefs => preferences
ederag Feb 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ autom4te.cache
m4
gnome-doc-utils.make
hamster-time-tracker-*.tar.gz
org.gnome.hamster.service
.lock-wscript
.lock-waf*
build
Expand Down
2 changes: 1 addition & 1 deletion data/hamster.metainfo.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
</ul>
</description>

<launchable type="desktop-id">hamster.desktop</launchable>
<launchable type="desktop-id">org.gnome.Hamster.GUI.desktop</launchable>

<screenshots>
<screenshot type="default">
Expand Down
10 changes: 10 additions & 0 deletions data/hamster.desktop.in → data/org.gnome.Hamster.GUI.desktop.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,15 @@ Terminal=false
Name=Hamster
Comment=Your personal time keeping tool
Icon=hamster
DBusActivatable=true
Exec=@BINDIR@/hamster
Categories=GNOME;GTK;Utility;
Actions=overview;add;

[Desktop Action overview]
Exec= @BINDIR@/hamster overview
Name=overview

[Desktop Action add]
Exec= @BINDIR@/hamster add
Name=add
2 changes: 1 addition & 1 deletion data/wscript_build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ bld.install_files('${DATADIR}/icons/hicolor/scalable/apps','art/scalable/hamster

bld.install_files('${DATADIR}/metainfo', 'hamster.metainfo.xml')

for filename in ["hamster.desktop"]:
for filename in ["org.gnome.Hamster.GUI.desktop"]:
bld(features = "subst",
source= "%s.in" % filename,
target= "%s" % filename,
Expand Down
3 changes: 3 additions & 0 deletions org.gnome.Hamster.GUI.service.in
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[D-BUS Service]
Name=org.gnome.Hamster.GUI
Exec=@BINDIR@/hamster --gapplication-service
0 org.gnome.hamster.Windows.service.in → org.gnome.Hamster.WindowServer.service.in
View file
Open in desktop
File renamed without changes.
0 org.gnome.hamster.service.in → org.gnome.Hamster.service.in
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion po/POTFILES.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
data/date_range.ui
data/edit_activity.ui
data/preferences.ui
data/hamster.desktop.in
data/org.gnome.Hamster.GUI.desktop.in
data/org.gnome.hamster.gschema.xml
src/hamster-cli.py
src/hamster/about.py
Expand Down
1 change: 0 additions & 1 deletion po/POTFILES.skip
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
data/hamster.desktop.in
build/
31 changes: 20 additions & 11 deletions src/hamster-cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,18 +92,23 @@ def fact_dict(fact_data, with_date):


class Hamster(gtk.Application):
"""Hamster gui.

Can be accessed across D-Bus with the 'org.gnome.Hamster.GUI' id.
"""

def __init__(self):
# inactivity_timeout: How long (ms) the service should stay alive
# after all windows have been closed.
gtk.Application.__init__(self,
application_id="org.gnome.Hamster.WindowServer",
application_id="org.gnome.Hamster.GUI",
#inactivity_timeout=10000,
register_session=True)

self.about_controller = None # 'about' window controller
self.add_controller = None # "add activity" window controller
self.overview_controller = None # overview window controller
self.prefs_controller = None # settings window controller
self.preferences_controller = None # settings window controller

self.connect("startup", self.on_startup)
self.connect("activate", self.on_activate)
Expand All @@ -114,7 +119,7 @@ def __init__(self):
self.add_actions()

def add_actions(self):
for name in ("about", "add", "overview", "prefs"):
for name in ("about", "add", "overview", "preferences"):
action = gio.SimpleAction.new(name, None)
action.connect("activate", self.on_activate_window)
self.add_action(action)
Expand Down Expand Up @@ -158,11 +163,11 @@ def _open_window(self, name, data=None):
self.overview_controller = Overview()
logger.debug("new Overview")
controller = self.overview_controller
elif name == "prefs":
if not self.prefs_controller:
self.prefs_controller = PreferencesEditor()
elif name == "preferences":
if not self.preferences_controller:
self.preferences_controller = PreferencesEditor()
logger.debug("new PreferencesEditor")
controller = self.prefs_controller
controller = self.preferences_controller

window = controller.window
if window not in self.get_windows():
Expand Down Expand Up @@ -376,7 +381,7 @@ def version(self):
* activities: List all the activities names, one per line.
* categories: List all the categories names, one per line.

* overview / prefs / add / about: launch specific window
* overview / preferences / add / about: launch specific window

* version: Show the Hamster version

Expand Down Expand Up @@ -430,10 +435,13 @@ def version(self):

if args.action in ("start", "track"):
action = "add" # alias
elif args.action == "prefs":
# for backward compatibility
action = "preferences"
else:
action = args.action

if action in ("about", "add", "overview", "prefs"):
if action in ("about", "add", "overview", "preferences"):
if action == "add" and args.action_args:
assert not unknown_args, "unknown options: {}".format(unknown_args)
# directly add fact from arguments
Expand All @@ -443,8 +451,9 @@ def version(self):
else:
app.register()
app.activate_action(action)
logger.debug("run")
status = app.run([sys.argv[0]] + unknown_args)
run_args = [sys.argv[0]] + unknown_args
logger.debug("run {}".format(run_args))
status = app.run(run_args)
logger.debug("app exited")
sys.exit(status)
elif hasattr(hamster_client, action):
Expand Down
29 changes: 17 additions & 12 deletions src/hamster-windows-service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
#!/usr/bin/env python3
# nicked off hamster-service

from gi.repository import GLib as glib
import dbus, dbus.service
import dbus
import dbus.service
import subprocess

from dbus.mainloop.glib import DBusGMainLoop
from gi.repository import GLib as glib

DBusGMainLoop(set_as_default=True)
loop = glib.MainLoop()
Expand All @@ -13,7 +16,12 @@
quit()


# maintain just one instance. this code feels hackish again
# Legacy server. Still used by the shell-extension.
# new code could access the org.gnome.Hamster.GUI actions directly
# http://lazka.github.io/pgi-docs/Gio-2.0/classes/Application.html#Gio.Application
# > The actions are also exported on the session bus,
# and GIO provides the Gio.DBusActionGroup wrapper
# to conveniently access them remotely.
class WindowServer(dbus.service.Object):
__dbus_object_path__ = "/org/gnome/Hamster/WindowServer"

Expand All @@ -29,24 +37,21 @@ def Quit(self):
"""Shutdown the service"""
self.mainloop.quit()


@dbus.service.method("org.gnome.Hamster.WindowServer")
def edit(self, id=None):
dialogs.edit.show(self.app, fact_id = id)
def _open_window(self, name):
subprocess.run("hamster {} &".format(name),
shell=True)

@dbus.service.method("org.gnome.Hamster.WindowServer")
def overview(self):
dialogs.overview.show(self.app)
self._open_window("overview")

@dbus.service.method("org.gnome.Hamster.WindowServer")
def about(self):
dialogs.about.show(self.app)
self._open_window("about")

@dbus.service.method("org.gnome.Hamster.WindowServer")
def preferences(self):
dialogs.prefs.show(self.app)


self._open_window("prefs")


if __name__ == '__main__':
Expand Down
14 changes: 10 additions & 4 deletions wscript
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,20 @@ def build(bld):
)

bld(features="subst",
source= "org.gnome.hamster.service.in",
target= "org.gnome.hamster.service",
source= "org.gnome.Hamster.service.in",
target= "org.gnome.Hamster.service",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't mind, I'd like to be sure I understand the difference between the org.gnome.Hamster and org.gnome.Hamster.GtkApp (currently called org.gnome.Hamster.Windows) services.

Because what they do might have an impact on the naming to follow if we want Hamster to work properly inside Flatpak. (which I'd very much want)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

org.gnome.Hamster is the database server:

hamster/src/hamster-service.py

Line 46 in 72a8e31

bus_name = dbus.service.BusName("org.gnome.Hamster", bus=self.bus)

org.gnome.Hamster.WindowServer is the legacy application windows server:

hamster/src/hamster-windows-service.py

Line 24 in 72a8e31

bus_name = dbus.service.BusName("org.gnome.Hamster.WindowServer", bus=self.bus)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be aware that this "little" change with "hamster" becoming "Hamster" can cause quite some headaches for users already having those files.
If users start complaining about services not starting any more for strange reasons, they need to perform a "sudo rm /usr/share/dbus-1/services/org.gnome.hamster.*" to remove the old files

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the warning,
but beware that postings on already merged issues are hard to remember.
So dbus launch is not case sensitive ?
Anyway there have been several other issues with old install remnants.
Warning set on the release page, in the wiki Troubleshooting section,
and updated README (ab5e9ea).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So dbus launch is not case sensitive ?

It is.

It's just that if people installed Hamster manually (i.e not with a package) then the old service files will be kept on the system, in parallel to the old ones, when those users updated their Hamster installation by simply running waf install from a source tree.

Given that you also renamed the desktop files, the old desktop files would also linger on the system in the same way.

That has the potential to cause lots of confusion.

Personally, I'm of the opinion that people who compile/install their software manually from sources, without going through the safer packaging routes, are expected to be capable of dealing with this kind of problems, and the notice you added to the wiki/readme seems perfectly sufficient to me.

Side note:

because to comply with freedesktop specifications some files are camel-cased now.

The spec mandates a few things like the dotted-notation, the absence of hyphens (-) and that each component starts with a letter. It does not mandate capitalizing the first letter of the last component. This is merely the emerging convention, but lots of apps use lower-case and that doesn't cause any problem and never should. I assume you did that on my suggestion, so I apologize for not having made that sufficiently clear.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... the notice you added to the wiki/readme seems perfectly sufficient to me.

Thanks for the discussion, it's reassuring.

I assume you did that on my suggestion

Partly, but your suggestions were perfectly inline with both
the freedesktop recommendations and the flatpak conventions.
Let's get to the bottom of it in #547.
If we can, it would be much better to make all file changes in one go.

install_path="${DATADIR}/dbus-1/services",
)

bld(features="subst",
source= "org.gnome.hamster.Windows.service.in",
target= "org.gnome.hamster.Windows.service",
source= "org.gnome.Hamster.GUI.service.in",
target= "org.gnome.Hamster.GUI.service",
install_path="${DATADIR}/dbus-1/services",
)

bld(features="subst",
source= "org.gnome.Hamster.WindowServer.service.in",
target= "org.gnome.Hamster.WindowServer.service",
install_path="${DATADIR}/dbus-1/services",
)

Expand Down