https://bugzilla.redhat.com/show_bug.cgi?id=1404043
--- Comment #5 from Jarod Wilson <ja...@redhat.com> --- (In reply to Jarod Wilson from comment #4) > Spec file comments from honli: > > Url: https://github.com/linux-rdma/rdma-core > honli: > https://fedoraproject.org/wiki/Packaging: > SourceURL#When_Upstream_uses_Prohibited_Code > honli: Please add comment for the "Source:" tag. > Source: rdma-core-%{version}.tgz I could, but this situation is temporary, there will be an upstream release and a full URL shortly. This was noted in comment #1, but I could add that same text to the spec until the release is out if need be. > BuildRequires: binutils > BuildRequires: cmake >= 2.8.11 > BuildRequires: gcc > BuildRequires: libudev-devel > BuildRequires: pkgconfig > BuildRequires: pkgconfig(libnl-3.0) > BuildRequires: pkgconfig(libnl-route-3.0) > BuildRequires: valgrind-devel > > honli: libnl3-devel is required for iwpmd libibverbs That's what you get from pkgconfig(libnl-3.0). > honli: systemd and dracut also needed. These already get pulled in. > honli: You also delete a few necessary "Requires:" tags. > honli: Please see review-comment.txt for details. From that doc: > honli: Please add "Requires:" entries against rsyslog, systemd, kmod, > honli: logrotate, initscripts, and dracut. None of these need to be listed explicitly, so far as I know. These are all things that are expected to be on the system, thus no need to call them out specifically. Several of them are already requirements of the kernel. > %description > RDMA core userspace infrastructure and documentation. > honli: I know this is an upstream issue. This is %description > honli: section is too simple, in other words, it is meaningless. > honli: If you do not have good %description section, I suggest > honli: you copy and paste > https://bugzilla.redhat.com/show_bug.cgi?id=1404043#c0 > honli: At least, user will know what is the package after read that. That #c0 text applies to the package as a whole though, including things that are in libibverbs. I'll expand on the text there a bit though. Proposed update: %description RDMA core userspace infrastructure and documentation, including initscripts, kernel driver-specific modprobe override configs, IPoIB network scripts, dracut rules, and the rdma-ndd utility. > %package -n libibverbs > Summary: A library and drivers for direct userspace use of RDMA > (InfiniBand/iWARP) hardware > Requires(post): /sbin/ldconfig > Requires(postun): /sbin/ldconfig > Requires: rdma-core > honli: Even over 99% files are stext files, rdma-core contains > /usr/sbin/rdma-ndd, > honli: which is an ELF file, so rdma-core is not a noarch rpm. Please > replace all > honli: "Requires: rdma-core" with "Requires: %{name}%{?_isa} = > %{version}-%{release}" Will do. > %description -n srp_daemon > In conjunction with the kernel ib_srp driver, srptools allows you to > honli: should replace srptools with srp_daemon in above line? > discover and use SCSI devices via the SCSI RDMA Protocol over InfiniBand. Done. > %prep > %setup > honli: %setup -q ??? > %build Why? I prefer verbose myself, so you can see more easily in logs if something goes wrong. > %postun -n ibacm > %systemd_postun_with_restart ibacm.service > > honli: Why you only run > systemd_post/systemd_preun/systemd_postun_with_restart > honli: against ibacm.service? How about iwpmd.service and srp_daemon.service? Looks like an oversight, will correct this. > %post -n libibcm -p /sbin/ldconfig > %postun -n libibcm -p /sbin/ldconfig > honli: Why not run ldconfig for other libraries, at least librdmacm has .so > honli: file in private dir. Another oversight. And on a related note, the ldconfigs for the base package serve no purpose, since there are no libs in it. This is probably a remnant of starting from the one-shot spec in the top level of the source dir. Will fix this up too. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
