https://bugzilla.redhat.com/show_bug.cgi?id=2165536 --- Comment #3 from Benson Muite <benson_muite@xxxxxxxxxxxxx> --- Thanks for your detailed suggestions. > TODO: I think this package should be called "bee2". It's the name used on the home page, on github and also the archive is called like that. Renamed package. > TODO: The libbee2 summary is very general. I see that the algorithm names are quite terrible, but couldn't the summary be made more specific? E.g. "STB cryptography"? The same applies to bsum summary. Expanded this a little. Much of the documentation is in Belorusian > FIX: Please correct the license tag for libbee2 and libbee2-devel packages to "GPL-3.0-only AND GPL-3.0-or-later". bsum should use "GPL-3.0-only". Done. > TODO: It's possible the GPL-3.0-or-later license is an upstream's mistake. Please report it. Done. https://github.com/agievich/bee2/issues/30 > FIX: Build-require 'sed' (libbee2.spec:45). Done. > TODO: You don't have to add -fPIE and -pie to build flags. Fedora default build flags linked from CFLAGS and LDFLAGS environment variables already do that. >From build log, build flags are: + CFLAGS='-O2 -flto=thin -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS --config /usr/lib/rpm/redhat/redhat-hardened-clang.cfg -fstack-protector-strong -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer' + export CFLAGS + CXXFLAGS='-O2 -flto=thin -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS --config /usr/lib/rpm/redhat/redhat-hardened-clang.cfg -fstack-protector-strong -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer' + export CXXFLAGS + FFLAGS='-O2 -flto=thin -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS --config /usr/lib/rpm/redhat/redhat-hardened-clang.cfg -fstack-protector-strong -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -I/usr/lib64/gfortran/modules' + export FFLAGS + FCFLAGS='-O2 -flto=thin -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS --config /usr/lib/rpm/redhat/redhat-hardened-clang.cfg -fstack-protector-strong -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -I/usr/lib64/gfortran/modules' + export FCFLAGS + VALAFLAGS=-g + export VALAFLAGS + LDFLAGS='-Wl,-z,relro -Wl,--as-needed -Wl,-z,now -flto=thin -fno-openmp-implicit-rpath -Wl,--build-id=sha1 ' Left as is to ensure do not get a warning from fedora-review > FIX: Build-require "coreutils" (libbee2.spec:67). Done. > The assembler code with SMD instructions is not built. Thus the resulting code is compatible with a minimal Fedora-supported CPU instruction set. OpenSSL has more checks in its build allowing for a variety of SIMD instructions, however will start with the base case. > FIX: A soname of the library is "libbee2.so.2.0". The %files section must name it explicitly <https://docs.fedoraproject.org/en-US/packaging-guidelines/#_listing_shared_library_files>, hence "%{_libdir}/libbee2.so.2.*" glob is not specific enough. Use "%{_libdir}/libbee2.so.2.0" and "%{_libdir}/libbee2.so.%{version}" to capture both files. Done. > FIX: Section 3 manual pages belong to devel subpackage. They document an API of the library. Done. > FIX: devel subpackage must own %{_includedir}/bee2, %{_includedir}/bee2/core etc. directories. I recommend simply using "%{_includedir}/bee2" in %files section instead of listing files separately. Added directory listings, find it easier to remember what is included. Hope this is ok. > FIX: Either fix the bug, or exclude the package from building on s390x with "ExcludeArch: s390x" <https://docs.fedoraproject.org/en-US/packaging-guidelines/#_architecture_build_failures>. Excluded this, and reported upstream. Needs further investigation. Some of the tests also fail with GCC compiler and Fedora build flags. Made the main package a libs package to more correctly indicate the content. Review template: https://download.copr.fedorainfracloud.org/results/fed500/libbee2/fedora-rawhide-x86_64/05521033-bee2/fedora-review/review.txt spec: https://download.copr.fedorainfracloud.org/results/fed500/libbee2/fedora-rawhide-x86_64/05521033-bee2/bee2.spec srpm: https://download.copr.fedorainfracloud.org/results/fed500/libbee2/fedora-rawhide-x86_64/05521033-bee2/bee2-2.1.0-3.fc39.src.rpm -- 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=2165536 _______________________________________________ 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, report it: https://pagure.io/fedora-infrastructure/new_issue
