[Bug 231861] Merge Review: cyrus-imapd - high-performance mail server (IMAP, POP3, ...)

[bugzilla at redhat.com]

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 #17 from Michal Hlavinka <mhlavink at redhat.com>  2009-11-26 08:39:45 EDT ---
(In reply to comment #16)
> Ok, another round of comments/questions:
> 
> MIT license? AFAICS the COPYRIGHT file is BSD. The CMU entry on
> http://fedoraproject.org/wiki/Licensing is internally inconsistent, and
> http://fedoraproject.org/wiki/Licensing/MIT#CMU_Style isn't the cyrus license -
> AFAIK that license has never applied to cyrus.

oops, you're right, it's BSD license

fixed

> _cyrusconf seems like a bit of overkill.

this was here to set config file version (prefork/no-prefork), but we did not
change that for ages, removed

fixed

> 
> Haven't all the versions in the requirements been irrelevant for ages?
> 

verified and removed

fixed

> Shouldn't most of the renamings and other file hackings in %build be in %prep?
> 

fixed

> Why this "if pkg-config openssl" thing? When and why does it apply?
> 

(afaik) this was there because there were problems with makefiles, but it's not
needed now, removed

fixed

> pushd before "perl -pi" and use of "$(ls *x)" isn't needed - I would prefer the
> simple version
> 

fixed

> It seems like there is no need for cleanup of "*~" and "*.html.*".
> 

fixed

> 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

fixed

> 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

fixed

> 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).

> 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

fixed

> Wouldn't it be better if csync were included in the setup packages
> /etc/services?
> 

I've asked setup maintainer to add csync in /etc/services

( http://git.fedorahosted.org/git/?p=setup.git;a=summary )

waiting for new setup release

> Is it relevant to include html versions of the man pages?

they are making no harm, but yes, they are not necessary, removed.

fixed

> 
> Much of README.RPM is outdated or irrelevant. I think "chkconfig cyrus-imapd
> on" should be mentioned. README.buildoptions doesn't exist. And postfix
> shouldn't be discriminated ;-)  

this file seems outdated and not too useful for Fedora, removed

fixed

--------
btw, there is "discussion" on mailing list where someone asked for including
autocreate patch in upstream's repository. I've added there my "me too".
Watching for upstream's decision.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.