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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

--- Comment #2 from Ralf Corsepius <rc040203@xxxxxxxxxx> 2011-11-04 13:17:59 EDT ---
(In reply to comment #1)
> 1. Are you planning on building for EL5?
Well, note the age of this submission (~ 1 year) - It predates this change in
Fedora - and the history of this package (Coin[12] packages's spec. This
rpm'specs origins are much older than Fedora - Can now be cleaned up ;)

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

The files with bogus permission repeatedly changed in Coin's history. 
I once decided to use the "generic permission fix axe", because it turned
tiresome to chase permissions ;)

> 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 don't recall the details - Could be an artefact from Coin1 and Coin2 or a
side effect of coin-config's habit to hard-code paths.

To be checked.

> I also got rid of the coin_includedir and coin_htmldir.
Your preference - mine is different, because these reduce size of the diffs
between Coin1, Coin2 and Coin3 and eases comparing the specs.

> 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*
OK, I'll look into this.

> Nit-picks:
> 
> 5. I like %{buildroot} but the guidelines say just be consistent :)
I hate %{buildroot} and am not using it anywhere inside of my specs ;)

> 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.
OK, will check.

> Other:
> 
> The alternatives doesn't bother me but you'll have to explain the i18n problems
> to me...
The source code is written using some ISO8859 variant as being used in Norway.

This causes doxygen to generate ISO8859 encoded docs and causes doxygen to get
confused on some characters. IIRC, there also where some doxygen constructs
inside of doxygen comments in the source-code, which newer doxygen handles
differently than older doxygens. - I'll try to check.

An updated package to come sometime next week.

-- 
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
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]