Bug 233139 - (SDLmm) Review Request: SDLmm - C++ interface for the popular SDL library
Review Request: SDLmm - C++ interface for the popular SDL library
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Christopher Stone
Fedora Package Reviews List
:
Depends On:
Blocks: 233142
  Show dependency treegraph
 
Reported: 2007-03-20 12:45 EDT by Hans de Goede
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-03-21 16:48:34 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
chris.stone: fedora‑review+
wtogami: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Hans de Goede 2007-03-20 12:45:46 EDT
Spec URL: http://people.atrpms.net/~hdegoede/SDLmm.spec
SRPM URL: http://people.atrpms.net/~hdegoede/SDLmm-0.1.8-1.fc7.src.rpm
Description:
SDLmm is a C++ glue for SDL, or the Simple DirectMedia Layer, which is a
generic API that provides low level access to audio, keyboard, mouse,
joystick, 3D hardware via OpenGL, and 2D framebuffer across multiple
platforms.

SDLmm aims to stay as close as possible to the C API while taking
advantage of native C++ features like object orientation.
Comment 1 Christopher Stone 2007-03-20 13:56:33 EDT
Please investigate these rpmlint warnings:

W: SDLmm undefined-non-weak-symbol /usr/lib64/libSDLmm-0.1.so.8.0.0
__cxa_pure_virtual
W: SDLmm undefined-non-weak-symbol /usr/lib64/libSDLmm-0.1.so.8.0.0
_ZNSs6assignEPKcm
W: SDLmm undefined-non-weak-symbol /usr/lib64/libSDLmm-0.1.so.8.0.0
__cxa_guard_release
W: SDLmm undefined-non-weak-symbol /usr/lib64/libSDLmm-0.1.so.8.0.0 _ZdlPv
W: SDLmm undefined-non-weak-symbol /usr/lib64/libSDLmm-0.1.so.8.0.0
_ZNSt8ios_base4InitD1Ev
W: SDLmm undefined-non-weak-symbol /usr/lib64/libSDLmm-0.1.so.8.0.0
__cxa_guard_acquire
W: SDLmm undefined-non-weak-symbol /usr/lib64/libSDLmm-0.1.so.8.0.0
_ZNSt8ios_base4InitC1Ev
W: SDLmm unused-direct-shlib-dependency /usr/lib64/libSDLmm-0.1.so.8.0.0
/lib64/libm.so.6
Comment 2 Hans de Goede 2007-03-20 16:11:08 EDT
Good catch, I really should teach myself to not only run rpmlint on the build
packages, but also on the installed packages.

Here is a new version fixing this:
Spec URL: http://people.atrpms.net/~hdegoede/SDLmm.spec
SRPM URL: http://people.atrpms.net/~hdegoede/SDLmm-0.1.8-2.fc7.src.rpm

rpmlint warnings still left on the installed package:
W: SDLmm unused-direct-shlib-dependency /usr/lib64/libSDLmm-0.1.so.8.0.0
/lib64/libpthread.so.0

This is caused by the output of "sdl-config --libs", and harmless since SDL
itself does indeed require pthreads.
Comment 3 Christopher Stone 2007-03-20 22:23:44 EDT
==== REVIEW CHECKLIST ====
- rpmlint output clean
- package named according to package naming guidelines
- spec file name matches %{name}
- package meets packaging guidelines
- licensed with open source compatible license
- license matches actual license
- license included in %doc
- spec written in American English
- spec file legible
- sources match upstream
0a05d27d1aed72af3c7a37b6378f50e5  SDLmm-0.1.8.tar.bz2
- package successfully compiles and builds on FC-6 x86_64
- all build dependencies listed in BR
- no locales
- proper use of ldconfig in %post/un sections
- package is not relocatable
- package owns all directories it creates
- directories it does not create owned by filesystem
- no duplicates in %files
- package contains proper %clean
- macro usage is consistent
- contains code
- no large documentation
- %doc does not affect runtime
- header files located in devel subpackage
- no static libraries
- no pkgconfig files
- libraries w/o suffix in devel
- devel has fully versioned requires
- no libtool archives
- not a GUI application
- package does not own files or directories owned by other packages


==== MUST FIX ====
- Source0 URL is incorrect, the directory is sdlmm, not SDLmm


==== SHOULD FIX ====
- use of -n %{name} is redundant in the %post/un sections

Comment 4 Hans de Goede 2007-03-21 06:23:15 EDT
Both fixed, see:
Spec URL: http://people.atrpms.net/~hdegoede/SDLmm.spec
SRPM URL: http://people.atrpms.net/~hdegoede/SDLmm-0.1.8-3.fc7.src.rpm
Comment 5 Christopher Stone 2007-03-21 12:35:14 EDT
I believe the new patch adds these warnings:
sdlmm_audio.cpp
sdlmm_srect.cpp: In function 'SDLmm::SRect SDLmm::Intersect(const SDLmm::SRect&,
const SDLmm::SRect&)':
sdlmm_srect.cpp:77: warning: comparison is always false due to limited range of
data type
sdlmm_srect.cpp:78: warning: comparison is always false due to limited range of
data type
sdlmm_srect.cpp: In function 'SDLmm::SRect SDLmm::Union(const SDLmm::SRect&,
const SDLmm::SRect&)':
sdlmm_srect.cpp:88: warning: comparison is always true due to limited range of
data type
sdlmm_srect.cpp:89: warning: comparison is always true due to limited range of
data type

Should investigate, but not a blocker.

All other must fix items have been fixed.

*** APPROVED ***
Comment 6 Hans de Goede 2007-03-21 14:52:11 EDT
New Package CVS Request
=======================
Package Name:      SDLmm
Short Description: C++ interface for the popular SDL library
Owners:            j.w.r.degoede@hhs.nl
Branches:          FC-6 devel
InitialCC:         <empty>
Comment 7 Hans de Goede 2007-03-21 16:48:34 EDT
Thanks for the review!

Imported and build, closing.

p.s.

I've updated (fixed) the patch to no longer cause those valid compiler warnings.

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