Bug 482216

Description is wrong both in spec and request will updated.

Description:

The Mayavi project includes two related packages for 3-dimensional
visualization:

    * Mayavi2: A tool for easy and interactive visualization of data.

    * TVTK: A Traits-based wrapper for the Visualization Toolkit, a
      popular open-source visualization library.

These libraries operate at different levels of abstraction. TVTK
manipulates visualization objects, while Mayavi2 lets you operate on
your data, and then see the results. Most users either use the Mayavi
user interface or program to its scripting interface; you probably
don't need to interact with TVTK unless you want to create a new
Mayavi module.


SPEC: http://rakesh.fedorapeople.org/spec/python-Mayavi.spec
SRPM: http://rakesh.fedorapeople.org/srpm/python-Mayavi-3.1.0-2.fc10.src.rpm

Created attachment 331546 [details]
Just a test, please ignore.

Created attachment 333684 [details]
Just a test please ignore

This one needs to be redone .. as it is a standalone application and its name cannot start with python-

Will redo.

All dependencies are imported and will be in updates-testing in some time .. till then I will repackage it as Mayavi

http://rakesh.fedorapeople.org/misc/Mayavi.spec
http://rakesh.fedorapeople.org/misc/Mayavi-3.2.0-1.fc11.src.rpm

Updated.

http://rakesh.fedorapeople.org/spec/Mayavi.spec
http://rakesh.fedorapeople.org/srpm/Mayavi-3.2.0-2.fc11.src.rpm

Updated.

Please use all dependencies from latest builds in koji (as I have just pushed them).

These packages are:
python-EnvisagePlugins-3.1.0
python-Traits-3.1.0
python-AppTools-3.2.0
python-Traits-3.1.0
python-EnthoughtBase-3.0.2
python-TraitsBackendQt-3.1.0
python-EnvisageCore-3.1.0
python-TraitsGUI-3.0.4

for all releases.

Here are my initial notes. I still have to check the licensing stuff, the doc files and a few other things:

* Please remove %{version} from URL. Isn't this the actual website?
   http://code.enthought.com/projects/mayavi/

* Source0 gives 404. Also, could you use %{name} and %{version} in Source0 and wherever else they can be used?

! Patches should be explained and be submitted to upstream and upstream tracker links should be given (if available) as comments

? Is the group tag correct?

* The BR's python-devel, python-setupdocs, python-Traits don't seem necessary. The package builds the same way without them.

* rpmlint says:
   Mayavi.x86_64: W: hidden-file-or-dir /usr/lib64/python2.6/site-packages/enthought/mayavi/html/.buildinfo
   Mayavi.x86_64: W: hidden-file-or-dir /usr/lib64/python2.6/site-packages/enthought/tvtk/html/.buildinfo
Can we get rid of these files?

? What is this line for?
   sed -i 's/\.dev$//g' $RPM_BUILD_ROOT/%{python_sitearch}/%{name}-%{version}-*.egg-info/requires.txt

? What is this file that gets packaged?
   /usr/lib64/python2.6/site-packages/Mayavi-3.2.0-py2.6-nspkg.pth

! I don't think we need this anymore:
   # unstripped-binary-or-object
   #chmod +x $RPM_BUILD_ROOT/%{python_sitearch}/enthought/tvtk/array_ext.so
If you want to keep it, please escape the macro with an extra %

- koji rawhide and F-10 seem fine
   http://koji.fedoraproject.org/koji/taskinfo?taskID=1408825
   http://koji.fedoraproject.org/koji/taskinfo?taskID=1408830

? There is this release dependent BR:
   %if 0%{?fedora} >= 11
   BuildRequires:  numpy-f2py
   %else
   BuildRequires:  numpy
   %endif
Also there is this release independent R:
   Requires:       numpy
Is there an inconsistency here?

! It seems like the description is made to span 70 columns. Can you make it span 80 columns instead (as far as possible)?

* Package requires 
   python-Traits >= 3.1.0
   python-TraitsBackendQt >= 3.1.0
Can you add these versioned dependencies?

* Packages containing GUI applications must include a %{name}.desktop file (which applies to both the mayavi2 and tvtk_doc executables), and that file must be properly installed with desktop-file-install in the %install section. Follow:
   http://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files
   http://fedoraproject.org/wiki/Packaging/ScriptletSnippets

* Packages must not own files or directories already owned by other packages. This package owns
   /usr/lib64/python2.6/site-packages/enthought/
which is also owned by python-Traits. This needs attention.

* tvtk_doc exits via segmentation fault.

Here is the rest of the review:

* As indicated by LICENSE_COLORBREWER.txt, parts of the file enthought/mayavi/core/lut/pylab_luts.py is distributed under ASL 1.1 and it should be added to the license tag.

* image_LICENSE.txt says
   "Crystal: LGPL license as described in icon_LICENSE_CP.txt"
but icon_LICENSE_CP.txt contains the full text of GPLv2. Can you ask this upstream for clarification? Even if they say it is LGPL, we cannot ship this icon_LICENSE_CP.txt file as it is wrong.

* image_LICENSE_OOo is LGPLv3+ and this has to be added to the license tag.

* docs/mayavi2.man should be packaged. Is this a man1?

? Is the documentation under docs/source/ worth packaging?

(In reply to comment #10)
> Here are my initial notes. I still have to check the licensing stuff, the doc
> files and a few other things:
> 
> * Please remove %{version} from URL. Isn't this the actual website?
>    http://code.enthought.com/projects/mayavi/
> 
> * Source0 gives 404. Also, could you use %{name} and %{version} in Source0 and
> wherever else they can be used?
> 
> ! Patches should be explained and be submitted to upstream and upstream tracker
> links should be given (if available) as comments
> 
> ? Is the group tag correct?
> 
> * The BR's python-devel, python-setupdocs, python-Traits don't seem necessary.
> The package builds the same way without them.
> 

python-setupdocs is required (build fails without it):
            distutils.errors.DistutilsError: Could not find suitable distribution for Requirement.parse('setupdocs>=1.0')

Other two are not required. Removed them.


> * rpmlint says:
>    Mayavi.x86_64: W: hidden-file-or-dir
> /usr/lib64/python2.6/site-packages/enthought/mayavi/html/.buildinfo
>    Mayavi.x86_64: W: hidden-file-or-dir
> /usr/lib64/python2.6/site-packages/enthought/tvtk/html/.buildinfo
> Can we get rid of these files?
> 

Done.

> ? What is this line for?
>    sed -i 's/\.dev$//g'
> $RPM_BUILD_ROOT/%{python_sitearch}/%{name}-%{version}-*.egg-info/requires.txt
> 

It removes .dev for versions in requires.txt, it is a known upstream problem .. and is being taken care in svn.

> ? What is this file that gets packaged?
>    /usr/lib64/python2.6/site-packages/Mayavi-3.2.0-py2.6-nspkg.pth
> 

Python script to locate enthought modules. Required, did not checked details.

> ! I don't think we need this anymore:
>    # unstripped-binary-or-object
>    #chmod +x $RPM_BUILD_ROOT/%{python_sitearch}/enthought/tvtk/array_ext.so
> If you want to keep it, please escape the macro with an extra %
> 

removed.

> - koji rawhide and F-10 seem fine
>    http://koji.fedoraproject.org/koji/taskinfo?taskID=1408825
>    http://koji.fedoraproject.org/koji/taskinfo?taskID=1408830
> 
> ? There is this release dependent BR:
>    %if 0%{?fedora} >= 11
>    BuildRequires:  numpy-f2py
>    %else
>    BuildRequires:  numpy
>    %endif
> Also there is this release independent R:
>    Requires:       numpy
> Is there an inconsistency here?
>

No, actually in F-11 distutils sub package was moved to numpy-f2py but numpy still has it for other stable releases. It is a BR. But mayavi still uses numpy for functioning (array calculations), which is a R.
 
> ! It seems like the description is made to span 70 columns. Can you make it
> span 80 columns instead (as far as possible)?
>

I tried doing it .. but it messes it up, my emacs adjusted it better so I have left it that way.
 
> * Package requires 
>    python-Traits >= 3.1.0
>    python-TraitsBackendQt >= 3.1.0
> Can you add these versioned dependencies?
>

fixed.
 
> * Packages containing GUI applications must include a %{name}.desktop file
> (which applies to both the mayavi2 and tvtk_doc executables), and that file
> must be properly installed with desktop-file-install in the %install section.
> Follow:
>    http://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files
>    http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
> 
> * Packages must not own files or directories already owned by other packages.
> This package owns
>    /usr/lib64/python2.6/site-packages/enthought/
> which is also owned by python-Traits. This needs attention.
>

Fixed.

> * tvtk_doc exits via segmentation fault.  

It works on my F11, tested on F10 also, may you paste the error message or any crash dump ? I was not able to reproduce.

Will update once issues in other comments are also fixed.

Alright thanks. This is the gdb backtrace when I exit via tvtk_doc OK or Cancel

  Program received signal SIGSEGV, Segmentation fault.
  0x000000320101362d in fini_context_translations () at setrans_client.c:243
  243             free(prev_r2t_trans);

It is selinux related. I don't know what is wrong. I made a fresh F-11 install last week.

I will try this on an F-10 box tomorrow.

(In reply to comment #10)
> ? What is this file that gets packaged?
>    /usr/lib64/python2.6/site-packages/Mayavi-3.2.0-py2.6-nspkg.pth
>

Removed.

(In reply to comment #11)
> Here is the rest of the review:
> 
> * As indicated by LICENSE_COLORBREWER.txt, parts of the file
> enthought/mayavi/core/lut/pylab_luts.py is distributed under ASL 1.1 and it
> should be added to the license tag.
>

Done.

 
> * image_LICENSE.txt says
>    "Crystal: LGPL license as described in icon_LICENSE_CP.txt"
> but icon_LICENSE_CP.txt contains the full text of GPLv2. Can you ask this
> upstream for clarification? Even if they say it is LGPL, we cannot ship this
> icon_LICENSE_CP.txt file as it is wrong.
>

Will remove after asking from upstream after clearing doubts.

> * image_LICENSE_OOo is LGPLv3+ and this has to be added to the license tag.
>

I think LGPLv2+ (as it has or later) covers LGPLv3+ also. Or am I wrong ?
 
> * docs/mayavi2.man should be packaged. Is this a man1?

Yes included.

> 
> ? Is the documentation under docs/source/ worth packaging?  

Yes included

Will updated, once I get confirmation from upstream about license.

Thanks,

(In reply to comment #13)
> Alright thanks. This is the gdb backtrace when I exit via tvtk_doc OK or Cancel
> 
>   Program received signal SIGSEGV, Segmentation fault.
>   0x000000320101362d in fini_context_translations () at setrans_client.c:243
>   243             free(prev_r2t_trans);
> 
> It is selinux related. I don't know what is wrong. I made a fresh F-11 install
> last week.
> 
> I will try this on an F-10 box tomorrow.  

Will try to reproduce and report upstream. Somehow I was not able to reproduce again. :(

Checking ...

> 
> > * image_LICENSE.txt says
> >    "Crystal: LGPL license as described in icon_LICENSE_CP.txt"
> > but icon_LICENSE_CP.txt contains the full text of GPLv2. Can you ask this
> > upstream for clarification? Even if they say it is LGPL, we cannot ship this
> > icon_LICENSE_CP.txt file as it is wrong.
> >
> 
> Will remove after asking from upstream after clearing doubts.
> 

The only issue here was that all files named icon_*.txt in file are actually image_*.txt files. But image_LICENSE_CP.txt is not GPL, it is LGPL ? May you recheck and confirm ?

Wrong naming, I have fixed. So, to me only issue remaining is crash .. I will am checking whether I can reproduce it. Once that is done will update.

Thanks,

http://rakesh.fedorapeople.org/srpm/Mayavi-3.2.0-3.fc11.src.rpm
http://rakesh.fedorapeople.org/spec/Mayavi.spec

Fixed All .. not able to reproduce crash :( May you check whether there is any other issues remaining ?

I tested this on F-10 and got the same segmentation fault. But the crash is on exit and doesn't affect the application during runtime.

Meanwhile, as we discussed, the missing things in the specfile are the .desktop files, the Group tag and the license thing ("... and LGPLv3+").

I was confused by the image_LICENSE_CP.txt file. Yes it is LGPL, so we don't have a problem there. What confused me was the license title inside the file is 
   "GNU General Public License"
whereas the text below is LGPL. It would be good to let upstream know about this confusion.

http://rakesh.fedorapeople.org/spec/Mayavi.spec
http://rakesh.fedorapeople.org/srpm/Mayavi-3.2.0-3.fc11.src.rpm

Updated.

Thanks for the update but there are a few problems:

* tvtk_doc.desktop and Mayavi.desktop are identical. I don't think this was intentional.

* Could you please add GenericName's to these .desktop files? (KDE uses GenericName, Gnome uses Comment)

* You used the desktop-database scriptlets from 
   http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#desktop-database
But those are for mime-types. What you need is to use the icon-cache scriptlets:
   http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Icon_Cache
(Don't forget to remove the Requires(post), Requires(postun) you added.)

http://rakesh.fedorapeople.org/spec/Mayavi.spec
http://rakesh.fedorapeople.org/srpm/Mayavi-3.2.0-4.fc11.src.rpm

Updated.

I have fixed Comment in tvtk_doc and added GenericName to bot,
fixed scriplets

Rakesh, I'm sorry I noticed a few things that I should have noticed before.

* The directory %{python_sitearch}/enthought/ is not owned!

* No icons specified in the .desktop files. This is not a blocker but it would be really nice to have an icon in the desktop menu for Mayavi. Also, without icons those scriptlets we used have little meaning.

I think we can use 
   ./docs/source/mayavi/images/mayavi2-48x48.png
from the source tree. Put it in
   %{_datadir}/icons/hicolor/48x48/apps/mayavi2.png
and add
   Icon=mayavi2
to both .desktop files (unless you want to use a different icon for tvtk)

* As for the Categories tag in the .desktop files, it would be good to add some more specific category names, so that the application ends up at a better place in the desktop menu. "Gnome" is too general for this kind of an application, also I have no idea how other DE's will handle it. I would suggest using

Science,DataVisualization

You may find other suitable categories at
   http://standards.freedesktop.org/menu-spec/latest/apa.html

Again, I'm sorry, I should have told you these things earlier.

1. python-Traits already owns %{python_sitearch}/enthought/

2. Using png file for both desktop files.

3. Added.

Updated:

http://rakesh.fedorapeople.org/spec/Mayavi.spec
http://rakesh.fedorapeople.org/srpm/Mayavi-3.2.0-5.fc11.src.rpm

* Package didn't build in koji
   http://koji.fedoraproject.org/koji/taskinfo?taskID=1436607

that's because we are missing a BR: desktop-file-utils


* Also please add 
   Icon=mayavi2
and
   Icon=tvtk_doc
to the .desktop files to associate the icons with desktop menu entries.


* rpmlint now says
Mayavi.x86_64: W: hidden-file-or-dir /usr/share/doc/Mayavi-3.2.0/source/mayavi/.static
Mayavi.x86_64: W: hidden-file-or-dir /usr/share/doc/Mayavi-3.2.0/source/mayavi/.templates

I don't know about the importance of these. I will leave them to your decision.


These are all trivial fixes. Please do them before committing.

-----------------------------------------
This package (Mayavi) is APPROVED by oget
-----------------------------------------

Thanks .. fixed all

http://rakesh.fedorapeople.org/spec/Mayavi.spec
http://rakesh.fedorapeople.org/srpm/Mayavi-3.2.0-6.fc11.src.rpm

New Package CVS Request
=======================
Package Name: Mayavi
Short Description: The Mayavi scientific data 3-dimensional visualizer
Owners: oget
Branches: F-10 F-11
InitialCC:

CVS done, except that I think you intended to make yourself the owner, not Orcan, so that's what I set up.  Otherwise, Orcan approved his own package, which is not appropriate.

Mayavi-3.2.0-6.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/Mayavi-3.2.0-6.fc10

Mayavi-3.2.0-6.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/Mayavi-3.2.0-6.fc11

Mayavi-3.2.0-6.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

Mayavi-3.2.0-6.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Package Change Request
======================
Package Name: Mayavi
New Branches: el6
Owners: orion
InitialCC:

Git done (by process-git-requests).