Bug 630778
Summary: | Review Request: llvm-py - Python bindings for LLVM | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Eric Smith <spacewar> |
Component: | Package Review | Assignee: | Michel Lind <michel> |
Status: | CLOSED NOTABUG | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora-package-review, michel, notting |
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: | 2011-10-27 02:33:49 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: |
Description
Eric Smith
2010-09-06 22:43:26 UTC
Just noticed the erroneous use of an apostrophe in "It's" in the description, which was cut-and pasted from the llvm-py web site. I'm assuming that I'll need to make some changes to the spec file as a result of package review comments, so I'll remove the apostrophe at that time. Swap with gedit-valencia ? https://bugzilla.redhat.com/show_bug.cgi?id=518892 It should be a straightforward review, but was stalled for months waiting for a new upstream release. - doc subpackage should probably be BuildArch: noarch. Changing it later is a bit painful so you might as well do it now - would it be less complicated to just use the default byte-compiler and then exclude the documentation files? It would involve less macro manipulation, and while it would unnecessarily compile some files, a beneficial side effect is that we make sure the example files are not completely broken. But I'm not too concerned either way. Complete review to follow. Koji F-14 build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2469802 Needs fixing: - -doc files should be in versioned directory. Instead of copying them manually you should probably just use relative paths when declaring them e.g. %doc test/*.py - -doc files should not be executable (see full review below) Suggestion for improvement: - add a %check section with the following: cd test PYTHONPATH=../build/lib.linux-*/ python testall.py * TODO Review [80%] ** DONE Names [2/2] *** DONE Package name *** DONE Spec name ** DONE Meets [[http://fedoraproject.org/wiki/Packaging/Guidelines][guidelines]] ** DONE source files match upstream $ md5sum llvm-py-0.6.tar.bz2 ~/rpmbuild/SOURCES/llvm-py-0.6.tar.bz2 eec62e4ce6f95f6e01edcba59747933d llvm-py-0.6.tar.bz2 eec62e4ce6f95f6e01edcba59747933d /home/michel/rpmbuild/SOURCES/llvm-py-0.6.tar.bz2 ** DONE License [3/3] *** DONE License is Fedora-approved *** DONE License field accurate *** DONE License included iff packaged by upstream ** TODO rpmlint [1/2] *** DONE on src.rpm llvm-py.src: W: spelling-error %description -l en_US backend -> backed, backbend, back end ==> ignore this llvm-py.src: W: invalid-url Source0: http://llvm-py.googlecode.com/files/llvm-py-0.6.tar.bz2 HTTP Error 404: Not Found ==> ignore -- spectool works fine, not sure what's wrong with rpmlint here 1 packages and 0 specfiles checked; 0 errors, 2 warnings. *** FAIL on x86_64.rpm llvm-py.x86_64: W: spelling-error %description -l en_US backend -> backed, backbend, back end ==> ignore this llvm-py.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/llvm/_core.so _core.so()(64bit) ==> ignore this llvm-py-doc.x86_64: W: spurious-executable-perm /usr/share/doc/llvm-py/uses.py ... llvm-py-doc.x86_64: W: spurious-executable-perm /usr/share/doc/llvm-py/operands.py ==> see below llvm-py-doc.x86_64: W: doc-file-dependency /usr/share/doc/llvm-py/uses.py /usr/bin/env ... llvm-py-doc.x86_64: W: doc-file-dependency /usr/share/doc/llvm-py/operands.py /usr/bin/env ==> can be ignored, they are example scripts after all 3 packages and 0 specfiles checked; 0 errors, 32 warnings. ** DONE Language & locale [3/3] *** DONE Spec in US English *** DONE Spec legible *** N/A Use %find_lang to handle locale files ** FAIL Build [2/3] *** DONE Koji results http://koji.fedoraproject.org/koji/taskinfo?taskID=2469802 *** DONE BRs complete *** FAIL Directory ownership /usr/share/doc/llvm-py not owned by any package ** TODO Spec inspection [6/8] *** FAIL ldconfig for libraries - State "FAIL" from "TODO" [2010-09-15 Wed 21:29] \\ ldconfig is called on %post and %postun even though there are no libraries *** DONE No duplicate files *** FAIL File permissions - State "FAIL" from "TODO" [2010-09-15 Wed 22:22] \\ -doc: files in %{_docdir} should not have executable permission *** DONE Filenames must be UTF-8 *** DONE Has %clean section (except F-13+: https://fedoraproject.org/wiki/Packaging/Guidelines#.25clean) Note: BuildRoot declaration can be removed on F-10+ and EL-6+ *** DONE %buildroot cleaned on %install *** DONE Macro usage consistent *** DONE Documentation [2/2] **** DONE If large docs, separate -doc note: should mark this as BuildArch: noarch **** DONE %doc files are non-essential ** N/A Desktop file validation ** DONE [[http://fedoraproject.org/wiki/Packaging/ScriptletSnippets][Scriptlets]] [4/4] *** N/A desktop-database (desktop entry has MimeType) *** N/A icon cache (icons in %{_datadir}/icons/) *** N/A info files *** N/A mimeinfo (file in %{_datadir}/mime/packages) I made the recommended changes, and updated to a Subversion snapshot from upstream to get support for LLVM 2.8. I had a bit of trouble with getting the environment variable set appropriately in your proposed python invocation for the %check section; maybe you know of a better solution than what I used. Spec URL: http://fedorapeople.org/~brouhaha/llvm-py/llvm-py.spec SRPM URL: http://fedorapeople.org/~brouhaha/llvm-py/llvm-py-0.7-0.1.20101105svn.fc14.src.rpm Koji scratch build for F14: http://koji.fedoraproject.org/koji/taskinfo?taskID=2697237 Thanks! Eric Closing this review request as upstream seems to be dead. |