Bug 1701810 - Review Request: perl-Net-BGP - Perl module for object-oriented API to the BGP protocol
Summary: Review Request: perl-Net-BGP - Perl module for object-oriented API to the BGP...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Petr Pisar
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2019-04-22 03:22 UTC by Robert Scheck
Modified: 2019-05-13 01:46 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-05-06 00:45:22 UTC
Type: Bug
ppisar: fedora-review+


Attachments (Terms of Use)

Description Robert Scheck 2019-04-22 03:22:27 UTC
Spec URL: https://labs.linuxnetz.de/bugzilla/perl-Net-BGP.spec
SRPM URL: https://labs.linuxnetz.de/bugzilla/perl-Net-BGP-0.16-1.src.rpm
Description: An implementation of the BGP-4 inter-domain routing protocol as perl module. It encapsulates all of the functionality needed to establish and maintain a BGP peering session and exchange routing update information with the peer. It aims to provide a simple API to the BGP protocol for the purposes of automation, logging, monitoring, testing, and similar tasks using the power and flexibility of perl. The module does not implement the functionality of a RIB (Routing Information Base) nor does it modify the kernel routing table of the host system. However, such operations could be implemented using the API provided by the module.

Comment 1 Petr Pisar 2019-04-24 14:32:43 UTC
URL and Source0 addresses are Ok.
Source0 archive (SHA-256: d0082f5df955f1e720f1f221d07ac8f1b38640444a2f410d46a1e607f4fbd89e) is original. Ok.
Summary is Ok.
Description is Ok.

TODO: Capitalize "perl" as "Perl" in the description text. "perl" denotes /usr/bin/perl program while "Perl" denotes a programing language.

License verified from README and Makefile.PL files. Ok.
No XS code found, noarch BuildArch is Ok.

FIX: Do not use %makeinstall macro. It's forbidden. Use %make_install instead.
TODO: If you constrain 'BuildRequires: perl(ExtUtils::MakeMaker)' with '>= 6.77', and append 'NO_PACKLIST=1 NOPERLLOCAL=1' to 'perl Makefile.PL' command, you can use recommended %make_build instead of 'make %{?_smp_mflags}' and you cane remove the 'find' command from %install section.

FIX: Package README using %doc macro. It's the only file that declares a license.

TODO: Constrain perl(Scalar::Util) dependency with '>= 1.01 (Makefile.PL:16).

FIX: Build-require perl(File::Find) (t/01-pod.t:3).
FIX: Build-require perl(File::Spec) (t/01-pod.t:2).
FIX: Remove a dependency on perl(Test::Harness). It's nowhere used.

TODO: Do not build-require perl(Test::Signature) and preferably remove t/00-Signature.t file. First it's pointless checking a signature after the signed code had already been executed. Second if Fedora needs patching the code, the signature will become invalid, third there is no public key preloaded and thus the signature cannot be validated. Fourth, the test fails for me even now.

FIX: Tests do not pass:

+ make test
PERL_DL_NONLAZY=1 "/usr/bin/perl" "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harness(0, 'blib/lib', 'blib/arch')" t/*.t
t/00-Signature.t ...... Old SIGNATURE detected. Please inform the module author to regenerate SIGNATURE using Module::Signature version 0.82 or newer.
gpg: WARNING: unsafe permissions on homedir '/home/test/.gnupg'
gpg: WARNING: unsafe permissions on homedir '/home/test/.gnupg'
WARNING: This key is not certified with a trusted signature!
Primary key fingerprint: BD2B 7528 2941 2518 0F94  543A A57B F5B2 9322 96DD
Not in MANIFEST: debugfiles.list
Not in MANIFEST: debuglinks.list
Not in MANIFEST: debugsourcefiles.list
Not in MANIFEST: debugsources.list
Not in MANIFEST: elfbins.list
==> MISMATCHED content between MANIFEST and distribution files! <==
t/00-Signature.t ...... 1/1
#   Failed test 'Valid signature'
#   at t/00-Signature.t line 13.
# Looks like you failed 1 test of 1.
t/00-Signature.t ...... Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/1 subtests

Please correct all the 'FIX' items, consider fixing 'TODO' items and provide an updated spec file.

Comment 2 Robert Scheck 2019-04-24 21:38:51 UTC
I'm sorry, I mixed up %makeinstall and %make_install. I didn't notice the t/00-Signature.t failure in a local mockbuild for EPEL 7, but your suggestion makes sense. Given the package is intended for EPEL as well, I've added two minor switches.

Spec URL: https://labs.linuxnetz.de/bugzilla/perl-Net-BGP.spec
SRPM URL: https://labs.linuxnetz.de/bugzilla/perl-Net-BGP-0.16-2.src.rpm

Comment 3 Petr Pisar 2019-04-25 09:02:08 UTC
Changes in the spec file:

--- perl-Net-BGP.spec.old       2019-04-22 05:22:35.000000000 +0200
+++ perl-Net-BGP.spec   2019-04-24 23:30:07.000000000 +0200
@@ -6,7 +6,7 @@
 Summary:        Perl module for object-oriented API to the BGP protocol
 Name:           perl-Net-BGP
 Version:        0.16
-Release:        1%{?dist}
+Release:        2%{?dist}
 License:        GPL+ or Artistic
 URL:            https://metacpan.org/release/%{pkgname}
 Source:         https://cpan.metacpan.org/authors/id/S/SS/SSCHECK/%{pkgname}-%{version}.tar.gz
@@ -14,7 +14,11 @@
 BuildRequires:  perl-generators
 BuildRequires:  perl-interpreter
 BuildRequires:  make
+%if 0%{?rhel} && 0%{?rhel} <= 7
 BuildRequires:  perl(ExtUtils::MakeMaker)
+%else
+BuildRequires:  perl(ExtUtils::MakeMaker) >= 6.77
+%endif
 # Run-time
 BuildRequires:  perl(bytes)
 BuildRequires:  perl(Carp)
@@ -23,23 +27,23 @@
 BuildRequires:  perl(IO::Select)
 BuildRequires:  perl(IO::Socket)
 BuildRequires:  perl(overload)
-BuildRequires:  perl(Scalar::Util)
+BuildRequires:  perl(Scalar::Util) >= 1.01
 BuildRequires:  perl(strict)
 BuildRequires:  perl(vars)
 # Tests
-BuildRequires:  perl(Test::Harness) >= 2.00
+BuildRequires:  perl(File::Find)
+BuildRequires:  perl(File::Spec)
 BuildRequires:  perl(Test::More) >= 0.47
-BuildRequires:  perl(Test::Signature)
 BuildRequires:  perl(Test::Pod) >= 0.95
 BuildArch:      noarch

 %description
-An implementation of the BGP-4 inter-domain routing protocol as perl module.
+An implementation of the BGP-4 inter-domain routing protocol as Perl module.
 It encapsulates all of the functionality needed to establish and maintain a
 BGP peering session and exchange routing update information with the peer.
 It aims to provide a simple API to the BGP protocol for the purposes of
 automation, logging, monitoring, testing, and similar tasks using the power
-and flexibility of perl. The module does not implement the functionality of
+and flexibility of Perl. The module does not implement the functionality of
 a RIB (Routing Information Base) nor does it modify the kernel routing table
 of the host system. However, such operations could be implemented using the
 API provided by the module.
@@ -48,23 +52,31 @@
 %setup -q -n %{pkgname}-%{version}

 %build
-perl Makefile.PL PREFIX=$RPM_BUILD_ROOT%{_prefix} INSTALLDIRS=vendor
-make %{?_smp_mflags}
+perl Makefile.PL INSTALLDIRS=vendor NO_PACKLIST=1 NO_PERLLOCAL=1
+%make_build

 %install
-%makeinstall
+%make_install
+%if 0%{?rhel} && 0%{?rhel} <= 7
 find $RPM_BUILD_ROOT \( -name perllocal.pod -o -name .packlist \) -exec rm -f {} \;
+%endif
 chmod -R u+w $RPM_BUILD_ROOT/*

+# Remove signature test (#1701810)
+rm -f t/00-Signature.t
+
 %check
 make test

 %files
-%doc Changes
+%doc Changes README
 %{perl_vendorlib}/Net/
 %{_mandir}/man3/*.3pm*

 %changelog
+* Wed Apr 24 2019 Robert Scheck <robert> 0.16-2
+- Add corrections from package review (#1701810 #c1)
+

All issues resolved. Ok.

TODO: The version in 'perl(ExtUtils::MakeMaker) >= 6.77' should be '6.76'. I made a typo. Sorry.
TODO: Build-require coreutils for the 'rm' command.
TODO: Conditionally build-require findutils for the 'find' command.

All tests pass. Ok.

$ rpmlint perl-Net-BGP.spec ../SRPMS/perl-Net-BGP-0.16-2.fc31.src.rpm ../RPMS/noarch/perl-Net-BGP-0.16-2.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-Net-BGP-0.16-2.fc31.noarch.rpm 
drwxr-xr-x    2 root    root                        0 Apr 25 10:53 /usr/share/doc/perl-Net-BGP
-rw-r--r--    1 root    root                     7647 Jan 28  2015 /usr/share/doc/perl-Net-BGP/Changes
-rw-r--r--    1 root    root                     1257 Jan 28  2015 /usr/share/doc/perl-Net-BGP/README
-rw-r--r--    1 root    root                     3279 Apr 25 10:52 /usr/share/man/man3/Net::BGP.3pm.gz
-rw-r--r--    1 root    root                     3349 Apr 25 10:52 /usr/share/man/man3/Net::BGP::ASPath.3pm.gz
-rw-r--r--    1 root    root                     3327 Apr 25 10:52 /usr/share/man/man3/Net::BGP::NLRI.3pm.gz
-rw-r--r--    1 root    root                     2045 Apr 25 10:52 /usr/share/man/man3/Net::BGP::Notification.3pm.gz
-rw-r--r--    1 root    root                     6796 Apr 25 10:52 /usr/share/man/man3/Net::BGP::Peer.3pm.gz
-rw-r--r--    1 root    root                     2548 Apr 25 10:52 /usr/share/man/man3/Net::BGP::Process.3pm.gz
-rw-r--r--    1 root    root                     1847 Apr 25 10:52 /usr/share/man/man3/Net::BGP::Refresh.3pm.gz
-rw-r--r--    1 root    root                     2104 Apr 25 10:52 /usr/share/man/man3/Net::BGP::Transport.3pm.gz
-rw-r--r--    1 root    root                     3040 Apr 25 10:52 /usr/share/man/man3/Net::BGP::Update.3pm.gz
drwxr-xr-x    2 root    root                        0 Apr 25 10:52 /usr/share/perl5/vendor_perl/Net
drwxr-xr-x    2 root    root                        0 Apr 25 10:52 /usr/share/perl5/vendor_perl/Net/BGP
-rw-r--r--    1 root    root                     5334 Jan 28  2015 /usr/share/perl5/vendor_perl/Net/BGP.pm
drwxr-xr-x    2 root    root                        0 Apr 25 10:52 /usr/share/perl5/vendor_perl/Net/BGP/ASPath
-rw-r--r--    1 root    root                    17098 Jan 28  2015 /usr/share/perl5/vendor_perl/Net/BGP/ASPath.pm
-rw-r--r--    1 root    root                     5596 Jan 28  2015 /usr/share/perl5/vendor_perl/Net/BGP/ASPath/AS.pm
-rw-r--r--    1 root    root                      581 Jan 28  2015 /usr/share/perl5/vendor_perl/Net/BGP/ASPath/AS_CONFED_SEQUENCE.pm
-rw-r--r--    1 root    root                      472 Jan 28  2015 /usr/share/perl5/vendor_perl/Net/BGP/ASPath/AS_CONFED_SET.pm
-rw-r--r--    1 root    root                      681 Jan 28  2015 /usr/share/perl5/vendor_perl/Net/BGP/ASPath/AS_SEQUENCE.pm
-rw-r--r--    1 root    root                      995 Jan 28  2015 /usr/share/perl5/vendor_perl/Net/BGP/ASPath/AS_SET.pm
-rw-r--r--    1 root    root                    13924 Jan 28  2015 /usr/share/perl5/vendor_perl/Net/BGP/NLRI.pm
-rw-r--r--    1 root    root                     6842 Jan 28  2015 /usr/share/perl5/vendor_perl/Net/BGP/Notification.pm
-rw-r--r--    1 root    root                    30083 Jan 28  2015 /usr/share/perl5/vendor_perl/Net/BGP/Peer.pm
-rw-r--r--    1 root    root                    11937 Jan 28  2015 /usr/share/perl5/vendor_perl/Net/BGP/Process.pm
-rw-r--r--    1 root    root                     4149 Jan 28  2015 /usr/share/perl5/vendor_perl/Net/BGP/Refresh.pm
-rw-r--r--    1 root    root                    43171 Jan 28  2015 /usr/share/perl5/vendor_perl/Net/BGP/Transport.pm
-rw-r--r--    1 root    root                    30217 Jan 28  2015 /usr/share/perl5/vendor_perl/Net/BGP/Update.pm
File location and permissions are Ok.

$ rpm -q --requires -p ../RPMS/noarch/perl-Net-BGP-0.16-2.fc31.noarch.rpm | sort -f | uniq -c
      1 perl(:MODULE_COMPAT_5.28.2)
      1 perl(bytes)
      1 perl(Carp)
      1 perl(Errno)
      1 perl(Exporter)
      1 perl(IO::Select)
      1 perl(IO::Socket)
      1 perl(Net::BGP::ASPath)
      1 perl(Net::BGP::ASPath::AS)
      1 perl(Net::BGP::ASPath::AS_CONFED_SEQUENCE)
      1 perl(Net::BGP::ASPath::AS_CONFED_SET)
      1 perl(Net::BGP::ASPath::AS_SEQUENCE)
      1 perl(Net::BGP::ASPath::AS_SET)
      1 perl(Net::BGP::NLRI)
      1 perl(Net::BGP::Notification)
      1 perl(Net::BGP::Peer)
      1 perl(Net::BGP::Refresh)
      1 perl(Net::BGP::Transport)
      1 perl(Net::BGP::Update)
      1 perl(overload)
      1 perl(Scalar::Util)
      1 perl(strict)
      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
TODO: Add '>= 1.01' contstraint to 'perl(Scalar::Util)' run-time dependency. After that you should also filter the unversioned one.

$ rpm -q --provides -p ../RPMS/noarch/perl-Net-BGP-0.16-2.fc31.noarch.rpm | sort -f | uniq -c
      1 perl(Net::BGP) = 0.16
      1 perl(Net::BGP::ASPath) = 0.16
      1 perl(Net::BGP::ASPath::AS) = 0.16
      1 perl(Net::BGP::ASPath::AS_CONFED_SEQUENCE) = 0.16
      1 perl(Net::BGP::ASPath::AS_CONFED_SET) = 0.16
      1 perl(Net::BGP::ASPath::AS_SEQUENCE) = 0.16
      1 perl(Net::BGP::ASPath::AS_SET) = 0.16
      1 perl(Net::BGP::NLRI) = 0.16
      1 perl(Net::BGP::Notification) = 0.07
      1 perl(Net::BGP::Peer) = 0.16
      1 perl(Net::BGP::Process) = 0.15
      1 perl(Net::BGP::Refresh) = 0.07
      1 perl(Net::BGP::Transport) = 0.16
      1 perl(Net::BGP::Update) = 0.16
      1 perl-Net-BGP = 0.16-2.fc31
Binary provides are Ok.

$ resolvedeps rawhide ../RPMS/noarch/perl-Net-BGP-0.16-2.fc31.noarch.rpm 
Binary dependencies are resolvable. Ok.

The package builds in F31 (https://koji.fedoraproject.org/koji/taskinfo?taskID=34417844). Ok.

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

Please consider fixing the 'TODO' items before building this package.
Resolution: Package APPROVED.

Comment 4 Gwyn Ciesla 2019-04-25 13:59:03 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/perl-Net-BGP

Comment 5 Fedora Update System 2019-04-25 22:22:48 UTC
perl-Net-BGP-0.16-2.fc30 has been submitted as an update to Fedora 30. https://bodhi.fedoraproject.org/updates/FEDORA-2019-0f787e7caf

Comment 6 Fedora Update System 2019-04-25 22:23:19 UTC
perl-Net-BGP-0.16-2.fc29 has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2019-1ec034c891

Comment 7 Fedora Update System 2019-04-25 22:28:16 UTC
perl-Net-BGP-0.16-2.fc28 has been submitted as an update to Fedora 28. https://bodhi.fedoraproject.org/updates/FEDORA-2019-bc11f71f51

Comment 8 Fedora Update System 2019-04-25 22:28:45 UTC
perl-Net-BGP-0.16-2.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2019-fa66ed2fed

Comment 9 Fedora Update System 2019-04-25 22:29:14 UTC
perl-Net-BGP-0.16-2.el6 has been submitted as an update to Fedora EPEL 6. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2019-c76200515c

Comment 10 Fedora Update System 2019-04-27 22:30:16 UTC
perl-Net-BGP-0.16-2.fc30 has been pushed to the Fedora 30 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-0f787e7caf

Comment 11 Fedora Update System 2019-04-27 23:11:39 UTC
perl-Net-BGP-0.16-2.fc28 has been pushed to the Fedora 28 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-bc11f71f51

Comment 12 Fedora Update System 2019-04-27 23:36:18 UTC
perl-Net-BGP-0.16-2.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2019-fa66ed2fed

Comment 13 Fedora Update System 2019-04-28 00:13:32 UTC
perl-Net-BGP-0.16-2.fc29 has been pushed to the Fedora 29 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-1ec034c891

Comment 14 Fedora Update System 2019-04-28 01:10:26 UTC
perl-Net-BGP-0.16-2.el6 has been pushed to the Fedora EPEL 6 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2019-c76200515c

Comment 15 Fedora Update System 2019-05-06 00:45:22 UTC
perl-Net-BGP-0.16-2.fc30 has been pushed to the Fedora 30 stable repository. If problems still persist, please make note of it in this bug report.

Comment 16 Fedora Update System 2019-05-06 03:10:50 UTC
perl-Net-BGP-0.16-2.fc28 has been pushed to the Fedora 28 stable repository. If problems still persist, please make note of it in this bug report.

Comment 17 Fedora Update System 2019-05-06 04:15:02 UTC
perl-Net-BGP-0.16-2.fc29 has been pushed to the Fedora 29 stable repository. If problems still persist, please make note of it in this bug report.

Comment 18 Fedora Update System 2019-05-13 01:05:16 UTC
perl-Net-BGP-0.16-2.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.

Comment 19 Fedora Update System 2019-05-13 01:46:38 UTC
perl-Net-BGP-0.16-2.el6 has been pushed to the Fedora EPEL 6 stable repository. If problems still persist, please make note of it in this bug report.


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