Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=231861
--- Comment #18 from Mads Kiilerich mads@kiilerich.com 2009-11-26 13:25:41 EDT ---
Shouldn't most of the renamings and other file hackings in %build be in %prep?
fixed
How about "Fix permissions on perl programs"? And the removal of cvs files?
Why do %pre stop the service but pretend it is running?
I guess it's for keeping condrestart working. I don't know why there is not just condrestart itself, but I thought it's because there were some race conditions. After searching through cvs history and filled bugs, I was not able to find any complain that condrestart alone does not work. removed
Right. It might be the same as bug 518753#c12 - perhaps it would be better to add a sleep in restart?
Is the chattr in %post OK? It might be a good idea, but is it OK that a package does it automatically? Shouldn't it be in README.RPM instead? Anyway, why not just chattr -R +S %{_var}/lib/imap/{user,quota} %{_var}/spool/imap
afaik cyrus-imapd expect this behaviour and I don't think this makes any problems. Simplified
Yes, but that behaviour isn't observable by cyrus anyway and "only" serves to "guarantee" the RFC requirement that mails must have reached the discs before being accepted - and that can't be guaranteed that way anyway. 10 years ago it was pushed as best practice by Brad Knowles & co, but I don't think it applies in modern times. AFAICS neither postfix nor sendmail nor dovecot does the same. I suggest asking some FS guru at RH.
Shouldn't the certificate creation be moved to service start where it is more transparent what goes on, just like /etc/rc.d/init.d/sshd does?
well, would it make any real difference? For example dovecot does the same think. Yes, I know it's bad example since I'm its owner, but I didn't add that part in specs and don't know any other package that creates certificates after installation (except dovecot, cyrus-imapd and openssh).
Isn't it more like database initialization - which AFAIK is done in startup script?
I think the killer argument is that for live-images and appliances it is very bad that the certificates is created at rpm-install-time.
Why is the user and group (and csync) created by the utils package %pre which doesn't use them? Move to main package?
well, just a little complicated here. cyrus-imapd requires utils and some of them are executed as cyrus user. But yes, user creation should be in main package. Moved
I noticed the "TODO: merge all sub-packages" - I assume that will solve it anyway? (Even though IIRC some the tools are imap client tools and can be used remotely.)
A couple of other nits:
Inconsistent use of xargs and -exec for file removal - shouldn't one of them be used consistently?
"Generate db config file" isn't exactly legible ...
"[ -x /usr/lib/rpm/brp-compress ]" - isn't that a mandatory build requirement?
Bashisms in cyrus-imapd.init which is /bin/sh - that is very common, but I guess it is bad?
I assume you will give a notice here when the attempt of getting patches upstreamed has been concluded somehow.