Bug 474149 - Review Request: chipmunk - A rigid body physics library
Review Request: chipmunk - A rigid body physics library
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Conrad Meyer
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-12-02 09:48 EST by Gwyn Ciesla
Modified: 2009-01-14 21:58 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-01-12 13:57:48 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
konrad: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)
Review. (9.17 KB, text/plain)
2009-01-01 18:45 EST, Conrad Meyer
no flags Details

  None (edit)
Description Gwyn Ciesla 2008-12-02 09:48:24 EST
Description:
    *  Designed for 2D video games.
    * Circle, convex polygon, and line segment collision primitives.
    * Multiple collision primitives can be attached to the same rigid body.
    * Fast collision detection by using a spatial hash for the broad phase.
    * Extremely fast impulse solving by utilizing Erin Catto’s contact persistence algorithm.
    * Support for collision callbacks based on object types.
    * Impulses applied to contact points can be retrieved after the impulse solver has finished.
    * Several kinds of joints available.
    * C99 implementation, no external dependencies.
    * Ruby extension available.
    * Simple, read the documentation.
    * Unrestrictive MIT license.

SRPM URL: http://zanoni.jcomserv.net/fedora/chipmunk/chipmunk-4.1.0-1.fc9.src.rpm
SPEC URL: http://zanoni.jcomserv.net/fedora/chipmunk/chipmunk.spec
Comment 1 Conrad Meyer 2009-01-01 18:45:31 EST
Created attachment 328048 [details]
Review.

Attached is my initial review. Please fix the problems indicated by rpmlint; also, I have a question. Otherwise looks good.
Comment 2 Gwyn Ciesla 2009-01-02 08:38:43 EST
The comment about *.so applies to *.so.*.

I'm actually not sure how to correct the debuginfo issue, which I was hoping to find someone to assist with.  I see strip mentioned a few places in the source tree, but my cmake-fu is insufficient to finding the most clueful way to remove it.

What's the question?
Comment 3 Conrad Meyer 2009-01-02 08:47:04 EST
(In reply to comment #2)
> The comment about *.so applies to *.so.*.

(This was my question.)

> I'm actually not sure how to correct the debuginfo issue, which I was hoping to
> find someone to assist with.  I see strip mentioned a few places in the source
> tree, but my cmake-fu is insufficient to finding the most clueful way to remove
> it.

Probably easiest just to remove strip wherever it's mentioned (my cmake-fu is no better than yours).
Comment 4 Gwyn Ciesla 2009-01-07 11:04:22 EST
SRPM URL: http://zanoni.jcomserv.net/fedora/chipmunk/chipmunk-4.1.0-2.fc10.src.rpm
SPEC URL: http://zanoni.jcomserv.net/fedora/chipmunk/chipmunk.spec

Fixed it, but now it complains that the ruby lib is unstripped.  I don't see anything in the ruby packaging guidelines that would help.
Comment 5 Conrad Meyer 2009-01-07 15:09:49 EST
Some more comments:
- Should this be System Environment/Libraries (and not Development/Libraries)?
- Line 46 of the spec has a tab instead of spaces.
- Line 61 should be %{cmake} . (sets everything automatically).
- Line 72, the mode should be 755 not 644. Rpm only strips 755 files when looking for debuginfo, this is why you are getting the warnings about unstripped binaries.
- Line 88, you should use %{_libdir} instead of /usr/lib
- Line 93, this should be "%{_includedir}/chipmunk" not "%{_includedir}/chipmunk/*.h" (the latter leaves an unowned directory).
- Line 87, should be "...*.so.*" (as opposed to "...*.so*"). This is because un-suffixed .so files belong in the -devel package.
- Add %{_libdir}/*.so to %files devel.
Comment 6 Conrad Meyer 2009-01-07 15:10:53 EST
Oh, and line 72, you might want to replace ".so.4" with ".so.*" so that your spec doesn't need to be updated when the SONAME gets bumped upstream.
Comment 8 Conrad Meyer 2009-01-07 15:55:45 EST
Looks good to me. APPROVED.
Comment 9 Conrad Meyer 2009-01-07 16:00:54 EST
Um, oops, this doesn't build in mock yet:
447 Installed (but unpackaged) file(s) found:
448    /usr/lib/libchipmunk.a
449    /usr/lib/libchipmunk.so
450    /usr/lib/libchipmunk.so.4

It seems that your package is somehow installing libraries to /usr/lib instead of %{_libdir}; please fix this before importing it to CVS. Thanks.
Comment 10 Gwyn Ciesla 2009-01-07 16:02:51 EST
Excellent, thanks!

New Package CVS Request
=======================
Package Name: chipmunk
Short Description: A rigid body physics library
Owners: limb
Branches: F-10
InitialCC:
Comment 11 Gwyn Ciesla 2009-01-07 16:10:50 EST
Will do, good catch.
Comment 12 Gwyn Ciesla 2009-01-07 16:30:16 EST
Weird, my mock build works.  Macros seem ok, I'll see what happened after import.
Comment 13 Conrad Meyer 2009-01-07 18:35:27 EST
(In reply to comment #12)
> Weird, my mock build works.  Macros seem ok, I'll see what happened after
> import.

I'm on x86_64. %{_libdir} != /usr/lib here, which is why rpmbuild (on x86_64) catches it.
Comment 15 Conrad Meyer 2009-01-08 12:32:51 EST
(In reply to comment #14)
> %if "%{?_lib}" == "lib64" 
>   %{cmake} -DLIB_SUFFIX=64
> %else
>   %{cmake}
> %endif 


%{cmake} already expands to something like this:
  CFLAGS="${CFLAGS:--O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic}" ; export CFLAGS ; 
  CXXFLAGS="${CXXFLAGS:--O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic}" ; export CXXFLAGS ; 
  FFLAGS="${FFLAGS:--O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic}" ; export FFLAGS ; 
  /usr/bin/cmake \
        -DCMAKE_INSTALL_PREFIX:PATH=/usr \
        -DCMAKE_INSTALL_LIBDIR:PATH=/usr/lib64 \
        -DINCLUDE_INSTALL_DIR:PATH=/usr/include \
        -DLIB_INSTALL_DIR:PATH=/usr/lib64 \
        -DSYSCONF_INSTALL_DIR:PATH=/etc \
        -DSHARE_INSTALL_PREFIX:PATH=/usr/share \
%if "lib64" == "lib64" 
        -DLIB_SUFFIX=64 \
%endif 
        -DBUILD_SHARED_LIBS:BOOL=ON


So I think what is passed to cmake is not the problem. As a "hack" solution you could move stuff from /usr/lib to %{_libdir} in %install, or else fix the CMakeLists.txt files.

But this is already approved and you will have trouble building in koji until this is fixed anyways, so no need to check back in with me unless you want help :).
Comment 16 Conrad Meyer 2009-01-08 12:33:44 EST
Or rather, it expands to that on lib64 arches. But you get the idea.
Comment 17 Kevin Fenzi 2009-01-09 00:54:23 EST
cvs done.
Comment 18 Gwyn Ciesla 2009-01-12 13:57:48 EST
I got it to build, but it's the ugly mv in %install method.  Bleah.

Imported and build in rawhide.
Comment 19 Fedora Update System 2009-01-12 14:05:14 EST
chipmunk-4.1.0-4.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/chipmunk-4.1.0-4.fc10
Comment 20 Conrad Meyer 2009-01-12 14:48:28 EST
Any chance you want to work with the maintainer of xmoto to get xmoto 0.5 to use the system chipmunk?
Comment 21 Conrad Meyer 2009-01-12 14:52:29 EST
Just kidding, it appears you are the xmoto maintainer. Good job :D.
Comment 22 Gwyn Ciesla 2009-01-12 14:57:36 EST
Had a reply half typed to #20 when I saw #21. . .:)

I've made a few attempts, but it looks like a lot of paths might reply on the presence of the bundled version.  I'm in conversation with upstream about it.  They have no problem using the system version, it's not not in any distros.  I let them know Fedora has it now, so we'll see what develops.  If I get it working before then, I will of course patch.
Comment 23 Conrad Meyer 2009-01-12 15:10:40 EST
I think there is a packaging guideline that is something along the lines of: MUST not use private copy of system libraries. So please do work this out with upstream, or failing that hack up a patch.
Comment 24 Gwyn Ciesla 2009-01-12 15:18:45 EST
I think it's actually a SHOULD, but I feel pretty strongly about it and have done quite a bit with several of my PHP packages to that end.  The sooner the better.
Comment 25 Fedora Update System 2009-01-14 21:58:47 EST
chipmunk-4.1.0-4.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Note You need to log in before you can comment on or make changes to this bug.