Bug 1797363

Summary: Review Request: perl-Text-Layout - Pango style text formatting
Product: [Fedora] Fedora Reporter: Johan Vromans <jvromans>
Component: Package ReviewAssignee: Petr Pisar <ppisar>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: package-review, ppisar
Target Milestone: ---Flags: ppisar: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: perl-Text-Layout-0.018.1-3.fc33 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2020-03-08 00:51:11 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 1797364    
Bug Blocks: 1797362    

Description Johan Vromans 2020-02-02 21:26:15 UTC 
Spec URL: https://www.chordpro.org/fedora/perl-Text-Layout.spec
SRPM URL: https://www.chordpro.org/fedora/ perl-Text-Layout-0.016-1.fc29.src.rpm

Description: 

This perl module implements Pango style text formatting, to be used with the chordpro package.

See https://bugzilla.redhat.com/show_bug.cgi?id=1797362.

I herewith request a review of this package for inclusion into Fedora.

Note that the chordpro package requires a package that is not yet part of Fedora:

perl-HarfBuzz-Shaper

I will make separate review requests for this.

Thanks for you attention.

Fedora Account System Username: sciurius
Comment 1 Petr Pisar 2020-02-24 16:32:21 UTC 
URL and Source addresses are usable. Ok.

TODO: The Url points to one of the documentation files. Please change it to <https://metacpan.org/release/Text-Layout> so that it points to the top level of the CPAN distribution.

Source file (SHA-256: 58f8ef35c2e75e2d428f988b35984e807131311628eba01367ee3ea10b349b6a) is original. Ok.
Summary verified from lib/Text/Layout.pm. Ok.
Description verified from README. Ok.

TODO: I recommend you to remove the status of the backends from the description. The status will evolve and the description could become outdated.

FIX: The license tag is wrong. lib/Text/Layout.pm declaes "Artistic 2.0", README declares "GPL+ or Artistic". Please mention both licenses in the tag as "(GPL+ or Artistic) and Artistic 2.0".

TODO: Do not use macros for simple commands. I.e. write "perl" instead of "%{__perl}". The same applies to %{__make}, %{__rm}, %{__chmod}.

FIX: Remove "%{__rm} -rf %{buildroot}" line from %install section. It has no effect because it's a default behavior.
FIX: Remove "find %{buildroot} -depth -type d -exec rmdir {} 2>/dev/null ';'" line from %install section. It has no effect because no supported ExtUtils::MakeMaker creates empty directories.
FIX: Remove %clean section. It has no effect because it's a default behavior.
FIX: Remove %defattr(-,root,root,-) line from %files section. It has no effect because it's a default behavior.
TODO: Call Makefile.PL with "NO_PACKLIST=1 NO_PERLLOCAL=1" arguments and use %{make_build} in %build section and %{make_install} in %install section as recommended in <https://fedoraproject.org/wiki/Perl/Tips#ExtUtils::MakeMaker> and <https://docs.fedoraproject.org/en-US/packaging-guidelines/#_parallel_make>. Then you don't need "find %{buildroot} -type f -name .packlist -exec rm -f {} ';'" line in %install section.

No XS code, noarch BuildArch is Ok.

FIX: Build-require "make" for (perl-Text-Layout.spec:66).
FIX: Constrain "perl(PDF::API2)" dependency with ">= 2.036" (META.json:44).
FIX: Build-require "perl(strict)" (Makefile.PL:3).
FIX: Build-require "perl(warnings)" (Makefile.PL:4).
FIX: "perl(ExtUtils::MakeMaker)" dependency version is incorrect. There should be ">= 6.46" (Makefile.PL:21).
FIX: Build-require "perl(Carp)" (lib/Text/Layout.pm:9).
FIX: Build-require "perl(constant)" (lib/Text/Layout.pm:115).
FIX: Build-require "perl(File::Basename) (lib/Text/Layout/FontConfig.pm:436).
FIX: Build-require "perl(overload)" (lib/Text/Layout/FontDescriptor.pm:197).
FIX: Build-require "perl(utf8)" (lib/Text/Layout.pm:5).

FATAL: "perl(HarfBuzz::Shaper) >= 0.018" dependency is not available in Fedora 33. Please package that dependency first.
Comment 2 Johan Vromans 2020-02-25 13:47:10 UTC 
Thanks. Updated:

Spec URL: https://www.chordpro.org/fedora/perl-Text-Layout.spec
SRPM URL: https://www.chordpro.org/fedora/ perl-Text-Layout-0.018-1.fc31.src.rpm
Comment 3 Johan Vromans 2020-02-27 11:14:43 UTC 
Spec URL: https://www.chordpro.org/fedora/perl-Text-Layout.spec
SRPM URL: https://www.chordpro.org/fedora/perl-Text-Layout-0.018-2.fc31.src.rpm
Comment 4 Petr Pisar 2020-02-27 12:54:32 UTC 
The package was rebased. I will review it again.

Url and Source0 addresses are usable. Ok.
Source0 archive (SHA-256: 65b0eada14bbfcf687fabc31d162bdf154c803ba8a10d83d118561d4f10c181d) is original. Ok.
License verified from lib/Text/Layout.pm and README. Ok.
Summary verified from lib/Text/Layout.pm. Ok.
Description verified from README. Ok.
No XS code, noarch BuildArch is Ok.

FIX: The minimal perl(ExtUtils::MakeMaker) version should be 6.76. Not 6.46. I made a mistake in my previous comment.

PDF::Builder is optional for tests. Ok.

TODO: Either correct perl(HarfBuzz::Shaper) minimal version to 0.019 (t/00-load.t:14) or remove the version, because the tests continue even without that.

All tests pass. Ok.

$ rpmlint perl-Text-Layout.spec ../SRPMS/perl-Text-Layout-0.018-2.fc33.src.rpm ../RPMS/noarch/perl-Text-Layout-0.018-2.fc33.noarch.rpm 
sh: /usr/bin/python2: No such file or directory
perl-Text-Layout.src: W: spelling-error Summary(en_US) Pango -> Tango, Mango, Pan go
perl-Text-Layout.src: W: spelling-error %description -l en_US Backends -> Back ends, Back-ends, Backhands
perl-Text-Layout.src: W: spelling-error %description -l en_US FontConfig -> Configuration
perl-Text-Layout.noarch: W: spelling-error Summary(en_US) Pango -> Tango, Mango, Pan go
perl-Text-Layout.noarch: W: spelling-error %description -l en_US Backends -> Back ends, Back-ends, Backhands
perl-Text-Layout.noarch: W: spelling-error %description -l en_US FontConfig -> Configuration
2 packages and 1 specfiles checked; 0 errors, 6 warnings.
rpmlint is Ok.

$ rpm -q -lv -p ../RPMS/noarch/perl-Text-Layout-0.018-2.fc33.noarch.rpm t@fedora-33 SPECS]$ rpm -q -lv -p ../RPMS/noarch/perl-Text-Layout-0.018-2.fc33.noarch.rpm
drwxr-xr-x    2 root     root                        0 Feb 27 12:52 /usr/share/doc/perl-Text-Layout
-rw-r--r--    1 root     root                      971 Feb  6 16:45 /usr/share/doc/perl-Text-Layout/Changes
-rw-r--r--    1 root     root                     2002 Jan 26 18:53 /usr/share/doc/perl-Text-Layout/README
-rw-r--r--    1 root     root                     6606 Feb 27 12:52 /usr/share/man/man3/Text::Layout.3pm.gz
-rw-r--r--    1 root     root                     2957 Feb 27 12:52 /usr/share/man/man3/Text::Layout::FontConfig.3pm.gz
-rw-r--r--    1 root     root                     1900 Feb 27 12:52 /usr/share/man/man3/Text::Layout::FontDescriptor.3pm.gz
drwxr-xr-x    2 root     root                        0 Feb 27 12:52 /usr/share/perl5/vendor_perl/Text
drwxr-xr-x    2 root     root                        0 Feb 27 12:52 /usr/share/perl5/vendor_perl/Text/Layout
-rw-r--r--    1 root     root                    26583 Feb  6 16:45 /usr/share/perl5/vendor_perl/Text/Layout.pm
-rw-r--r--    1 root     root                      154 Feb  6 16:45 /usr/share/perl5/vendor_perl/Text/Layout/Cairo.pm
-rw-r--r--    1 root     root                    13480 Feb  6 16:45 /usr/share/perl5/vendor_perl/Text/Layout/FontConfig.pm
-rw-r--r--    1 root     root                     3378 Feb  6 16:45 /usr/share/perl5/vendor_perl/Text/Layout/FontDescriptor.pm
-rw-r--r--    1 root     root                      967 Feb  6 16:45 /usr/share/perl5/vendor_perl/Text/Layout/Markdown.pm
-rw-r--r--    1 root     root                     5484 Feb  6 16:45 /usr/share/perl5/vendor_perl/Text/Layout/PDFAPI2.pm
-rw-r--r--    1 root     root                      154 Feb  6 16:45 /usr/share/perl5/vendor_perl/Text/Layout/Pango.pm
A file layout and the permissions are Ok.

$ rpm -q --requires -p ../RPMS/noarch/perl-Text-Layout-0.018-2.fc33.noarch.rpm | sort -f | uniq -c
      1 perl(:MODULE_COMPAT_5.30.1)
      1 perl(:VERSION) >= 5.10.1
      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
FIX: Run-require:
perl(Carp) lib/Text/Layout.pm:9
perl(constant) lib/Text/Layout.pm:115
perl(File::Basename) lib/Text/Layout/FontConfig.pm:437
perl(overload) lib/Text/Layout/FontDescriptor.pm:197
perl(strict) lib/Text/Layout.pm:3
perl(utf8) lib/Text/Layout.pm:5
perl(warnings) lib/Text/Layout.pm:4
Alternatively stop disabling the depenency generators (AutoReqProv) and they will appear automatically.

$ rpm -q --provides -p ../RPMS/noarch/perl-Text-Layout-0.018-2.fc33.noarch.rpm | sort -f | uniq -c
      1 perl(Font::Config) = 0.018
      1 perl(Font::Descriptor) = 0.018
      1 perl(Text::Layout) = 0.018
      1 perl-Text-Layout = 0.018-2.fc33
FIX: Stop providing 'perl(Font::Config)' and 'perl(Font::Descriptor)'. They are not delivered by this package.
TODO: Provide all the other Text::Layout::* modules. They are there, they have documentation, they are indexed by CPAN.

Maybe should keep AutoReqProv enabled and filter only those that you don't need <https://docs.fedoraproject.org/en-US/packaging-guidelines/AutoProvidesAndRequiresFiltering/>.

$ resolvedeps f33-build ../RPMS/noarch/perl-Text-Layout-0.018-2.fc33.noarch.rpm 
Binary dependencies are resolvable. Ok.

The package builds in F33 (https://koji.fedoraproject.org/koji/taskinfo?taskID=41966469). 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: Package NOT approved.
Comment 5 Johan Vromans 2020-02-27 15:09:11 UTC 
Thanks for your valuable review.

All FIX and TODO items have been addressed.

* The requirement for HarfBuzz::Shaper 0.19 was wrong, fixed in source t/00-load.t.
* AutoReqProv has been enabled and unwanted requires/provides are now dealt with using __requires_exclude and __provides_exclude_from . For the time being I do not want to explicitly provide the backend modules.

New SPEC and SRPMS:
https://www.chordpro.org/fedora/perl-Text-Layout-0.018.1-3.fc31.src.rpm
https://www.chordpro.org/fedora/perl-Text-Layout.spec
Comment 6 Petr Pisar 2020-02-27 16:07:03 UTC 
The package was rebased. I will review it again.

Url and Source0 addresses are usable. Ok.
Source0 archive (SHA-256: 781b3d3b7e7b249702f1580b95051762723d2c4ef3941ee8b56661b64d87ee5a) is original. Ok.
Summary verified from lib/Text/Layout.pm. Ok.
Description verified from README. Ok.
License verified from lib/Text/Layout.pm and README. Ok.
No XS code, noarch BuildArch is Ok.

PDF::Builder is optional for tests. Ok.

All tests pass. Ok.

$ rpmlint perl-Text-Layout.spec ../SRPMS/perl-Text-Layout-0.018.1-3.fc33.src.rpm ../RPMS/noarch/perl-Text-Layout-0.018.1-3.fc33.noarch.rpm 
sh: /usr/bin/python2: No such file or directory
perl-Text-Layout.src: W: spelling-error Summary(en_US) Pango -> Tango, Mango, Pan go
perl-Text-Layout.src: W: spelling-error %description -l en_US Backends -> Back ends, Back-ends, Backhands
perl-Text-Layout.src: W: spelling-error %description -l en_US FontConfig -> Configuration
perl-Text-Layout.noarch: W: spelling-error Summary(en_US) Pango -> Tango, Mango, Pan go
perl-Text-Layout.noarch: W: spelling-error %description -l en_US Backends -> Back ends, Back-ends, Backhands
perl-Text-Layout.noarch: W: spelling-error %description -l en_US FontConfig -> Configuration
2 packages and 1 specfiles checked; 0 errors, 6 warnings.
rpmlint is Ok.

$ rpm -q -lv -p ../RPMS/noarch/perl-Text-Layout-0.018.1-3.fc33.noarch.rpm 
drwxr-xr-x    2 root     root                        0 Feb 27 16:58 /usr/share/doc/perl-Text-Layout
-rw-r--r--    1 root     root                      973 Feb 27 14:45 /usr/share/doc/perl-Text-Layout/Changes
-rw-r--r--    1 root     root                     2002 Jan 26 18:53 /usr/share/doc/perl-Text-Layout/README
-rw-r--r--    1 root     root                     6606 Feb 27 16:58 /usr/share/man/man3/Text::Layout.3pm.gz
-rw-r--r--    1 root     root                     2957 Feb 27 16:58 /usr/share/man/man3/Text::Layout::FontConfig.3pm.gz
-rw-r--r--    1 root     root                     1900 Feb 27 16:58 /usr/share/man/man3/Text::Layout::FontDescriptor.3pm.gz
drwxr-xr-x    2 root     root                        0 Feb 27 16:58 /usr/share/perl5/vendor_perl/Text
drwxr-xr-x    2 root     root                        0 Feb 27 16:58 /usr/share/perl5/vendor_perl/Text/Layout
-rw-r--r--    1 root     root                    26585 Feb 27 14:45 /usr/share/perl5/vendor_perl/Text/Layout.pm
-rw-r--r--    1 root     root                      154 Feb 27 14:45 /usr/share/perl5/vendor_perl/Text/Layout/Cairo.pm
-rw-r--r--    1 root     root                    13482 Feb 27 14:45 /usr/share/perl5/vendor_perl/Text/Layout/FontConfig.pm
-rw-r--r--    1 root     root                     3380 Feb 27 14:45 /usr/share/perl5/vendor_perl/Text/Layout/FontDescriptor.pm
-rw-r--r--    1 root     root                      967 Feb 27 14:45 /usr/share/perl5/vendor_perl/Text/Layout/Markdown.pm
-rw-r--r--    1 root     root                     5484 Feb 27 14:45 /usr/share/perl5/vendor_perl/Text/Layout/PDFAPI2.pm
-rw-r--r--    1 root     root                      154 Feb 27 14:45 /usr/share/perl5/vendor_perl/Text/Layout/Pango.pm
A file layout and the permissions are Ok.

$ rpm -q --requires -p ../RPMS/noarch/perl-Text-Layout-0.018.1-3.fc33.noarch.rpm | sort -f | uniq -c
      1 perl(:MODULE_COMPAT_5.30.1)
      1 perl(:VERSION) >= 5.10.1
      1 perl(Carp)
      1 perl(constant)
      1 perl(File::Basename)
      1 perl(overload)
      1 perl(strict)
      1 perl(utf8)
      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(PayloadIsZstd) <= 5.4.18-1
Binary requires are Ok.

$ rpm -q --provides -p ../RPMS/noarch/perl-Text-Layout-0.018.1-3.fc33.noarch.rpm | sort -f | uniq -c
      1 perl(Text::Layout) = 0.018.1
      1 perl(Text::Layout::FontConfig) = 0.018.1
      1 perl(Text::Layout::FontDescriptor) = 0.018.1
      1 perl-Text-Layout = 0.018.1-3.fc33
Binary provides are Ok.

$ resolvedeps f33-build ../RPMS/noarch/perl-Text-Layout-0.018.1-3.fc33.noarch.rpm 
Binary dependencies are resolvable. Ok.

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

The package is in line with Fedora and Perl packaging guide lines.
Resolution: Package APPROVED.
Comment 7 Gwyn Ciesla 2020-02-27 16:45:58 UTC 
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/perl-Text-Layout
Comment 8 Fedora Update System 2020-02-28 07:48:03 UTC 
FEDORA-2020-64b8c66486 has been submitted as an update to Fedora 32. https://bodhi.fedoraproject.org/updates/FEDORA-2020-64b8c66486
Comment 9 Fedora Update System 2020-02-29 01:22:34 UTC 
perl-Text-Layout-0.018.1-3.fc32 has been pushed to the Fedora 32 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-2020-64b8c66486
Comment 10 Fedora Update System 2020-02-29 03:11:38 UTC 
perl-Text-Layout-0.018.1-3.fc31 has been pushed to the Fedora 31 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-2020-1b09a7ad0f
Comment 11 Fedora Update System 2020-03-08 00:51:11 UTC 
perl-Text-Layout-0.018.1-3.fc31 has been pushed to the Fedora 31 stable repository. If problems still persist, please make note of it in this bug report.
Comment 12 Fedora Update System 2020-03-16 20:17:14 UTC 
perl-Text-Layout-0.018.1-3.fc32 has been pushed to the Fedora 32 stable repository. If problems still persist, please make note of it in this bug report.
Comment 13 Fedora Update System 2020-03-16 20:29:54 UTC 
perl-Text-Layout-0.018.1-3.fc32 has been pushed to the Fedora 32 stable repository. If problems still persist, please make note of it in this bug report.