Bug 678221

Taking.

 - = N/A
 x = Check
 ! = Problem
 ? = Not evaluated

=== REQUIRED ITEMS ===
 [x] Package is named according to the Package Naming Guidelines.
 [x] Spec file name must match the base package %{name}, in the format
%{name}.spec.
 [x] Package meets the Packaging Guidelines including the Perl specific items
 [x] Package successfully compiles and builds into binary rpms on at least one
supported architecture.
     Tested on: http://koji.fedoraproject.org/koji/taskinfo?taskID=2854928

 [x] Rpmlint output:
perl-EV.src: W: spelling-error Summary(en_US) libev -> liber, libel, Liberia
perl-EV.src: W: spelling-error %description -l en_US libev -> liber, libel, Liberia
perl-EV.src: W: spelling-error %description -l en_US http -> HTTP
perl-EV.src: W: spelling-error %description -l en_US schmorp -> schmoes, schmooze, schmo
perl-EV.src: W: spelling-error %description -l en_US html -> HTML, ht ml, ht-ml
perl-EV.src: W: spelling-error %description -l en_US cvs -> cs, vs, cts
perl-EV.src: W: spelling-error %description -l en_US backends -> backbends, back ends, back-ends
perl-EV.src: W: spelling-error %description -l en_US backend -> backed, backbend, back end
perl-EV.x86_64: W: spelling-error Summary(en_US) libev -> liber, libel, Liberia
perl-EV.x86_64: W: spelling-error %description -l en_US libev -> liber, libel, Liberia
perl-EV.x86_64: W: spelling-error %description -l en_US http -> HTTP
perl-EV.x86_64: W: spelling-error %description -l en_US schmorp -> schmoes, schmooze, schmo
perl-EV.x86_64: W: spelling-error %description -l en_US html -> HTML, ht ml, ht-ml
perl-EV.x86_64: W: spelling-error %description -l en_US cvs -> cs, vs, cts
perl-EV.x86_64: W: spelling-error %description -l en_US backends -> backbends, back ends, back-ends
perl-EV.x86_64: W: spelling-error %description -l en_US backend -> backed, backbend, back end
perl-EV.x86_64: W: private-shared-object-provides /usr/lib64/perl5/vendor_perl/auto/EV/EV.so EV.so()(64bit)
perl-EV.x86_64: W: devel-file-in-non-devel-package /usr/lib64/perl5/vendor_perl/EV/EVAPI.h
perl-EV.x86_64: W: devel-file-in-non-devel-package /usr/lib64/perl5/vendor_perl/EV/ev.h
2 packages and 1 specfiles checked; 0 errors, 19 warnings.

We'll ignore the "spelling errors".

The private-shared-object-provides will go away if you use the Perl default
filter (as you should for all packages of Perl modules). See
https://fedoraproject.org/wiki/Perl_default_filter for the gory details.

Is there a compelling reason for including include files in the main package?
If not, I'ld rather you split those two files in a -devel sub-package

 [x] Package is not relocatable.
 [x] Buildroot is correct
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

Note that Buildroot is ignored for semi-recent versions of Fedora and EPEL.

 [x] Package is licensed with an open-source compatible license and meets other
legal requirements as defined in the legal section of Packaging Guidelines.
 [!] License field in the package spec file matches the actual license.
     License type: GPLv2+ and Artistic

From where did you get "BSD or GPLv2+"?

 [-] If (and only if) the source package includes the text of the license(s) in
its own file, then that file, containing the text of the license(s) for the
package is included in %doc.
 [x] Spec file is legible and written in American English.
 [x] Sources used to build the package matches the upstream source, as provided
in the spec URL.
33088705bc34bf66bccde50205c15e9f  EV-4.03.tar.gz

 [x] Package is not known to require ExcludeArch
 [x] All build dependencies are listed in BuildRequires, except for any that
are listed in the exceptions section of Packaging Guidelines.
 [-] The spec file handles locales properly.
 [-] ldconfig called in %post and %postun if required.
 [x] Package must own all directories that it creates.
 [-] Package requires other packages for directories it uses.
 [x] Package does not contain duplicates in %files.
 [x] Permissions on files are set properly.
 [x] Package consistently uses macros.
 [x] Package contains code, or permissable content.
 [-] Large documentation files are in a -doc subpackage, if required.
 [-] Package uses nothing in %doc for runtime.
 [!] Header files in -devel subpackage, if present.
 [x] Static libraries in -devel subpackage, if present.
 [-] Package requires pkgconfig, if .pc files are present.
 [x] Development .so files in -devel subpackage, if present.
 [-] Fully versioned dependency in subpackages, if present.
 [-] Package does not contain any libtool archives (.la).
 [-] Package contains a properly installed %{name}.desktop file if it is a GUI
application.
 [x] Package does not own files or directories owned by other packages.

=== SUGGESTED ITEMS ===
 [x] Latest version is packaged.
 [x] Package does not include license text files separate from upstream.
 [-] Description and summary sections in the package spec file contains
translations for supported Non-English languages, if available.
 [x] Reviewer should test that the package builds in mock.
     Tested on: rawhide.x86_64
 [x] Package should compile and build into binary rpms on all supported
architectures.
     Tested on: http://koji.fedoraproject.org/koji/taskinfo?taskID=2854928
 [?] Package functions as described.
 [-] Scriptlets must be sane, if used.
 [-] The placement of pkgconfig(.pc) files is correct.
 [-] File based requires are sane.
 [x] %check is present and the tests pass

All tests successful.
Files=11, Tests=6875,  6 wallclock secs ( 0.91 usr  0.03 sys +  0.17 cusr  0.07 csys =  1.18 CPU)
Result: PASS

There are a number of things to fix, here.

Thanks for the review, President!

Here is an updated package:
Spec URL: http://bochecha.fedorapeople.org/packages/perl-EV.spec
SRPM URL: http://bochecha.fedorapeople.org/packages/perl-EV-4.03-2.fc16.src.rpm

Full diff:
diff --git a/perl-EV.spec b/perl-EV.spec
index 00baa90..2b29f94 100644
--- a/perl-EV.spec
+++ b/perl-EV.spec
@@ -1,6 +1,6 @@
 Name:           perl-EV
 Version:        4.03
-Release:        1%{?dist}
+Release:        2%{?dist}
 Summary:        Wrapper for the libev high-performance event loop library

 Group:          Development/Libraries
@@ -8,7 +8,6 @@ License:        (GPL+ or Artistic) and (BSD or GPLv2+)
 URL:            http://search.cpan.org/dist/EV/
 Source0:        http://search.cpan.org/CPAN/authors/id/M/ML/MLEHMANN/EV-%{version}.tar.gz
 Patch0:         perl-EV-4.03-Don-t-ask-questions-at-build-time.patch
-BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

 BuildRequires:  perl(ExtUtils::MakeMaker)
 BuildRequires:  perl(common::sense)
@@ -18,6 +17,9 @@ BuildRequires:  perl(AnyEvent) => 2.6
 Requires:       perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version))


+%{?perl_default_filter}
+
+
 %description
 This module provides an interface to libev
 (<http://software.schmorp.de/pkg/libev.html>). While the included documentation
@@ -28,6 +30,15 @@ specific backend with "LIBEV_FLAGS", or just about in any case because it has
 much more detailed information.


+%package devel
+Summary:        Wrapper for the libev high-performance event loop library
+Requires:       %{name}%{?_isa} = %{version}-%{release}
+
+
+%description devel
+This package provides the development headers for the Perl EV module.
+
+
 %prep
 %setup -q -n EV-%{version}

@@ -69,11 +80,24 @@ rm -rf $RPM_BUILD_ROOT
 %doc Changes COPYING README
 %{perl_vendorarch}/auto/*
 %{perl_vendorarch}/EV.pm
-%{perl_vendorarch}/EV/
+%{perl_vendorarch}/EV
+%exclude %{perl_vendorarch}/EV/*.h
 %{_mandir}/man3/*.3*


+%files devel
+%defattr(-,root,root,-)
+%{perl_vendorarch}/EV/*.h
+
+
 %changelog
+* Wed Feb 23 2011 Mathieu Bridon <bochecha> - 4.03-2
+- Fixes asked during the review process:
+  - Filter the private shared EV.so out of the automatic Provides
+  - Put the header files in a -devel package
+- Removed the Buildroot line since it's useless for newer versions of Fedora
+  and this package can only go in Fedora >= 15 due to its libev dependency)
+
 * Mon Jan 24 2011 Mathieu Bridon <bochecha> - 4.03-1
 - Update to 4.03.
 - Use the system libev instead of the bundled one.

---

(In reply to comment #2)
>  - = N/A
>  x = Check
>  ! = Problem
>  ? = Not evaluated
> 
> === REQUIRED ITEMS ===
>  [x] Rpmlint output:
[... snip ...]
> The private-shared-object-provides will go away if you use the Perl default
> filter (as you should for all packages of Perl modules). See
> https://fedoraproject.org/wiki/Perl_default_filter for the gory details.

Fixed.

> Is there a compelling reason for including include files in the main package?
> If not, I'ld rather you split those two files in a -devel sub-package

No reason, those rpmlint warnings just escaped me in the middle all those spelling suggestions.

Fixed.

>  [x] Buildroot is correct
> %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
> 
> Note that Buildroot is ignored for semi-recent versions of Fedora and EPEL.

Since this is only going to Fedora >= 15 anyway (dependency on libev), I removed the Buildroot line.

>  [!] License field in the package spec file matches the actual license.
>      License type: GPLv2+ and Artistic

That's GPLv2+ or Artistic, not and.

> From where did you get "BSD or GPLv2+"?

There's a bundled copy of libev in the source RPM.

At build-time, I remove this folder and use instead the sources coming from the Fedora libev-source package, to avoid building against the bundled copy (this is what is done by other packages such as tigervnc that uses the sources from Xorg).

The sources of libev are BSD or GPLv2+, hence the License tag.

However, I must admit that I'm not sure I did it right license-wise:
- since I'm building against the system-provided sources of libev, should I just remove the part related to the libev sources?
- since the sources are still included in the source rpm, should I leave it as is?

Not sure, I felt it was safer to keep all the licenses of all the included sources.

>  [!] Header files in -devel subpackage, if present.

I believe that was the same as above (rpmlint warning), so this is fixed.

(In reply to comment #3)
>
> Since this is only going to Fedora >= 15 anyway (dependency on libev), I
> removed the Buildroot line.

Note that, while you're at it, the %clean section is not required for F-13 and above.

> The sources of libev are BSD or GPLv2+, hence the License tag.
> 
> However, I must admit that I'm not sure I did it right license-wise:
> - since I'm building against the system-provided sources of libev, should I
> just remove the part related to the libev sources?
> - since the sources are still included in the source rpm, should I leave it as
> is?

I'm going to go with the first solution.

Fix this and I'll approve your package.

Sorry it took so long, $dayjob got in the way.

As you asked, I used only the perl-EV flicense in the license tag. I added a comment just above it with more background on the licensing of the included sources.

Here is an update package:
Spec URL: http://bochecha.fedorapeople.org/packages/perl-EV.spec
SRPM URL: http://bochecha.fedorapeople.org/packages/perl-EV-4.03-3.fc15.src.rpm

Full diff:
diff --git a/perl-EV.spec b/perl-EV.spec
index 2b29f94..4f244f9 100644
--- a/perl-EV.spec
+++ b/perl-EV.spec
@@ -1,10 +1,13 @@
 Name:           perl-EV
 Version:        4.03
-Release:        2%{?dist}
+Release:        3%{?dist}
 Summary:        Wrapper for the libev high-performance event loop library
 
 Group:          Development/Libraries
-License:        (GPL+ or Artistic) and (BSD or GPLv2+)
+# Note: The source archive includes a libev/ folder which contents are licensed
+#       as "BSD or GPLv2+". However, those are removed at build-time and
+#       perl-EV is instead built against the system-provided libev.
+License:        GPL+ or Artistic
 URL:            http://search.cpan.org/dist/EV/
 Source0:        http://search.cpan.org/CPAN/authors/id/M/ML/MLEHMANN/EV-%{version}.tar.gz
 Patch0:         perl-EV-4.03-Don-t-ask-questions-at-build-time.patch
@@ -59,7 +62,6 @@ make %{?_smp_mflags}
 
 
 %install
-rm -rf $RPM_BUILD_ROOT
 make pure_install PERL_INSTALL_ROOT=$RPM_BUILD_ROOT
 find $RPM_BUILD_ROOT -type f -name .packlist -exec rm -f {} ';'
 find $RPM_BUILD_ROOT -type f -name '*.bs' -a -size 0 -exec rm -f {} ';'
@@ -91,6 +93,13 @@ rm -rf $RPM_BUILD_ROOT
 
 
 %changelog
+* Tue Mar 08 2011 Mathieu Bridon <bochecha> - 4.03-3
+- Some more fixes as part of the review process:
+  - Fix the license tag to be only the license of perl-EV, and add a note about
+    the included libev sources.
+- Removed manual cleaning of the buildroot since it has been useless since
+  Fedora 10 and even EPEL (>=6) doesn't need it now.
+
 * Wed Feb 23 2011 Mathieu Bridon <bochecha> - 4.03-2
 - Fixes asked during the review process:
   - Filter the private shared EV.so out of the automatic Provides

(In reply to comment #5)
>
> Sorry it took so long, $dayjob got in the way.

Tell me about it.
The changes work for me. APPROVED.

Thanks Emmanuel!

New Package SCM Request
=======================
Package Name: perl-EV
Short Description: Wrapper for the libev high-performance event loop library
Owners: bochecha
Branches: f15
InitialCC: perl-sig

Git done (by process-git-requests).

Thanks Jason for the branch.

Package was built in Rawhide and will soon be submitted to F15, closing.

Package Change Request
======================
Package Name: perl-EV
New Branches: el6
Owners: bochecha
InitialCC: perl-sig

Git done (by process-git-requests).