https://bugzilla.redhat.com/show_bug.cgi?id=1955394 Ben Beasley <code@xxxxxxxxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(code@musicinmybra | |in.net) | --- Comment #18 from Ben Beasley <code@xxxxxxxxxxxxxxxxxx> --- Well, it’s a little hard to review a hypothetical spec file, especially because I no longer remember much about this package, but I’ll try! > 1. Revise configure script to provide all of the installation directories explicitly > Fix as comment #4 > > 2. Revise configure script to hold fedora compile flags > […] It sounds like you understand my suggestion and plan to implement it, so if the details are correct, I would approve this. ;-) Check the actual compiler command lines used in the build and compare against the output of “rpm -E '%{optflags}'” to be sure the flags are what they ought to be. > - Qatzip just use these 2 flags set by %set_build_flags, other flags such as FFLAGS are not honored here. Obviously, you do only need to handle the environment variables that apply to your build; no need to consider FFLAGS when there are no Fortran sources, or CXXFLAGS when there are no C++ sources. For a C library, handling CFLAGS and LDFLAGS should be sufficient. Make sure you are not adding any other optimization flags such as -O3 on top of these without justification (https://docs.fedoraproject.org/en-US/packaging-guidelines/#_compiler_flags). > 3. Split library package from main package into sub-package > - Split as comment #8 > - Main package will not depend on the lib package, but the devel package does. > Main package only contains binary file and is not linked to libqatzip.so. > The libqatzip.so is provided to other applications, so the devel package depends on the libs package. This sounds right. (https://docs.fedoraproject.org/en-US/packaging-guidelines/#_requiring_base_package) It’s common for the subpackage to be called -libs even when there is only one library file, but -lib would be acceptable too. Remember to you use a fully-versioned, arch-specific dependency like: > Requires: %{name}-libs%{?_isa} = %{version}-%{release} > 4. About the %check section > The upstream test source code are not invoked by the qatzip itself and is not called during the setup process. > It maybe used by some benchmark tools. > So I think we can add a brief comment in the spec file(in the spec file?) to explain it, such as > # Check section is not available for these functional and performance tests require special hardware. Agreed, this clearly justifies the lack of a %check section. -- 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 Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
