https://bugzilla.redhat.com/show_bug.cgi?id=1814682 --- Comment #47 from Michal Schmidt <mschmidt@xxxxxxxxxx> --- (In reply to lsun from comment #32) > As for the "systemd-units", it appears to be required by koji. I got some > failures like below once removed it. So I'll leave it for now (please > correct me if I am incorrect). > https://koji.fedoraproject.org/koji/taskinfo?taskID=43163313 OK, in that case leaving a BuildRequires there is acceptable. Note that the "systemd-units" package was merged into the main "systemd" package in 2012 (before Fedora 18). Fedora packaging guidelines removed the last reference to "systemd-units" in 2016, keeping "BuildRequires: systemd" as the preferred way. There is another option. You can remove the BR and instead tell your configure script to not autodetect the systemd units directory, using: %configure --with-systemdsystemunitdir=%{_unitdir} The minor advantage of this would be a smaller buildroot. Use whatever option you prefer there. (In reply to lsun from comment #46) > Spec URL: https://github.com/Mellanox/rshim-user-space/releases/download/rshim-2.0.3/rshim.spec > Requires: psmisc Why does the rshim.service use killall in the first place? There is: > KillMode=process Why this mode? Do you need child processes to be left running in the cgroup after the service is stopped? If yes, commenting on it in the unit file would be nice. > ExecStop=/usr/bin/killall -SIGKILL rshim Referencing processes to kill by name is not good. It would kill unrelated processes with the same name. Is none of systemd's kill modes suitable for stopping the service without additional help? And why SIGKILL? Does it not stop on SIGTERM? > Requires: kernel-modules-extra This is always going to be imperfect, because nothing guarantees that the installed package corresponds to the actually running kernel (different versions, variants like -debug, custom unpackaged kernels, ...). There is precedent for depending on kernel-modules-extra in Fedora packages (usbip, xl2tpd, ...) though. I would just make it more explicit which module the package needs by instead using this: Requires: kmod(cuse.ko) # Hint for dnf to prefer kernel-modules-extra over kernel-debug-modules-extra: Suggests: kernel-modules-extra BTW, is it necessary to call 'system("modprobe cuse");' in src/rshim.c? I would expect the module to get autoloaded during the call to cuse_lowlevel_setup. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
