Bug 1713315 - Review Request: perl-AnyEvent-HTTP-Server - AnyEvent HTTP/1.1 Server
Summary: Review Request: perl-AnyEvent-HTTP-Server - AnyEvent HTTP/1.1 Server
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Petr Pisar
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2019-05-23 11:56 UTC by Yanko Kaneti
Modified: 2019-06-08 11:15 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-06-08 11:15:12 UTC
Type: ---
Embargoed:
ppisar: fedora-review+


Attachments (Terms of Use)

Description Yanko Kaneti 2019-05-23 11:56:26 UTC
Spec URL: http://declera.com/~yaneti/perl-AnyEvent-HTTP-Server/perl-AnyEvent-HTTP-Server.spec
SRPM URL: http://declera.com/~yaneti/perl-AnyEvent-HTTP-Server/perl-AnyEvent-HTTP-Server-1.99981-1.20190523gitb09c2c7.fc31.src.rpm
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

Comment 2 Petr Pisar 2019-06-06 14:39:59 UTC
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.

Comment 3 Yanko Kaneti 2019-06-06 16:39:01 UTC
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-Server.spec
SRPM URL: http://declera.com/~yaneti/perl-AnyEvent-HTTP-Server/perl-AnyEvent-HTTP-Server-1.99981-3.20190523gitb09c2c7.fc31.src.rpm

Comment 4 Petr Pisar 2019-06-07 07:23:34 UTC
>  %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.

Comment 5 Yanko Kaneti 2019-06-07 07:39:58 UTC
(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?

Comment 6 Petr Pisar 2019-06-07 08:13:54 UTC
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.

Comment 7 Yanko Kaneti 2019-06-08 11:15:12 UTC
Thanks.

Imported. Fixed all your latest suggestions. Built


Note You need to log in before you can comment on or make changes to this bug.