Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 222569 - Review Request: pychess - Chess game for GNOME
Summary: Review Request: pychess - Chess game for GNOME
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Peter Gordon
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2007-01-14 18:20 UTC by Sindre Pedersen Bjørdal
Modified: 2007-11-30 22:11 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-01-14 22:43:50 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Sindre Pedersen Bjørdal 2007-01-14 18:20:54 UTC
Spec URL: http://folk.ntnu.no/sindrb/packages/pychess.spec
SRPM URL: http://folk.ntnu.no/sindrb/packages/pychess-0.6.0-0.1.beta5.src.rpm

Description: 
PyChess is a gtk chessgame for Linux. It is designed to a the same time be easy
to use, beautyful to look at and provide advanced functions for advanced 
players

rpmlint output that I'm not sure how to solve:

SRPM: 
W: pychess setup-not-quiet

RPM:
E: pychess non-executable-script /usr/lib/python2.4/site-packages/pychess/Utils/Move.py 0644
E: pychess non-executable-script /usr/lib/python2.4/site-packages/pychess/Players/PyChess.py 0644
E: pychess non-executable-script /usr/lib/python2.4/site-packages/pychess/Utils/History.py 0644

Comment 1 Peter Gordon 2007-01-14 18:34:55 UTC
(In reply to comment #0)
> rpmlint output that I'm not sure how to solve:
> 
> SRPM: 
> W: pychess setup-not-quiet

This one means that you're not passing "-q" to the %setup call in your %prep
section. This parameter tells rpmbuild not to output the list of files it is
extracting and whatnot.
 
> RPM:
> E: pychess non-executable-script
/usr/lib/python2.4/site-packages/pychess/Utils/Move.py 0644
> E: pychess non-executable-script
/usr/lib/python2.4/site-packages/pychess/Players/PyChess.py 0644
> E: pychess non-executable-script
/usr/lib/python2.4/site-packages/pychess/Utils/History.py 0644

These three mean that the scripts are not executable. A simple `chmod +x` at the
end of your %build section on each of these should resolve those qualms.

(Full review to come later today.)

Comment 2 Mamoru TASAKA 2007-01-14 18:55:02 UTC
Just one comment

(In reply to comment #0)
> E: pychess non-executable-script
/usr/lib/python2.4/site-packages/pychess/Utils/Move.py 0644
> E: pychess non-executable-script
/usr/lib/python2.4/site-packages/pychess/Players/PyChess.py 0644
> E: pychess non-executable-script
/usr/lib/python2.4/site-packages/pychess/Utils/History.py 0644

(In reply to comment #1)
> These three mean that the scripts are not executable. A simple `chmod +x` at the
> end of your %build section on each of these should resolve those qualms.

Please check if
* these files should have executable flags (i.e. really should executable)
* or shebangs on these files should be removed (i.e. 0644 is correct)
The scripts only called by other applications/scripts and not
directly called by user should not have shebangs.

Comment 3 Sindre Pedersen Bjørdal 2007-01-14 19:20:21 UTC
Fixed the issues mentioned, rpmlint is now silent.

Updated: 

Spec URL: http://folk.ntnu.no/sindrb/packages/pychess.spec
SRPM URL: http://folk.ntnu.no/sindrb/packages/pychess-0.6.0-0.2.beta5.src.rpm

Comment 4 Mamoru TASAKA 2007-01-14 19:25:43 UTC
Well, adding executable permission means that
these scripts are sometimes called directly by user?

Comment 5 Sindre Pedersen Bjørdal 2007-01-14 20:18:03 UTC
I don't know. Wouldn't think so.

Comment 6 Peter Gordon 2007-01-14 21:37:34 UTC
Thanks for the catch, Mamoru. However, this is behavior similar to my packaging
of Scribes; and it was APPROVED with such changes. Seeing as how such
permissions cause no apparent security issues or bugs with the software, I don't
consider this a blocker. 


== Formal Review of pychess 0.6.0-0.2.beta5 == 

GOOD: rpmlint is silent on both the source and binary (noarch) RPMs.
GOOD: The package follows the naming guidelines, and the spec file is named
accordingly ("%{name}.spec"). 
GOOD: BuildRoot is "%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u}
-n)", following the packaging guidelines. 
GOOD: No duplicate BuildRequires are included; and all necessary BuildRequires
are listed.
GOOD: Included documentation (%doc) is OK.
GOOD: Package builds and runs against system copies of installed tools and
libraries; and does not include its own local copies thereof.
GOOD: Package includes an appropriate .desktop file since it is a graphical
application; and desktop-file-install is properly used to install it. A
BuildRequires: desktop-file-utils is also present.
GOOD: Macros are used instead of harcoded file names, and usage of these macros
(including $RPM_BUILD_ROOT) is consistent throughout the spec file.
GOOD: Locale files are handled correctly, using %find_lang appropriately.
GOOD: Package is not relocatable.
GOOD: Package includes appropriate code and content; and final directory and
file ownership is OK.
GOOD: Package does not own any system files/directories or any files/directories
that conflict with another package.   
GOOD: Package license (GPL) is OK; and a copy of it is included in the package
as documentation (%doc LICENSE). The License field in the spec file properly
reflects this.
GOOD: Spec file is nicely legible.
GOOD: The source tarball matches that of upstream:
  $ md5sum SOURCES/pychess-0.6.0-beta5*
  ed2cdca72465c4b529a1caf6960745be  SOURCES/pychess-0.6.0_beta5-srpm.tar.gz
  ed2cdca72465c4b529a1caf6960745be  SOURCES/pychess-0.6.0_beta5-upstream.tar.gz
GOOD: The package successfully builds in mock into noarch binary RPMs on both
FC6 and devel/FC7. 
GOOD: No duplicates are listed in the %files section; and its %defattr line is good.
GOOD: Package has an appropriate %clean section, which contains simply "rm -rf
$RPM_BUILD_ROOT"
GOOD: When installed, the application runs well with no apparent segfaults or
major bugs.

N/A: No non-ASCII characters are needed, so encoding is OK.
N/A: This is a noarch package, so compiler flags are not needed. 
N/A: No static libraries or rPath exclusions are needed.
N/A: Package includes no configuration files or data, so %config markings are
not needed.
N/A: Package uses Python distutils for building; so using `make %{?_smp_flags}`
is not needed.
N/A: Package is not a web application.
N/A: Package is noarch; thus no ExcludeArch/ExclusiveArch tweaking is required.
N/A: Package installs no shared libraries; thus %post/%postun scriplets of
/sbin/ldconfig are not needed.
N/A: No large (neither in size nor in quantity) documentation is included, thus
no -doc subpackage is needed.
N/A: No headers, no pkgconfig files, and no static or unsuffixed shared
libraries are included. Thus, no -devel subpackage is needed.
N/A: Package contains no libtool archives (*.la files) 
N/A: Package contains no %description or Summary translations.
N/A: Scriplets are not required for this package.

I see only one thing that needs fixing with this package and that is that the
%description contains what appear to be a lot of simple typos. The following
would be more appropriate:
"PyChess is a GTK+ chess game for Linux. It is designed to at the same time
be easy to use, beautiful to look at, and provide advanced functions for
advanced players."

However, that is something you can fix after importing it into CVS. Therefore
(under the condition that this is done), this package is APPROVED. Remember to
also close this bug as NEXTRELEASE when it is imported and pushed through the
build system. 


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