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(a)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.
--
Configure bugmail:
https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.