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@xxxxxxxxxxxxx> 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. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review