Bug 198834 (sage)

Summary: Review Request: sage - OpenGL extensions library using SDL
Product: [Fedora] Fedora Reporter: Wart <wart>
Component: Package ReviewAssignee: Christopher Stone <chris.stone>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: che666
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2006-07-26 16:49:42 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On:    
Bug Blocks: 163779, 198839    

Description Wart 2006-07-13 18:54:53 EDT
Spec URL: http://www.kobold.org/~wart/fedora/sage.spec
SRPM URL: http://www.kobold.org/~wart/fedora/sage-0.1.2-1.src.rpm
Description:
Sage is an OpenGL extensions library using SDL. It aims to simplify the use of
checking for and loading OpenGL extensions in an application.  This library is used by some WorldForge clients.
Comment 1 Christopher Stone 2006-07-14 04:07:22 EDT
- rpmlint output clean
- package name meets package naming guidelines
- spec file name matches package %{name}
- package meets packaging guidelines
- package licensed with open source compatible license
O license does not match upstream license
- license included in %doc
- spec file in American english
- spec file is legible
- sources match upstream
4eea72b30a88dbe5d512009913462fc3  sage-0.1.2.tar.gz
- package successfully compiles and builds on x86_64 FC-5
- all dependencies are listed in BuildRequires
- package properly containst %post/%postun ldconfig
- package is not relocatable
- package owns all directories it creates
- package does not contain duplicate files
- file permissions set properly
- package contains proper %clean section
- macro usage is consistent
- package contains permissible content
- package does not contain large documentation
- files in %doc do not affect runtime
- header files are in devel
- pkgconfig files are in devel
- libraries w/o suffix are in devel
- devel package requires base package
- package does not contain any .la files
- package is not a GUI needing .desktop files
- package does not own files or directories owned by other packages


==== MUST ====
- devel package should Requires: pkgconfig
- use %{_mandir} instead of %{_datadir}/man
- notify upstream about --disable-static failure
- fix license to match upstream license
- had to download source from:
http://dl.sourceforge.net/sourceforge/worldforge/sage-0.1.2.tar.gz
Source0 should be updated accordingly

==== SHOULD =====
- add a %check even though it doesnt do anything now, it may in future
Comment 2 Wart 2006-07-14 14:56:06 EDT
(In reply to comment #1)
> ==== MUST ====
> - devel package should Requires: pkgconfig

Done.

> - use %{_mandir} instead of %{_datadir}/man

Done.

> - notify upstream about --disable-static failure

I don't think all configure scripts support --disable-static anyway.  I'll just
remove the flag and the comment.

> - fix license to match upstream license

Fixed.

> - had to download source from:
> http://dl.sourceforge.net/sourceforge/worldforge/sage-0.1.2.tar.gz
> Source0 should be updated accordingly

The current URL works for me?  This might be caused by some SF mirror selection
nonsense.  It seems that some mirrors insert 'sourceforge/' into the download
url paths.

> ==== SHOULD =====
> - add a %check even though it doesnt do anything now, it may in future

Done.

http://www.kobold.org/~wart/fedora/sage-0.1.2-2.src.rpm
http://www.kobold.org/~wart/fedora/sage.spec
Comment 3 Ralf Corsepius 2006-07-14 22:51:14 EDT
(In reply to comment #2)
> > - notify upstream about --disable-static failure
> 
> I don't think all configure scripts support --disable-static anyway. 
> I'll just remove the flag and the comment.
1. All autoconf-based packages must accept all --disable/enable flags.
This package does.

2. This package supports --disable-static (==--enable-static=no)
./configure --help
..
  --enable-static[=PKGS]
                          build static libraries [default=yes]
..

3. I do not see that --disable-static would not work for this package.


BLOCKER:
The package does not honor RPM_OPT_FLAGS correctly. It appends -O3 to CFLAGS.
Comment 4 Wart 2006-07-15 02:31:57 EDT
(In reply to comment #3)
> (In reply to comment #2)
> > > - notify upstream about --disable-static failure
> > 
> > I don't think all configure scripts support --disable-static anyway. 
> > I'll just remove the flag and the comment.
> 1. All autoconf-based packages must accept all --disable/enable flags.
> This package does.

If you want to be pendantic, then yes, they all support it.  But not all of them
are guaranteed to do anything useful with it.

> 2. This package supports --disable-static (==--enable-static=no)
> ./configure --help

You're right.  The 'rm -f *.a' at the end of %install was hiding this fact from
me.  I'll re-add --disable-static.
Comment 5 Wart 2006-07-16 00:36:51 EDT
Updated package that includes --disable-static and removes -O3 from CFLAGS:

http://www.kobold.org/~wart/fedora/sage-0.1.2-3.src.rpm
http://www.kobold.org/~wart/fedora/sage.spec
Comment 6 Christopher Stone 2006-07-26 14:36:19 EDT
Please add "make {?_smp_mflags} check" to the %check.

APPROVED
Comment 7 Wart 2006-07-26 16:49:42 EDT
Imported, added the 'make %{?_smp_mflags}', tagged, and built.

Thanks!