projecthamster / hamster

GNOME time tracker
http://projecthamster.org
GNU General Public License v3.0
1.07k stars 250 forks source link

Add `--replace-all` option for development #751

Open matthijskooijman opened 9 months ago

matthijskooijman commented 9 months ago

This PR adds a --replace-all commandline option to make hamster send quit commands to all running services and replace them with new version. This is convenient during development, to test a new version without having to manually kill the old version (and hope it is not autorestarted by dbus in the meantime).

I'm marking this as a draft, since the daemonization added seems to interfere with the glib mainloop somehow.

What I did was have the hamster-service and hamster-windows-service commands daemonize after startup (e.g. after they successfully claimed their dbus name, or gave up trying to). This allows hamster --replace-all, which calls hamster-service --replace and hamster-windows-service --replace, to wait until both background services are started before continuing, and also report failure if either one fails to be replaced.

However, it seems that doing the fork() call needed for this after the from gi.repository import Gtk as gtk import (which is done in hamster/__init__.py early for "performance"), breaks some Glib internals (I noticed this because the file monitor for changes to the python source stopped working, and later also got timeouts during startup (Error creating proxy: Error calling StartServiceByName for org.gtk.vfs.Daemon: Timeout was reached (g-io-error-quark, 24)).

I'm not quite sure how to resolve this yet - Messing up glib is obviously unacceptable. Reordering things so the fork happens before the Gtk import, but still after startup (or at least after dbus name claim) could work, but is not going to pretty and is probably fragile for future refactoring. Not doing the fork at all seems the most likely option, but that could mean --replace-all would just have to do a fixed timeout, or wait for some message on stdout or another file descriptor, or a signal or some other IPC, which are not so nice and/or more complex. Maybe waiting for a dbus NameOwnerCange could work (and maybe we could match the pid, or just be ok with any change) and be somewhat elegant.

This needs a bit more thought, but I'm out of time for today (and tomorrow). I did want to share the progress so far, hence this PR.

github-actions[bot] commented 9 months ago

Automatically generated build artifacts for commit 4109d79b63919ad64952450aba629dfa11738392 (note: these links will expire after some time):

To test this PR, download and unzip the flatpak application above and then install with:

flatpak install Hamster.flatpak