[Bug 665733] Review Request: Coin3 - High-level 3D visualization library

[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=665733

Richard Shaw <hobbes1069 at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #1 from Richard Shaw <hobbes1069 at gmail.com> 2011-11-01 09:34:42 EDT ---
1. Are you planning on building for EL5? If not then the following can go away.

BuildRoot:, rm -rf $RPM_BUILD_ROOT from %install, %clean entirely, %defarrt
from %files


2. I had managed to get a build before getting your email response. Just to see
how minimal I could go I stripped out a bunch of stuff including:

# bogus permissions
find . \( -name '*.h' -o -name '*.cpp' -o -name '*.c' \) -a -executable -exec
chmod -x {} \;

rpmlint only complained about files in one directory so I replaced the above
with:

chmod -x src/xml/expat/*.c

It reduced the build time quite a bit. Nothing that would matter for koji, but
significant for experimental building on my own machine :)


3. Ok, in %configure I made quite a few changes. I went ahead and took
advantage of all the options that seemed appropriate. Here's my version, keep
in mind I removed all the alternative stuff for my personal build:

%configure \
        --includedir=%{_includedir}/Coin3 \
        --htmldir=%{_datadir}/doc/Coin3 \
        --disable-dependency-tracking \
        --enable-shared \
        --disable-dl-libbzip2 \
        --disable-dl-glu \
        --disable-dl-zlib \
        --disable-dl-freetype \
        --disable-dl-fontconfig \
        --disable-spidermonkey \
        --enable-man \
        --enable-html \
        --enable-3ds-import \
        CPPFLAGS=$(pkg-config --cflags freetype2)

The htmldir environment variable worked but since it offered a --htmldir option
I went ahead and used it. If this is truly documentation, should it not go in
/usr/share/doc/... and not /usr/share/...?

I also got rid of the coin_includedir and coin_htmldir. They're not used very
much and if you're not familiar with the package and you're looking in %files
you have to go back to the top of the spec to see how they're defined. 

4. %files:

%doc AUTHORS COPYING README* LICENSE* THANKS FAQ*

- The README* also grabs readme's for windows and mac so change to:

%doc AUTHORS COPYING README README.UNIX LICENSE* THANKS FAQ*


Nit-picks:

5. I like %{buildroot} but the guidelines say just be consistent :)

6. There's a blank line in the middle of your BuildRequires. It looks like
you're separating the GL/X stuff from everything else. If you're not going to
put in a comment, it would be better to remove the blank line.


Other:

The alternatives doesn't bother me but you'll have to explain the i18n problems
to me...

Thanks,
Richard

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