Bug 514105 - (courier-imap) Review Request: courier-imap - The Courier IMAP server
Review Request: courier-imap - The Courier IMAP server
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Rex Dieter
Fedora Extras Quality Assurance
:
Depends On: courier-authlib
Blocks: FE-DEADREVIEW KyaPanel
  Show dependency treegraph
 
Reported: 2009-07-27 22:02 EDT by Aldrey Galindo
Modified: 2017-01-27 10:35 EST (History)
9 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-04-19 08:29:49 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
SPEC file (2.68 KB, text/plain)
2009-07-27 22:02 EDT, Aldrey Galindo
no flags Details

  None (edit)
Description Aldrey Galindo 2009-07-27 22:02:07 EDT
Created attachment 355345 [details]
SPEC file

Spec Attached

Description: This package contains the standalone Courier IMAP server, which is used to provide IMAP access
to local mailboxes
Comment 1 Itamar Reis Peixoto 2009-07-27 22:56:59 EDT
is this your first package ?
Comment 2 Aldrey Galindo 2009-07-28 06:42:51 EDT
yes, my first package in bugzilla
Comment 3 Itamar Reis Peixoto 2009-07-28 06:49:25 EDT
looking at your spec file, I can see.

BuildRequires: courier-authlib-devel courier-authlib


these lines makes your package dependes on courier-authlib-devel courier-authlib, but these packages are not in fedora repos. so If you want to package courier-imap then you need to package the dependencies first.

courier-authlib-devel courier-authlib


:-)
Comment 4 Itamar Reis Peixoto 2009-07-28 06:58:57 EDT
you can grab the courier-authlib review here

https://bugzilla.redhat.com/show_bug.cgi?id=486570

and continue the work.
Comment 5 Susi Lehtola 2009-07-29 05:03:40 EDT
Make the spec and SRPM available via http (or ftp) as instructed in
http://fedoraproject.org/wiki/PackageMaintainers/Join#Upload_Your_Package
Comment 6 Susi Lehtola 2009-07-29 05:24:38 EDT
You can drop most of the %attr straight away, since the attributes given to the files in %install are preserved in the RPM (644,root,root for 'normal' files and 755,root,root for directories, libraries and executables).

The config files
 %attr(0600,root,root) %{_sysconfdir}/imapd-ssl.dist
 %attr(0600,root,root) %config(noreplace) %{_sysconfdir}/imapd.cnf
 %attr(0600,root,root) %{_sysconfdir}/imapd.dist
 %attr(0600,root,root) %{_sysconfdir}/pop3d-ssl.dist
 %attr(0600,root,root) %config(noreplace) %{_sysconfdir}/pop3d.cnf
 %attr(0600,root,root) %{_sysconfdir}/pop3d.dist
 %attr(0600,root,root) %{_sysconfdir}/quotawarnmsg.example
may need to have their attributes set separately, but I recommend doing it in %install and dropping the %attr lines from %files which are IMHO a bit messy:
 find $RPM_BUILD_ROOT%{_sysconfdir} -type f -exec chmod 600 {} \;
(This will find all files in /etc and set the permissions to 600.)
Comment 7 Peter Lemenkov 2009-07-29 05:30:30 EDT
(In reply to comment #5)
> Make the spec and SRPM available via http (or ftp) as instructed in
> http://fedoraproject.org/wiki/PackageMaintainers/Join#Upload_Your_Package  

I'm afraid, he cannot. User can get space on Fedorapeople only after someone will sponsor him.
Comment 8 Susi Lehtola 2009-07-29 05:43:49 EDT
(In reply to comment #7)
> (In reply to comment #5)
> > Make the spec and SRPM available via http (or ftp) as instructed in
> > http://fedoraproject.org/wiki/PackageMaintainers/Join#Upload_Your_Package  
> 
> I'm afraid, he cannot. User can get space on Fedorapeople only after someone
> will sponsor him.  

There's plenty of other (free) hosting available on the Internet, if he doesn't have a place yet.
Comment 9 Itamar Reis Peixoto 2009-07-29 07:55:03 EDT
(In reply to comment #8)
courier-imap dependes on courier-authlib, feel free to review it first at.

https://bugzilla.redhat.com/show_bug.cgi?id=486570
Comment 11 Rafael Gomes 2009-09-06 10:21:13 EDT
Please don't forget this bug!
Comment 12 Rex Dieter 2010-04-09 10:26:07 EDT
I can help review this.
Comment 13 Rex Dieter 2010-04-09 11:22:17 EDT
For starters,

1. SHOULD: 
This has some potential conflicts or at least confusing bits with uw-imap (and other imap/pop3 providers):

I'd suggest renaming man page items for starters including
%{_mandir}/man8/imapd.8.gz => %{_mandir}/man8/imapd.8courier.gz

Preferably too, generic binaries like:
%{_bindir}/imapd => %{_bindir}/courier-imapd
%{_bindir}/pop3d => %{_bindir}/courier-pop3d

and generic config items, move something like:
%{_sysconfdir}/* => %{_sysconfdir}/courier/

if at all possible.


2. MUST:
License: GPLv3 with openssl exception
(see COPYING)


3. SHOULD: include a startup script in /etc/rc.d/init.d/ ?
See courier-imap.spec in tarball for an example

4. SHOULD: include pam configs ?
See courier-imap.spec in tarball for an example
Comment 14 Rex Dieter 2010-04-21 11:47:03 EDT
ping?
Comment 15 Aldrey Galindo 2010-04-23 22:03:50 EDT
Rafael Gomes asked to post SPEC and RPMS in your account, I have no account to submit them. Once I update it post.
I would anticipate that any recommendations were made.
Comment 17 Rafael Gomes 2010-05-04 23:39:43 EDT
Ping! Any news? This package is very important for a Brazilian project, please help us.

Thanks!
Comment 18 Giandomenico De Tullio 2011-04-10 03:59:34 EDT
"Time passes and eternity approaches", 

Any News!?
Comment 19 Rex Dieter 2011-09-02 10:59:54 EDT
courier-imap.src: E: description-line-too-long C This package contains the standalone Courier IMAP server, which is used to provide IMAP access
courier-imap.src: E: description-line-too-long C to local mailboxes. Courier-IMAP is provided here as a separate package that can be used with
courier-imap.src: W: non-standard-group System/Servers
courier-imap.src: W: invalid-license GPLv3 with openssl exception
courier-imap.src: W: invalid-license see COPYING
courier-imap.src: W: strange-permission courier-imap.init 0755L

SHOULD: drop or fix Group tag

MUST: fix license, use something like this in .spec instead:
# SSL exception, see COPYING
License: GPLv3 with exceptions

naming: ok

SHOULD: drop unecessary
BuildRequires: courier-authlib
(which should already get pulled in from courier-authlib-devel)

MUST: fix remaining generic namespace collisions, and use courier- prefix everywhere.  examples include:
/etc/pam.d/imap
(since this currently Conflicts with cyrus-imap, grr, it should get fixed too )

SHOULD: other nice-to-have-fixed namespace issues include:
%{_sysconfdir}/imapd-ssl.dist
%{_sysconfdir}/imapd.cnf
%{_sysconfdir}/imapd.dist
%{_sysconfdir}/pop3d-ssl.dist
%{_sysconfdir}/pop3d.cnf
%{_sysconfdir}/pop3d.dist
%{_datadir}/mkimapdcert
%{_datadir}/mkpop3dcert
%{_libexecdir}/imapd-ssl.rc
%{_libexecdir}/imapd.rc
%{_libexecdir}/pop3d-ssl.rc
%{_libexecdir}/pop3d.rc

(libexecdir could potentially just use %{_libexecdir}/courier/ subdir, for example)

macros: NOT OK, MUST fix.  See:
http://fedoraproject.org/wiki/Packaging/Guidelines#Macros
in particular,
"Macro forms of system executables SHOULD NOT be used except when there is a
need to allow the location of those executables to be configurable. For
example, rm should be used in preference to %{__rm}, but %{__python} is
acceptable."

init: SHOULD: update to include systemd unit file, instead of sysv init
http://fedoraproject.org/wiki/Packaging/Guidelines#Systemd
Comment 20 Rex Dieter 2012-01-07 11:42:32 EST
ping reporter, been another couple of months... (sorry, for my own delays)
Comment 21 Rex Dieter 2012-04-19 08:29:49 EDT
marking dead review, original reporter hasn't been heard from in years.
Comment 22 Rex Dieter 2017-01-27 10:35:44 EST
clearing review flag

Note You need to log in before you can comment on or make changes to this bug.