Spec URL: http://people.atrpms.net/~hdegoede/coldet.spec SRPM URL: http://people.atrpms.net/~hdegoede/coldet-1.1-1.src.rpm Description: This library is an effort to provide a free collision detection library for generic polyhedra. Its purpose is mainly for 3D games where accurate detection is needed between two non-simple objects. Features: * Works on any model, even polygon soups. * Uses bounding box hierarchies for fast detection. * Uses additional triangle intersection tests for 100% accuracy. * Provides (upon request) exact point of collision, plus the pair of triangles that collided. * Supports timeout setting, to limit detection time. * Model-Model collision test. * Ray-Model collision test. * Segment-Model collision test. * Sphere-Model collision test. * Ray-Sphere and Sphere-Sphere primitive collision tests.
The *-devel packages installs its headers to /usr/include, resulting into this: /usr/include/coldet.h /usr/include/math3d.h To me, /usr/include/math3d.h is too general to justify installing it there[1]. I strongly recommend to install the headers into /usr/include/coldet instead. Also, the package suffers from quite an amount of rather serious GCC warnings (esp. punned pointers), which are not unlikely to break its functionality. [1] I am working on 3d simulations myself and have several packages providing /usr/include/*/math3d.h installed.
(In reply to comment #1) > The *-devel packages installs its headers to /usr/include, resulting into this: > /usr/include/coldet.h > /usr/include/math3d.h > > To me, /usr/include/math3d.h is too general to justify installing it there[1]. > I strongly recommend to install the headers into /usr/include/coldet instead. > I was thinking this at first too, then I though it would be ok and that if it wouldn't be ok I would hear so during the review :) > Also, the package suffers from quite an amount of rather serious GCC warnings > (esp. punned pointers), which are not unlikely to break its functionality. > Huh, quite an amount? on x86_64 devel I count 3 "dereferencing type-punned pointer will break strict-aliasing rules" warnings. I've once spend a days fixing all those warnings in Glide3. But I've given up there are simple to many of these warnings and 99.9% is not a problem. I know howto fix these, but I don't see how this package is any different from all the other packages we have with these kind of warnings. Here is a new version soon moving the headers to /usr/include/coldet: Spec URL: http://people.atrpms.net/~hdegoede/coldet.spec SRPM URL: http://people.atrpms.net/~hdegoede/coldet-1.1-2.src.rpm
(In reply to comment #2) > (In reply to comment #1) > Huh, quite an amount? on x86_64 devel I count 3 "dereferencing type-punned > pointer will break strict-aliasing rules" warnings. I've once spend a days > fixing all those warnings in Glide3. But I've given up there are simple to > many of these warnings => Crappy, non portable code. > and 99.9% is not a problem. They might currently not be a problem, but they are very likely to become in future, once GCC should start using more agressive optimizations. I know howto fix these, but I > don't see how this package is any different from all the other packages > we have with these kind of warnings. The only difference is this package consisting of very few source files.
(In reply to comment #2) Hans, I think, I need to clarify my comment: I would approve this package if you install the headers to /usr/include/coldet. My remark on the "punned pointers" was not meant to be blocker to the review. It was meant as a "BIG FAT WARNING" to you that you will not unlikely be facing "weird runtime errors" related to this package in future.
MUST ==== * Package/spec named appropriately * Source matches upstream: 26c2db12ec5ad2d7a0b1d0fe2597ed4a coldet_11.zip * LGPL license ok, license file included * rpmlint output clean * spec file legible and in Am. English * Builds ok in mock on FC4, FC5, and FC6 for both i386 and x86_64 * No excessive BR: (no BR: at all!) * ldconfig called in %post/%postud as needed * headers and unversioned .so properly located in -devel subpackage * Owns all directories that it creates; doesn't own directories that it should not. * No locales * No .desktop file needed * Not relocatable * No duplicate %files * File permissions look ok * No libtool archives MUSTFIX ======= * -devel subpackage missing the -%{release} component of e-v-r when requiring the base package.
(In reply to comment #5) > MUSTFIX > ======= > * -devel subpackage missing the -%{release} component of e-v-r when requiring > the base package. Fixed, new version is here: Spec URL: http://people.atrpms.net/~hdegoede/coldet.spec SRPM URL: http://people.atrpms.net/~hdegoede/coldet-1.1-2.src.rpm
Correction that should be: Spec URL: http://people.atrpms.net/~hdegoede/coldet.spec SRPM URL: http://people.atrpms.net/~hdegoede/coldet-1.1-3.src.rpm
All MUSTFIX items addressed. APPROVED
Thanks! Imported & build, closing.