Bug 1797364 - Review Request: perl-HarfBuzz-Shaper - Access to a small subset of the native HarfBuzz library
Summary: Review Request: perl-HarfBuzz-Shaper - Access to a small subset of the native...
Keywords:
Status: CLOSED ERRATA
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: 1797362 1797363
TreeView+ depends on / blocked
 
Reported: 2020-02-02 21:36 UTC by Johan Vromans
Modified: 2020-03-16 20:29 UTC (History)
2 users (show)

Fixed In Version: perl-HarfBuzz-Shaper-0.018.4-2.fc33
Clone Of:
Environment:
Last Closed: 2020-03-12 21:55:44 UTC
Type: ---
Embargoed:
ppisar: fedora-review+


Attachments (Terms of Use)

Description Johan Vromans 2020-02-02 21:36:18 UTC
Spec URL: https://www.chordpro.org/fedora/perl-HarfBuzz-Shaper.spec
SRPM URL: https://www.chordpro.org/fedora/perl-HarfBuzz-Shaper-0.018.4-1.fc29.src.rpm

Description:

This package provides a number of Perl bindings for the harfbuzz library.

This package is required by perl-Text-Layout, see https://bugzilla.redhat.com/show_bug.cgi?id=1797363.

Fedora Account System Username: sciurius

Comment 1 Petr Pisar 2020-02-25 12:25:43 UTC
The review bug report must match the spec file.

Comment 2 Johan Vromans 2020-02-25 12:44:07 UTC
(In reply to Petr Pisar from comment #1)
> The review bug report must match the spec file.

Excuse my ignorance, but this is a bit too cryptic to me...

Comment 3 Petr Pisar 2020-02-25 12:46:18 UTC
(In reply to Johan Vromans from comment #2)
> (In reply to Petr Pisar from comment #1)
> > The review bug report must match the spec file.
> 
> Excuse my ignorance, but this is a bit too cryptic to me...

The review bug report summary. I corrected it.

Comment 4 Petr Pisar 2020-02-25 13:22:46 UTC
Url and Source addresses are usable. Ok.
Source0 archive (SHA-256: 6b115945e94797c7f1e1cba35961a8c8e24e2a8505f57c387a79579e73530250) is original. Ok.

TODO: Change the Url to <https://metacpan.org/release/HarfBuzz-Shaper>. That's the top link for the CPAN distribution.

Summary verified from lib/HarfBuzz/Shaper.pm. Ok.
Descripton verified from lib/HarfBuzz/Shaper.pm. Ok.

TODO: Remove the last sentence (Feel free...). It's not descriptive.
FIX: Spell "perl" as "Perl" in the description.

License verified from inc/Devel/CheckLib.pm (GPL+ or Artistic), hb_src/harfbuzz/hb-ot-cff2-table.hh (MIT), hb_src/harfbuzz/hb-ucd.cc (MIT), lib/HarfBuzz/Shaper.pm (GPL+ or Artistic), README (GPL+ or Artistic).

FIX: Delete ./hb_src directory in %prep section to make sure the bundled hurfbuzz code is never used.

TODO: Use program names instead macros. E.g. "perl" instead of %{__perl}". The same applies to %{__make} and %{__rm}.
TODO: Use "%{make_build}" instead of "%{__make} %{?_smp_mflags}".
FIX: Do not use "%{__rm} -rf %{buildroot}" in the %install section. This is the default rpmbuild behavior.
TODO: Use "%{make_install} instead of "%{__make} install DESTDIR=%{buildroot}" in the %install section.
FIX: Remove %clean section. This is the default rpmbuild behavior.
FIX: Remove "%defattr(-,root,root,-)" from the %files section. This is the default rpmbuild behavior.

FIX: Change perl(ExtUtils::MakeMaker) dependency constrain from ">= 6.5503" to ">= 6.76". 6.5503 is nowhere specified in the sources. 6.76 is needed for the NO_PACKLIST and NO_PERLLOCAL Makefile.PL arguments.

FIX: Unbundle Devel::CheckLib (remove ./inc content and build-require perl(Devel::CheckLib). Or build-require it's dependencies: perl(Config), perl(Exporter), perl(File::Spec), perl(File::Temp), perl(Text::ParseWords), perl(vars).

FIX: Do not build-require harfbuzz. I believe it's used only indirectly vie harbuzz-devel package.

FIX: Build-require perl(lib) (Makefile.PL:6) or remove the "use lib" line from after unbundling Devel::CheckLib.
FIX: Build-require perl(strict) (Makefile.PL:3).
FIX: Build-require perl(warnings) (Makefile.PL:4).
FIX: Build-require perl(Carp) (lib/HarfBuzz/Shaper.pm:8).
FIX: Build-require perl(Encode) (lib/HarfBuzz/Shaper.pm:9).
FIX: Build-require perl(XSLoader) (lib/HarfBuzz/Shaper.pm:13).
FIX: Build-require perl(charnames) (t/20_shaper.t:6).
FIX: BUild-require perl(utf8) (t/10_basic.t:5).

All tests pass. Ok.

$ rpmlint perl-HarfBuzz-Shaper.spec ../SRPMS/perl-HarfBuzz-Shaper-0.018.4-1.fc33.src.rpm ../RPMS/x86_64/perl-HarfBuzz-Shaper-*
sh: /usr/bin/python2: No such file or directory
4 packages and 1 specfiles checked; 0 errors, 0 warnings.
rpmlint is Ok.

$ rpm -q -lv -p ../RPMS/x86_64/perl-HarfBuzz-Shaper-0.018.4-1.fc33.x86_64.rpm
drwxr-xr-x    2 root     root                        0 Feb 25 14:11 /usr/lib/.build-id
drwxr-xr-x    2 root     root                        0 Feb 25 14:11 /usr/lib/.build-id/36
lrwxrwxrwx    1 root     root                       70 Feb 25 14:11 /usr/lib/.build-id/36/3c7e35ccddb8116501cef14f547e81249057a3 -> ../../../../usr/lib64/perl5/vendor_perl/auto/HarfBuzz/Shaper/Shaper.so
drwxr-xr-x    2 root     root                        0 Feb 25 14:11 /usr/lib64/perl5/vendor_perl/HarfBuzz
-rw-r--r--    1 root     root                     5189 Feb  2 10:22 /usr/lib64/perl5/vendor_perl/HarfBuzz/Shaper.pm
drwxr-xr-x    2 root     root                        0 Feb 25 14:11 /usr/lib64/perl5/vendor_perl/auto/HarfBuzz
drwxr-xr-x    2 root     root                        0 Feb 25 14:11 /usr/lib64/perl5/vendor_perl/auto/HarfBuzz/Shaper
-rwxr-xr-x    1 root     root                    20056 Feb 25 14:11 /usr/lib64/perl5/vendor_perl/auto/HarfBuzz/Shaper/Shaper.so
drwxr-xr-x    2 root     root                        0 Feb 25 14:11 /usr/share/doc/perl-HarfBuzz-Shaper
-rw-r--r--    1 root     root                     1231 Jan 31 15:41 /usr/share/doc/perl-HarfBuzz-Shaper/Changes
-rw-r--r--    1 root     root                     1182 Jan 31 09:43 /usr/share/doc/perl-HarfBuzz-Shaper/README
-rw-r--r--    1 root     root                     2523 Feb 25 14:11 /usr/share/man/man3/HarfBuzz::Shaper.3pm.gz
File layout and permissions are Ok.

$ rpm -q --requires -p ../RPMS/x86_64/perl-HarfBuzz-Shaper-0.018.4-1.fc33.x86_64.rpm | sort -f | uniq -c
      1 harfbuzz >= 1.7.7
      1 libc.so.6()(64bit)
      1 libc.so.6(GLIBC_2.2.5)(64bit)
      1 libc.so.6(GLIBC_2.4)(64bit)
      1 libharfbuzz.so.0()(64bit)
      1 libperl.so.5.30()(64bit)
      1 libpthread.so.0()(64bit)
      1 perl(:MODULE_COMPAT_5.30.1)
      2 perl(:VERSION) >= 5.10.1
      1 perl(Carp)
      1 perl(Encode)
      1 perl(strict)
      1 perl(warnings)
      1 perl(XSLoader)
      1 perl-generators
      1 perl-interpreter
      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
      1 rtld(GNU_HASH)
FIX: Do not run-require perl(:VERSION) explicitly. It's detected automatically.
FIX: Do not run-require "harfbuzz >= 1.7.7". This is ensured by an automatically generated dependency on the harfbuzz dynamic library.
FIX: Do not run-require perl-generators and perl-interpreter. They are not used at run-time.

$ rpm -q --provides -p ../RPMS/x86_64/perl-HarfBuzz-Shaper-0.018.4-1.fc33.x86_64.rpm | sort -f | uniq -c
      1 perl(HarfBuzz::Shaper) = 0.018.4
      1 perl-HarfBuzz-Shaper = 0.018.4-1.fc33
      1 perl-HarfBuzz-Shaper(x86-64) = 0.018.4-1.fc33
Binary provides are Ok.

$ resolvedeps rawhide ../RPMS/x86_64/perl-HarfBuzz-Shaper-0.018.4-1.fc33.x86_64.rpm 
Binary dependencies are resolvable. Ok.

The package builds in F33 (https://koji.fedoraproject.org/koji/taskinfo?taskID=41887382). 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-25 14:34:05 UTC
Thanks for your valuable feedback. All FIX and TODO items have been addressed.

Spec URL: https://www.chordpro.org/fedora/perl-HarfBuzz-Shaper.spec
SRPM URL: https://www.chordpro.org/fedora/perl-HarfBuzz-Shaper-0.018.4-2.fc31.src.rpm

Comment 6 Petr Pisar 2020-02-26 08:38:59 UTC
TODO: There is still %{__perl} in 'Requires: perl(:MODULE_COMPAT_'. Use plain "perl".
TODO: I recommend you to sort the BuildRequires lexicographically. It will make upgrading the package later easier.

TODO: To suppress these warnings:

Warning: the following files are missing in your kit:
        hb_src/harfbuzz/check-c-linkage-decls.sh
        [...]
        inc/Devel/CheckLib.pm
Please inform the author.

you can remove the file entries from MANIFEST file. E.g. this command in %prep section will do it:

perl -i -ne 'print $_ unless m{^(hb_src|inc)/}' MANIFEST

All tests pass. Ok.

$ rpmlint perl-HarfBuzz-Shaper.spec ../SRPMS/perl-HarfBuzz-Shaper-0.018.4-2.fc33.src.rpm ../RPMS/x86_64/perl-HarfBuzz-Shaper-*
sh: /usr/bin/python2: No such file or directory
4 packages and 1 specfiles checked; 0 errors, 0 warnings.
rpmlint is Ok.

$ rpm -q --requires -p ../RPMS/x86_64/perl-HarfBuzz-Shaper-0.018.4-2.fc33.x86_64.rpm | sort -f | uniq -c
      1 harfbuzz
      1 libc.so.6()(64bit)
      1 libc.so.6(GLIBC_2.2.5)(64bit)
      1 libc.so.6(GLIBC_2.4)(64bit)
      1 libharfbuzz.so.0()(64bit)
      1 libperl.so.5.30()(64bit)
      1 libpthread.so.0()(64bit)
      1 perl(:MODULE_COMPAT_5.30.1)
      1 perl(:VERSION) >= 5.10.1
      1 perl(Carp)
      1 perl(Encode)
      1 perl(strict)
      1 perl(warnings)
      1 perl(XSLoader)
      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
      1 rtld(GNU_HASH)
FIX: Do not run-require 'harfbuzz' at all. The dependency comes automatically via 'libharfbuzz.so.0()(64bit)'. It's redundant.

$ resolvedeps rawhide ../RPMS/x86_64/perl-HarfBuzz-Shaper-0.018.4-2.fc33.x86_64.rpm 
Binary dependencies are resolvable. Ok.

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

Comment 7 Johan Vromans 2020-02-26 10:03:09 UTC
FIX and TODO addressed. Thanks for the review.

Comment 8 Gwyn Ciesla 2020-02-26 15:13:54 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/perl-HarfBuzz-Shaper

Comment 9 Fedora Update System 2020-02-28 07:53:51 UTC
FEDORA-2020-be6cf240e6 has been submitted as an update to Fedora 32. https://bodhi.fedoraproject.org/updates/FEDORA-2020-be6cf240e6

Comment 10 Fedora Update System 2020-02-28 07:54:04 UTC
FEDORA-2020-3d85348df3 has been submitted as an update to Fedora 31. https://bodhi.fedoraproject.org/updates/FEDORA-2020-3d85348df3

Comment 11 Fedora Update System 2020-03-03 08:21:42 UTC
FEDORA-2020-dc44d79108 has been submitted as an update to Fedora 31. https://bodhi.fedoraproject.org/updates/FEDORA-2020-dc44d79108

Comment 12 Fedora Update System 2020-03-03 20:28:20 UTC
perl-HarfBuzz-Shaper-0.021-4.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-dc44d79108

Comment 13 Fedora Update System 2020-03-12 21:55:44 UTC
perl-HarfBuzz-Shaper-0.021-4.fc31 has been pushed to the Fedora 31 stable repository. If problems still persist, please make note of it in this bug report.

Comment 14 Fedora Update System 2020-03-16 20:15:37 UTC
perl-HarfBuzz-Shaper-0.018.4-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 15 Fedora Update System 2020-03-16 20:29:31 UTC
perl-HarfBuzz-Shaper-0.018.4-3.fc32 has been pushed to the Fedora 32 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.