Bug 783149

Summary:	Review Request: perl-POE-Component-Resolver - Non-blocking getaddrinfo() resolver
Product:	[Fedora] Fedora	Reporter:	Petr Šabata <psabata>
Component:	Package Review	Assignee:	Petr Pisar <ppisar>
Status:	CLOSED RAWHIDE	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	notting, package-review, ppisar
Target Milestone:	---	Flags:	ppisar: fedora-review+ gwync: fedora-cvs+
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:	perl-POE-Component-Resolver-0.914-1.fc17	Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2012-01-20 14:39:51 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Bug Depends On:
Bug Blocks:	781402

Description Petr Šabata 2012-01-19 14:26:15 UTC

Spec URL: http://psabata.fedorapeople.org/pkgs/perl-POE-Component-Resolver/perl-POE-Component-Resolver.spec
SRPM URL: http://psabata.fedorapeople.org/pkgs/perl-POE-Component-Resolver/perl-POE-Component-Resolver-0.914-1.fc16.src.rpm
Description:
POE::Component::Resolver performs Socket::GetAddrInfo::getaddrinfo() calls
in subprocesses where they're permitted to block as long as necessary.

Comment 1 Petr Pisar 2012-01-19 15:06:33 UTC

Source file is original. Ok.
Summary verified from lib/POE/Component/Resolver.pm. Ok.
Description verified from lib/POE/Component/Resolver.pm. Ok.
License verified from lib/POE/Component/Resolver.pm and LICENSE. Ok.

perl(ExtUtils::MakeMaker) is unversioned because all Fedoras satisfy. Ok.

TODO: BuildRequire `perl(base)' for tests (lib/POE/Component/Resolver.pm:18)

FIX: BuildRequire `perl(Socket::GetAddrInfo) >= 0.19' is not fulfilled in F17. perl-Socket-GetAddrInfo package needs upgrade.

TODO: Package only README or only REAMDE.mkdn. Their identical but formatting.

Due to unsatisfied dependencies this review has been postponed. Please drop o notice here once the F17/rawhide becomes more up-to-date.

Comment 2 Petr Šabata 2012-01-19 15:16:46 UTC

I built Socket::GetAddrInfo 0.19 about two hours ago; your mirrors are slow again :P
http://koji.fedoraproject.org/koji/buildinfo?buildID=294298

I'll post an updated package with fixed TODO's shortly.

Comment 3 Petr Šabata 2012-01-19 15:19:48 UTC

The updated package is available on the same URLs.

Comment 4 Petr Pisar 2012-01-19 15:47:13 UTC

I see. Resuming.

No XS code, noarch BuildArch is Ok.

FIX: Remove perl(POE::Filter::Reference) from build-requires or make it run-time dependency too (lib/POE/Component/Resolver.pm:219).
FIX: Remove perl(POE::Filter::Reference) from build-requires or make it run-time dependency too (lib/POE/Component/Resolver.pm:218).

TODO: Remove `perl(Scalar::Util) >= 1.23' from run-time dependencies. It's used obviously nowhere in the code.

FIX: Build-Require `perl(POE::Component::Resolver::Sidecar)' for tests (lib/POE/Component/Resolver.pm:15).

All tests pass. Ok.

$ rpmlint perl-POE-Component-Resolver.spec ../SRPMS/perl-POE-Component-Resolver-0.914-1.fc17.src.rpm ../RPMS/noarch/perl-POE-Component-Resolver-0.914-1.fc17.noarch.rpm
perl-POE-Component-Resolver.src: W: spelling-error Summary(en_US) getaddrinfo
perl-POE-Component-Resolver.src: W: spelling-error %description -l en_US GetAddrInfo
perl-POE-Component-Resolver.src: W: spelling-error %description -l en_US getaddrinfo
perl-POE-Component-Resolver.src: W: spelling-error %description -l en_US subprocesses -> sub processes, sub-processes, processes
perl-POE-Component-Resolver.noarch: W: spelling-error Summary(en_US) getaddrinfo
perl-POE-Component-Resolver.noarch: W: spelling-error %description -l en_US GetAddrInfo
perl-POE-Component-Resolver.noarch: W: spelling-error %description -l en_US getaddrinfo
perl-POE-Component-Resolver.noarch: W: spelling-error %description -l en_US subprocesses -> sub processes, sub-processes, processes
perl-POE-Component-Resolver.noarch: E: incorrect-fsf-address /usr/share/doc/perl-POE-Component-Resolver-0.914/LICENSE
2 packages and 1 specfiles checked; 1 errors, 8 warnings.
rpmlint is Ok.

$ rpm -q -lv -p ../RPMS/noarch/perl-POE-Component-Resolver-0.914-1.fc17.noarch.rpm 
drwxr-xr-x    2 root    root                        0 Jan 19 16:11 /usr/share/doc/perl-POE-Component-Resolver-0.914
-rw-r--r--    1 root    root                     6863 Sep 15 07:51 /usr/share/doc/perl-POE-Component-Resolver-0.914/CHANGES
-rw-r--r--    1 root    root                    18252 Sep 15 07:51 /usr/share/doc/perl-POE-Component-Resolver-0.914/LICENSE
-rw-r--r--    1 root    root                     9561 Sep 15 07:51 /usr/share/doc/perl-POE-Component-Resolver-0.914/README
-rw-r--r--    1 root    root                     7909 Sep 15 07:51 /usr/share/doc/perl-POE-Component-Resolver-0.914/README.mkdn
-rw-r--r--    1 root    root                     5122 Jan 19 16:11 /usr/share/man/man3/POE::Component::Resolver.3pm.gz
-rw-r--r--    1 root    root                     2357 Jan 19 16:11 /usr/share/man/man3/POE::Component::Resolver::Sidecar.3pm.gz
drwxr-xr-x    2 root    root                        0 Jan 19 16:11 /usr/share/perl5/vendor_perl/POE
drwxr-xr-x    2 root    root                        0 Jan 19 16:11 /usr/share/perl5/vendor_perl/POE/Component
drwxr-xr-x    2 root    root                        0 Jan 19 16:11 /usr/share/perl5/vendor_perl/POE/Component/Resolver
-rw-r--r--    1 root    root                    19562 Sep 15 07:51 /usr/share/perl5/vendor_perl/POE/Component/Resolver.pm
-rw-r--r--    1 root    root                     2415 Sep 15 07:51 /usr/share/perl5/vendor_perl/POE/Component/Resolver/Sidecar.pm
File layout and permissions are Ok.

$ rpm -q --requires -p ../RPMS/noarch/perl-POE-Component-Resolver-0.914-1.fc17.noarch.rpm |sort |uniq -c
      1 perl(base)  
      1 perl(bytes)  
      1 perl(Carp)  
      1 perl(Config)  
      1 perl(Exporter)  
      1 perl(:MODULE_COMPAT_5.14.2)  
      1 perl(POE) >= 1.311
      1 perl(POE::Component::Resolver::Sidecar)  
      1 perl(Scalar::Util) >= 1.23
      1 perl(Socket)  
      1 perl(Socket::GetAddrInfo) >= 0.19
      1 perl(Storable) >= 2.18
      1 perl(strict)  
      1 perl(Time::HiRes) >= 1.9711
      1 perl(warnings)  
      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
TODO: Remove `perl(Scalar::Util) >= 1.23' from run-time dependencies. It's used obviously nowhere in the code.

$ rpm -q --provides -p ../RPMS/noarch/perl-POE-Component-Resolver-0.914-1.fc17.noarch.rpm |sort |uniq -c
      1 perl(POE::Component::Resolver) = 0.914
      1 perl-POE-Component-Resolver = 0.914-1.fc17
      1 perl(POE::Component::Resolver::Sidecar) = 0.914
Binary provides are Ok.

Binaries dependencies are resolvable. Ok.

FIX: Package does not build in F17 (http://koji.fedoraproject.org/koji/taskinfo?taskID=3715139).
It looks like of-by-one error if $#got_families == 1 at $i declaration:

            my $i = $#got_families;
            while ($i--) {
                splice(@got_families, $i, 1) if (
→                   $got_families[$i] == $got_families[$i+1]
                );
            }

Otherwise package is in line with Fedora and Perl packaging guidelines.


Please correct all `FIX' items, consider fixing `TODO' issues, and provide new spec file.

Resolution: Package NOT approved.

Comment 5 Petr Šabata 2012-01-20 11:21:03 UTC

(In reply to comment #4)
> I see. Resuming.
> 
> No XS code, noarch BuildArch is Ok.
> 
> FIX: Remove perl(POE::Filter::Reference) from build-requires or make it
> run-time dependency too (lib/POE/Component/Resolver.pm:219).
> FIX: Remove perl(POE::Filter::Reference) from build-requires or make it
> run-time dependency too (lib/POE/Component/Resolver.pm:218).
> 

I've added POE::Filter::Reference and POE::Wheel::Run (which is what you meant, I suppose) to Requires.

> TODO: Remove `perl(Scalar::Util) >= 1.23' from run-time dependencies. It's used
> obviously nowhere in the code.

Removed.

> FIX: Build-Require `perl(POE::Component::Resolver::Sidecar)' for tests
> (lib/POE/Component/Resolver.pm:15).

Won't do.  It's is provided by this package.

> 
> FIX: Package does not build in F17
> (http://koji.fedoraproject.org/koji/taskinfo?taskID=3715139).
> It looks like of-by-one error if $#got_families == 1 at $i declaration:
> 
>             my $i = $#got_families;
>             while ($i--) {
>                 splice(@got_families, $i, 1) if (
> →                   $got_families[$i] == $got_families[$i+1]
>                 );
>             }
> 

This test fails only in koji; the package works fine and all test pass in mock.
I've disabled the failing test for now.

--

Again, the updated package in available on the same URLs.

Comment 6 Petr Pisar 2012-01-20 13:28:55 UTC

Spec file changes:

--- perl-POE-Component-Resolver.spec.old        2012-01-19 15:19:28.000000000 +0100
+++ perl-POE-Component-Resolver.spec    2012-01-20 12:17:43.000000000 +0100
@@ -7,6 +7,7 @@
 URL:            http://search.cpan.org/dist/POE-Component-Resolver/
 Source0:        http://www.cpan.org/authors/id/R/RC/RCAPUTO/POE-Component-Resolver-%{version}.tar.gz
 BuildArch:      noarch
+BuildRequires:  perl(base)
 BuildRequires:  perl(Carp)
 BuildRequires:  perl(Exporter)
 BuildRequires:  perl(ExtUtils::MakeMaker)
@@ -20,7 +21,8 @@
 BuildRequires:  perl(Test::More) >= 0.96
 BuildRequires:  perl(Time::HiRes) >= 1.9711
 Requires:       perl(POE) >= 1.311
-Requires:       perl(Scalar::Util) >= 1.23
+Requires:       perl(POE::Filter::Reference)
+Requires:       perl(POE::Wheel::Run)
 Requires:       perl(Socket::GetAddrInfo) >= 0.19
 Requires:       perl(Storable) >= 2.18
 Requires:       perl(Time::HiRes) >= 1.9711
@@ -28,7 +30,6 @@
 
 %{?perl_default_filter}
 %global __requires_exclude %{?__requires_exclude:__requires_exclude|}^perl\\(POE\\)
-%global __requires_exclude %__requires_exclude|^perl\\(Scalar::Util\\)
 %global __requires_exclude %__requires_exclude|^perl\\(Socket::GetAddrInfo\\)
 %global __requires_exclude %__requires_exclude|^perl\\(Storable\\)
 %global __requires_exclude %__requires_exclude|^perl\\(Time::HiRes\\)
@@ -51,10 +52,12 @@
 %{_fixperms} %{buildroot}/*
 
 %check
+# Remove resolver test which doesn't work in koji
+rm -f t/01-basic.t
 make test
 
 %files
-%doc CHANGES LICENSE README README.mkdn
+%doc CHANGES LICENSE README
 %{perl_vendorlib}/*
 %{_mandir}/man3/*
 

> TODO: BuildRequire `perl(base)' for tests (lib/POE/Component/Resolver.pm:18)
+BuildRequires:  perl(base)
Ok.

> TODO: Package only README or only REAMDE.mkdn. Their identical but formatting.
 %files
-%doc CHANGES LICENSE README README.mkdn
+%doc CHANGES LICENSE README
Ok.

> FIX: Remove perl(POE::Filter::Reference) from build-requires or make it
> run-time dependency too (lib/POE/Component/Resolver.pm:219).
> FIX: Remove perl(POE::Filter::Reference) from build-requires or make it
> run-time dependency too (lib/POE/Component/Resolver.pm:218).
+Requires:       perl(POE::Filter::Reference)
+Requires:       perl(POE::Wheel::Run)
Ok.

> TODO: Remove `perl(Scalar::Util) >= 1.23' from run-time dependencies. It's
> used obviously nowhere in the code.
-Requires:       perl(Scalar::Util) >= 1.23
Ok.

> > FIX: Build-Require `perl(POE::Component::Resolver::Sidecar)' for tests
> > (lib/POE/Component/Resolver.pm:15).
> Won't do.  It's is provided by this package.
You are right. Ok.

All tests pass. Ok.

$ rpmlint perl-POE-Component-Resolver.spec ../SRPMS/perl-POE-Component-Resolver-0.914-1.fc17.src.rpm ../RPMS/noarch/perl-POE-Component-Resolver-0.914-1.fc17.noarch.rpm 
perl-POE-Component-Resolver.src: W: spelling-error Summary(en_US) getaddrinfo 
perl-POE-Component-Resolver.src: W: spelling-error %description -l en_US GetAddrInfo 
perl-POE-Component-Resolver.src: W: spelling-error %description -l en_US getaddrinfo 
perl-POE-Component-Resolver.src: W: spelling-error %description -l en_US subprocesses -> sub processes, sub-processes, processes
perl-POE-Component-Resolver.noarch: W: spelling-error Summary(en_US) getaddrinfo 
perl-POE-Component-Resolver.noarch: W: spelling-error %description -l en_US GetAddrInfo 
perl-POE-Component-Resolver.noarch: W: spelling-error %description -l en_US getaddrinfo 
perl-POE-Component-Resolver.noarch: W: spelling-error %description -l en_US subprocesses -> sub processes, sub-processes, processes
perl-POE-Component-Resolver.noarch: E: incorrect-fsf-address /usr/share/doc/perl-POE-Component-Resolver-0.914/LICENSE
2 packages and 1 specfiles checked; 1 errors, 8 warnings.
rpmlint is Ok.

> TODO: Remove `perl(Scalar::Util) >= 1.23' from run-time dependencies. It's used
> obviously nowhere in the code.
$ rpm -q --requires -p ../RPMS/noarch/perl-POE-Component-Resolver-0.914-1.fc17.noarch.rpm | sort |uniq -c
      1 perl(base)  
      1 perl(bytes)  
      1 perl(Carp)  
      1 perl(Config)  
      1 perl(Exporter)  
      1 perl(:MODULE_COMPAT_5.14.2)  
      1 perl(POE) >= 1.311
      1 perl(POE::Component::Resolver::Sidecar)  
      1 perl(POE::Filter::Reference)  
      1 perl(POE::Wheel::Run)  
      1 perl(Socket)  
      1 perl(Socket::GetAddrInfo) >= 0.19
      1 perl(Storable) >= 2.18
      1 perl(strict)  
      1 perl(Time::HiRes) >= 1.9711
      1 perl(warnings)  
      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.

Binaries dependencies are resolvable. Ok.

> > FIX: Package does not build in F17
> > (http://koji.fedoraproject.org/koji/taskinfo?taskID=3715139).
> This test fails only in koji; the package works fine and all test pass in
> mock. I've disabled the failing test for now.
Package builds in F17 (http://koji.fedoraproject.org/koji/taskinfo?taskID=3717450). Ok.


Resolution: Package APPROVED.

Comment 7 Petr Šabata 2012-01-20 13:38:31 UTC

New Package SCM Request
=======================
Package Name: perl-POE-Component-Resolver
Short Description: Non-blocking getaddrinfo() resolver
Owners: psabata mmaslano ppisar
Branches: 
InitialCC: perl-sig

Comment 8 Gwyn Ciesla 2012-01-20 14:10:37 UTC

Git done (by process-git-requests).

Comment 9 Petr Šabata 2012-01-20 14:33:09 UTC

Thank you, guys.