feat: Start wfx service on package installation on systems using syst… #324
Conversation
|
What about the (default) configuration? Is that sufficient for "typical" use or does it make more sense not to auto-start wfx on installation but instead having it configured specifically/properly before starting it, then manually? |
Default configuration should be fine. The idea was "sane defaults" :) Personally, I don't like services being auto-enabled, but it seems that this is Debian's policy/convention, so I'd be fine with it. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #324 +/- ##
=======================================
Coverage 79.83% 79.83%
=======================================
Files 94 94
Lines 4413 4413
=======================================
Hits 3523 3523
Misses 645 645
Partials 245 245 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Basically, debian helper scripts perform those actions, based on systemd unit files included during package creation: |
|
@pocketbroadcast: Your latest two commits are missing a |
| priority: extra | ||
| scripts: | ||
| postinstall: .ci/goreleaser/postinstall.sh | ||
| preremove: .ci/goreleaser/preremove.sh |
There was a problem hiding this comment.
As this patch set applies to deb and rpm formats, how is the convention on RPM-based distros, e.g., Fedora, RHEL, ... regarding starting a service after installation? If that differs from Debian's, we need to switch-case here.
There was a problem hiding this comment.
At least fedora does not start services on install, so we need to distinguish here.
One can provide different scripts for rpm and deb, but, unfortunately, that would cause either, some code duplication on the user handling in existing scripts, or (as suggested by nfpm - https://nfpm.goreleaser.com/docs/tips/) adding some quirks to detect on which package system is the actual script is executed:
if you need to know the packaging system I have found it useful to add a /etc/path-to-cfg/package.env that contains _INSTALLED_FROM=apk|deb|rpm which can be sourced into the pre/post install/remove scripts
Further, it seems only wfx is built for deb and rpm, but not wfxctl and wfx-viewer. Is this intentional?
There was a problem hiding this comment.
Further, it seems only wfx is built for deb and rpm, but not wfxctl and wfx-viewer. Is this intentional?
These are released in extra packages, see, e.g., the release artifacts for v0.4.1. This way, one can decide fine-grained what's needed or not for the application scenario at hand.
There was a problem hiding this comment.
Hm, OK, either we open the if-then-else hell or opt for one package format. I would go for Debian only and drop RPM support for the time being (requiring a breaking note and reasoning in the CHANGELOG).
If RPM turns out to be needed, we have at least a good argument for bringing in hard-to-maintain code :)
| # | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| if command -v systemctl &> /dev/null; then |
There was a problem hiding this comment.
Hm, this script – as the others – use /bin/sh as shell which could effectively be dash, bash, zsh, fish, .... and command -v also resolves shell functions: So if there's a shell function aptly named systemctl defined, e.g., in the global config file, that shell function is called instead of the systemctl executable on disk, which could be OK or not. On the other hand, in strictly POSIX shells, one cannot use type -p (which resolves in $PATH only) as parameters (as -p) to type are not POSIX defined/mandated. Well, there's no silver bullet here...
There was a problem hiding this comment.
Will replace current scripts with autogenerated code from dh_installsystemd, this does not solve the issue you mentioned, but at least it's aligned with debians maintainer scripts.
There was a problem hiding this comment.
With going Debian only (see below comment), that may be an option, as long as it is easily maintainable.
…emd. Signed-off-by: Alexander Heinisch <[email protected]>
Enable service if systemctl is available and start service only if systemd is used as a running init process. This split is required, since creating the link to the unit file (enabling the service) is also desired to be created when creating a rootfs-image "offline" (without systemd running at that time). Signed-off-by: Alexander Heinisch <[email protected]>
Signed-off-by: Alexander Heinisch <[email protected]>
pocketbroadcast
force-pushed
the
feat-start-wfx-service
branch
from
fd2b6fa to
4667435
Compare
3 participants
…emd.
Description
For Debian packages it is common to start services included directly after installation.
Typically, this behavior is enforced by dh_* scripts added to the rules files.
(e.g.: https://manpages.debian.org/bookworm/debhelper/dh_systemd_enable.1.en.html )
I decided against using a rules file as this seems non idiomatic for nFPM.
Issues Addressed
I added systemd support to enable+start wfx on installation and disable+stop it on removal.
Change Type
Please select the relevant options:
if users expect wfx not to be started automatically.
Checklist
Since I could not find a next branch, I raised this PR for main
I did a local build of deb package and manual installation / removal using dpkg