Bug 439100 - Review Request: octaviz - 3D visualization system for Octave
Review Request: octaviz - 3D visualization system for Octave
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Ed Hill
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-03-26 19:33 EDT by Claudio Tomasoni
Modified: 2010-01-04 05:58 EST (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-01-04 05:58:33 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
ed: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)
add BRs for mock and some path cleanups for x86_64 (2.13 KB, text/x-patch)
2008-04-06 18:40 EDT, Ed Hill
no flags Details

  None (edit)
Description Claudio Tomasoni 2008-03-26 19:33:27 EDT
Spec URL: http://www.claudiotomasoni.it/files/RPMS/octaviz.spec
SRPM URL: http://www.claudiotomasoni.it/files/RPMS/octaviz-0.4.7-1.fc8.src.rpm
Description: Octaviz is a visualization system for Octave. It is a wrapper that makes all  VTK classes accessible from within  Octave using  the same object-oriented syntax as in C++  or  Python. Octaviz also provides high-level functions for 2D and 3D  visualization.  Using those  functions, most common visualization tasks (3D surface plots, contour plots etc) can be accomplished without any knowledge about VTK.

The spec file creates the main package and a subpackage which contains examples of usage of octaviz classes.

rpmlint output is clean. I'm not sponsored, so, in case, I would need a sponsor.
Comment 1 Claudio Tomasoni 2008-03-29 12:27:17 EDT
New revision of the spec file. Many examples in the subpackage octaviz-examples
complain if vtkdata is not installed, so vtkdata as been added as requirement
for this subpackage only.

New Spec URL: http://www.claudiotomasoni.it/files/RPMS/octaviz.spec
New SRPM URL: http://www.claudiotomasoni.it/files/RPMS/octaviz-0.4.7-2.fc8.src.rpm

Still looking for a reviewer...
Comment 2 Ed Hill 2008-04-06 18:40:13 EDT
Created attachment 301443 [details]
add BRs for mock and some path cleanups for x86_64
Comment 3 Ed Hill 2008-04-06 19:00:47 EDT
Hi Claudio, I haven't sponsored anyone in a while so I'll have to re-read 
the guidelines.  In the mean time, I've attached a patch that fixes some 
problems for the octaviz build in mock for F8-x86_64.  Please put together 
a new revision and confirm.   Also, we need to verify that octaviz builds 
in mock for devel (F9) and can do that next.

Before sponsoring you I'd like to ask that you please do a "pre-review" 
for at least one new package submission as described at:

  http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

So could you please pick one from the "new" list at the bottom of:

  http://fedoraproject.org/wiki/PackageReviewProcess

and point me towards your comments?
Comment 4 Ed Hill 2008-04-06 20:53:33 EDT
With the patch in comment #2 the mock F8-x86_64 build fails with the error 
messages:

  Checking for unpackaged file(s): /usr/lib/rpm/check-files
    /var/tmp/octaviz-0.4.7-3.fc8-root-mockbuild
  error: Installed (but unpackaged) file(s) found:
  
/usr/libexec/octave/3.0.0/site/oct/x86_64-redhat-linux-gnu/octaviz/vtk3DSImporter.oct
  
/usr/libexec/octave/3.0.0/site/oct/x86_64-redhat-linux-gnu/octaviz/vtk3DWidget.oct


so it looks like the specfile needs to be edited so that:

  %{_libexecdir}/octave/3.0.0/site/oct/i386-redhat-linux-gnu/octaviz

becomes:

  %{_libexecdir}/octave/3.0.0/site/oct/%{_arch}-redhat-linux-gnu/octaviz
Comment 5 Claudio Tomasoni 2008-04-10 15:44:22 EDT
Hi Ed, I'm very sorry for the delay (I've been very busy these days).
Here is a new release with your patches applied and some other fixes (paths
cleanup).

New SPEC URL: http://www.claudiotomasoni.it/files/RPMS/octaviz.spec
New SRPM URL: http://www.claudiotomasoni.it/files/RPMS/octaviz-0.4.7-3.fc8.src.rpm

Could you try it, please? (it works at home for both i686 and x86_64).

I will be very happy to do a "pre-review", but I won't be able to do it until
next monday. I will point you to my comments as soon as possible.
Thanks indeed.
Comment 6 Ed Hill 2008-04-10 16:24:26 EDT
Hi Claudio, that's cool -- and please don't feel that there is a big hurry!  
We're both volunteers and we work on these Fedora things as our free time 
allows.  I'll do a more thorough octaviz review probably over the weekend.  
Hope you have a good weekend!  :-)
Comment 7 Ed Hill 2008-04-13 09:22:58 EDT
Hi Claudio, here's a more thorough review:

good:
 + rpmlint is silent
 + license is correctly included
 + specfile is legible and macros appear to be sane
 + source matches upstream sha1sum:
     b9bd87453b30696cbc175158c4a74b5db42ae126  octaviz-0.4.7.tar.gz
     b9bd87453b30696cbc175158c4a74b5db42ae126  octaviz-0.4.7.tar.gz.orig
 + builds in mock for F8 x86_64
 + works (with some demo path issues below) when installed on F8 x86_64
 + proper use of ldconfig
 + no duplicates in %files listing
 + no need for -devel
 + file permissions appear to be correct
 + spec has %clean and does "rm -rf $RPM_BUILD_ROOT' before install
 + no *.la
 + no need for %{name}.desktop since its an octave add-on

needswork / suggestions:
 - According to the guidelines
     http://fedoraproject.org/wiki/Packaging/NamingGuidelines
   octaviz is an addon package and should be named "octave-octaviz".
   I think this is a good idea but I don't consider it a "blocker"
   (that is, if you have good reasons to keep the name as-is then
   we can discuss/consider it).

 - license is GPLv2+ (please note the "+") per the COPYING and README
   files

 - VTK_DATA_ROOT should be set to %{_datadir}/vtkdata-5.0.4 not
   %{_datadir}/vtkdata-5.0.3 on F8 -- and perhaps this can be fixed
   more automatically within the spec-file using
     BuildRequires: vtkdata
   and then adding a macro such as $(find %{_datadir} -name vtkdata*)
   or similar logic

 - Should octaviz require vtkdata?  I think it should since many
   of the octaviz demo scripts depend upon data (images) provided by
   vtkdata.  The only downside to requiring it the ~20MB vtkdata
   download which is big-ish.  So please don't consider this a 
   blocker, its more of a judgment call.


I think the package is in good shape and the remaining changes are pretty 
minor.  Can you please comment on the above?

Also, I'm willing to sponsor you if you'll demonstrate an ability to review 
packages by doing at least one "pre-review" as described above.
Comment 8 Mamoru TASAKA 2008-05-03 19:26:05 EDT
(In reply to comment #7)
> Also, I'm willing to sponsor you if you'll demonstrate an ability to review 
> packages by doing at least one "pre-review" as described above.

I guess I can sponsor him (see bug 438750)
Comment 9 Ed Hill 2008-05-03 22:57:11 EDT
Hi Mamoru, if you'd like to sponsor Claudio then that's fine with me.

And Claudio -- can you please respond to comment #7?  If you fix the 
license and improve the VTK_DATA_ROOT situation then I'll approve it.
Comment 10 Claudio Tomasoni 2008-05-07 16:43:39 EDT
Here I am! and here are the new SPEC and SRPM:

New SPEC URL: http://www.claudiotomasoni.it/files/RPMS/octaviz.spec
New SRPM URL: http://www.claudiotomasoni.it/files/RPMS/octaviz-0.4.7-4.fc8.src.rpm

The fixes are:
- license is GPLv2+ (GPL 2 or later in README)
- vtk paths are detected automatically at package compiling time (both
/usr/lib/vtk-5.0 and /usr/share/vtkdata-5.0.4 - but I'm still thinking to a more
reliable way)
- in order to be able to determine the paths, both vtk and vtkdata have been
added as build requirements

Non fixed:
- vtkdata is required only for octaviz-examples, since the most popular
functions (vtk_plot, vtk_mesh, vtk_surf, and related) do not need it, while many
examples included in the examples subpackage can't run without it.
- name not changed in octave-octaviz. I know octaviz is an octave addon, but
many people would simply look for "octaviz" since this is the official name of
the project (the name people read in the official site and in ?many? forums or
newsgroups dedicated to engineering). Anyway, if someone consider this a
blocker, I will change the name (I don't know how to explain it, the name is a
sort of placard).

Waiting for new comments...
Comment 11 Ed Hill 2008-05-07 22:04:34 EDT
Hi Claudio,

The 0.4.7-4 srpm built for me in mock (F8 x86_64) and is working nicely 
on my laptop.   I don't see any blockers here so this package is approved.

That leaves the issue of sponsorship.  You and Mamoru started working on 
bug # 438750 before this one so I think its fair that Mamoru gets the 
first chance to sponsor you.  After (or should I say assuming?) that 
happens, please go ahead and import, build, etc. this package according 
to the directions at:

  http://fedoraproject.org/wiki/PackageMaintainers

And if you have any questions please feel free to ask!
Comment 12 Claudio Tomasoni 2008-05-12 18:25:27 EDT
Many thanks for the review and for your helpfulness!
I will proceed as soon as I'm sponsored (I hope soon...)
Comment 13 Mamoru TASAKA 2008-05-13 13:58:12 EDT
(Removing NEEDSPONSOR: I will sponsor Claudio)
Comment 14 Claudio Tomasoni 2008-06-01 13:23:55 EDT
New Package CVS Request
=======================
Package Name: octaviz
Short Description: 3D visualization system for Octave
Owners: claudiotomasoni
Branches: F-7 F-8
InitialCC:
Cvsextras Commits: yes
Comment 15 Kevin Fenzi 2008-06-01 13:28:51 EDT
We are no longer doing F-7 branches. 
Do you really not want t F-9 branch? 
Comment 16 Claudio Tomasoni 2008-06-02 05:27:39 EDT
Oops, sorry! My fault.

New Package CVS Request
=======================
Branches: F-8 F-9

The same for qtoctave.
Comment 17 Kevin Fenzi 2008-06-02 11:31:43 EDT
cvs done.
Comment 18 Claudio Tomasoni 2008-08-24 10:19:21 EDT
I am very sorry for the long absence, but I just stepped out from a very annoying health problem.
The old package didn't build for ppc and ppc64, so here is a new revision:

New SPEC URL: http://www.claudiotomasoni.it/files/RPMS/octaviz.spec
New SRPM URL: http://www.claudiotomasoni.it/files/RPMS/octaviz-0.4.7-5.fc9.src.rpm

If the package is still approved, I will build and submit it as soon as possible.
Comment 19 Fabian Affolter 2009-01-15 07:30:12 EST
Ed, is this package still approved after the fix for the archs?
Comment 20 Fabian Affolter 2009-01-15 07:34:42 EST
Claudio, I think that the package is still approved.  Please update the spec file because as mentioned in Comment #18 the old one didn't build on ppc/ppc64.

F-10 http://koji.fedoraproject.org/koji/taskinfo?taskID=649644
F-9  http://koji.fedoraproject.org/koji/taskinfo?taskID=649642
Comment 21 Fabian Affolter 2009-04-08 01:58:39 EDT
ping?
Comment 22 Henrique "LonelySpooky" Junior 2009-05-19 06:57:52 EDT
I'd like to see this package in Fedora.
Comment 23 Fabian Affolter 2009-07-27 07:29:31 EDT
Claudio,
Comment 24 Fabian Affolter 2009-07-27 07:30:59 EDT
Sorry again...Claudio, can you please make the changes from Comment #18?
Comment 25 Fabian Affolter 2009-09-18 03:29:20 EDT
Claudio, any progress?

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