*** Bug 455152 has been marked as a duplicate of this bug. ***

Just a quick comment, i haven't looked at the package closely, but the wrong-file-end-of-line-encoding can probably be fixed by either dos2unix or (preferably) using:
sed -i 's/\r//' <file_to_strip> 

See also https://fedoraproject.org/wiki/PackageMaintainers/Common_Rpmlint_Issues#wrong-file-end-of-line-encoding

Thanks Stefan.

$ rpmlint vpython.spec vpython-5.13-1.fc12.src.rpm noarch/vpython-doc-5.13-1.fc12.noarch.rpm x86_64/vpython-*
vpython.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 3, tab: line 13)
vpython.src: W: mixed-use-of-spaces-and-tabs (spaces: line 3, tab: line 13)
vpython.x86_64: E: devel-dependency vpython-devel
vpython-devel.x86_64: W: no-documentation
vpython-devel.x86_64: W: dangling-relative-symlink /usr/lib64/python2.6/site-packages/cvisualmodule.so cvisualmodule.so.3.0.0
5 packages and 1 specfiles checked; 1 errors, 4 warnings.


Ignorable issues from my side:
- tabs-vs-spaces
- dangling-relative-symlink

Remaining issues:
- devel-file-depency: Should I delete the devel package and ship the *.so file
  with the main package?
  Nothing is mentioned in Guidelines:Python about this.

Spec URL: http://tomspur.fedorapeople.org/review/vpython.spec
SRPM URL: http://tomspur.fedorapeople.org/review/vpython-5.13-2.fc12.src.rpm

With boost in the F12-testing repository, the boost patch is not needed anymore.

Furthermore, I removed the devel package and ship the *.so file in the main package, because without this program won't work anyway and allways needs to be 'Require'd.

________________________________


Rpmlint is now completely clean:
$ rpmlint vpython.spec vpython-5.13-3.fc12.src.rpm noarch/* x86_64/*
4 packages and 1 specfiles checked; 0 errors, 0 warnings.

________________________________


Spec URL: http://tomspur.fedorapeople.org/review/vpython.spec
SRPM URL: http://tomspur.fedorapeople.org/review/vpython-5.13-3.fc12.src.rpm

I will review your package.

%changelog
* Thu Mar 11 2010 Thomas Spura <tomspur> - 5.22-1
- new version
- move python_sitelib/* to python_sitearch in %%install
  (if not, patch always needs to regenerated)

Spec URL: http://tomspur.fedorapeople.org/review/vpython.spec
SRPM URL: http://tomspur.fedorapeople.org/review/vpython-5.22-1.fc13.src.rpm

(In reply to comment #5)
> With boost in the F12-testing repository, the boost patch is not needed
> anymore.
> 
> Furthermore, I removed the devel package and ship the *.so file in the main
> package, because without this program won't work anyway and allways needs to be
> 'Require'd.
> 
> ________________________________
> 
> 
> Rpmlint is now completely clean:
> $ rpmlint vpython.spec vpython-5.13-3.fc12.src.rpm noarch/* x86_64/*
> 4 packages and 1 specfiles checked; 0 errors, 0 warnings.
> 
> ________________________________
> 
> 
> Spec URL: http://tomspur.fedorapeople.org/review/vpython.spec
> SRPM URL: http://tomspur.fedorapeople.org/review/vpython-5.13-3.fc12.src.rpm    

Maybe I did the wrong thing, but I took your vpython-5.13-3.fc12.src.rpm and did this:
 rpmbuild --rebuild vpython-5.13-3.fc12.src.rpm
 then install the resulting vpython-5.13-3.fc12.i686.rpm to my system

Then open a console and enter a python session:
 Python 2.6.2 (r262:71600, Jan 25 2010, 18:46:45) 
 [GCC 4.4.2 20091222 (Red Hat 4.4.2-20)] on linux2
 Type "help", "copyright", "credits" or "license" for more information.
 >>> import visual
 >>> from visual import *
 >>> sphere()
 Speicherzugriffsfehler (Speicherabzug geschrieben)

Speicherzugriffsfehler = SEGFAULT

So I would be very happy to get this working... What to do now?

Greetings

$ python
Python 2.6.4 (r264:75706, Apr  1 2010, 02:55:51) 
[GCC 4.4.3 20100226 (Red Hat 4.4.3-8)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import visual
>>> from visual import *
>>> sphere()
<visual.primitives.sphere object at 0x1b52838>
>>> 

I'm running on F-13 and here it seems to work as expected and I also didn't note an issue on F-12, yet...

I *guess* (because I don't know abrt internals), that abrt doesn't catch this segfault, because it's happening inside of python.

Could you try to put the lines from above into a script and try to get a abrt crash?

It that doesn't work another possibility would be:
https://fedoraproject.org/wiki/QA:Testcase_ABRT_python

If you overwrite another existing file (like the smolt one) and execute that, abrt should catch it in any way.

Hopefully that helps.

##################################################

Thanks for pining, I just noted, that upstream is a newer version ;)

Spec URL: http://tomspur.fedorapeople.org/review/vpython.spec
SRPM URL: http://tomspur.fedorapeople.org/review/vpython-5.31-1.fc13.src.rpm   

I tied to do a koji scratch build for F-12:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2099992

Unfortunately it fails, so it's likely, that you can't build this...
I can't figure it out, why it doesn't built on F-12 atm. Here on F-13 anything is fine.
But you could try it anyway and have a look to src/build.log, which error occures...

> I *guess* (because I don't know abrt internals), that abrt doesn't catch this
> segfault, because it's happening inside of python.
> 
> Could you try to put the lines from above into a script and try to get a abrt
> crash?

I don't understand what you are exactly meaning but abrt caught those crashes everytime, but since I was neither sure if this is a relevant crash ;) nor if I should *really* post such a long backtrace, I trashed them. But as you asked for it I reproduced it and reported the crash with abrt to bug 580984, I hope this is what you like to get.

> ##################################################
> 
> Thanks for pining, I just noted, that upstream is a newer version ;)
> 

Had no time to test this until now - sorry! But if it is important I will do it for you - no problem!

...now what next?

Thanks for your answer and greetings

Created attachment 405696 [details]
fc13 compile test (on fc12)

As wished here the results of compilation test of the fc13 package, sorry for the German error messages! It does not compile, e.g. because user 'tom' is missing...

Created attachment 405697 [details]
build.log

Created attachment 405698 [details]
config.log

(In reply to comment #10)
> > I *guess* (because I don't know abrt internals), that abrt doesn't catch this
> > segfault, because it's happening inside of python.
> > 
> > Could you try to put the lines from above into a script and try to get a abrt
> > crash?
> 
> I don't understand what you are exactly meaning but abrt caught those crashes
> everytime, but since I was neither sure if this is a relevant crash ;) nor if I
> should *really* post such a long backtrace, I trashed them. But as you asked
> for it I reproduced it and reported the crash with abrt to bug 580984, I hope
> this is what you like to get.

abrt didn't get the backtrace everytime, when something is crashing for me in e.g. ipython, so I just guessed, that abrt didn't get the crash. If it worked even better...

The first one wasn't relevant, because the debuginfo were missing, but the second one is quite usefull. Hopefully the new version will work for you and we'll not need it ;)

> 
> > ##################################################
> > 
> > Thanks for pining, I just noted, that upstream is a newer version ;)
> > 
> 
> Had no time to test this until now - sorry! But if it is important I will do it
> for you - no problem!
> 
> ...now what next?
> 
> Thanks for your answer and greetings    

It will be better to test with the new version. I try to get the packages for F-12, so you don't need to build them on your own.


(In reply to comment #11)
> Created an attachment (id=405696) [details]
> fc13 compile test (on fc12)
> 
> As wished here the results of compilation test of the fc13 package, sorry for
> the German error messages! It does not compile, e.g. because user 'tom' is
> missing...    

Thanks for the logs.
I am German - so no problem at all ;)
(If you run anything with 'LC_ALL=C $and_the_command' prefixed, it will be in English - just a side note.)

That's a really strange issue. I thought, there would be a missing header or a compile error...

I'll try to get a src.rpm package without 'tom:tom' in it.

Another possibility would be:
Just download the spec file and run 'spectool -g python-visual.spec'. That will download the sourcefile from upstream and you can simply run 'rpmbuild -ba python-visual.spec' and will have the RPM in ~/rpmbuild/RPMS.
I found out, what's the reason for not building on F-12 (at least on my machine):
bug 185079 comment 4
mock is complaining:
../include/python/num_util.hpp:73:31: warning: numpy/arrayobject.h: No such file or directory

Could you try to install the F-13 version? Usually there won't be a problem with it. When this package is reviewed, I'll ping the maintainer of numpy, to ship an update for F-12, so this can be build on F-12 too.

(Don't know why your build didn't showed the missing numpy/arrayobject.h lines...)

########################

There is a new version from upstream. But, because on ubuntu there is python-visual and you later do a 'import visual' I choosed now to rename the package to python-visual. (see: http://vpython.org/contents/download_linux.html)
This later also matches to python3-visual, once there will be python3 support. I did a 'Obsoletes: vpython' for now because of this...

$ rpmlint noarch/python-visual-doc-5.32-1.fc13.noarch.rpm x86_64/python-visual-* python-visual-5.32-1.fc13.src.rpm 
4 packages and 0 specfiles checked; 0 errors, 0 warnings.

Spec URL: http://tomspur.fedorapeople.org/review/python-visual.spec
SRPM URL: http://tomspur.fedorapeople.org/review/python-visual-5.32-1.fc13.src.rpm

RPMS http://tomspur.fedorapeople.org/review/rpms/ :

http://tomspur.fedorapeople.org/review/rpms/python-visual-5.32-1.fc13.x86_64.rpm
http://tomspur.fedorapeople.org/review/rpms/python-visual-debuginfo-5.32-1.fc13.x86_64.rpm

Could you please try to do as root:
'rpm -U http://tomspur.fedorapeople.org/review/rpms/python-visual-5.32-1.fc13.x86_64.rpm http://tomspur.fedorapeople.org/review/rpms/python-visual-debuginfo-5.32-1.fc13.x86_64.rpm'

(In reply to comment #14)

Sorry for the delay, I was stunned by your post :) and so it took me a while to digest... ;)

> It will be better to test with the new version. I try to get the packages for
> F-12, so you don't need to build them on your own.

What is the state here - I want to badger you, just curious...

> Thanks for the logs.
> I am German - so no problem at all ;)

Good to now, just in case... ;)

> (If you run anything with 'LC_ALL=C $and_the_command' prefixed, it will be in
> English - just a side note.)

Thanks for the hint! Was useful, as you will see next.

> Another possibility would be:
> Just download the spec file and run 'spectool -g python-visual.spec'. That will
> download the sourcefile from upstream and you can simply run 'rpmbuild -ba
> python-visual.spec' and will have the RPM in ~/rpmbuild/RPMS.
> I found out, what's the reason for not building on F-12 (at least on my
> machine):
> bug 185079 comment 4
> mock is complaining:
> ../include/python/num_util.hpp:73:31: warning: numpy/arrayobject.h: No such
> file or directory

I tried this, for me good to see was; no tom issue, but an error while trying to generate the rpm, output is attached (I think we had this error already).

> Could you try to install the F-13 version? Usually there won't be a problem
> with it. When this package is reviewed, I'll ping the maintainer of numpy, to
> ship an update for F-12, so this can be build on F-12 too.

I tried but was not able to install the F-13 version, package dependencies, a lot... (shall I go the hard way and install them all one by one manually?!? please say no...)

The numpy update would be cool, when will it be available?

> There is a new version from upstream. But, because on ubuntu there is
> python-visual and you later do a 'import visual' I choosed now to rename the
> package to python-visual. (see:
> http://vpython.org/contents/download_linux.html)
> This later also matches to python3-visual, once there will be python3 support.
> I did a 'Obsoletes: vpython' for now because of this...

Yes, I recognized this already and think this a good idea! More consistent (logic) and user friendly. And why give it a new name, when there is already one...
Whan does the 'obsolete' do?

> Could you please try to do as root:
> 'rpm -U
> http://tomspur.fedorapeople.org/review/rpms/python-visual-5.32-1.fc13.x86_64.rpm
> http://tomspur.fedorapeople.org/review/rpms/python-visual-debuginfo-5.32-1.fc13.x86_64.rpm'    

I don't think this will work, since I'm running an i386 system... What to do?

Created attachment 406060 [details]
output of: "rpmbuild -ba python-visual.spec"

(In reply to comment #15)
> I don't think this will work, since I'm running an i386 system... What to 

Sorry not "i386" but "i686"...

(In reply to comment #15)
> (In reply to comment #14)
> > It will be better to test with the new version. I try to get the packages for
> > F-12, so you don't need to build them on your own.
> 
> What is the state here - I want to badger you, just curious...

Currently I can't build that, because of the numpy issue (don't know why you don't seem to hit this issue). So I'll wait for the numpy update now.

 
> > Could you try to install the F-13 version? Usually there won't be a problem
> > with it. When this package is reviewed, I'll ping the maintainer of numpy, to
> > ship an update for F-12, so this can be build on F-12 too.
> 
> I tried but was not able to install the F-13 version, package dependencies, a
> lot... (shall I go the hard way and install them all one by one manually?!?
> please say no...)

Uhrm.. 'No' ;)
Better try to build it on your own (hopefully now the latest time).

> 
> The numpy update would be cool, when will it be available?

I just pinged the maintainer of numpy, could be done in a few secs or in a week ;)
But because it seems to build for you, I don't think this is an issue.
Most probably the moving of the files (see below).
 
> > I did a 'Obsoletes: vpython' for now because of this...
> 
> Whan does the 'obsolete' do?

It uninstalls your vpython package, when you install this one. I'll delete this line again, when this is reviewed (or sometime else).

> > Could you please try to do as root:
> > 'rpm -U
> > http://tomspur.fedorapeople.org/review/rpms/python-visual-5.32-1.fc13.x86_64.rpm
> > http://tomspur.fedorapeople.org/review/rpms/python-visual-debuginfo-5.32-1.fc13.x86_64.rpm'    
> 
> I don't think this will work, since I'm running an i386 system... What to do?    


Thanks for pointing out...
It seems to be my fault ;)

I needed to move some files around, because on 64bit systems, the files should be in /usr/lib64* and not in /usr/lib/*. I never needed to restrict some commands to a arch before, so I *guess* it works now ;)

Could you try again to build it?
(Just update the spec file and rpmbuild again)

Spec URL: http://tomspur.fedorapeople.org/review/python-visual.spec
SRPM URL:
http://tomspur.fedorapeople.org/review/python-visual-5.32-2.fc13.src.rpm

(In reply to comment #18)
> > What is the state here - I want to badger you, just curious...

(to clearify I meant: "I DO NOT want to badger you..." ;)

> Currently I can't build that, because of the numpy issue (don't know why you
> don't seem to hit this issue). So I'll wait for the numpy update now.

Me too - sorry, that I am no help here, but I'm lost with that much of C code,
if all would be python, I could may be give some sugestions...

> Uhrm.. 'No' ;)

Thanks! :))

> I just pinged the maintainer of numpy, could be done in a few secs or in a week
> ;)
> But because it seems to build for you, I don't think this is an issue.
> Most probably the moving of the files (see below).

The old storry... Ok, so in this case I will ignore that.

> Could you try again to build it?
> (Just update the spec file and rpmbuild again)
> 
> Spec URL: http://tomspur.fedorapeople.org/review/python-visual.spec
> SRPM URL:
> http://tomspur.fedorapeople.org/review/python-visual-5.32-2.fc13.src.rpm    

So I did the rpmbuild from spec file as you mentioned; GOOD WORK! It builds now completely and works also to install, as the f12 package before, so this issue is solved (at least for me)! Thank you for this.

But there is still the SEGFAULT issue. Here I have (may be) a hint for you; since we have another machine here running ubuntu and there is, as you mentioned, already a 'python-visual' package and I have already tried this out with the same SEGFAULT issue. Now a strange thing, I got a hint from a workmate of mine to install 'libgtkglextmm-x11-1.2-dev' since there is a packaging error - and a miracle happened and the SEGFAULT vanished... (it did not help me since I have already installed the similar f12 package, I had to because this is needed to build the rpm)

By the way I wanted also to thank you for your informative explanations!

(In reply to comment #19)
> But there is still the SEGFAULT issue. Here I have (may be) a hint for you;
> since we have another machine here running ubuntu and there is, as you
> mentioned, already a 'python-visual' package and I have already tried this out
> with the same SEGFAULT issue. Now a strange thing, I got a hint from a workmate
> of mine to install 'libgtkglextmm-x11-1.2-dev' since there is a packaging error
> - and a miracle happened and the SEGFAULT vanished... (it did not help me since
> I have already installed the similar f12 package, I had to because this is
> needed to build the rpm)
> 
> By the way I wanted also to thank you for your informative explanations!    

You're welcome :)

It's pretty strange, that you can reproduce this issue also on another machine!!
I never saw a SEGFAULT here...

I just pinged upstream about this issue, but they have some problems to see, what happens in the backtrace. So it could take a while, till someone has a solution for that :(

At least the problem is now 'known' and hopefully someone from the upstream mailinglist is able to find the bug.

Sorry, being of no help here either ;-)

Spec URL: http://tomspur.fedorapeople.org/review/python-visual.spec
SRPM URL:
http://tomspur.fedorapeople.org/review/python-visual-5.32-3.fc13.src.rpm   

Changes:
- move examples subfolder to the doc package

I also unmarked the segfault in bug #580984 a blocker of this review.
It would indeed be great to get that bug fixed before importing, but unfortunately this is unlikely to happen 'soon'.

Spec URL: http://tomspur.fedorapeople.org/review/python-visual.spec
SRPM URL:
http://tomspur.fedorapeople.org/review/python-visual-5.32-4.fc13.src.rpm  

Changes:
- doc subpackage needs to be non-noarch

Mohamed: Do you still plan a review here?

I'm still following this challenging review ^^.

Maybe you should not use the %exclude tag in %files, files tagges with it are included into the calculated size whereas they are not installed.

About the arch-ed doc package, I'm quite disappointed. If the examples, as supposed by their name, are not required to run visual, they should be considered to be documentation also, and set as %doc too.

(In reply to comment #23)
> I'm still following this challenging review ^^.

Good to read :)
 
> Maybe you should not use the %exclude tag in %files, files tagges with it are
> included into the calculated size whereas they are not installed.
> 
> About the arch-ed doc package, I'm quite disappointed. If the examples, as
> supposed by their name, are not required to run visual, they should be
> considered to be documentation also, and set as %doc too.    


The reason, why I used %exclude was that, this is the easiest way to include the examples into the doc package, without to differ from upstream.

Now I simply delete the examples and docs completely and include them as %doc in the doc package.

This way it's slightly different from upstream, but the doc package is noarch again and %exclude is gone now:

Spec URL: http://tomspur.fedorapeople.org/review/python-visual.spec
SRPM URL:
http://tomspur.fedorapeople.org/review/python-visual-5.32-5.fc13.src.rpm

Hope that is better this way... :)

Better indeed :-)
Technically only one things is blocking its approval:

* The latest version of rpmlint has some surprises:
$ rpmlint -i python-visual-5.32-5.fc13.x86_64.rpm 
python-visual.x86_64: W: private-shared-object-provides /usr/lib64/python2.6/site-packages/cvisualmodule.so.3.0.0 cvisualmodule.so.3()(64bit)
A shared object soname provides is provided by a file in a path from which
other packages should not directly load shared objects from.  Such shared
objects should thus not be depended on and they should not result in provides
in the containing package.  Get rid of the provides if appropriate, for
example by filtering it out during build.  Note that in some cases this may
require disabling rpmbuild's internal dependency generator.

python-visual.x86_64: W: private-shared-object-provides /usr/lib64/python2.6/site-packages/cvisualmodule.so.3.0.0 cvisualmodule.so.3(CVISUAL_0_0)(64bit)
A shared object soname provides is provided by a file in a path from which
other packages should not directly load shared objects from.  Such shared
objects should thus not be depended on and they should not result in provides
in the containing package.  Get rid of the provides if appropriate, for
example by filtering it out during build.  Note that in some cases this may
require disabling rpmbuild's internal dependency generator.


By the way, the build is quite silent, I had to check a generated log file to check flags and such. I don(t know if it can be considered to be an issue, but its quite annoying for debugging.

(In reply to comment #25)
> Better indeed :-)
> Technically only one things is blocking its approval:

Great :)
 
> * The latest version of rpmlint has some surprises:
> $ rpmlint -i python-visual-5.32-5.fc13.x86_64.rpm 
> python-visual.x86_64: W: private-shared-object-provides
> /usr/lib64/python2.6/site-packages/cvisualmodule.so.3.0.0
> cvisualmodule.so.3()(64bit)

[snip]

I found a solution for this, but it's a bit hacky ;)

%define _use_internal_dependency_generator 0
%define %__find_provides    %{_rpmconfigdir}/find-provides | grep -v cvisualmodule
%define %__find_requires    %{_rpmconfigdir}/find-requires | grep -v cvisualmodule

Tell me, if you have a better (easier) version for this. I don't think, it will be possible in another way...

> By the way, the build is quite silent, I had to check a generated log file to
> check flags and such. I don(t know if it can be considered to be an issue, but
> its quite annoying for debugging.    

Yes, it's anoying... Unfortunately, there is not 'the one patch', I could apply to make the buildsystem more verbose. That would change all the time, and that's too much effort atm.

Once this is approved, I plan to work with upstream to maybe get back to scons and get away from autotools and the like.
Hopefully that will be better some time.
(On my TODO list for now, but that won't change 'soon', sorry for being annoying ;-).)


Spec URL: http://tomspur.fedorapeople.org/review/python-visual.spec
SRPM URL:
http://tomspur.fedorapeople.org/review/python-visual-5.32-6.fc13.src.rpm

(In reply to comment #26)
> > By the way, the build is quite silent, I had to check a generated log file to
> > check flags and such. I don(t know if it can be considered to be an issue, but
> > its quite annoying for debugging.    
> 
> Yes, it's anoying... Unfortunately, there is not 'the one patch', I could apply
> to make the buildsystem more verbose. That would change all the time, and
> that's too much effort atm.
The patch is indeed not a solution. Anyway, what about this, in %setup ?
   sed -i 's/2\?>> \?\$(LOGFILE).*//' src/Makefile.in
It will skip all redirections to the log file in src/Makefile.in

> I found a solution for this, but it's a bit hacky ;)
> 
> %define _use_internal_dependency_generator 0
> %define %__find_provides    %{_rpmconfigdir}/find-provides | grep -v
> cvisualmodule
> %define %__find_requires    %{_rpmconfigdir}/find-requires | grep -v
> cvisualmodule
Maybe you mean:
   %global _use_internal_dependency_generator 0
   %global __find_provides    %{_rpmconfigdir}/find-provides | grep -v cvisualmodule
   %global __find_requires    %{_rpmconfigdir}/find-requires | grep -v cvisualmodule
(otherwise I get "error: Macro % has illegal name (%define)", and by the way %global is preferred over %define ;))
I rebuilt your package with these lines, rpmlint is silent now.

(In reply to comment #27)
> (In reply to comment #26)
> > > By the way, the build is quite silent, I had to check a generated log file to
> > > check flags and such. I don(t know if it can be considered to be an issue, but
> > > its quite annoying for debugging.    
> > 
> > Yes, it's anoying... Unfortunately, there is not 'the one patch', I could apply
> > to make the buildsystem more verbose. That would change all the time, and
> > that's too much effort atm.
> The patch is indeed not a solution. Anyway, what about this, in %setup ?
>    sed -i 's/2\?>> \?\$(LOGFILE).*//' src/Makefile.in
> It will skip all redirections to the log file in src/Makefile.in

Thanks! I'm not used to such complicated sed commands, mostly just s/foo/bar/g and that's it...

> 
> > I found a solution for this, but it's a bit hacky ;)
> > 
> > %define _use_internal_dependency_generator 0
> > %define %__find_provides    %{_rpmconfigdir}/find-provides | grep -v
> > cvisualmodule
> > %define %__find_requires    %{_rpmconfigdir}/find-requires | grep -v
> > cvisualmodule
> Maybe you mean:
>    %global _use_internal_dependency_generator 0
>    %global __find_provides    %{_rpmconfigdir}/find-provides | grep -v
> cvisualmodule
>    %global __find_requires    %{_rpmconfigdir}/find-requires | grep -v
> cvisualmodule
> (otherwise I get "error: Macro % has illegal name (%define)", and by the way
> %global is preferred over %define ;))
> I rebuilt your package with these lines, rpmlint is silent now.    

Changed to your version.


Spec URL: http://tomspur.fedorapeople.org/review/python-visual.spec
SRPM URL:
http://tomspur.fedorapeople.org/review/python-visual-5.32-7.fc13.src.rpm

Just a few words about the license: it seems some files are under the MIT license too. Maybe you could add it to the "License" field.

Moreover, in the license.txt file, the part relative to the Polygon module is quite worrying (typically nonfree)... But I didn't find this module in the sources, I suppose it was removed from previous versions.

Changed to 'Boost and MIT'.

Furthermore, I asked upstream to clarify, where it is to find or if it was already deleted.
(Maybe you can read that somewhere at [1], but I couldn't find it right now. Maybe generating the non-email archives takes some time)

[1] http://sourceforge.net/mailarchive/forum.php?forum_name=visualpython-users&max_rows=25&style=nested&viewmonth=201006

Spec URL: http://tomspur.fedorapeople.org/review/python-visual.spec

(In reply to comment #29)
> Just a few words about the license: it seems some files are under the MIT
> license too. Maybe you could add it to the "License" field.
> 
> Moreover, in the license.txt file, the part relative to the Polygon module is
> quite worrying (typically nonfree)... But I didn't find this module in the
> sources, I suppose it was removed from previous versions.

Answer from upstream:
"""
I guess the confusion is that in the case of the Windows and Mac installers,
the modules are included in the VPython installers. On the Linux download
page at vpython.org you will see "In support of the 3D text object, you will
need to install the font-handling modules FontTools, ttfquery (version 1.0.4
or later), and Polygon (all available from pypi.python.org), for which the
following conditions apply:" followed by an extract from the Visual
license.

Also, in INSTALL.txt included in the Visual tarball you will see "For Visual
5.3 and later, you need the Python modules FontTools, ttfquery, and
Polygon. Be sure to get ttfquery 1.0.4 or later." This is followed by the
specific information about Polygon.

If you don't install these components on Linux the new text object won't
work.
"""

So the license mentioned in that file applies to the polygon module, which is not (yet) in fedora, and probably won't be here too, because of the nonfree impression.

So I think Boost and MIT is ok here, what do you think?

Changes:
- follow Packaging:AutoProvidesAndRequiresFiltering for hiding the cvisual.so module

SPEC: http://tomspur.fedorapeople.org/review/python-visual.spec
SRPM: http://tomspur.fedorapeople.org/review/python-visual-5.32-9.fc13.src.rpm

Ok for the licensing. At last the review:

MUST: rpmlint must be run on every package.
->OK, no warning for all the RPMs

MUST: The package must be named according to the Package Naming Guidelines.
->OK

MUST: The spec file name must match the base package %{name}, in the format %{name}.spec unless your package has an exemption.
->OK

MUST: The package must meet the Packaging Guidelines.
->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.
->OK

MUST: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %doc.
->OK

MUST: The spec file must be written in American English.
->OK

MUST: The spec file for the package MUST be legible.
->OK

MUST: The sources used to build the package must match the upstream source, as provided in the spec URL.
-> OK, md5sum = 866f51f0c49b60086dbe3581f508d30b

MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture.
->OK, tested on x86_64 with mock

MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch.
->N/A

MUST: All build dependencies must be listed in BuildRequires, except for any that are listed in the exceptions section of the Packaging Guidelines ; inclusion of those as BuildRequires is optional.
->OK

MUST: The spec file MUST handle locales properly. This is done by using the %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.
->N/A

MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun.
->N/A

MUST: Packages must NOT bundle copies of system libraries.
->N/A

MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package. Without this, use of Prefix: /usr is considered a blocker.
->N/A

MUST: A package must own all directories that it creates. If it does not create a directory that it uses, then it should require a package which does create that directory.
->OK

MUST: A Fedora package must not list a file more than once in the spec file's %files listings. 
->OK

MUST: Permissions on files must be set properly. Executables should be set with executable permissions, for example. Every %files section must include a %defattr(...) line.
->OK

MUST: Each package must consistently use macros.
->OK

MUST: The package must contain code, or permissable content.
->OK

MUST: Large documentation files must go in a -doc subpackage.
->OK

MUST: If a package includes something as %doc, it must not affect the runtime of the application. To summarize: If it is in %doc, the program must run properly if it is not present.
->OK

MUST: Header files must be in a -devel package.
->N/A

MUST: Static libraries must be in a -static package.
->N/A

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.
->N/A

MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release}.
->N/A

MUST: Packages must NOT contain any .la libtool archives, these must be removed in the spec if they are built.
->OK

MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section.
->N/A

MUST: Packages must not own files or directories already owned by other packages.
->OK

MUST: All filenames in rpm packages must be valid UTF-8.
->OK

This package is _at last_ APPROVED!

(In reply to comment #33)
> This package is _at last_ APPROVED!

Thanks for the review!

___________________________________________

New Package SCM Request
=======================
Package Name: python-visual
Short Description: 3D Programming
Owners: tomspur
Branches: f13 f14
InitialCC:

Git done (by process-git-requests).

Build in rawhide:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2554024

Will do the updates to other branches, when python3-numpy is ready, so I can build a python3-visual package too.

python-visual-5.32-10.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/python-visual-5.32-10.fc14

python-visual-5.32-10.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/python-visual-5.32-10.fc13

python-visual-5.32-10.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

python-visual-5.32-10.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.