Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
Bug ID: 890491 Summary: Review Request: perl-Mail-Procmail - Procmail-like facility for creating easy mail filters Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Reporter: strobert@strobe.net
Spec URL: http://rpm.ysl.org/fedora-review/perl-Mail-Procmail.spec SRPM URL: http://rpm.ysl.org/fedora-review/perl-Mail-Procmail-1.08-1.el6.src.rpm Description: procmail is a great mail filter program, but it has weird recipe format. It's pattern matching capabilities are basic and often insufficient. I wanted something flexible whereby I could filter my mail using the power of Perl. Fedora Account System Username: strobert
Koji scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=4818076
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
Trond H. Amundsen t.h.amundsen@usit.uio.no changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |t.h.amundsen@usit.uio.no
--- Comment #1 from Trond H. Amundsen t.h.amundsen@usit.uio.no --- Not an official review, I just have some comments on the spec file:
* These are only needed if rpmbuild doesn't find those dependencies automatically:
Requires: perl(LockFile::Simple) Requires: perl(Mail::Internet) Requires: perl(Test::More)
* Unless EPEL5 is one of the intended targets you can (and should) omit the BuildRoot and the whole %clean section
* The description is weird... "I wanted something etc." should be rephrased
* You don't need a macro for %{__perl} (I think). I'm not 100% sure of this one.
* Spec files don't belong in %doc, at least not spec files for other packages
* You should use %perl_default_filter, see URL: https://fedoraproject.org/wiki/Perl_default_filter
-trond
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
--- Comment #2 from Steven Roberts strobert@strobe.net --- What I get for trying to squeeze this in before I went to bed last night :). Obviously was a bit too tired. The wrong version got sync'd up which addresses some of the items... I was fighting my local fedora dev env last night (mock stopped working after latest updates) so was shuffling files around.
- I'll drop the requires. Thought I had.
- EPEL5 is a planned target. we have been using a local generated rpm for a bit know, but thinking getting it upstream would be a good thing.
- that description is straight from the perl module's author when describing his module. I'll reqord it a bit to make a bit more sense as a package description.
- %{__perl} is what cpanspec spits out and what the MODEUL_COMPAT define listed uses at: https://fedoraproject.org/wiki/Packaging:Perl?rd=Packaging/Perl#Versioned_MO... so got the impression that was normal best practice.
- I'll have to see where cpanspec grabbed the extra spec files from. that is really weird. I'm actually working with the cpanspec author at the moment on a new release so sounds like a bug to get fixed :)
- thank you for the pointer to the %perl_default_filter. the link to it (https://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering#Pe...) from the perl packaging page is a little hidden. I'll see about tweaking the wiki pages to point it out more.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
--- Comment #3 from Steven Roberts strobert@strobe.net --- new spec,srpm uploaded Spec URL: http://rpm.ysl.org/fedora-review/perl-Mail-Procmail.spec SRPM URL: http://rpm.ysl.org/fedora-review/perl-Mail-Procmail-1.08-2.el6.src.rpm
Did the adjustments mentioned above. I figured out where those spec files came from, they are actually bundled in the tarball from upstream (including the ones for the other packages).
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
Petr Pisar ppisar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |ppisar@redhat.com Assignee|nobody@fedoraproject.org |ppisar@redhat.com Flags| |fedora-review?
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
--- Comment #4 from Petr Pisar ppisar@redhat.com --- URL and Source0 are usable. Ok. Source file is original (SHA-256: 09ff5367e83a752f1279f31d8ac05b62360222bb438dc58a71f7e39506298c70). Ok. Summary verified from lib/Mail/Procmail.pm. Ok. Description verified from lib/Mail/Procmail.pm. Ok. License verified from lib/Mail/Procmail.pm. Ok. No XS code, noarch BuildArch is Ok.
TODO: Remove BuildRoot definition. It's not needed any more. TODO: Remove explicit build root cleaning, it's not needed any more. TODO: Remove deleting empty directories in %install section. Modern ExtUtils::MakeMaker does not leave empty directories. TODO: Remove explicit %clean, it's not needed any more. TODO: Remove explicit defattr in %files section. It's not needed any more.
TODO: Build-require `perl(constant)' (lib/Mail/Procmail.pm:155). TODO: Build-require `perl(Exporter)' (lib/Mail/Procmail.pm:162).
FIX: Build-require `perl(Carp)' (lib/Mail/Procmail.pm:189).
IO::File is not needed on current perl. Ok.
All tests pass. Ok.
$ rpmlint perl-Mail-Procmail.spec ../SRPMS/perl-Mail-Procmail-1.08-2.fc19.src.rpm ../RPMS/noarch/perl-Mail-Procmail-1.08-2.fc19.noarch.rpm 2 packages and 1 specfiles checked; 0 errors, 0 warnings. rpmlint is Ok.
$ rpm -q -lv -p ../RPMS/noarch/perl-Mail-Procmail-1.08-2.fc19.noarch.rpm drwxr-xr-x 2 root root 0 Jan 21 16:03 /usr/share/doc/perl-Mail-Procmail-1.08 -rw-r--r-- 1 root root 2339 Sep 19 2004 /usr/share/doc/perl-Mail-Procmail-1.08/CHANGES -rw-r--r-- 1 root root 3729 Sep 19 2004 /usr/share/doc/perl-Mail-Procmail-1.08/README -rw-r--r-- 1 root root 6956 Jan 21 16:03 /usr/share/man/man3/Mail::Procmail.3pm.gz drwxr-xr-x 2 root root 0 Jan 21 16:03 /usr/share/perl5/vendor_perl/Mail -rw-r--r-- 1 root root 22214 Sep 19 2004 /usr/share/perl5/vendor_perl/Mail/Procmail.pm File permissions and layout are Ok.
$ rpm -q --requires -p ../RPMS/noarch/perl-Mail-Procmail-1.08-2.fc19.noarch.rpm |sort |uniq -c 1 perl >= 0:5.005 1 perl(Carp) 1 perl(constant) 1 perl(Exporter) 1 perl(Fcntl) 1 perl(LockFile::Simple) 1 perl(Mail::Internet) 1 perl(:MODULE_COMPAT_5.16.2) 1 perl(strict) 1 perl(Sys::Hostname) 1 perl(vars) 1 rpmlib(CompressedFileNames) <= 3.0.4-1 1 rpmlib(FileDigests) <= 4.6.0-1 1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 1 rpmlib(PayloadIsXz) <= 5.2-1 Binary requires are Ok.
$ rpm -q --provides -p ../RPMS/noarch/perl-Mail-Procmail-1.08-2.fc19.noarch.rpm |sort |uniq -c 1 perl(Mail::Procmail) = 1.08 1 perl-Mail-Procmail = 1.08-2.fc19 Binary provides are Ok.
$ resolvedeps rawhide ../RPMS/noarch/perl-Mail-Procmail-1.08-2.fc19.noarch.rpm Binary dependencies resolvable. Ok.
Package builds in F19 (http://koji.fedoraproject.org/koji/taskinfo?taskID=4889603). ???
Otherwise package is in line with Fedora and Perl packaging guidelines.
Please correct all `FIX' issues, consider fixing `TODO' items, and provide new spec file. Resolution: NOT approved.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
--- Comment #5 from Steven Roberts strobert@strobe.net --- thank you for the review. what are you using? it looks different than at last the version of fedora-review I have been running. I would like to prescreen future packages to save folks some time.
EPEL5 is a target so the following are still required: TODO: Remove BuildRoot definition. It's not needed any more. TODO: Remove explicit build root cleaning, it's not needed any more. TODO: Remove deleting empty directories in %install section. Modern ExtUtils::MakeMaker does not leave empty directories. TODO: Remove explicit %clean, it's not needed any more. TODO: Remove explicit defattr in %files section. It's not needed any more.
Fixed these: TODO: Build-require `perl(constant)' (lib/Mail/Procmail.pm:155). TODO: Build-require `perl(Exporter)' (lib/Mail/Procmail.pm:162). FIX: Build-require `perl(Carp)' (lib/Mail/Procmail.pm:189).
I'm working with the spanspec author on a new version, I'll see about why those weren't caught by it.
new spec,srpm uploaded Spec URL: http://rpm.ysl.org/fedora-review/perl-Mail-Procmail.spec SRPM URL: http://rpm.ysl.org/fedora-review/perl-Mail-Procmail-1.08-3.src.rpm
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
Petr Pisar ppisar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? | Flags| |fedora-review+
--- Comment #6 from Petr Pisar ppisar@redhat.com --- Spec file changes: --- perl-Mail-Procmail.spec.old 2012-12-31 04:08:04.000000000 +0100 +++ perl-Mail-Procmail.spec 2013-01-24 04:54:40.000000000 +0100 @@ -1,6 +1,6 @@ Name: perl-Mail-Procmail Version: 1.08 -Release: 2%{?dist} +Release: 3%{?dist} Summary: Procmail-like facility for creating easy mail filters License: GPL+ or Artistic Group: Development/Libraries @@ -12,6 +12,9 @@ BuildRequires: perl(LockFile::Simple) BuildRequires: perl(Mail::Internet) BuildRequires: perl(Test::More) +BuildRequires: perl(Carp) +BuildRequires: perl(constant) +BuildRequires: perl(Exporter) Requires: perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version))
%{?perl_default_filter} @@ -52,6 +55,9 @@ %{_mandir}/man3/*
%changelog +* Wed Jan 23 2013 Steven D. Roberts strobert@strobe.net 1.08-3 +- added some missing buildreqs + * Sun Dec 30 2012 Steven D. Roberts strobert@strobe.net 1.08-2 - spec cleanup.
TODO: Remove BuildRoot definition. It's not needed any more. TODO: Remove explicit build root cleaning, it's not needed any more. TODO: Remove deleting empty directories in %install section. Modern ExtUtils::MakeMaker does not leave empty directories. TODO: Remove explicit %clean, it's not needed any more. TODO: Remove explicit defattr in %files section. It's not needed any more.
Not addressed.
TODO: Build-require `perl(constant)' (lib/Mail/Procmail.pm:155).
+BuildRequires: perl(constant) Ok.
TODO: Build-require `perl(Exporter)' (lib/Mail/Procmail.pm:162).
+BuildRequires: perl(Exporter) Ok.
FIX: Build-require `perl(Carp)' (lib/Mail/Procmail.pm:189).
+BuildRequires: perl(Carp) Ok.
$ rpmlint perl-Mail-Procmail.spec ../SRPMS/perl-Mail-Procmail-1.08-3.fc19.src.rpm ../RPMS/noarch/perl-Mail-Procmail-1.08-3.fc19.noarch.rpm 2 packages and 1 specfiles checked; 0 errors, 0 warnings. rpmlint is Ok.
Package builds in F19 (http://koji.fedoraproject.org/koji/taskinfo?taskID=4898744). Ok
Package is good. Resolution: Package APPROVED.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
--- Comment #7 from Petr Pisar ppisar@redhat.com --- (In reply to comment #5)
thank you for the review. what are you using? it looks different than at last the version of fedora-review I have been running. I would like to prescreen future packages to save folks some time.
No tool, manually. We have plan to write the test as plug-in for fedora-review, but fedora-review is broken now.
EPEL5 is a target so the following are still required:
TODO: Remove explicit defattr in %files section. It's not needed any more.
This is not needed since rpm 4.4 which presents in RHEL-5 https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions.
Fixed these: TODO: Build-require `perl(constant)' (lib/Mail/Procmail.pm:155). TODO: Build-require `perl(Exporter)' (lib/Mail/Procmail.pm:162). FIX: Build-require `perl(Carp)' (lib/Mail/Procmail.pm:189).
I'm working with the spanspec author on a new version, I'll see about why those weren't caught by it.
cpanspec investigates META.yml only. And even so it removes modules which are bundled with perl.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
--- Comment #8 from Steven Roberts strobert@strobe.net --- ah. unfortunate about fedora-review.
good catch on the defattr. one of my biggest stumbles has been finding all of the old items I used to do in the RH7.3 days (which we were on for WAY too long). I'll drop that before I do the official builds
good to know on cpanspec. we (the cpanspec author and a couple of us who expressed interest in it recently) have been discussing how it find buildreqs, so sounds like a good thing to look into further.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
Steven Roberts strobert@strobe.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-cvs?
--- Comment #9 from Steven Roberts strobert@strobe.net --- New Package SCM Request ======================= Package Name: perl-Mail-Procmail Short Description: Procmail-like facility for creating easy mail filters Owners: strobert Branches: f17 f18 el5 el6 InitialCC: perl-sig
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
--- Comment #10 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
--- Comment #11 from Fedora Update System updates@fedoraproject.org --- perl-Mail-Procmail-1.08-4.el5 has been submitted as an update for Fedora EPEL 5. https://admin.fedoraproject.org/updates/perl-Mail-Procmail-1.08-4.el5
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
--- Comment #12 from Fedora Update System updates@fedoraproject.org --- perl-Mail-Procmail-1.08-4.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/perl-Mail-Procmail-1.08-4.fc17
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
--- Comment #13 from Fedora Update System updates@fedoraproject.org --- perl-Mail-Procmail-1.08-4.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/perl-Mail-Procmail-1.08-4.fc18
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
--- Comment #14 from Fedora Update System updates@fedoraproject.org --- perl-Mail-Procmail-1.08-4.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/perl-Mail-Procmail-1.08-4.el6
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
--- Comment #15 from Fedora Update System updates@fedoraproject.org --- perl-Mail-Procmail-1.08-4.el6 has been pushed to the Fedora EPEL 6 testing repository.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |CURRENTRELEASE Last Closed| |2013-02-21 00:42:47
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
--- Comment #16 from Fedora Update System updates@fedoraproject.org --- perl-Mail-Procmail-1.08-4.fc18 has been pushed to the Fedora 18 stable repository.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
--- Comment #17 from Fedora Update System updates@fedoraproject.org --- perl-Mail-Procmail-1.08-4.fc17 has been pushed to the Fedora 17 stable repository.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
--- Comment #18 from Fedora Update System updates@fedoraproject.org --- perl-Mail-Procmail-1.08-4.el5 has been pushed to the Fedora EPEL 5 stable repository.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|CURRENTRELEASE |ERRATA
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=890491
--- Comment #19 from Fedora Update System updates@fedoraproject.org --- perl-Mail-Procmail-1.08-4.el6 has been pushed to the Fedora EPEL 6 stable repository.
package-review@lists.fedoraproject.org