Bug 188178 - Review Request: gauche-gl - OpenGL binding for Gauche
Review Request: gauche-gl - OpenGL binding for Gauche
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
:
Depends On: 188168
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-04-06 13:56 EDT by Gérard Milmeister
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: 2006-05-09 16:52:44 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Gérard Milmeister 2006-04-06 13:56:27 EDT
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 11:35:34 EDT
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 11:45:06 EDT
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-06 20:05:25 EDT
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 17:09:44 EDT
* 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 12:03:41 EDT
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 12:23:28 EDT
(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 16:52:44 EDT
Built on FC4, FC5 and FC6.
Added to owners.list.

Thanks for the review.

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