https://bugzilla.redhat.com/show_bug.cgi?id=2085444 Daniel Berrangé <berrange@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |berrange@xxxxxxxxxx --- Comment #1 from Daniel Berrangé <berrange@xxxxxxxxxx> --- I'm not taking this on as a formal review, but some low hanging fruit I noticed after a quick look at the spec & test build * "BuildRequires: perl" can be replaced with "perl-base" as I don't see a need for the historical full perl module set * The "%description" is terse to the say the least, just repeating the Summary - "Intel(R) SGX SDK" - this needs to be more verbose - a few sentences * The section find %{?buildroot}/license -type f -print0 | \ xargs -0 -n1 cat >> %{?buildroot}%{_docdir}/%{name}/COPYING rm -fr %{?buildroot}/license echo "%{_install_path}" > %{_specdir}/listfile find %{?buildroot} -type d -exec \ sh -c '(ls -p "{}"|grep />/dev/null)||echo "{}"' \; | \ sed -e "s#^%{?buildroot}##" | \ grep -v "^%{_libdir}" | \ grep -v "^%{_bindir}" | \ grep -v "^%{_install_path}" | \ sed -e "s#^#%dir #" >> %{_specdir}/listfile find %{?buildroot} -type f | \ sed -e "s#^%{?buildroot}##" | \ grep -v "^%{_install_path}" >> %{_specdir}/listfile %files -f %{_specdir}/listfile Is rather unpleasantly obscuring what is actually being packaged. With this kind of magic it is way too easy to accidentally ship undesirable files in the final RPM. IMHO the %file section should be listing stuff explicitly, with few wildcards, to make it clear what is being shipped. ie it is reasonable to wildcard header files *.h, but not wildcard the nested trees. As a case in point, this current magic obscures the fact that the majority of files in this RPM has been put into /opt/intel/sgxsdk. Fedora RPMs are not expected to use /opt except in rare cases https://docs.fedoraproject.org/en-US/packaging-guidelines/#_limited_usage_of_opt_etcopt_and_varopt so if there's a good reason for use of /opt it needs a justification to be provided * The license text is duplicated in two different locations /usr/share/doc/sgx-sdk/COPYING /usr/share/licenses/sgx-sdk/License.txt * The pkg-config files are corrupt - the start of all of them looks like /opt/intel/sgxsdk includedir=${prefix}/include libdir=${prefix}/lib64 I believe that first line is supposed to be 'prefix=/opt/intel/sgxsdk' * sgx-gdb is shipped twice in two different locations * The package possibly ought to have a -devel sub-RPM for the include files & .pc files, so they're separate from the runtime .so files * sample code would likely be better in a -samples sub-RPM since there's quite alot of it -- 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 https://bugzilla.redhat.com/show_bug.cgi?id=2085444 _______________________________________________ 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 Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
