Bug 439100

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...

Created attachment 301443 [details]
add BRs for mock and some path cleanups for x86_64

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?

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

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.

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!  :-)

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.

(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)

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.

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...

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!

Many thanks for the review and for your helpfulness!
I will proceed as soon as I'm sponsored (I hope soon...)

(Removing NEEDSPONSOR: I will sponsor Claudio)

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

We are no longer doing F-7 branches. 
Do you really not want t F-9 branch? 

Oops, sorry! My fault.

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

The same for qtoctave.

cvs done.

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.

Ed, is this package still approved after the fix for the archs?

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

ping?

I'd like to see this package in Fedora.

Claudio,

Sorry again...Claudio, can you please make the changes from Comment #18?

Claudio, any progress?

http://koji.fedoraproject.org/koji/buildinfo?buildID=51848