Bug 557948 - Review Request: PyAIML - A Python AIML Interpreter
Summary: Review Request: PyAIML - A Python AIML Interpreter
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Susi Lehtola
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-Legal FedoraOLPCDelta SOAS-3
TreeView+ depends on / blocked
 
Reported: 2010-01-22 21:15 UTC by Sebastian Dziallas
Modified: 2010-02-10 22:43 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-02-10 22:43:19 UTC
Type: ---
Embargoed:
susi.lehtola: fedora-review?


Attachments (Terms of Use)

Description Sebastian Dziallas 2010-01-22 21:15:12 UTC
Spec URL: http://sdz.fedorapeople.org/rpmbuild/PyAIML.spec
SRPM URL: http://sdz.fedorapeople.org/rpmbuild/PyAIML-0.8.5-1.fc12.src.rpm
Koji Scratch Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1939424

Description: This package is a requirement for the latest version of the Sugar Speak activity.

Comment 1 Eric Smith 2010-01-24 02:41:04 UTC
I'm not yet sponsored as a packager, but here's an unofficial review:

rpmlint output:
1 packages and 1 specfiles checked; 0 errors, 0 warnings.

source tarball verified against upstream
compiles and builds on F12 x86_64 and i386 using mock
verified that upstream is public domain, so no license text file needed

When I try to verify the functionality of the package using the example in the README.txt, and get to the point of doing 'k.respond("load aiml b")', I get 'WARNING: No match found for input: load aiml b'.  I don't really know anything about AIML so I'm not certain what is going wrong, but it doesn't match the expected behaviour.

I'm not sure about this one, but none of the examples in the Packaging:Python  guidelines include the whole sitelib directory, so rather than having the files section contain %{python_sitelib}/*, perhaps it might be better to have:
%{python_sitelib}/%{name}-%{version}-*.egg-info
%{python_sitelib}/aiml/

Comment 2 Susi Lehtola 2010-01-24 10:04:32 UTC
Assigning.

Comment 3 Susi Lehtola 2010-01-24 10:21:12 UTC
I'm going to sponsor Eric, here's a verbose review:


rpmlint output is clean.


MUST: The package does not yet exist in Fedora. The Review Request is not a duplicate. OK
 - Be sure to check for older reviews on the review queue, first (or that if the package already is in Fedora).

MUST: The spec file for the package is legible and macros are used consistently. OK
 - Spec files are legible, no mixing of styles ($RPM_BUILD_ROOT vs %{buildroot}), macro usage is sane.

MUST: The package must be named according to the Package Naming Guidelines. OK
- Naming guidelines state that Python libraries must be named python-foo, unless the name is pyfoo or Pyfoo.

MUST: The spec file name must match the base package %{name}. OK
MUST: The package must be licensed with a Fedora approved license and meet the  Licensing Guidelines. OK

MUST: The License field in the package spec file must match the actual license. NEEDSWORK
- Nothing in the tarball actually specifies a license or defines the code to be in the public domain.
- All I can find is
 License: UNKNOWN
in PKG-INFO.
- Upstream needs to add a license statement in the tarball before this package can be approved.

MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. NEEDSWORK
- Source URL is malformed:
 $ spectool -g PyAIML.spec 
 (clip)
 2010-01-24 12:08:58 (117 KB/s) - “./index.html” saved [65180/65180]
- Source URL does not adhere to source URL guidelines
 http://fedoraproject.org/wiki/Packaging/SourceURL
- Correct Source URL is
 http://downloads.sourceforge.net/pyaiml/PyAIML%20%28unstable%29/PyAIML-%{version}.tar.bz2

MUST: The package MUST successfully compile and build into binary rpms. OK
MUST: The spec file MUST handle locales properly. N/A
MUST: Optflags are used and time stamps preserved. OK
MUST: Packages containing shared library files must call ldconfig. N/A

MUST: A package must own all directories that it creates or require the package that owns the directory. OK
- As Eric stated in comment #1, be more verbose in %files since you might miss if the egg is not built for some reason or another.

MUST: Files only listed once in %files listings. OK
MUST: Debuginfo package is complete. OK
MUST: Permissions on files must be set properly. OK
MUST: Clean section exists. OK
MUST: Large documentation files must go in a -doc subpackage. N/A

MUST: All relevant items are included in %doc. Items in %doc do not affect runtime of application. ~OK
- You could add TODO.txt to %doc.

MUST: Header files must be in a -devel package. N/A
MUST: Static libraries must be in a -static package. N/A
MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. N/A
MUST: If a package contains library files with a suffix then library files ending in .so must go in a -devel package. N/A
MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency. N/A
MUST: Packages does not contain any .la libtool archives. N/A
MUST: Desktop files are installed properly. N/A
MUST: No file conflicts with other packages and no general names. OK
MUST: Buildroot cleaned before install. OK
SHOULD: %{?dist} tag is used in release. OK

SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. NEEDSWORK
- No license file is included, please ask upstream to include one.

SHOULD: The package builds in mock. OK

****

The source URL and egg stuff are minor issues. The license issue is bigger: there is no license specified, so we can't include this in Fedora - blocking FE-LEGAL.

If upstream e.g. adds in README.TXT (or in the comment of the source code files) a statement

"This code is in the public domain."

everything would be OK. However, the latest (unstable!) release has been in 2004, so I don't have high hopes of this happening...

Comment 4 Sebastian Dziallas 2010-01-24 16:59:27 UTC
D'oh! You're right, indeed. I've pinged the upstream author to see if we've any chances there. Thanks for the review already.

Comment 5 Sebastian Dziallas 2010-02-10 22:43:19 UTC
Argh! Upstream is not responsive, so I guess I'll have to close this for now. If he replies, I'll reopen here.


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