Bug 2217967 - Review Request: perl-Devel-StackTrace-Extract - Extract a stack trace from an exception object
Summary: Review Request: perl-Devel-StackTrace-Extract - Extract a stack trace from an...
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: 2217660
TreeView+ depends on / blocked
 
Reported: 2023-06-27 17:04 UTC by Xavier Bachelot
Modified: 2023-07-03 15:24 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2023-07-03 15:24:07 UTC
Type: ---
Embargoed:
ppisar: fedora-review+


Attachments (Terms of Use)

Description Xavier Bachelot 2023-06-27 17:04:20 UTC
Spec URL: https://www.bachelot.org/fedora/SPECS/perl-Devel-StackTrace-Extract.spec
SRPM URL: https://www.bachelot.org/fedora/SRPMS/perl-Devel-StackTrace-Extract-1.000000-1.fc39.src.rpm
Description:
It's popular to store stack traces in objects that are thrown as
exceptions, but, this being Perl, there's more than one way to do it. This
module provides a simple interface to attempt to extract the stack trace
from various well known exception classes that are on the CPAN.
Fedora Account System Username: xavierb

Comment 1 Petr Pisar 2023-06-28 10:43:58 UTC
URL and Source0 address are Ok.
TODO: Consider using <https://cpan.metacpan.org/authors/id/M/MA/MAXMIND/Devel-StackTrace-Extract-1.000000.tar.gz> for Source0. It's the canonical path listed on the home page.
Source0 archive (SHA-512: 4e90aeafc410c69680b062206cf42010cd2f5182c54c9c830adf61058798cf9b7fb0e5c6fe47e08d3dc8750318600dc803a7d975bba764a4b29fb6ae0f924953) is original. Ok.
Summary verified from lib/Devel/StackTrace/Extract.pm. Ok.
Description verified from lib/Devel/StackTrace/Extract.pm. Ok.
License verified from README.md, Makefile.PL, LICENSE, and lib/Devel/StackTrace/Extract.pm. Ok.
No XS code, noarch BuildArch is Ok.

FIX: Replace 'perl >= 0:5.006' build-dependency with 'perl(VERSION) >= 5.6' to declare a minimal version of Perl language and with 'perl-interpreter' to declare a dependency on a perl executable. 'perl' itself is unnecessarily large as it includes all core modules, header files, C compiler etc. '>= 0:5.006' is wrong because the epoch number does not align with Fedora history.

TODO: Consider build-requiring all exception classes which t/extract.t exhibits and Fedora contains (e.g. Moose::Exception). Otherwise, that test tests nothing and is susceptible to random failures when any of the modules appears in a buildroot.

TODO: Consider replacing '%{__perl}' macro with a plain 'perl' command. There is no benefit in hiding it behind a macro.

TODO: Consider passing NOPACKLIST=1 and NOPERLLOCAL=1 arguments to Makefile.PL. Then you will have to build-require 'perl(ExtUtils::MakeMaker) >= 6.76', but you will be able to place "make pure_install..." with '%{make_install}' macro and remove both find commands. See <https://fedoraproject.org/wiki/Perl/Tips#ExtUtils::MakeMaker>.

TODO: Do not use top-level globs in %files section <https://docs.fedoraproject.org/en-US/packaging-guidelines/#_explicit_lists>.

All tests pass. Ok.

$ rpmlint perl-Devel-StackTrace-Extract.spec ../SRPMS/perl-Devel-StackTrace-Extract-1.000000-1.fc39.src.rpm ../RPMS/noarch/perl-Devel-StackTrace-Extract-1.000000-1.fc39.noarch.rpm 
======================================== rpmlint session starts =======================================
rpmlint: 2.4.0
configuration:
    /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora-legacy-licenses.toml
    /etc/xdg/rpmlint/fedora-spdx-licenses.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
checks: 31, packages: 3

========= 2 packages and 1 specfiles checked; 0 errors, 0 warnings, 0 badness; has taken 0.2 s ========
rpmlint is Ok.

$ rpm -qlvp ../RPMS/noarch/perl-Devel-StackTrace-Extract-1.000000-1.fc39.noarch.rpm 
drwxr-xr-x    2 root     root                        0 Jun 27 02:00 /usr/share/doc/perl-Devel-StackTrace-Extract
-rw-r--r--    1 root     root                       90 Mar 30  2016 /usr/share/doc/perl-Devel-StackTrace-Extract/Changes
-rw-r--r--    1 root     root                     2987 Mar 30  2016 /usr/share/doc/perl-Devel-StackTrace-Extract/README.md
drwxr-xr-x    2 root     root                        0 Jun 27 02:00 /usr/share/licenses/perl-Devel-StackTrace-Extract
-rw-r--r--    1 root     root                    18349 Mar 30  2016 /usr/share/licenses/perl-Devel-StackTrace-Extract/LICENSE
-rw-r--r--    1 root     root                     1989 Jun 27 02:00 /usr/share/man/man3/Devel::StackTrace::Extract.3pm.gz
drwxr-xr-x    2 root     root                        0 Jun 27 02:00 /usr/share/perl5/vendor_perl/Devel
drwxr-xr-x    2 root     root                        0 Jun 27 02:00 /usr/share/perl5/vendor_perl/Devel/StackTrace
-rw-r--r--    1 root     root                     5168 Mar 30  2016 /usr/share/perl5/vendor_perl/Devel/StackTrace/Extract.pm
File permissions and layout are Ok.

$ rpm -q --requires -p ../RPMS/noarch/perl-Devel-StackTrace-Extract-1.000000-1.fc39.noarch.rpm | sort -f | uniq -c
      1 perl(base)
      1 perl(Devel::StackTrace)
      1 perl(Devel::StackTrace::Frame)
      1 perl(Exporter)
      1 perl(Scalar::Util)
      1 perl(strict)
      1 perl(warnings)
      1 perl-libs
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsZstd) <= 5.4.18-1
Binary requires are Ok.

$ rpm -q --provides -p ../RPMS/noarch/perl-Devel-StackTrace-Extract-1.000000-1.fc39.noarch.rpm | sort -f | uniq -c
      1 perl(Devel::StackTrace::Extract) = 1.000000
      1 perl-Devel-StackTrace-Extract = 1.000000-1.fc39
Binary provides are Ok.

$ resolvedeps rawhide ../RPMS/noarch/perl-Devel-StackTrace-Extract-1.000000-1.fc39.noarch.rpm 
Binary dependencies are resolvable. Ok.

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

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

Please correct the FIX item, consider fixing TODO items, and provide a new spec file.

Comment 2 Xavier Bachelot 2023-06-28 12:18:40 UTC
Thanks for the review Petr.

I believe I have addressed most of the points you raised.
I'm unsure about the Source0 URL. I thought using a more stable URL than the one using the authors/ would be better ?
Also I'm unsure about BR: perl(VERSION) >= 5.6, it doesn't resolve for me. I used BR: perl-interpreter instead.

Spec URL: https://www.bachelot.org/fedora/SPECS/perl-Devel-StackTrace-Extract.spec
SRPM URL: https://www.bachelot.org/fedora/SRPMS/perl-Devel-StackTrace-Extract-1.000000-2.fc39.src.rpm

Comment 3 Petr Pisar 2023-06-28 15:38:10 UTC
Thanks for the update.

Your URL is more stable. That's right. However, I prefer the authors URL for two reasons: First, it matches an address the upstream advertises, hence it's more credible and easier to compare. Second, any PAUSE user can upload a release of the distribution to PAUSE. If the uploading users is not on a list of maintainers of all the contained modules, the release is marked as "unauthorized", yet it is (or was?) kept publicly available on CPAN. CPAN maybe got better at this, but I remember that it happened few times that an unauthorized release was reported down to Fedora. Then a failing Source0 requires the Fedora maintainer to go to CPAN to find out who is the new author and at that time he can notice that the release in unauthorized (there is a red label Unauthorized printed next to the release name). Maybe I'm only overly cautious.

The "perl(VERSION) >= 5.6" does not work because I made a typo. Correct one is "perl(:VERSION) >= 5.6".


======================================== rpmlint session starts =======================================
rpmlint: 2.4.0
configuration:
    /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora-legacy-licenses.toml
    /etc/xdg/rpmlint/fedora-spdx-licenses.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
checks: 31, packages: 3

========= 2 packages and 1 specfiles checked; 0 errors, 0 warnings, 0 badness; has taken 0.2 s ========
rpmlint is Ok.

The package builds in Rawhide (https://koji.fedoraproject.org/koji/taskinfo?taskID=102708290). Ok.
This package is APPROVED.

Comment 4 Xavier Bachelot 2023-06-29 14:42:10 UTC
Ok, got it. I've switched to the canonical URL and added BR: perl(:VERSION) >= 5.6.
Thanks again for the review.

Here's the updated spec and srpm, I'll file the repo request shortly.

Spec URL: https://www.bachelot.org/fedora/SPECS/perl-Devel-StackTrace-Extract.spec
SRPM URL: https://www.bachelot.org/fedora/SRPMS/perl-Devel-StackTrace-Extract-1.000000-3.fc39.src.rpm

Comment 5 Fedora Admin user for bugzilla script actions 2023-06-29 14:42:41 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/perl-Devel-StackTrace-Extract


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