Bug 188178
Summary: | Review Request: gauche-gl - OpenGL binding for Gauche | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Gérard Milmeister <gemi> |
Component: | Package Review | Assignee: | 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
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. 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. 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. * 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 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. (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. Built on FC4, FC5 and FC6. Added to owners.list. Thanks for the review. |