https://bugzilla.redhat.com/show_bug.cgi?id=1663618 Jitka Plesnikova <jplesnik@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+ --- Comment #4 from Jitka Plesnikova <jplesnik@xxxxxxxxxx> --- > Is there a tool you used to find the missing dependencies? I am using tool tangerine and then I check the files manually. > > BuildRequires > > FIX: Please add following BRs > > - perl(charnames) - lib/App/ccdiff.pm:7 > > - perl(Getopt::Long) - lib/App/ccdiff.pm:10 > > - perl(Term::ANSIColor) - lib/App/ccdiff.pm:9 > > - perl(warnings) - used in Makefile.PL:2, lib/App/ccdiff.pm:6, ... > > - make - spec file lines 43, 50 +BuildRequires: make +BuildRequires: perl(warnings) +BuildRequires: perl(charnames) +BuildRequires: perl(Term::ANSIColor) +BuildRequires: perl(Getopt::Long) Ok > Done. It looked to me like the build succeeded without these dependencies, > so what was the rationale for including them? I couldn't see anything > relevant in the packaging guidelines > (https://docs.fedoraproject.org/en-US/packaging-guidelines/Perl/ > #_build_dependencies). Is it just that, since the script is being tested > during the build, the BuildRequires should include all of the Requires? The build succeeded, because the dependencies were loaded by other packages. However, if some dependencies change, it could caused the build failure. The BRs contain all dependencies which are needed for compile-time and tests. Requires should listed all dependencies which can be needed by user. > > FIX: Please add following deps to Requires > > - perl(Pod::Usage) - ccdiff:106, lib/App/ccdiff.pm:106 > > - perl-podlators - ccdiff:105, lib/App/ccdiff.pm:105 > > - groff-base - ccdiff:105, lib/App/ccdiff.pm:105 > > Done. +Requires: perl-podlators +Requires: groff-base +Requires: perl(Pod::Usage) Ok > > > If you want to add the package to EPEL, please ignore these two TODO > > TODO: The easier way to remove .packlist is used NO_PACKLIST option, > > which is part of perl(ExtUtils::MakeMaker) >= 6.76. It can be > > used in all Fedoras. The command is > > perl Makefile.PL INSTALLDIRS=vendor NO_PACKLIST=1 > > > > TODO: Remove the deleting empty directories in %install section. This is > > default behavior for Fedoras. > > Ah, thanks. Both of the find commands were added automatically by > `rpmdev-newspec -t perl`. I'll remove them for now, so that the spec is more > appropriate for Fedora. -perl Makefile.PL INSTALLDIRS=vendor +perl Makefile.PL INSTALLDIRS=vendor NO_PACKLIST=1 -find %{buildroot} -type f -name .packlist -exec rm -f {} ';' -find %{buildroot} -depth -type d -exec rmdir {} 2>/dev/null ';' Ok. TODO: Please add version constrain 'perl(ExtUtils::MakeMaker) >= 6.76' Please consider fixing 'TODO' item. The package looks good. Approved -- You are receiving this mail because: You are on the CC list for the bug. 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 Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
