https://bugzilla.redhat.com/show_bug.cgi?id=1904696 --- Comment #4 from Richard Shaw <hobbes1069@xxxxxxxxx> --- (In reply to Mikel Olasagasti Uranga from comment #2) > Some notes: > > - Requires Perl and perl modules are missing: > > https://git.fws.fr/fws/virt-backup/src/branch/master/virt-backup.spec#L16 He had "AutoRep: no" but removing it found even more requirements that he listed, I assumed the automatic detection was sufficient? I did add a require for perl itself. > - Why patch the .spec file from tarball with virt-backup-update.patch if it > not going to be installed/used? Because Daniel provides a spec file and I'm working with him to modernize it. I emailed him the same patch. > - _sharedstatedir can be used instead of _localstatedir Fixed. > - Is the changelog correct? I understand that you took Daniel's version and > changed it. You can change Release to 2 and add a new Changelog entry. Yes, I'll do that with the updates. (In reply to Mikel Olasagasti Uranga from comment #3) > - Check missing perl deps: As I mentioned before, the automatic dep findinging seems to be sufficient. The Perl packaging guidelines could be more explicit here but subsection, https://docs.fedoraproject.org/en-US/packaging-guidelines/Perl/#_manual_requires_and_provides "Under some circumstances, RPM’s automatic dependency generator can miss dependencies that should be added." Seems to indicate that it's ok to rely on the automatic dependency generator. I did add a BR: on perl-generators though. > https://git.fws.fr/fws/virt-backup/src/branch/master/virt-backup#L26 > > - Missing dep on qemu-img Fixed. > https://git.fws.fr/fws/virt-backup/src/branch/master/virt-backup#L571 > > - Script maybe should also depend, not 100% sure, on > libvirt-daemon-driver-qemu for /var/lib/libvirt/qemu/ ownership and > libvirt-libs for /var/lib/libvirt/ ownership Unless we're sure I'd rather not. The packaging guideline requirement is to require packages that own directories into which you're installing files. I don't think there's a requirement to pull in directories the script may "use". That being said, I think at least requiring libvirt-libs is a good idea. Spec URL: https://hobbes1069.fedorapeople.org/virt-backup.spec SRPM URL: https://hobbes1069.fedorapeople.org/virt-backup-0.2.25-2.fc33.src.rpm %changelog * Sun Dec 06 2020 Richard Shaw <hobbes1069@xxxxxxxxx> - 0.2.25-2 - Update spec per reviewer feedback. -- 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
