Bug 428559 - Review Request: mypaint - A simple and easy paint program
Summary: Review Request: mypaint - A simple and easy paint program
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Parag AN(पराग)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-01-13 12:05 UTC by Marc Wiriadisastra
Modified: 2008-01-21 22:26 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2008-01-21 22:26:41 UTC
Type: ---
Embargoed:
panemade: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Marc Wiriadisastra 2008-01-13 12:05:29 UTC
Spec URL: http://mwiriadi.fedorapeople.org/packages/mypaint/mypaint.spec
SRPM URL: http://mwiriadi.fedorapeople.org/packages/mypaint/mypaint-0.5.0-1.fc8.src.rpm
Description: Mypaint is a fast and easy/simple painter app focused on the painter, 
so you can only focus on the art and not the program itself. 
Currently MyPaint does not have a layer system, also mypaint is 
using pygtk with C extensions.

Comment 1 Parag AN(पराग) 2008-01-14 04:03:34 UTC
1)missing BR: pygtk2-devel and remove pygtk2 from BR.
2)drop static package. Any reason you packaged it?
3) rpmlint output is
mypaint.i386: W: file-not-utf8
/usr/share/doc/mypaint-0.5.0/html/prev-template.xcf.gz
The character encoding of this file is not UTF-8.  Consider converting it
in the specfile for example using iconv(1).
==> I think ok to accepet here.

mypaint.i386: W: spurious-executable-perm
/usr/share/doc/mypaint-0.5.0/html/generate.py
The file is installed with executable permissions, but was identified as one
that probably should not be executable.  Verify if the executable bits are
desired, and remove if not.
==> Can you drop executable permissions here?


Comment 2 Marc Wiriadisastra 2008-01-14 09:41:50 UTC
I have left prev-template.xcf.gz since it is gzip'ed. Here is the updated package.

http://mwiriadi.fedorapeople.org/packages/mypaint/mypaint-0.5.0-2.fc8.src.rpm
http://mwiriadi.fedorapeople.org/packages/mypaint/mypaint.spec

Comment 3 Parag AN(पराग) 2008-01-14 12:05:41 UTC
1)drop .la using
find $RPM_BUILD_ROOT -name '*.la' -exec rm -f {} ';'
in %install

2) I think I have not seen such case which installs libraries in python packages.
/usr/lib/python2.5/site-packages/mypaint
/usr/lib/python2.5/site-packages/mypaint/mydrawwidget.so
/usr/lib/python2.5/site-packages/mypaint/mydrawwidget.so.0
/usr/lib/python2.5/site-packages/mypaint/mydrawwidget.so.0.0.0

But I can see that .so is generated using .c code so as per review guideline
MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1),
then library files that end in .so (without suffix) must go in a -devel package.


Comment 4 Marc Wiriadisastra 2008-01-14 12:17:21 UTC
http://mwiriadi.fedorapeople.org/packages/mypaint/mypaint-0.5.0-3.fc8.src.rpm
http://mwiriadi.fedorapeople.org/packages/mypaint/mypaint.spec

I saw that but I thought for one file it was pointless but oh well.

Comment 5 Parag AN(पराग) 2008-01-14 12:35:59 UTC
mypaint-devel.i386: W: no-dependency-on mypaint
mypaint-devel.i386: W: summary-not-capitalized mypaint devel files
Summary doesn't begin with a capital letter.

I am still not sure about these install locations
/usr/lib/python2.5/site-packages/mypaint/mydrawwidget.so.0
/usr/lib/python2.5/site-packages/mypaint/mydrawwidget.so.0.0.0

Comment 6 Parag AN(पराग) 2008-01-18 11:11:03 UTC
Ok. we can do some simple things here.
lets drop -devel package and add following lines to %install
=====================================================================
#following is needed for mydrawwidget.so module so that it can be readily 
#accessible to other people trying to import it.
touch $RPM_BUILD_ROOT%{_datadir}/mypaint/python/__init__.py

##Remove unnecessary links for .so.0.0.0 file
rm -f  $RPM_BUILD_ROOT%{python_sitearch}/%{name}/mydrawwidget.so
rm -f  $RPM_BUILD_ROOT%{python_sitearch}/%{name}/mydrawwidget.so.0
mv $RPM_BUILD_ROOT%{python_sitearch}/%{name}/mydrawwidget.so.0.0.0
$RPM_BUILD_ROOT%{python_sitearch}/%{name}/mydrawwidget.so
==========================================================================


Kindly check any rpmlint output on new package then.

Comment 7 Marc Wiriadisastra 2008-01-19 14:38:26 UTC
http://mwiriadi.fedorapeople.org/packages/mypaint/mypaint.spec
http://mwiriadi.fedorapeople.org/packages/mypaint/mypaint-0.5.0-5.fc8.src.rpm

Added the sections.

I had a close look at the xcf file and while it's not utf-8 compliant it's a
binary picture I believe so I'm not to sure how to fix it.

Comment 8 Parag AN(पराग) 2008-01-21 04:50:54 UTC
you missed to change sitelib to sitearch. Use following patch and increase
release and submit new SRPM for final review.


--- mypaint.spec        2008-01-21 10:13:45.000000000 +0530
+++ mypaint-new.spec    2008-01-21 09:28:08.000000000 +0530
@@ -1,5 +1,5 @@
 %{!?python_sitearch: %define python_sitearch %(%{__python} -c "from
distutils.sysconfig import get_python_lib; print get_python_lib(1)")}
-%{!?python_sitelib: %define python_sitelib %(%{__python} -c "from
distutils.sysconfig import get_python_lib; print get_python_lib()")}
+
 Name:           mypaint
 Version:        0.5.0
 Release:        5%{?dist}
@@ -57,8 +57,8 @@
 %{_datadir}/%{name}/
 %{_datadir}/pixmaps/%{name}*
 %{_datadir}/applications/fedora-%{name}.desktop
-%dir %{python_sitelib}/%{name}/
-%{python_sitelib}/%{name}/mydrawwidget.so
+%dir %{python_sitearch}/%{name}/
+%{python_sitearch}/%{name}/mydrawwidget.so
 
 
 %changelog


Comment 10 Parag AN(पराग) 2008-01-21 09:45:51 UTC
Review:
+ package builds in mock (rawhide i386).
+ rpmlint is silent for SRPM.
- rpmlint is NOT silent for RPM.
mypaint.i386: W: file-not-utf8
/usr/share/doc/mypaint-0.5.0/html/prev-template.xcf.gz
==> Ok to accept.
+ source files match upstream.
9547f8ed4f0e5e6382d4295a47889c45  mypaint-0.5.0.tar.bz2
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.
+ License text COPYING is included in package.
+ %doc is small so no need of -doc subpackage.
+ BuildRequires are proper.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code, not content.
+ no static libraries.
+ no .pc files are present.
+ no -devel subpackage exists.
+ no .la files.
+ no translations are available.
+ Does owns the directories it creates.
+ no duplicates in %files.
+ file permissions are appropriate.
+ Desktop file installed correctly.
+ scriptlets are used.
+ GUI app.
APPROVED.


Comment 11 Marc Wiriadisastra 2008-01-21 09:53:05 UTC
New Package CVS Request
=======================
Package Name: mypaint
Short Description: A simple and easy paint program
Owners: mwiriadi
Branches: F-8 devel
InitialCC: mwiriadi
Cvsextras Commits: yes


Comment 12 Kevin Fenzi 2008-01-21 17:02:33 UTC
cvs done.


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