https://bugzilla.redhat.com/show_bug.cgi?id=2165536



--- Comment #3 from Benson Muite <[email protected]> ---
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 -- [email protected]
To unsubscribe send an email to [email protected]
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/[email protected]
Do not reply to spam, report it: 
https://pagure.io/fedora-infrastructure/new_issue

Reply via email to