https://bugzilla.redhat.com/show_bug.cgi?id=1310092 --- Comment #9 from Ralf Senderek <fedora@xxxxxxxxxxx> --- (In reply to Richard Shaw from comment #8) > Ok, another round of spec comments. You may have to do some explaining as this > is not an area I'm familiar with but you're doing a lot of interesting things > with this package. I'd be glad to explain every aspect of the cryptobone to you. In order to make things easy for the user and at the same time not compromising on security, that leads to a system with a few bells and whistles that need explanation. > 1. Have you considered a systemd timer rather than a cron job? Not yet, a cron job running every minute seemed a good trade-off to me, as I try to get new incoming messages fast without putting too much strain on the system. The user should see a new message popping up in the RAM disk automatically. I could have avoided the cron job if I burden the user with triggering the mail polling which I don't want to do. Switching to systemd timers might be a good idea if I wanted to increase the frequency of incoming email polling. Do you have a reason to suggest switching to a systemd timer that has any advantage in this context? > 2. %global cryptobonedir %{_prefix}/lib/%{name} done. > 3. It would be good to add a comment on why this is necessary: > > if ! systemctl is-active postfix > /dev/null ; then > systemctl enable postfix > fi That is something I have thought about recently. The main reason is to make sure that email is sent out. I've decided to require postfix for this package, but anyone already using another MTA would not really need that and what's worse postfix might create a conflict with the installed alternative MTA. The best solution would be to require any MTA be it postfix or other. If "Requires: sendmail" would work I'd be happy enough. But I need to test that before I can change this aspect of my spec file. There's only one place in the source code where it is needed: (line 67 in the script "sendmail") 67 /usr/sbin/sendmail -f "$SENDER" "$RECIPIENT" < /usr/lib/cryptobone/cryptobone/encryptedmessage.asc No other dependency is required for outgoing messages. > 4. This should be done in %install Of course. > 5. chkconfig --add cryptoboned As the package now requires systemd, this is unnecessary. > 6. echo "Please reboot your computer as the crypto bone daemon will start only > while booting." The cryptobone daemon is unusual in the sense that it needs access to secret information that is only available for a tiny period while the system is booting, I tried to inform the user that a re-boot is necessary after installation. Users will find that the GUI alerts them if the cryptobone daemon does not run, which will also be the case, if it is stopped and restarted like ordinary daemons. So the GUI will alert the user in any case. If the message can remain in the spec file it might be better to place it in a %posttrans script so that it gets displayed as the last line? > 7. What is the purpose of this? Placing systemd unit files in /etc is intended > for the end user so they can override the files in /usr/lib/systemd/system. I'm > pretty sure mucking around with /etc/systemd/system is forbidden. > > if [ -f /etc/systemd/system/cryptoboned.service ] ; then > # re-install symlinks > systemctl disable cryptoboned > rm -f /etc/systemd/system/cryptoboned.service 2> /dev/null > systemctl enable cryptoboned > fi Yes, because I have previously (mistakenly) installed the unit file under /etc I included this code only to get a clean system after update. In future versions I will delete this code as it is no longer necessary. be cause I use %{_unitdir} now. > 8. %files done. > I think that's enough for now! Thanks for your helpful comments. -- 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 https://admin.fedoraproject.org/mailman/listinfo/package-review