https://bugzilla.redhat.com/show_bug.cgi?id=1713315
Bug ID: 1713315 Summary: Review Request: perl-AnyEvent-HTTP-Server - AnyEvent HTTP/1.1 Server Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: yaneti@declera.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: http://declera.com/~yaneti/perl-AnyEvent-HTTP-Server/perl-AnyEvent-HTTP-Serv... SRPM URL: http://declera.com/~yaneti/perl-AnyEvent-HTTP-Server/perl-AnyEvent-HTTP-Serv... Description: AnyEvent::HTTP::Server is a very fast asynchronous HTTP server written in perl. It has been tested in high load production environments and may be considered both fast and stable.
Fedora Account System Username: yaneti
https://bugzilla.redhat.com/show_bug.cgi?id=1713315
--- Comment #1 from Yanko Kaneti yaneti@declera.com --- -2 files cleanup
Spec URL: http://declera.com/~yaneti/perl-AnyEvent-HTTP-Server/perl-AnyEvent-HTTP-Serv... SRPM URL: http://declera.com/~yaneti/perl-AnyEvent-HTTP-Server/perl-AnyEvent-HTTP-Serv...
https://bugzilla.redhat.com/show_bug.cgi?id=1713315
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?
https://bugzilla.redhat.com/show_bug.cgi?id=1713315
--- Comment #2 from Petr Pisar ppisar@redhat.com --- FIX: The upstream calls the software AnyEvent-HTTP-Server-II. Name this package perl-AnyEvent-HTTP-Server-II. There is no reason for diverging.
Url and Source addresses are Ok. Source archive (SHA-256: 461a4f3bc3897ab9e26cf3f0db51d193370f150fe95dce3daf1a65f8e19f7542) is original. Ok.
TODO: Remove a Group tag. It should not be used in Fedora.
License verified from lib/AnyEvent/HTTP/Server/Req.pm, lib/AnyEvent/HTTP/Server.pm, README, Makefile.PL, LICENSE. Ok.
FIX: Build-require 'perl-interpreter' (perl-AnyEvent-HTTP-Server.spec:29). TODO: You should use plain 'perl' commands instead of '%{__perl}' macro. FIX: Build-require 'make' (perl-AnyEvent-HTTP-Server.spec:41).
FIX: Either build-require findutils (perl-AnyEvent-HTTP-Server.spec:46), or build-require 'perl(ExtUtils::MakeMaker) >= 6.76' and supply 'NO_PACKLIST=1 NO_PERLLOCAL=1' arguments Makefile.PL. Then you don't have to run the two find commands and instead of 'make pure_install...' you can just call '%{make_install}'.
TODO: Use '%{make_build} instead of 'make %{?_smp_mflags}'.
TODO: Do not package META.json file. It does not bring any new data to the users. TODO: Package 'ex' directory as a documentation.
TODO: Unset AUTHOR environment variable before runnig Makefile.PL. Otherwise the build would require additional dependencies.
FIX: Build-require 'perl(:VERSION) >= 5.10' (lib/AnyEvent/HTTP/Server/WS.pm:3). FIX: Build-require 'perl(Data::Dumper)', 'perl(feature)', 'perl(strict)','perl(utf8)', 'perl(warnings)' (Kit.pm.PL). FIX: Build-require 'perl(AnyEvent::Handle)' (t/01-basic.t:7). FIX: Build-require 'perl(AnyEvent::Socket)' (t/01-basic.t:6). FIX: Build-require 'perl(Test::More)' (COMPILE t/01-basic.t:11). FIX: Build-require 'perl(AnyEvent::Util) (lib/AnyEvent/HTTP/Server.pm:28). FIX: Build-require 'perl(Config)' (lib/AnyEvent/HTTP/Server/WS.pm:7). FIX: Build-require 'prl(constant)' (lib/AnyEvent/HTTP/Server/Req.pm:57). FIX: Build-require 'perl(Encode)' (lib/AnyEvent/HTTP/Server.pm:31). FIX: Build-require 'perl(MIME::Base64)' (lib/AnyEvent/HTTP/Server.pm:33). FIX: Build-require 'perl(overload)' (lib/AnyEvent/HTTP/Server/Req.pm:19). FIX: Build-require 'perl(Scalar::Util)' (lib/AnyEvent/HTTP/Server.pm:26). FIX: Build-require 'perl(Socket)' (lib/AnyEvent/HTTP/Server.pm:29). FIX: Build-require 'perl(Time::HiRes) (lib/AnyEvent/HTTP/Server.pm:34).
$ rpmlint perl-AnyEvent-HTTP-Server.spec ../SRPMS/perl-AnyEvent-HTTP-Server-1.99981-2.20190523gitb09c2c7.fc31.src.rpm ../RPMS/noarch/perl-AnyEvent-HTTP-Server-1.99981-2.20190523gitb09c2c7.fc31.noarch.rpm /usr/share/rpmlint/Pkg.py:168: UnicodeWarning: decode() called on unicode string, see https://bugzilla.redhat.com/show_bug.cgi?id=1693751 s.decode('UTF-8') /usr/share/rpmlint/Pkg.py:168: UnicodeWarning: decode() called on unicode string, see https://bugzilla.redhat.com/show_bug.cgi?id=1693751 s.decode('UTF-8') 2 packages and 1 specfiles checked; 0 errors, 0 warnings. rpmlint is Ok.
$ rpm -q -lv -p ../RPMS/noarch/perl-AnyEvent-HTTP-Server-1.99981-2.20190523gitb09c2c7.fc31.noarch.rpm drwxr-xr-x 2 root root 0 Jun 6 16:24 /usr/share/doc/perl-AnyEvent-HTTP-Server -rw-r--r-- 1 root root 525 Oct 28 2018 /usr/share/doc/perl-AnyEvent-HTTP-Server/Changes -rw-r--r-- 1 root root 941 Oct 28 2018 /usr/share/doc/perl-AnyEvent-HTTP-Server/META.json -rw-r--r-- 1 root root 2551 Oct 28 2018 /usr/share/doc/perl-AnyEvent-HTTP-Server/README.md drwxr-xr-x 2 root root 0 Jun 6 16:24 /usr/share/licenses/perl-AnyEvent-HTTP-Server -rw-r--r-- 1 root root 157 Oct 28 2018 /usr/share/licenses/perl-AnyEvent-HTTP-Server/LICENSE -rw-r--r-- 1 root root 3374 Jun 6 16:24 /usr/share/man/man3/AnyEvent::HTTP::Server.3pm.gz -rw-r--r-- 1 root root 3037 Jun 6 16:24 /usr/share/man/man3/AnyEvent::HTTP::Server::Req.3pm.gz drwxr-xr-x 2 root root 0 Jun 6 16:24 /usr/share/perl5/vendor_perl/AnyEvent drwxr-xr-x 2 root root 0 Jun 6 16:24 /usr/share/perl5/vendor_perl/AnyEvent/HTTP drwxr-xr-x 2 root root 0 Jun 6 16:24 /usr/share/perl5/vendor_perl/AnyEvent/HTTP/Server -rw-r--r-- 1 root root 31675 Oct 28 2018 /usr/share/perl5/vendor_perl/AnyEvent/HTTP/Server.pm -rw-r--r-- 1 root root 2713 Jun 6 16:24 /usr/share/perl5/vendor_perl/AnyEvent/HTTP/Server/Kit.pm -rw-r--r-- 1 root root 20105 Oct 28 2018 /usr/share/perl5/vendor_perl/AnyEvent/HTTP/Server/Req.pm -rw-r--r-- 1 root root 8958 Oct 28 2018 /usr/share/perl5/vendor_perl/AnyEvent/HTTP/Server/WS.pm File permissions and locations are Ok.
$ rpm -q --requires -p ../RPMS/noarch/perl-AnyEvent-HTTP-Server-1.99981-2.20190523gitb09c2c7.fc31.noarch.rpm | sort -f | uniq -c 1 perl(:MODULE_COMPAT_5.30.0) 1 perl(:VERSION) >= 5.10.0 1 perl(AnyEvent) 1 perl(AnyEvent) >= 5 1 perl(AnyEvent::Handle) 1 perl(AnyEvent::HTTP::Server::Kit) 1 perl(AnyEvent::HTTP::Server::Req) 1 perl(AnyEvent::HTTP::Server::WS) 1 perl(AnyEvent::Socket) 1 perl(AnyEvent::Util) 1 perl(Compress::Zlib) 1 perl(Config) 1 perl(constant) 1 perl(Data::Dumper) 1 perl(Digest::SHA1) 1 perl(Digest::SHA1) >= 2 1 perl(Encode) 1 perl(Errno) 1 perl(JSON::XS) 1 perl(JSON::XS) >= 3 1 perl(MIME::Base64) 1 perl(overload) 1 perl(Scalar::Util) 1 perl(Socket) 1 perl(strict) 1 perl(Time::HiRes) 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 the unversioned dependencies on 'perl(AnyEvent)', 'perl(Digest::SHA1)', and 'perl(JSON::XS)'. They are redundant.
$ rpm -q --provides -p ../RPMS/noarch/perl-AnyEvent-HTTP-Server-1.99981-2.20190523gitb09c2c7.fc31.noarch.rpm | sort -f | uniq -c 1 perl(AnyEvent::HTTP::Server) = 1.99981 1 perl(AnyEvent::HTTP::Server::Form) = 1.97 1 perl(AnyEvent::HTTP::Server::Kit) 1 perl(AnyEvent::HTTP::Server::Req) 1 perl(AnyEvent::HTTP::Server::WS) 1 perl(AnyEvent::HTTP::Server::WS::CLOSING) 1 perl-AnyEvent-HTTP-Server = 1.99981-2.20190523gitb09c2c7.fc31 Binary provides are Ok.
$ resolvedeps f31-build ../RPMS/noarch/perl-AnyEvent-HTTP-Server-1.99981-2.20190523gitb09c2c7.fc31.noarch.rpm Binary dependencies are resolvable. Ok.
The package builds in F31 (https://koji.fedoraproject.org/koji/taskinfo?taskID=35338134). Ok.
Otherwise the package is in line with Fedora and Perl packaging guidelines.
Please correct all 'FIX' items, consider fixing 'TODO' items and provide a new spec file. Resolution: NOT approved.
https://bugzilla.redhat.com/show_bug.cgi?id=1713315
--- Comment #3 from Yanko Kaneti yaneti@declera.com --- Thanks for the review and sorry for having so many issues to begin with. I've followed most of your recommendation except (maybe the most important one ;):
- > The upstream calls the software AnyEvent-HTTP-Server-II. Name this package perl-AnyEvent-HTTP-Server-II. There is no reason for diverging.
I think the only reason there is a -II repository is for the authors desire to stress that the module has been mostly rewritten. For all other intents and purposes this is perl-AnyEvent-HTTP-Server version 2.0 (a 1.9... prerelease) and the name conflict is intentional, even the readme says so.
- TODO: Remove the unversioned dependencies on 'perl(AnyEvent)', 'perl(Digest::SHA1)', and 'perl(JSON::XS)'. They are redundant.
This sounds to me like a regression in rpm-build, the release notes for rpm 4.13 explicitly say "Filter automatic unversioned dependencies when versioned ones exist (RhBug:678605)" with rationale being a perl related bug
-3 Incorporate review feedack (#1713315)
Spec URL: http://declera.com/~yaneti/perl-AnyEvent-HTTP-Server/perl-AnyEvent-HTTP-Serv... SRPM URL: http://declera.com/~yaneti/perl-AnyEvent-HTTP-Server/perl-AnyEvent-HTTP-Serv...
https://bugzilla.redhat.com/show_bug.cgi?id=1713315
--- Comment #4 from Petr Pisar ppisar@redhat.com ---
%install +perl -pi -e 's,#!.*perl,#!%{__perl},' ex/*.pl
TODO: I recommend moving the command to %prep section because it patches the sources. I also recommend using $Config{startperl} Perl variable from a Config module instead of %{__perl} RPM macro. The macro is a shell command and can contain any shell code. While the shell bang line can contain only an execve() command.
FIX: Build-require 'perl(feature)', 'perl(strict)','perl(utf8)', 'perl(warnings)'. It seems you forgot to add them.
$ rpmlint perl-AnyEvent-HTTP-Server.spec ../SRPMS/perl-AnyEvent-HTTP-Server-1.99981-3.20190523gitb09c2c7.fc31.src.rpm ../RPMS/noarch/perl-AnyEvent-HTTP-Server-1.99981-3.20190523gitb09c2c7.fc31.noarch.rpm /usr/share/rpmlint/Pkg.py:168: UnicodeWarning: decode() called on unicode string, see https://bugzilla.redhat.com/show_bug.cgi?id=1693751 s.decode('UTF-8') /usr/share/rpmlint/Pkg.py:168: UnicodeWarning: decode() called on unicode string, see https://bugzilla.redhat.com/show_bug.cgi?id=1693751 s.decode('UTF-8') 2 packages and 1 specfiles checked; 0 errors, 0 warnings. rpmlint is Ok.
I think the only reason there is a -II repository is for the authors desire to stress that the module has been mostly rewritten. For all other intents and purposes this is perl-AnyEvent-HTTP-Server version 2.0 (a 1.9... prerelease) and the name conflict is intentional, even the readme says so.
The readme says:
This is a second verson available as AnyEvent-HTTP-Server-II. The first version is now obsolette.
If the author wanted to obsolete AnyEvent-HTTP-Server, he would use the same name and uploaded it to CPAN under that name. None of that happened. It's a different project with a different name. Also imagine that somebody wanted to package the real AnyEvent-HTTP-Server from CPAN. Occupying the name prevents him from doing it. Also naming it perl-AnyEvent-HTTP-Server makes an impression that it is the AnyEvent-HTTP-Server project, but that's not true. It has a completely different author.
I really think this should be named perl-AnyEvent-HTTP-Server-II. Once upstream renames it to AnyEvent-HTTP-Server, you can also rename the package. Please do not make any assumptions about author's intentions and follow the current naming. If you don't agree, we can ask Fedora Packaging Committee for their opinion.
This sounds to me like a regression in rpm-build, the release notes for rpm 4.13 explicitly say "Filter automatic unversioned dependencies when versioned ones exist (RhBug:678605)" with rationale being a perl related bug
Minimizing dependencies among manually specified and generated ones has never worked. That's my experience. I believe RPM maintainers never intended to fix bug #678605 completely. Instead they only remove unversioned generated dependencies if a versioned generated dependency exists. But that's more or less irrelevant now because the dependency generator for Perl is maintained in perl-generators now and perl-generators itself does not produce redundant dependencies. You can reopen the bug if you are not content with the rpm-build behavior.
In this package, you can either patch sources to list the minimal version at places where they load the modules, or you can use https://docs.fedoraproject.org/en-US/packaging-guidelines/AutoProvidesAndRequiresFiltering/ filtering like this:
%global __requires_exclude %{?__requires_exclude:%{__requires_exclude}|}^perl\(AnyEvent|Digest::SHA1|JSON::XS)\)$
Please add the missing build-requires, rename the package and provide a new spec file.
https://bugzilla.redhat.com/show_bug.cgi?id=1713315
--- Comment #5 from Yanko Kaneti yaneti@declera.com --- (In reply to Petr Pisar from comment #4)
I think the only reason there is a -II repository is for the authors desire to stress that the module has been mostly rewritten. For all other intents and purposes this is perl-AnyEvent-HTTP-Server version 2.0 (a 1.9... prerelease) and the name conflict is intentional, even the readme says so.
The readme says:
This is a second verson available as AnyEvent-HTTP-Server-II. The first version is now obsolette.If the author wanted to obsolete AnyEvent-HTTP-Server, he would use the same name and uploaded it to CPAN under that name. None of that happened. It's a different project with a different name. Also imagine that somebody wanted to package the real AnyEvent-HTTP-Server from CPAN. Occupying the name prevents him from doing it. Also naming it perl-AnyEvent-HTTP-Server makes an impression that it is the AnyEvent-HTTP-Server project, but that's not true. It has a completely different author.
I really think this should be named perl-AnyEvent-HTTP-Server-II. Once upstream renames it to AnyEvent-HTTP-Server, you can also rename the package. Please do not make any assumptions about author's intentions and follow the current naming. If you don't agree, we can ask Fedora Packaging Committee for their opinion.
There is no AnyEvent::HTTP::Server in CPAN now and it was either never on CPAN or it was there by the same author who later decided to remove it from there ... The only other AnyEvent::HTTP::Server I can find is https://github.com/Mons/AnyEvent-HTTP-Server i.e same author
What other AnyEvent::HTTP::Server are you refering to?
https://bugzilla.redhat.com/show_bug.cgi?id=1713315
Petr Pisar ppisar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #6 from Petr Pisar ppisar@redhat.com --- I see. I beg your pardon. The modern MetaCPAN did a fuzzy search and provided me https://metacpan.org/release/Test-HTTP-AnyEvent-Server. And I overlooked the difference in the name.
In the light of this news, I take back my request for renaming the package. Although, technically the upstream has a different name, I think it's acceptable to coerce the name to perl-AnyEvent-HTTP-Server.
Please add the missing dependencies before building this package. Resolution: ACCEPTED.
https://bugzilla.redhat.com/show_bug.cgi?id=1713315
Yanko Kaneti yaneti@declera.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution|--- |RAWHIDE Last Closed| |2019-06-08 11:15:12
--- Comment #7 from Yanko Kaneti yaneti@declera.com --- Thanks.
Imported. Fixed all your latest suggestions. Built
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com