Bug 469972 - Review Request: libglfw - A portable framework for OpenGL
Summary: Review Request: libglfw - A portable framework for OpenGL
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: David Nielsen
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 462181 469969
TreeView+ depends on / blocked
 
Reported: 2008-11-04 22:41 UTC by Paul F. Johnson
Modified: 2009-01-11 22:01 UTC (History)
8 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2009-01-08 14:29:19 UTC
Type: ---
Embargoed:
lkundrak: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Paul F. Johnson 2008-11-04 22:41:24 UTC
Spec URL: http://pfj.fedorapeople.org/glfw.spec
SRPM URL: http://pfj.fedorapeople.org/glfw-2.6-1.fc10.src.rpm
Description: GLFW is a free, Open Source, portable framework for OpenGL application development. In short, it is a single library providing a powerful, portable API for otherwise operating system specific tasks such as opening an OpenGL window, and reading keyboard, time, mouse and joystick input.

Comment 1 David Nielsen 2008-11-07 22:08:09 UTC
I'll take this

Comment 2 David Nielsen 2008-11-08 14:05:10 UTC
bad rpmlint output:

glfw.x86_64: W: file-not-utf8 /usr/share/doc/glfw-2.6/glfwug.tex
glfw.x86_64: W: file-not-utf8 /usr/share/doc/glfw-2.6/glfwrm.tex
glfw.x86_64: E: no-binary
(if it has no binary wouldn't libglfw be a better name - also documentation goes in the documentation package)

glfw-debuginfo.x86_64: E: empty-debuginfo-package
(see below)

glfw-devel.x86_64: W: unstripped-binary-or-object /usr/lib64/libglfw.so
glfw-devel.x86_64: W: no-soname /usr/lib64/libglfw.so

glfw.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 9)

according to
http://glfw.sourceforge.net/license.html
license is zlib/libpng not bsd

and as usual a few comments on your voodoo is nice for those who maintain your packages while you are unable.

You should split out all the documentation into a subpackage or at the very least put it in the -devel package (though there seems to be a lot of it, a doc package would be preferable)

Good news though, it builds in mock (x86_64 f10)

Comment 3 Paul F. Johnson 2008-11-08 14:35:39 UTC
Spec URL: http://pfj.fedorapeople.org/glfw.spec
SRPM URL: http://pfj.fedorapeople.org/libglfw-2.6-2.fc10.src.rpm

Okay, sorted most of the problems except for the spec file - I can do that on the next run (somewhat rushed today!!!!)

Comment 4 Paul F. Johnson 2008-11-12 18:59:29 UTC
Ping!!!!

Comment 5 David Nielsen 2008-11-12 19:15:52 UTC
oh when you said "the next run" I thought you meant another update was coming so I didn't look at it

Generally I like this one, let's call it approved

Comment 6 Paul F. Johnson 2008-11-12 21:50:51 UTC
Thanks :-)

New Package CVS Request
=======================
Package Name: libglfw
Short Description: A portable OpenGL framework
Owners: pfj
Branches: devel, f9
InitialCC: pfj

Comment 7 Paul F. Johnson 2008-11-12 21:51:25 UTC
I'll rebuild for tao in the morning and update that :-)

Comment 8 Kevin Fenzi 2008-11-14 05:33:07 UTC
David: I know it's a formality, but could you set the fedora-review flag to + ? 
I know you approved in comment #5, but can you just confirm? 

Also, Paul: would it be possible to re-upload the spec so others could look? 
It's gone from the above links.

Comment 9 David Nielsen 2008-11-14 08:39:42 UTC
dear Einstein I hate the new bugzilla, it's confusing layout keeps letting me forget these details. die die die..

Comment 10 Kevin Fenzi 2008-11-16 20:20:03 UTC
Paul: I assume you want a F10 branch as well?

cvs done (with F-10 branch as well).

Comment 11 Ville Skyttä 2008-12-01 13:59:11 UTC
Looks like there's still some work here to do, reopening:

What's the rationale for the empty main package?  Shouldn't the *.so be included in it instead of -devel?

The ldconfig calls are useless in the now empty main package.  I suppose they'd be useless even if the *.so would be in the main one because there's no soname in the *.so.

Specfile should be named libglfw.spec, not glfw.spec.

Empty debuginfo package and unstripped objects remain unfixed.  Fix for both would probably be a matter of installing the *.so with 755 permissions instead of 644.

Comment 12 Lubomir Rintel 2009-01-01 16:23:37 UTC
Ping

The .so file should really go to the main package and the static library should be eliminated, see http://fedoraproject.org/wiki/Packaging/Guidelines#Exclusion_of_Static_Libraries

Ville: By the way -- is it ok for a shared library to have no soname? Without it we can't depend on exact ABI version. Probably the right way to fix it to persuade upstream into using a SONAME themselves (they already consider package versions to be API versions, according to comments in source code). But anyway, if upstream did not do that, shouldn't we just make up a soname?

Comment 13 Ville Skyttä 2009-01-01 19:01:41 UTC
Persuading upstream to implement versioning would be best.  I don't think it's necessarily a good idea to invent versioning independently of upstream in the Fedora packages - there may be a good deal of confusion when/if upstream later implements versioning as well and ships a version using the same soname that was earlier used in Fedora for something that's not ABI compatible.

And additionally, if the shared lib is left unversioned at least for now, I don't think adding an unversioned soname would add any value either - AFAIK sonames are all about versioning.

Comment 14 Robert Scheck 2009-01-02 00:08:50 UTC
Degrading successful fedora-cvs flag as the review wasn't that successful;
please do a real and full formal review solving all the mentioned issues.

Comment 15 Simon 2009-01-07 23:52:56 UTC
ping?

Comment 16 Lubomir Rintel 2009-01-08 14:26:13 UTC
I went through the outstanding issues and corrected the package according to Guidelines, since the maintainer seemed irresponsive.

David, please pay more attention to reviews, ask your sponsor or #fedora-devel if you are unsure about something. Thanks!

Comment 17 Robert Scheck 2009-01-11 22:01:33 UTC
Lubomir, thanks for doing the work. I had a short look to it in CVS and it
now looks fine to me. Can you also please prepare a same well done update
for F-9 if not already done?


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