Add --replace-all option for development
#751
Draft
Commits on Dec 27, 2023
-
Fix quit dbus action for the GUI
This action was registered, but did not work because the handler accepted too few arguments and also called itself recursively. This fixes both, calling the 'quit' action (as exposed through dbus by gtk automatically) now actually quits the GUI.
-
hamster-cli: Flatten commandline handling flow
This simplifies the flow a bit by putting the handling of "add" with arguments at the same level of all other application actions instead of below it, needing one fewer level of indentation. Behavior should be fully unchanged. Diff best viewed while ignoring whitespace changes.
-
Print "hamster-service up" later and use logger
This logs the message after the storage class was actually instantiated and registered on the bus, rather than before.
-
hamster-windows-service: Use logger
This converts the single print in this script to use a logger instance (like hamster-service already uses).
-
Fix dbus name claiming race condition
This refactors the way that dbus names are claimed for hamster-service and hamster-window-service. Instead of checking if the name is already claimed beforehand, this tries to claim the and checks the result. This fixes a minor race condition when a service is started twice at the same time. Before, the name would appear to be free when it was checked, but when the script proceeded to actually claim the name, it could have been claimed by another instance already. Because do_not_queue was not yet used, this means that the claim would probably not even fail, but leave a pointless process running. This commit refactors the code structure to introduce a claim_bus_name helper, which claims the name with do_not_queue=True so it either fails when the name is already taken, or is sure the name is claimed. This also raises the bus claiming code out of the Storage and WindowServer classes into the main script, since that is a better place to decide to abort startup and log messages about this. This refactoring also prepares for implementing a `--replace` option in a subsequent commit.
-
Implement --replace option for all three binaries
This makes them replace any currently running GUI, storage service or windows service. This is primarily useful during development, to prevent having to kill these services manually. For the two services, this is implemented properly by trying to claim the bus name and if it is taken, calling the quit method on the current instance while the existing claim is kept in the dbus request queue. This ensures that as soon as the name is released, it is claimed again by the new process (preventing a race condition where a service autostart could occur in between). For the GUI, this race condition is not prevented due to the way Gtk/Gio.Application claims its name, but this should be minor enough an issue to not be problematic (especially since the user can always easily quit the GUI manually beforehand). This commit implements most of projecthamster#746.
-
Remove .py extension from main scripts
This makes the filenames equal to the name they get when installed, which is more consistent and makes it easier to call them programmatically (this prepares for implementing `--replace-all` in subsequent commit). The .py extension was added in commit 774c315 (give .py extension to hamster-*), on account of letting xgettext understand these are python files. Currently, there is no working (or at least not documented) xgettext flow anyway, and xgettext seems to support passing an explicit language parameter, so this can probably be used, removing the need for these explicit extensions.
-
-
Revert "Remove .py extension from main scripts"
This reverts commit 767f80a22593757aec10b3f6ab3a4ef3ab8a0809.
-
Detach/daemonize service processes
This lets the service commands (hamster-service and hamster-windows-service) daemonize after initialization. This ensures they are detached from their parent process, and allows a caller to wait for them to return to know whether startup succeeded or not.
-
Implement --replace-all option
This replaces the GUI and two background services (by using the recently added `--replace` options). This is helpful during development to easily replace the existing deamons (even when installed system-wide) with a development version, without having to resort to fragile pkill commands. This also updates the README to recommend using this option instead of kill commands. This also removes a note about not calling windows via dbus, since that does not seem to be true with the current codebase. This fixes projecthamster#746
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.