Bug 428559 - Review Request: mypaint - A simple and easy paint program
Review Request: mypaint - A simple and easy paint program
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-01-13 07:05 EST by Marc Wiriadisastra
Modified: 2008-01-21 17:26 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-01-21 17:26:41 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
panemade: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Marc Wiriadisastra 2008-01-13 07:05:29 EST
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-13 23:03:34 EST
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 04:41:50 EST
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 07:05:41 EST
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 07:17:21 EST
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 07:35:59 EST
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 06:11:03 EST
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 09:38:26 EST
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-20 23:50:54 EST
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 04:45:51 EST
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 04:53:05 EST
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 12:02:33 EST
cvs done.

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