Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


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

Jerry James <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |[email protected]
         AssignedTo|[email protected]    |[email protected]
               Flag|                            |fedora-review?

--- Comment #1 from Jerry James <[email protected]> 2012-05-04 11:10:16 EDT 
---
I'll take this review.  Here are a some initial comments in advance of a full
review.

You don't need "BuildRequires: gzip".  See
https://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions_2.  Also, is
help2man actually used in the build?  It doesn't seem to be, in which case you
can also drop "BuildRequires: help2man".

Use %global instead of %define: see
https://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define.

Unless you plan to build this package for EPEL 5, you can remove the defattr()
statements from the %files sections.  Those statements became optional with the
release of rpm 4.4.

Looking at the source code shows that USE_SSE really means that SSE2
instructions can be emitted into the binary.  This is fine for x86_64, but is
not fine for %{ix86}.  Some CPUs in that class do not support SSE2.  You have 2
options for that platform: (1) don't build with SSE2 support at all, or (2)
build 2 shared libraries, one without SSE2 support in %{_libdir}, and one with
SSE2 support in %{_libdir}/sse2.  On CPUs with SSE2 support, ld.so will pick up
the library in %{_libdir}/sse2 at runtime.  You can see examples of doing this
in the atlas, gmp, gmp-ecm, and m4ri packages, among others.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
_______________________________________________
package-review mailing list
[email protected]
https://admin.fedoraproject.org/mailman/listinfo/package-review

Reply via email to