https://bugzilla.redhat.com/show_bug.cgi?id=498390 Petr Pisar <ppisar@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+ --- Comment #52 from Petr Pisar <ppisar@xxxxxxxxxx> --- Reviewing release 4: > TODO: Remove the leading `A' from the summary. Ok. > TODO: Correct typos in description. The first sentence does not have any verb. > The second sentence starts with "Is is". Summary text should end a full stop > symbol. Ok. > TODO: The `Obsoletes: rakudo-star <= 0.0.2017.01-2' should use `<' operator > as 0.0.2017.01 is the last one in Fedora. Ok. > FIX: Build-require `perl' (rakudo.spec:54). Ok. > FIX: Use %{__prefix} in `Configure.pl --prefix=/usr' instead of `/usr'. Ok. > FIX: Build-require `make' (rakudo.spec:55). > FIX: Build-require `perl-podlators' (rakudo.spec:63). > FIX: Build-require `coreutils' (rakudo.spec:82). Ok. > FIX: Build-require `perl(base)' (tools/lib/NQP/Configure.pm:8). > FIX: Build-require `perl(Cwd)' (Configure.pl:10). > FIX: Build-require `perl(Exporter)' (tools/lib/NQP/Configure.pm:8). > FIX: Build-require `perl(File::Copy)' (tools/lib/NQP/Configure.pm:5). > FIX: Build-require `perl(File::Spec)' (Configure.pl:9). > FIX: Build-require `perl(Getopt::Long)' (Configure.pl:8). > FIX: Build-require `perl(lib)' (Configure.pl:11). > FIX: Build-require `perl(strict)' (Configure.pl:5). > FIX: Build-require `perl(Text::ParseWords)' (Configure.pl:7). > FIX: Build-require `perl(warnings)' (Configure.pl:6). Ok. > FIX: Build-require `gcc'. Ok. > FIX: Build-require `perl(FindBin)' for tests (t/harness5:9). > FIX: Build-require `perl(List::Util)' for tests (t/harness5:11). > FIX: Build-require `perl(Test::Harness)' for tests (t/harness5:15). Ok. FIX: Do not build-require `perl(ExtUtils::Command)'. It's not used. Configure.pl use it only on non-UNIX platforms and t/spec/test_summary contains it only in a documentation. > FIX: Remove %defattr from %files section. Ok. > TODO: Package docs/ChangeLog as a documentation. Ok. > TODO: The `-ltommath -latomic_ops -luv -lm -lpthread -lrt -ldl' linker > options come from `/usr/bin/nqp-m --show-config' tool, therefore I think it > would make more sense to move the dependency on libtommath-devel and other > libraries from rakudo.spec to nqp or moarvm-devel. I grepped for header files > included by rakudo and I could not find any usage of them. (Maybe the are not > needed at all). Not addressed. Ok. > TODO: Is the build-time dependency on readline-devel needed? It's nowhere > used and docs/ChangeLog reads it was removed. Ok. $ rpmlint rakudo.spec ../SRPMS/rakudo-0.2017.01-4.fc27.src.rpm ../RPMS/x86_64/rakudo-* rakudo.src: W: spelling-error %description -l en_US http -> HTTP rakudo.x86_64: W: spelling-error %description -l en_US http -> HTTP rakudo.x86_64: W: self-obsoletion rakudo-star < 0.0.2017.01-3 obsoletes rakudo-star = 0.0.0.2017.01-4.fc27 rakudo.x86_64: W: hidden-file-or-dir /usr/lib64/perl6/precomp/.lock rakudo.x86_64: E: zero-length /usr/lib64/perl6/precomp/.lock rakudo.x86_64: E: zero-length /usr/lib64/perl6/repo.lock rakudo.x86_64: W: no-manual-page-for-binary perl6-debug-m rakudo.x86_64: W: no-manual-page-for-binary perl6-gdb-m rakudo.x86_64: W: no-manual-page-for-binary perl6-valgrind-m 3 packages and 1 specfiles checked; 2 errors, 14 warnings. FIX: The `rakudo-star' Provides has a superfluous "0" because %{version} already contains one. TODO: I have no idea how the .lock files are used. But if they are used for write-locking, then it will not work if /usr is a readonly file system. In that case they should be placed into /var or /run with tmpfiles.d technique <https://fedoraproject.org/wiki/Packaging:Tmpfiles.d> and rakudo should have be patched accordingly. I thing this a good candidate for a bug report to upstream. > FIX: Move the libperl6_ops_moar.so out of /usr/share (to /usr/lib64/perl6 or somewhere else). > FIX: Are the *.moarvm files stored under /usr/share differ on each architecture. They must be moved somewhere > else. Quick fix would be to install everything under %{__libbdir}. But if the 'share' component comes from nqp > or moarvm, then they need to be moved first. Ok. > TODO: I grep binary package of some libuv.so.1 symbols and I could not find any. I suspect that many of > libraries are not needed. Not addressed. Ok. Should be reported to upstream. TODO: You can remove `Group:' tag. It's usage is discouraged. $ rpm -q --provides -p ../RPMS/x86_64/rakudo-0.2017.01-4.fc27.x86_64.rpm | sort -f | uniq -c 1 libperl6_ops_moar.so()(64bit) 1 rakudo = 0.2017.01-4.fc27 1 rakudo(x86-64) = 0.2017.01-4.fc27 1 rakudo-star = 0.0.0.2017.01-4.fc27 TODO: The libperl6_ops_moar.so()(64bit) Provides comes from non-standard shared library path (/usr/lib64/perl6/runtime/dynext/). As such it's not a public rakudo ABI and it should not be provided by the RPM package. You can filter the autogenerated dependency with __provides_exclude_from <https://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering>. Otherwise the package looks good. Thank you very much for implementing the changes. This was a tough review. Please correct the `FIX' items and consider fixing the `TODO' item before building this package. Resolution: Package APPROVED. -- You are receiving this mail because: You are always notified about changes to this product and component _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx