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

Ben Beasley <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|needinfo?(code@musicinmybra |
                   |in.net)                     |



--- Comment #18 from Ben Beasley <[email protected]> ---
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 -- [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 on the list, report it: 
https://pagure.io/fedora-infrastructure

Reply via email to