Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: postfix https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226307 kevin@xxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|kevin@xxxxxxxxx |twoerner@xxxxxxxxxx Flag|fedora-review? |fedora-review- ------- Additional Comments From kevin@xxxxxxxxx 2007-02-11 17:36 EST ------- OK - Package meets naming and packaging guidelines OK - Spec file matches base package name. OK - Spec has consistant macro usage. OK - Meets Packaging Guidelines. OK - License (IBM Public License) OK - License field in spec matches OK - License file included in package OK - Spec in American English OK - Spec is legible. OK - Sources match upstream md5sum: 24f3a076a2a1af0ca8dcb9bac3f145fa postfix-2.3.6.tar.gz 24f3a076a2a1af0ca8dcb9bac3f145fa postfix-2.3.6.tar.gz.1 See below - BuildRequires correct OK - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. See below - Package has correct buildroot %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) OK - Package is code or permissible content. OK - Packages %doc files don't affect runtime. OK - Package compiles and builds on at least one arch. OK - Package has no duplicate files in %files. OK - Package doesn't own any directories other packages own. See below - Package owns all the directories it creates. See below - No rpmlint output. See below - final provides and requires are sane: SHOULD Items: OK - Should build in mock. OK - Should build on all supported archs OK - Should function as described. See below - Should have subpackages require base package with fully versioned depend. See below - Should have dist tag See below - Should package latest version 18 outstanding bugs - check for outstanding bugs on package. Issues: 1. Suggest: add a %{?dist} to release? Makes keeping several branches in sync much easier. 2. You should use the approved buildroot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) 3. 2.3.7 is out. Perhaps upgrade to that? 4. On mysql and pgsql support. Could both of those be enabled in subpackages? ie, postfix-pgsql, postfix-mysql? They can be quite usefull, and I don't know of any reason to not include them. 5. Shouldn't pflogsumm be it's own package? It has a separate upstream source. Was there a reason to include it in this package? Also, it has no dependency on postfix, but it installs a doc file into the postfix docdir, meaning if postfix wasn't installed and this package was installed, then removed, you will be left with a lingering postfix doc dir with nothing in it. If it was separate, it could also be noarch. 6. Our pal rpmlint is doing quite a bit of talking on this package. a) W: postfix prereq-use /sbin/chkconfig, /sbin/service, sh-utils W: postfix prereq-use fileutils, textutils, W: postfix prereq-use /usr/sbin/alternatives W: postfix prereq-use %{_sbindir}/groupadd, %{_sbindir}/useradd The use of PreReq is deprecated. In the majority of cases, a plain Requires is enough and the right thing to do. Sometimes Requires(pre), Requires(post), Requires(preun) and/or Requires(postun) can also be used instead of PreReq. Take a look at: http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-97754e2c646616c5f6222f0cfc6923c60765133e b) W: postfix unversioned-explicit-provides MTA W: postfix unversioned-explicit-provides smtpd W: postfix unversioned-explicit-provides smtpdaemon W: postfix unversioned-explicit-provides /usr/bin/newaliases W: postfix unversioned-explicit-provides /usr/sbin/sendmail W: postfix unversioned-explicit-provides /usr/bin/mailq W: postfix unversioned-explicit-provides /usr/bin/rmail E: postfix hardcoded-library-path in $RPM_BUILD_ROOT/usr/lib E: postfix hardcoded-library-path in $RPM_BUILD_ROOT/usr/lib E: postfix hardcoded-library-path in /usr/lib/sendmail.postfix E: postfix hardcoded-library-path in /usr/lib/sendmail.postfix These are due to the alternatives setup. I think this can be ignored. c) E: postfix use-of-RPM_SOURCE_DIR Suggest: replace %{_sourcedir} with $SOURCE1 ? d) W: postfix macro-in-%changelog _docdir Suggest: change macro to use %% in the changelog instead of % e) W: postfix mixed-use-of-spaces-and-tabs (spaces: line 27, tab: line 315) Suggest: fix to all tabs or all spaces? f) E: postfix only-non-binary-in-usr-lib This is the pfloggsum subpackage. If it was split out to it's own package it could be noarch. g) W: postfix conffile-without-noreplace-flag /etc/rc.d/init.d/postfix E: postfix executable-marked-as-config-file /etc/rc.d/init.d/postfix Suggest: should not mark that init file as conf? h) E: postfix file-in-usr-marked-as-conffile /usr/lib64/sasl/smtpd.conf E: postfix file-in-usr-marked-as-conffile /usr/lib64/sasl2/smtpd.conf Might be a bug in cyrus-sasl putting it's config files there, but not postfix's fault. i) W: postfix service-default-enabled /etc/rc.d/init.d/postfix Suggest: Do no enable by default in the init file. j) All these can be ignored I think. Please examine them and confirm that the permissions and uids are correct: E: postfix non-standard-uid /var/spool/postfix/trace postfix E: postfix non-standard-dir-perm /var/spool/postfix/trace 0700 E: postfix non-standard-gid /usr/sbin/postqueue postdrop E: postfix setgid-binary /usr/sbin/postqueue postdrop 02755 E: postfix non-standard-executable-perm /usr/sbin/postqueue 02755 E: postfix non-standard-uid /var/spool/postfix/defer postfix E: postfix non-standard-dir-perm /var/spool/postfix/defer 0700 E: postfix non-standard-uid /var/spool/postfix/saved postfix E: postfix non-standard-dir-perm /var/spool/postfix/saved 0700 W: postfix non-conffile-in-etc /etc/postfix/postfix-files E: postfix non-standard-uid /var/spool/postfix/corrupt postfix E: postfix non-standard-dir-perm /var/spool/postfix/corrupt 0700 E: postfix non-standard-uid /var/spool/postfix/deferred postfix E: postfix non-standard-dir-perm /var/spool/postfix/deferred 0700 E: postfix non-standard-uid /var/spool/postfix/flush postfix E: postfix non-standard-dir-perm /var/spool/postfix/flush 0700 E: postfix non-standard-uid /var/spool/postfix/public postfix E: postfix non-standard-gid /var/spool/postfix/public postdrop E: postfix non-standard-dir-perm /var/spool/postfix/public 0710 W: postfix non-conffile-in-etc /etc/postfix/TLS_LICENSE E: postfix non-standard-gid /usr/sbin/postdrop postdrop E: postfix setgid-binary /usr/sbin/postdrop postdrop 02755 E: postfix non-standard-executable-perm /usr/sbin/postdrop 02755 E: postfix non-standard-uid /var/spool/postfix/private postfix E: postfix non-standard-dir-perm /var/spool/postfix/private 0700 W: postfix non-conffile-in-etc /etc/postfix/LICENSE E: postfix non-standard-uid /var/spool/postfix/incoming postfix E: postfix non-standard-dir-perm /var/spool/postfix/incoming 0700 E: postfix non-standard-uid /var/spool/postfix/bounce postfix E: postfix non-standard-dir-perm /var/spool/postfix/bounce 0700 E: postfix non-standard-uid /var/spool/postfix/active postfix E: postfix non-standard-dir-perm /var/spool/postfix/active 0700 W: postfix non-conffile-in-etc /etc/postfix/main.cf.default E: postfix non-standard-uid /var/spool/postfix/hold postfix E: postfix non-standard-dir-perm /var/spool/postfix/hold 0700 E: postfix non-standard-uid /var/spool/postfix/maildrop postfix E: postfix non-standard-gid /var/spool/postfix/maildrop postdrop E: postfix non-standard-dir-perm /var/spool/postfix/maildrop 0730 7. The dependencies on perl here all seem to be due to the qshape script. Would it be worth splitting that script out as well? That would remove: perl(File::Find) perl(Getopt::Std) perl(IO::File) perl(strict) /usr/bin/perl Requires from the postfix package. 8. 18 outstanding bugs. Several of them seem related directly to the packaging. Perhaps address those as part of the review here? 9. The LDAP conditionals mention redhat 8.0. Can we now just drop that stuff for f7/fc6/fc5? 10. Some unneeded Buildrequires: BuildRequires: gawk, perl, sed, ed, db4-devel, pkgconfig, zlib-devel See: http://www.fedoraproject.org/wiki/Packaging/Guidelines#Exceptions Can remove perl, sed at least, and possibly gawk and ed. 11. In the pfloggsumm subpackage Requires: perl-Date-Calc is not needed. rpm picks up on that itself. 12. Why is the umask 022 line there in %build and %install and several other sections? 13. Might use %{?_smp_mflags} in the make line to speed up build times? Once you have addressed these items (either by making the suggested changes, or by explaining why they don't make sense), please reassign this review back to me, and change the 'fedora-review' flag back to ? for me to take action. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review