https://bugzilla.redhat.com/show_bug.cgi?id=2085444 --- Comment #2 from xiangquan.liu@xxxxxxxxx --- (In reply to Daniel Berrangé from comment #1) > 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 [Xiangquan] I would like to explain the reason why using lines of scripts here. We want to share some scripts/files between rpm and debian. In this case, we just need to update one place for both rpm and debian. For example, if one file is added or deleted, only BOM file is updated. If it is not acceptable, it will be updated. > > > * The license text is duplicated in two different locations > > /usr/share/doc/sgx-sdk/COPYING > /usr/share/licenses/sgx-sdk/License.txt > [Xiangquan] Will remove one of them. > > * 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' > [Xiangquan] Will check it. > * sgx-gdb is shipped twice in two different locations > [Xiangquan] Will check it. > * 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 > [Xiangquan] Actually SGXSDK is package just for developers. > * sample code would likely be better in a -samples sub-RPM since there's > quite alot of it [Xiangquan] Actually SGXSDK is package just for developers. -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. 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
