https://bugzilla.redhat.com/show_bug.cgi?id=1814682 --- Comment #5 from lsun@xxxxxxxxxxxx --- (In reply to Michal Schmidt from comment #2) > Commenting on a few lines in rshim.spec: > > > URL: https://github.com/mellanox/rshim-user-space > > Seeing the name "rshim-user-space" made me curious if there's a kernel part > and I found: > https://github.com/mellanox/rshim > "BlueField RSHIM driver" > > What is the relation between the kernel and the userspace drivers? > Was the kernel driver the original approach which later got obsoleted by the > userspace implementation? Yes, the kernel driver is the original approach, which has upstreaming difficulties due to mixed functionalities. This 'rshim-user-space' driver intends to replace the kernel driver with exactly the same user interface. > > > Obsoletes: %{name} < 2.0 > > This line says: rshim-2.0 obsoletes rshim < 2.0. That seems redundant. Will remove it. > > > %global with_systemd %(if ( test -d "%{_unitdir}" > /dev/null); then echo -n '1'; else echo -n '0'; fi) > > Why not assume systemd always? The driver might be used on some older OS version (like centos 6) which doesn't have systemd. The check here is to prevent source RPM build issues for such case. This package will also be included in Mellanox OFED package which needs to support different build environments. > > > %prep > > rm -fr %{name}-%{version} > > mkdir %{name}-%{version} > > tar -axf %{SOURCE0} -C %{name}-%{version} --strip-components 1 > > %setup -q -D -T > > Would this simpler way work too?: > %prep > %setup -n rshim-user-space-%{name}-%{version} > > Or maybe, since you are the upstream maintainer, you could change the > upstream tarball generation to avoid the odd directory name? I had a little trouble with the name. The current tarball with such top-level odd name was generated by GitHub automatically when a release/tag is created. But you're right. I could actually upload the tarball and use it instead. Will make the change in next post. > > > %install > > %makeinstall -C src INSTALL_DIR="%{buildroot}%{_bindir}" > > %if "%{with_systemd}" == "1" > > %{__install} -d %{buildroot}%{_unitdir} > > %{__install} -m 0644 bfrshim.service %{buildroot}%{_unitdir} > > %endif > > %{__install} -d %{buildroot}%{_mandir}/man1 > > %{__install} -m 0644 man/bfrshim.1 %{buildroot}%{_mandir}/man1 > > Why don't you let %makeinstall install these? Will update it in next post. > > > %__spec_install_post > > This looks like duplicating rpm-build's work. Will update it in next post. > > > %post > > %if "%{with_systemd}" == "1" > > systemctl daemon-reload > > systemctl enable bfrshim.service > > %endif > > No, you need to use the macros from the packaging guidelines: > https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/ > #_systemd Will update it in next post. > > > %files > > %license LICENSE > > %defattr(-,root,root,-) > > This looks like the default, so the %defattr line is redundant. Will remove it in next post. > > > %%doc README.md > > Double % sign, typo? Will fix it in next post. > > > %changelog > > * Fri Mar 13 2020 Liming Sun <lsun@xxxxxxxxxxxx> - 2.0-1 > >- Update the spec file according to fedora packaging-guidelines > > * Mon Dec 16 2019 Liming Sun <lsun@xxxxxxxxxxxx> > >- Initial packaging > > Please add an empty line between changelog entries. Will fix it in next post. -- 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
