Spec URL: http://laiskiainen.org/tmp/driverctl/driverctl.spec SRPM URL: http://laiskiainen.org/tmp/driverctl/driverctl-0.74-1.fc24.src.rpm Description: driverctl is a tool for manipulating and inspecting the system device driver choices. Fedora Account System Username: pmatilai
> Group: System Environment/Kernel not needed > License: LGPLv2 LGPLv2+ actually > # "make archive" from git checkout of tag %{version} or > # https://gitlab.com/pmatilai/driverctl/repository/archive.tar.gz?ref=%{version} > Source0: %{name}-%{version}.tar.gz Source0: %{url}/repository/archive.tar.gz?ref=%{version}#/%{name}-%{version}.tar.gz > Requires(post,postun): systemd %{?systemd_requires} > make install DESTDIR=%{buildroot} %make_install > /usr/lib/udev/vfio_name Can you file a bug against systemd to add macro %{_udevdir}? > %{_sysconfdir}/bash_completion.d/driverctl-bash-completion.sh 1. it should go to %{_datadir}/bash-completion/ 2. directory is not owned * Description should end with "." * install is invocated without "-p", so fix it in upstream as well ;)
(In reply to Igor Gnatenko from comment #1) > > Group: System Environment/Kernel > not needed Not harmful either. I'm old school and prefer to keep it, thanks. > > > License: LGPLv2 > LGPLv2+ actually The README states the license is LGPL v2.1 specifically. > > > # "make archive" from git checkout of tag %{version} or > > # https://gitlab.com/pmatilai/driverctl/repository/archive.tar.gz?ref=%{version} > > Source0: %{name}-%{version}.tar.gz > Source0: > %{url}/repository/archive.tar.gz?ref=%{version}#/%{name}-%{version}.tar.gz I dont get the point, if anything that's LESS readable. > > Requires(post,postun): systemd > %{?systemd_requires} Actually not. The requirement is there for udevadm which at some point lived in systemd, but in current Fedora its in systemd-udev. I fixed it upstream to be a file dependency to cover for these differences, and RHEL-7 matters too. > > > make install DESTDIR=%{buildroot} > %make_install Shrug, both work. Changed to %make_install upstream since RHEL-7 seems to have %make_install already afterall. > > /usr/lib/udev/vfio_name > Can you file a bug against systemd to add macro %{_udevdir}? > > > %{_sysconfdir}/bash_completion.d/driverctl-bash-completion.sh > 1. it should go to %{_datadir}/bash-completion/ > 2. directory is not owned Sigh. %{_sysconfdir}/bash_completion.d/ was owned by filesystem to avoid the directory ownership issues, and now I'd need to own two extra directories. Is there an actual requirement documented for this someplace? > > * Description should end with "." > * install is invocated without "-p", so fix it in upstream as well ;) Both fixed upstream. Thanks for the review, I'll post an updated version once we agree on the nitpicks :)
Updated version which should address all the complaints: SPEC: http://laiskiainen.org/tmp/driverctl/driverctl.spec SRPM: http://laiskiainen.org/tmp/driverctl/driverctl-0.91-1.fc24.src.rpm
Hi Igor, could you please take a look at the newer version? Thanks!
ping Igor?
Drop scriptlets to update udev rules since it triggers unnecessary calls for udevadm which are not needed (systemd uses something like inotify and does this automagically).
The last I remember on the subject, systemd deliberately would NOT do such things because of atomicity issues: eg if a package drops multiple (dependent) rule files into the directory then the rules should only be updated after they all are there. It's explained somewhere on a public mailing list, but of course its also possible things have changed since then.
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/driverctl
FWIW, I imported what was reviewed, ie http://laiskiainen.org/tmp/driverctl/driverctl-0.91-1.fc24.src.rpm as-is. Deciding what (if anything) to do wrt comment #6 is up to the new maintainer.