Bug 188178

Summary: Review Request: gauche-gl - OpenGL binding for Gauche
Product: [Fedora] Fedora Reporter: Gérard Milmeister <gemi>
Component: Package ReviewAssignee: Jason Tibbitts <j>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhide   
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-05-09 20:52:44 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 188168    
Bug Blocks: 163779    

Description Gérard Milmeister 2006-04-06 17:56:27 UTC
Spec Url: http://math.ifi.unizh.ch/fedora/spec/gauche-gl.spec
SRPM Url: http://math.ifi.unizh.ch/fedora/5/i386/SRPMS.gemi/gauche-gl-0.4.1-2.fc5.src.rpm info here>
Description:
OpenGL binding for Gauche.

Comment 2 Brian Pepple 2006-05-04 15:35:34 UTC
Couple of quick notes:

1. You have ownership problems with the %{_infodir}.
2. Bunch of unnecessary BuildRequires: libGL-devel (by freeglut-devel),
libGLU-devel (by freeglut-devel), libICE-devel (by libSM-devel), libX11-devel
(by libXext-devel)
3. Requires for install-info should probably follow the scriptlet example from
the wiki.
http://fedoraproject.org/wiki/ScriptletSnippets#head-117e9450bc166ceb4251bf8d87a9dd4e862442a4
4. Requires for gauche is unnecessry, since the devel package soname should pull
this in.
5. I'd drop the VERSION & README docs, since they don't provide any useful
information.

If I've got some time later today, I'll try to do a more thorough review.

Comment 3 Jason Tibbitts 2006-05-04 15:45:06 UTC
I was originally planning on doing both of these, but the problems with
gauche-gtk necessitating a gauche update and the devel repo being busted set me
back.  You're certainly welcome to take this if you like.

Comment 4 Jason Tibbitts 2006-05-07 00:05:25 UTC
On Brian's remarks:

1) I don't see problems with ownership of %{_infodir}; the package will own
/usr/share/info/gauche-gl-refe.info.gz but doesn't own the directory.

2) Yep, those could be trimmed although this isn't a blocker.

3) I think it's important that Requires(post): and Requires(postun): for
install-info be listed separately.

4) RPM won't pick up the versioned dependency.  I don't know about Gauche
internals but it's possible that the soname alone is not sufficient; there may
be scheme code dependencies as well.  Gérard would be the best one to decide on
that.

5) I can't argue about VERSION; I looked for other rather content-free README
files and found a couple quickly (axis and bug-buddy) so there seems to be some
precedent for including that kind of thing even when it doesn't say much.  It's
a coin toss.

My own issues:
You're missing BR: texinfo; without it, no info files are generated and the
build fails in %files.

rpmlint complains:
W: gauche-gl incoherent-version-in-changelog 0.4.1-1 0.4.1-3

Please add a changelog entry when you bump the revision.

W: gauche-gl wrong-file-end-of-line-encoding
/usr/share/doc/gauche-gl-0.4.1/examples/slbook/ogl2particle/particle.vert
W: gauche-gl wrong-file-end-of-line-encoding
/usr/share/doc/gauche-gl-0.4.1/examples/slbook/ogl2particle/particle.frag
W: gauche-gl wrong-file-end-of-line-encoding
/usr/share/doc/gauche-gl-0.4.1/examples/slbook/ogl2particle/3Dlabs-License.txt
W: gauche-gl wrong-file-end-of-line-encoding
/usr/share/doc/gauche-gl-0.4.1/examples/slbook/ogl2brick/README.txt
W: gauche-gl wrong-file-end-of-line-encoding
/usr/share/doc/gauche-gl-0.4.1/examples/slbook/ogl2brick/brick.frag
W: gauche-gl wrong-file-end-of-line-encoding
/usr/share/doc/gauche-gl-0.4.1/examples/slbook/ogl2brick/3Dlabs-License.txt
W: gauche-gl wrong-file-end-of-line-encoding
/usr/share/doc/gauche-gl-0.4.1/examples/slbook/ogl2particle/ogl2particle.scm
W: gauche-gl wrong-file-end-of-line-encoding
/usr/share/doc/gauche-gl-0.4.1/examples/slbook/ogl2particle/README.txt
W: gauche-gl wrong-file-end-of-line-encoding
/usr/share/doc/gauche-gl-0.4.1/examples/slbook/ogl2brick/brick.vert

I suggest you run these through sed to strip the carriage returns.

W: gauche-gl hidden-file-or-dir /usr/share/gauche/0.8.7/lib/.packages
W: gauche-gl hidden-file-or-dir /usr/share/gauche/0.8.7/lib/.packages

Same issue as with gauche-gtk; it's your decision on handling this.

W: gauche-gl devel-file-in-non-devel-package
/usr/lib64/gauche/0.8.7/include/gauche/math3d.h

Is this needed at runtime?  The guidelines indicate that this should be in a
-devel package, but it seems a waste for just one file.

W: gauche-gl doc-file-dependency
/usr/share/doc/gauche-gl-0.4.1/examples/glbook/run /usr/bin/env

Probably the same issue as with gauche-gtk.

Review:
* package meets naming and packaging guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* license field matches the actual license.
* license is open source-compatible; text is included in the package.
* source files match upstream:
   a51b19a0f16f88ed6cd85c6e49cc6e75  Gauche-gl-0.4.1.tgz
   a51b19a0f16f88ed6cd85c6e49cc6e75  Gauche-gl-0.4.1.tgz-srpm
* latest version is being packaged.
X BuildRequires are more than necessary (which is not a blocker) and missing
texinfo (which is a blocker)
* package builds in mock (FC5, x86_64) (after fixing RB: texinfo)
X rpmlint complains; see above.
X final provides are sane; final requires include extra /usr/bin/env and
install-info should be in Requires(post) and Requires(postun).
O shared libraries are present, but are internal to gauche.
* package is not relocatable.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
X file permissions: executable file in %doc.
* %clean is present.
O %check is not present; no test suite upstream.
* scriptlets are sane.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
X no headers.
* no pkgconfig files.
* no libtool .la droppings.
* not a GUI app.

Comment 5 Gérard Milmeister 2006-05-07 21:09:44 UTC
* Brian: how did you do to link to a place in the middle of the wiki page?
* In a way it is safer to specify more precisely the info file, because
  then the package fails when /usr/share/info/dir had been installed.
* The package is build specifially for a version of gauche, if there is
  a new version, the package has to be rebuilt.
* I decided to leave the directory ".packages". There are many other
  packages that have dotted directories, for example eclipse...
* The run files in the doc directories are not necessary, I removed them.
* Normally the header file should not be necessary. I removed it. Maybe
  there is a case when one wants to develop a plugin in C
  that uses native functionality from gauche-gl, but this seems a bit
  farfetched.

http://math.ifi.unizh.ch/fedora/5/i386/SRPMS.gemi/gauche-gl-0.4.1-4.fc5.src.rpm

Comment 6 Jason Tibbitts 2006-05-09 16:03:41 UTC
This new package builds fine in mock (development, x86_64) and the open issues
have been taken care of.

APPROVED

BTW, to link to the middle of a wiki page you have to find the cryptic anchor
name, usually from following a link from a table of contents.

Comment 7 Paul Howarth 2006-05-09 16:23:28 UTC
(In reply to comment #6)
 BTW, to link to the middle of a wiki page you have to find the cryptic anchor
> name, usually from following a link from a table of contents.

However, if you have write access to that wiki page, you could create a named
anchor using the wiki [[Anchor(anchorname)]] facility - see
http://fedoraproject.org/wiki/HelpOnLinking

Such a link would work more reliably, since the auto-generated anchor names may
vary if the page is edited.

Comment 8 Gérard Milmeister 2006-05-09 20:52:44 UTC
Built on FC4, FC5 and FC6.
Added to owners.list.

Thanks for the review.