[Bug 814804] Review Request: OpenColorIO - Enables color transforms and image display across graphics apps

[bugzilla at redhat.com]

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=814804

--- Comment #7 from Kalev Lember <kalevlember at gmail.com> 2012-04-25 21:13:21 EDT ---
Fedora review OpenColorIO-1.0.7-3.fc16.src.rpm 2012-04-25

+ OK
! needs attention

rpmlint output:
$ rpmlint OpenColorIO \
          OpenColorIO-debuginfo \
          OpenColorIO-devel \
          OpenColorIO-doc \
          OpenColorIO-1.0.7-3.fc16.src.rpm
OpenColorIO-devel.x86_64: W: no-documentation
OpenColorIO.src: W: invalid-url Source0:
imageworks-OpenColorIO-v1.0.7-0-g87da508.tar.gz
5 packages and 0 specfiles checked; 0 errors, 2 warnings.

+ Rpmlint warnings are harmless and can be ignored
+ The package is named according to Fedora packaging guidelines
+ The spec file name matches the base package name.
+ The package meets the Packaging Guidelines
+ The package is licensed with a Fedora approved license and meets the
  Licensing Guidelines.
+ The license field in the spec file matches the actual license
+ The package contains the license file (LICENSE)
+ Spec file is written in American English
+ Spec file is legible
+ Upstream sources match the sources in the srpm.
+ The package builds in koji
n/a ExcludeArch bugs filed
+ BuildRequires look sane
n/a The spec file handles locales properly
+ ldconfig in %post and %postun
+ Package does not bundle copies of system libraries
n/a Package isn't relocatable
+ Package owns all the directories it creates
+ No duplicate files in %files
+ Permissions are properly set
+ Consistent use of macros
+ The package must contain code or permissible content
+ Large documentation files should go in -doc subpackage
+ Files marked %doc should not affect package
+ Header files should be in -devel
n/a Static libraries should be in -static
+ Library files that end in .so must go in a -devel package
+ -devel must require the fully versioned base
+ Packages should not contain libtool .la files
n/a Proper .desktop file handling
+ Doesn't own files or directories already owned by other packages
+ Filenames are valid UTF-8

Only issue I see is that it is passing -msse2 to the compiler when building for
the i686 architecture. Not all i686 processors have SSE support and as this
architecture is for compatibility for older processors, we should avoid
building binaries that don't run on all supported i686 processors.

>From a quick look at the CMakeLists.txt file, looks like passing
-DOCIO_USE_SSE=OFF should be enough to turn this off.

Usually it would be enough to unconditionally disable the -msse2 flag, as this
is enabled by default for the x86-64 with gcc. But in this case, it looks like
there's also some SSE-specific code in OpenColorIO itself, so just
unconditionally turning off all SSE support with -DOCIO_USE_SSE=OFF wouldn't
work, if we want to keep the SSE-specific code for x86-64.

Could perhaps use:
%ifnarch x86_64
  -DOCIO_USE_SSE=OFF
%endif

to disable SSE on all architectures where we don't want SSE support (including
arm and other secondary arches without SSE)?

-- 
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.