Bug 557948

Summary: Review Request: PyAIML - A Python AIML Interpreter
Product: [Fedora] Fedora Reporter: Sebastian Dziallas <sebastian>
Component: Package ReviewAssignee: Susi Lehtola <susi.lehtola>
Status: CLOSED WONTFIX QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, notting, pbrobinson, spacewar, susi.lehtola
Target Milestone: ---Flags: susi.lehtola: fedora-review?
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-02-10 22:43:19 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:    
Bug Blocks: 182235, 462625, 558617    

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.