(In reply to comment #0)
> Expected rpmlint output:
> 
> o E: vtk-* script-without-shellbang *.cmake
>   I'm not sure why rpmlint detects cmake files as scripts.

Perhaps they are installed with exec bits set?

> o E: vtk-testing.* script-without-shellbang .*tcl
>   Is it worth fixing this?

Do they need to be installed executable?

> o W: vtk-devel unstripped-binary-or-object <wrapper dirs>/*.so
>   debuginfo processing missed this as these are place in non-standard folders.
>   Manually strip and have only half a debuginfo worth of symbols, hack into
>   debuginfo generation, or maybe simply allow these devel files to carry the
>   symbols?

What permissions are they installed with?

> o wrong-file-end-of-line-encoding on an windows file in the examples doc.
>   The examples contain a couple of non-Linux parts, but I don't want to castrate  
>   them. Maybe someone will develop on several archs.

Not a bad excuse, that one :-)

(In reply to comment #1)
> (In reply to comment #0)
> > Expected rpmlint output:
> > 
> > o E: vtk-* script-without-shellbang *.cmake
> >   I'm not sure why rpmlint detects cmake files as scripts.
> 
> Perhaps they are installed with exec bits set?

Ah, yes, that makes sense. I'll kill all executable bits on non-scripts hoping
that vtk doesn't really interprete them. Thanks!

(In reply to comment #1)
> > o W: vtk-devel unstripped-binary-or-object <wrapper dirs>/*.so
> >   debuginfo processing missed this as these are place in non-standard folders.
> >   Manually strip and have only half a debuginfo worth of symbols, hack into
> >   debuginfo generation, or maybe simply allow these devel files to carry the
> >   symbols?
> 
> What permissions are they installed with?

Only 644, seems this is why debuginfo didn't grab them. I'll fix that, too.
Thanks, Paul!

Spec URL: http://people.atrpms.net/~athimm/fedorasubmit/vtk/vtk.spec
SRPM URL: http://people.atrpms.net/~athimm/fedorasubmit/vtk/vtk-5.0.1-9.at.src.rpm

* Wed Jul 19 2006 Axel Thimm <Axel.Thimm>
- Fix some permissions for rpmlint and debuginfo.

Fixing the permissions removed 90% of rpmlint output (unstripped binaries,
executable ASCII files which are no scripts).

Here is my comments (aka review) on the spec file:

Minor:
* BRs: gcc-c++ and sbin/ldconfig should not be needed
* Group: I think System/Libraries would be more appropriate
* Install: I digged into the cmake build and found that you can simplify the
install part adding:
-DCMAKE_INSTALL_PREFIX=%{buildroot} \
-DVTK_INSTALL_ROOT=%{_prefix} \
This will install stuff in the proper places w/o need for moving stuff around
manually.
* make %{?_smp_mflags} works for me w/o a hitch. I think it should be there

Major:
* non standard buildroot: I believe you asked for a revision on this rule.
Anyway, I saw many other packages were forced to stick to current guidelines, so
I think you have to fix that until the rule changes.

* Duplicate libs: there are a number of libraries included in the package which
we can replaced with system ones. This is done with the flags:

        -DVTK_USE_SYSTEM_EXPAT=ON \
        -DVTK_USE_SYSTEM_FREETYPE=ON \
        -DVTK_USE_SYSTEM_JPEG=ON \
        -DVTK_USE_SYSTEM_PNG=ON \
        -DVTK_USE_SYSTEM_TIFF=ON \
        -DVTK_USE_SYSTEM_ZLIB=ON \

Of course this also adds up for more BRs...

* There is a qt widget and designer plugin available. This is useful for
building qt applications with vtk windows/widgets. To activate it, I added:

        -DVTK_USE_QVTK=ON \
        -DVTK_INSTALL_QT_DIR=%{qt_plugins_prefix} \

where %{qt_plugins_prefix} is:
%define qt_plugins_prefix %(qmake -query QT_INSTALL_PREFIX)/plugins/designer


Moreover, I would appreciate some help understanding the purpose of some
snippets which I am sure you (and many others) understand at first sight, but I
don't. For example I think a comment before:

grep -rl '\.\./\.\./\.\./\.\./VTKData' . | xargs perl -pi
-e's,\.\./\.\./\.\./\.\./VTKData,%{_datadir}/vtkdata-%{version},g'

would help a lot...

another quick Q. 
What's the purpose of the second "cmake ." at line 98 in the build section?

Thanks for the review:

Spec URL: http://people.atrpms.net/~athimm/fedorasubmit/vtk/vtk.spec
SRPM URL: http://people.atrpms.net/~athimm/fedorasubmit/vtk/vtk-5.0.1-10.at.src.rpm

* Sun Jul 23 2006 Axel Thimm <Axel.Thimm> - 5.0.1-10
- Embed feedback from bug 199405 comment 5.
- Fix some Group entries.
- Remove redundant dependencies.
- Use system libs.
- Comment specfile more.
- Change buildroot handling with CMAKE_INSTALL_PREFIX.
- Enable qt designer plugin.

rpmlint is almost quiet now:
E: vtk-examples zero-length
/usr/share/doc/vtk-examples-5.0.1/Examples/Build/vtkMy/Wrapping/hints
W: vtk-examples wrong-file-end-of-line-encoding
/usr/share/doc/vtk-examples-5.0.1/Examples/GUI/Win32/SampleMFC/Sample.rc
E: vtk-examples zero-length
/usr/share/doc/vtk-examples-5.0.1/Examples/Build/vtkLocal/hints
W: vtk-python no-documentation
W: vtk-tcl no-documentation
W: vtk-tcl devel-file-in-non-devel-package /usr/lib/vtk-5.0/tcl/vtktcl.c
E: vtk-testing only-non-binary-in-usr-lib
W: vtk-testing no-documentation

The netcdf support is built only if cmake is recent enough, so for
reproducible builds it may be relevant to BuildRequires an higher
cmake version (at least 2.0.0?)

The srpm doesn't rebuild on my devel FC, I guess because 
%{python_libdir} isn't expanded. Moreover the .pyo are 
included. You could use the snippet at 
http://fedoraproject.org/wiki/Packaging/Python#head-01f2df6d8ffc1100844fce3a24b2449fa3bbe679
to get rid of them.

And instead of %{python_libdir} why don't use use 
%{!?python_sitearch: %define python_sitearch %(%{__python} -c "from
distutils.sysconfig import get_python_lib; print get_python_lib(1)")}
as in the spectemplate-python.spec?

Oops, thanks for noticing, fixed. Spec/packages are at

Spec URL: http://people.atrpms.net/~athimm/fedorasubmit/vtk/vtk.spec
SRPM URL: http://people.atrpms.net/~athimm/fedorasubmit/vtk/vtk-5.0.1-11.at.src.rpm

[%{python_libdir} is something defined in ATrpms' buildsystem, and it slipped
into the package, sorry.]

I also fixed the pyo issue.

About cmake's versioned dependency: I didn't find the required version for cmake
for netcdf at first. Then I found it as well as a yet stricter requirement for
building all examples (2.0.4).

Since it isn't really a blocker for Fedora (there is no older cmake than 2.4.3
and some people even suggest not putting a versioned dependency at all in such
cases) and the rebuilds take a long time, the package + spec above don't have
this versioned dependency yet, but I'll put it in place in the next package
update, because apart from technical considerations at the very least this is
properly documenting a non-trivial dependency information.

Should -11 be the one that get's approved I'll upload a -12 package with this
trivial fix.

1) undefined-non-weak-symbols
=============================

rpmlint on installed packages reports 
undefined-non-weak-symbols, they should certainly be 
reported upstream:

rpmlint vtk
W: vtk undefined-non-weak-symbol /usr/lib/libvtkMPEG2Encode.so.5.0.1 sqrt
...
So it seems that there is a missing -lm for vtkmpeg2encode

rpmlint vtk-python:
many undefined-non-weak-symbol, maybe should be linked against
a python library?


2) rpmlint on debug package
===========================

There are still many rpmlint errors in debuginfo package due to
bad perms. You could do something along

find . \( -name \*.txt -o -name '*.[ch]' -o -name '*.[ch][px][px]' \) -print0 |
xargs -0 chmod -x


3) included sources
===================

Now comes the included sources issues. There are still copies 
of other projects codes included, that aren't solved as easily
as the other that were solved above, since there doesn't seems
to be support for that upstream. I checked in Utilities.

Those 2 are not problematic:
* Exodus2: I didn't found a clear upstream on the web, so it could
  be kept as is
* the internal gl2ps is not used, and cannot be used for licence
  reasons (either vtk becomes LGPL, or proprietary...). That's not
  a problem, but having upstream able to link against an external 
  gl2ps would be nice (it could be rather easily packaged for extras)
gl2ps is at http://www.geuz.org/gl2ps/

Those 3 are outdated. At least it should be reported and discussed
upstream:
* the netcdf part is based on an netcdf library older than what is
  in extras
* the included DICOMParser library is older that the one on
http://sourceforge.net/projects/dicomparser/ 
* the included FTGL is older then the one on
http://homepages.paradise.net.nz/henryj/code/

This one is certainly a blocker:
* the code in vtkmpeg2encode for mpeg2 encoding is covered by 
  patents, and it doesn't seems that it is possible to sold the
  corresponding software in the USA.

If I'm not wrong, I don't see any other way to handle that issue 
(except from reporting upstream) than removing the offending code 
from the tarball.

(In reply to comment #10)
> undefined-non-weak-symbols

I can't reproduce that on my builds (see http://ATrpms.net/name/vtk/ for binary
builds for several platforms). What distro/arch and build tool did you use?

> * the code in vtkmpeg2encode for mpeg2 encoding is covered by 
>   patents, and it doesn't seems that it is possible to sold the
>   corresponding software in the USA.

I'll check that.

(In reply to comment #11)

> I can't reproduce that on my builds (see http://ATrpms.net/name/vtk/ for binary
> builds for several platforms). What distro/arch and build tool did you use?

I have the same rpmlint error with 
http://dl.atrpms.net/all/vtk-5.0.1-10.fc5.90.at.i386.rpm

I'm on rawhide
rpmlint-0.77-1.fc6

it only shows up when running rpmlint against the installed rpm.

Also the .so associated with Python and Tcl may not be necessary
if no program should link against those libraries. This way 
it may become possible to remove the dependency of the devel 
package on the python and tcl subpackages.

Some days ago my rpmlint on FC5 even refused to recognize the objformat in FC6
and I have seen similar reports in bugzilla. Now it will pass the FC6 packages
with the same output like the FC5 packages.

Maybe this is an rpmlint false alarm? rpmlint-0.77-1.fc5@x86_64 doesn't show any
undefined weak symbols, neither on FC5 builds, not FC6.

> it only shows up when running rpmlint against the installed rpm.

OK, I can't test that at the moment only external rpmlint application is
possible for me.

But that sounds like an rpmlint bug the more. The output should be the same
whether applied on an external package or on an installed one.

Did you invoke an example on rawhide? Did the executable puke on missing sqrt?
That would display whether the rpmlint error is flase or not.

It looks strange, but for the pupose of the review I wouldn't invest more time
into understanding latest rawhide and rpmlint changes. FWIW I'm rebuilding all
of ATrpms with a disttag of fc5.91 for test2. Maybe it makes a difference, let's
 see.

(In reply to comment #14)

> OK, I can't test that at the moment only external rpmlint application is
> possible for me.

It is not a blocker, it is just an information that something could
be ameliorated upstream (and there may be portability issues, but
that's not our problem).

> But that sounds like an rpmlint bug the more. The output should be the same
> whether applied on an external package or on an installed one.

No, it can't. Basically rpmlint invokes "ldd -d -r" on libs.

> Did you invoke an example on rawhide? Did the executable puke on missing sqrt?
> That would display whether the rpmlint error is flase or not.

If I recall well, on fedora the weak symbols don't break executables, 
but prelinking is less efficient.

> It looks strange, but for the pupose of the review I wouldn't invest more time
> into understanding latest rawhide and rpmlint changes. FWIW I'm rebuilding all
> of ATrpms with a disttag of fc5.91 for test2. Maybe it makes a difference, let's
>  see.

I guess it won't, but as I said above it is just a remark.

Ville explains all that here:
https://www.redhat.com/archives/fedora-extras-list/2006-July/msg00569.html

Thanks for the pointer, now I understand the issue a bit better and it's
reproducable even on FC5, so nothing is rawhide-relavant.

Since it only occurs in a part that is anyway under investigation of being
cropped I'll defer that until it's clear what happens to vtkmpeg2encode.

What is the status of this bug report?

Patrice do you want to do a formal review?

Some points that I think should be addressed before this packages is accepted, 
those referred by Patrice like the removal of the patented code are a must.

Review for release 11.at:
* RPM name is OK
* Source vtk-5.0.1.tar.gz is the same as upstream
* This is the latest version
* Builds fine in mock

Needs work:
* BuildRoot should 
be %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
  (wiki: PackagingGuidelines#BuildRoot)
* BuildRequires: gcc-c++ should not be included
  (wiki: PackagingGuidelines#Exceptions)
* Spec file: some paths are not replaced with RPM macros
  (wiki: QAChecklist item 7)

  This is relevant in the case of /usr/lib/, is this on purpose?
This shows in rpmlint of the source rpm:
E: vtk hardcoded-library-path in %{_prefix}/lib/*`"
E: vtk hardcoded-library-path in %{_prefix}/lib/*

Minor:
* QT environment variable are not sourced

  Looking to the debug file I see that there are source files there:
/usr/src/debug/VTK

  Is this intended?

(In reply to comment #17)
> What is the status of this bug report?

The licence issue is a blocker, and maybe also the inclusion of 
other packages within vtk. 

> Patrice do you want to do a formal review?

I am waiting for the current issues to be solved before I
continue reviewing, but I don't really care whether I am
the formal reviewer or not. 


> Needs work:
> * BuildRoot should 
> be %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
>   (wiki: PackagingGuidelines#BuildRoot)

The BuildRoot in the spec file is also right now.

> * BuildRequires: gcc-c++ should not be included
>   (wiki: PackagingGuidelines#Exceptions)

It could be included (this is not a blocker), but I 
agree that it is better without.

>   This is relevant in the case of /usr/lib/, is this on purpose?
> This shows in rpmlint of the source rpm:
> E: vtk hardcoded-library-path in %{_prefix}/lib/*`"
> E: vtk hardcoded-library-path in %{_prefix}/lib/*

Seems like there is something unclean in the code...

> Minor:
> * QT environment variable are not sourced

This may not be so minor, since it may lead to issues
on lib64 architectures with some qt versions.

This is not the latest version, 5.0.2 has recently been published and I'm
testing builds with it.

The hardcoded lib are a wrong positive from rpmlint. Is this what you refer to
with "some paths are not replaced with RPM macros"?

"QT environment variable are not sourced": There is no need to do so, qt-devel
takes care of that.

debuginfo: Why shouldn't it contain source files? I'm no debuginfo expert/user,
but it seems that this is what find-debuginfo.sh wants to place in debuginfo.

On legal/patent issues: I tried to contact upstream, as some people claim that
the algorithms used are also patented, but there was no response yet. Previous
releases contained a list of functions containing patented material, now that
list is gone.

Packages update to 5.0.2 are at
http://people.atrpms.net/~athimm/fedorasubmit/vtk/vtk.spec
http://people.atrpms.net/~athimm/fedorasubmit/vtk/vtk-5.0.2-13.at.src.rpm

Wrt patents: There are some patented algorithms were the patents are held by the
authors. What is the proper procedure? If the authors would be willing to allow
a free distribution through fedora would that be OK? If yes, I would ask them to.

I'm trying to build a VTK application and I saw that a package is being
reviewed, so I built your SRPM and tried it. I'm running into compilation errors
associated with the other application, and I'm starting to think the bug is in
VTK. It's looking for headers in paths that include the build path for the VTK
SRPM. I started greping the libraries and found that there are a lot of strings
in the libraries that contain the build path. The following example illustrates
the problem:

strings /usr/lib/libvtkCommon.so | grep VTK/Common

It seems that either some cmake configure option is missing, or there is some
bug in VTK itself. Unfortunately this is the first time I have used cmake, so
I'm not able to debug this any further.

The strings I see from the command above are harmless in the sense that they are
assertion messages. E.g. if the code hits an internal bug it would issue a
warning/error revealing the original path of the source file.

But you could post the actual errors you see in the dependent application.
Although technically one should wait until this package get through the review -
due to patent issues it might even not make it.

OK, it looks like you were right about the strings. However, there is still a
bug that was inserting the package build path when trying to build another
project that uses VTK. Apparently this is a known bug:

http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=388736

Regarding the patent issues, I haven't fully researched the issue, but I was
recently referred to these links by someone who uses VTK:

http://public.kitware.com/pipermail/vtk-developers/2005-February/003131.html
http://public.kitware.com/pipermail/vtk-developers/2002-July/001757.html

It sounds to me like the patented subdirectory was removed from the source tree
with release 5.0 because of the problems it was causing. If you have trouble
getting a response from the upstream developers, the package is in Debian, so
you might consider contacting the Debian packager for more information.

(In reply to comment #10)
> Those 3 are outdated. At least it should be reported and discussed
> upstream:
> * the netcdf part is based on an netcdf library older than what is
>   in extras
> * the included DICOMParser library is older that the one on
> http://sourceforge.net/projects/dicomparser/ 
> * the included FTGL is older then the one on
> http://homepages.paradise.net.nz/henryj/code/

I have filed upstream bug report against this, see:
http://www.vtk.org/Bug/bug.php?op=show&bugid=3824

(In reply to comment #23)
> there is still a bug that was inserting the package build path when trying
> to build another project that uses VTK. Apparently this is a known bug:
> 
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=388736

Thanks, I'll fix that!

> Regarding the patent issues, I haven't fully researched the issue, but I was
> recently referred to these links by someone who uses VTK:
> 
> http://public.kitware.com/pipermail/vtk-developers/2005-February/003131.html
> http://public.kitware.com/pipermail/vtk-developers/2002-July/001757.html
> 
> It sounds to me like the patented subdirectory was removed from the source
> tree with release 5.0 because of the problems it was causing.

That's half the story. Apparently the authors themselves have patents on
algorithms in the code, they only removed foreign algorithms. I *assume* that
the authors would grant rights to use within Fedora and similar projects, but
I'd only like to ask if Fedora Legal would accept such a modus. I'm going to
raise that issue on the extras list.

Hi folks, I've built and am now testing the vtk-5.0.2-13.at.src.rpm 
listed in comment #20.

It still has problems with the following items which were listed in 
comment #10 :

 - undefined-non-weak-symbols
 - patented MPEG2 bits will have to be removed:
    [ http://en.wikipedia.org/wiki/MPEG-2 ]
 - many files in the -debug package have permissions issues

and I'd like to add one additional observation:

 - The vtk-devel package places a large number of files in /usr/include/.  
   Almost all are called "vtk*" and are thus unlikely to result in name 
   collisions with other software.   However, the "/usr/include/internal" 
   dir is problematic.  Please consider placing all the vtk include 
   files and subdirs in a directory such as /usr/include/vtk

Created attachment 139933 [details]
silence warnings about deprecated headers

Here is a small patch that silences the deprecated headers [otherwise you 
get a lot of compier warnings when building VTK apps].	Please see:

  http://www.vtk.org/Bug/bug.php?op=vote&bugid=1457

where upstream so far refuses to address the issue.  Also, the Debian VTK 
packages have a similar patch.

Apparently the patch in comment #27 silences most of the warnings that I get
in *my* VTK-using application but it doesn't go anywhere far enough.  Please 
see:

  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=396283
  http://vtk.org/Bug/bug.php?op=show&bugid=1953

and please ignore the patch in comment #27 since its not a real fix.

Comment on attachment 139933 [details]
silence warnings about deprecated headers

OK, I marked the patch in comment #27 as obsolete.

I just repinged the authors and other patent holders on what the situation is
with vtk and patents (other than the "trivial" mpeg bits):

http://public.kitware.com/pipermail/vtkusers/2006-December/088816.html

(note that I don't expect an answer this year anymore, so you should follow up
the archives into January if you are viewing this in 2007).

I'm not sure whether this is the proper action yet, perhaps this package should
be marked as blocking FE-LEGAL.

Hi Axel, I saw your comments on the VTK list.  I'm an occasional VTK 
user and hope that it can be included in Fedora.  I'm also a little 
concerned about the possible legal problems stemming from past (have 
they expired?) patents.

Would you be willing to continue with the review process (that is, 
addressing the items in comments #10 and #26) while, in parallel, we 
contact FESCO, Fedora Legal, or whoever it is within Fedora who can 
provide an authoritative response to the legal issues?

Thanks for picking this up for review, it's been w/o a reviewer for more than 5
months. :)

The next thing that needs to be done on a technical level is to remove the mpeg
source code (which will also remove any weak symbol issues). I already contacted
fedora-packaging for some advice on what the proper procedure is to have a
documented way for a reviewer to check that the remaining source is still what
upstream created and not some trojan horse ;)

But let's wait until both kitware and GE make their comments on the patents
(especially GE and Marching Cubes). I wouldn't like to find us placing lots of
efforts which some of the patent holders may send to /dev/null for fun.

BTW this package depends on bug #199402, which is needed for testing this one.
Bug #199402 also needs a review. :)

> BTW this package depends on bug #199402, which is needed for testing this one.
> Bug #199402 also needs a review. :)

Well, putting my brain back into shape:

a) the bug in question is bug #199406, not bug #199402 ...
b) technically the dependencies are reverse, e.g. the vtkdata package requires
   this one, but for vtkdata is a very good test for vtk.

Upstream comments in
http://public.kitware.com/pipermail/vtkusers/2007-January/088901.html

> In VTK, the only code that is patented is the mpeg2 encoder. All the
> other patented algorithms were either replaced or their patents
> expired. The inclusion of mpeg2 in VTK is an oversight. We will make
> it an external dependency in the next patch of VTK 5. I would suggest
> waiting until then to include VTK in a distribution.

Hi Axel, thank you for your persistence and for the update!  I'll be glad 
to review the next version (mentioned above) as soon as its ready.

Created attachment 147683 [details]
vtk.spec version 14 with changes noted in comment

Some comments:

- With cmake projects, I like to do:

make VERBOSE=1 %{?_smp_mflags}

to see the compile command lines.  Helps with debugging builds.  Produces
copious output though.

- Are there really cmakes < 2.0.4 out there in the wild?

- You can use DESTDIR with the cmakes in Fedora.

- Usually you do out of tree builds with cmake.

- Why not build Java, Qt Designer, GL2PS, and with OSMesa support?  OSMesa
isn't avail in FC5 but is in FC6, though it may be buggy.

- I find all the automatic generating of file lists confusing and prone to
problems, but it's a matter of taste.

- I'd like to get the test suite run with ctest, but it currently tries to run
tests that need a display.  You can turn that off with VTK_USE_DISPLAY=OFF, but
then the tests/examples are not built and so cannot be installed in the
examples subpackage.

What java did you use?

$ grep /usr/lib/jvm/java vtk.spec.orion 
export JAVA_HOME=/usr/lib/jvm/java
$ rpm -q gcc-java libgcj-devel libgcj
gcc-java-4.1.1-51.fc6
libgcj-devel-4.1.1-51.fc6
libgcj-4.1.1-51.fc6
libgcj-4.1.1-51.fc6
$ rpm -ql gcc-java libgcj-devel libgcj | grep -E '/usr/lib.*/jvm/java($/)'
$ ls -l /usr/lib*/jvm/java
ls: /usr/lib*/jvm/java: No such file or directory

[root@cynosure log]# ls -l /usr/lib/jvm/java
lrwxrwxrwx 1 root root 26 Jan 23 10:20 /usr/lib/jvm/java ->
/etc/alternatives/java_sdk
[root@cynosure log]# ls -l /etc/alternatives/java_sdk
lrwxrwxrwx 1 root root 27 Jan 23 10:20 /etc/alternatives/java_sdk ->
/usr/lib/jvm/java-1.4.2-gcj
[root@cynosure log]# rpm -qf /usr/lib/jvm/java-1.4.2-gcj
java-1.4.2-gcj-compat-devel-1.4.2.0-40jpp.110
[root@cynosure log]# rpm -q --provides java-1.4.2-gcj-compat-devel-1.4.2.0-40jpp.110
java-1.4.2-devel
java-devel = 1.4.2
java-devel-gcj = 0:1.4.2.0
java-gcj-compat-devel = 1.0.68
java-sdk = 1.4.2
java-sdk-1.4.2
java-sdk-1.4.2-gcj = 0:1.4.2.0
java-sdk-gcj = 0:1.4.2.0
java-1.4.2-gcj-compat-devel = 0:1.4.2.0-40jpp.110

So the BR: should probably be "java-devel" rather than gcc-java.  Or you might
be able to use a different JAVA_HOME, but I think you're stuck using versioned
directories if you do.

I'm currently preparing new packages for 5.0.3, I stumbled over the following:

[ 97%] Building CXX object Wrapping/Java/CMakeFiles/VTKJavaExecutable.dir/VTKJava.o
Linking CXX executable ../../bin/VTKJavaExecutable
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to `_Jv_CheckCast'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to
`_Jv_JNI_PopSystemFrame'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to `_Jv_MonitorEnter'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to `_Jv_byteClass'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to `_Jv_InitClass'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to
`_Jv_GetJNIEnvNewFrame'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to
`__gcj_personality_v0'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to `_Jv_NewPrimArray'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to `_Jv_divI'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to
`_Jv_CheckArrayStore'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to
`_Jv_LookupJNIMethod'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to
`_Jv_LookupInterfaceMethodIdx'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to `_Jv_charClass'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to
`java::lang::Math::floor(double)'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to `_Jv_intClass'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to
`_Jv_ThrowBadArrayIndex'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to `vtable for
java::lang::Class'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to `_Jv_IsInstanceOf'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to `_Jv_doubleClass'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to `_Jv_AllocObject'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to
`_Jv_AllocObjectNoFinalizer'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to
`_Jv_ThrowNullPointerException'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to `_Jv_Throw'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to
`_Jv_UnwrapJNIweakReference'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to
`_Jv_ThrowNoSuchFieldError'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to `_Jv_floatClass'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to `_Jv_MonitorExit'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to
`_Jv_ResolvePoolEntry'
/usr/lib64/lib-gnu-java-awt-peer-gtk.so.7: undefined reference to
`_Jv_NewObjectArray'
collect2: ld returned 1 exit status

(of course it has to be at 97% and not 3% ;)

That's on FC5, so I guess for FC5 I'll just turn off the java sub-package.

On further note: Upon request the vtk developers split the mpeg sources out of
the main tarball and verified that there are no patent issues with any
algorithms in vtk (see also comment #34), so the review will be able to proceed
w/o patent troubling.

I also merged in most changes of Orion in comment #36.

OK, so all that is left is finishing the builds, verifiying it still works, and
post new URLs. :)

Hi Axel, I'll be glad to continue with this review as soon as you post
an update.  I've been using the SRPM posted in comment #20 and it works 
nicely for me on a few different (i386 and x86_64) machines.

> I'll be glad to continue with this review as soon as you post an update.

OK, here it comes. For the lazy one there are binary packages at ATrpms. Thanks
to Paulo and Orion for several fixes.

http://dl.atrpms.net/all/vtk-5.0.3-17.at.src.rpm
http://dl.atrpms.net/all/vtk.spec

 Mon Apr 16 2007 Axel Thimm <Axel.Thimm> - 5.0.3-17
- Make java build conditional.
- Add ldconfig %%post/%%postun for java/qt subpackages.

* Sun Apr 15 2007 Axel Thimm <Axel.Thimm> - 5.0.3-16
- Remove %%ghosting pyc/pyo.

* Wed Apr 04 2007 Paulo Roma <roma.br> - 5.0.3-15
- Update to 5.0.4.
- Added support for qt4 plugin.

* Wed Feb  7 2007 Orion Poplawski <orion.com> - 5.0.2-14
- Enable Java, Qt, GL2PS, OSMESA

BTW if you're reviewing this package, it may make sense to do so also for bug
#199406, the respective data file with examples for vtk (it's a trivial review
compared to this one). Thanks!

Hi Axel, I'm working through a review right now and I've run into a problem
where it fails to build in mock for a "fedora-6-x86_64-core" buildroot.  The
reported error is:

  RPM build errors:
    File must begin with "/": %{python_sitearch}/vtk

which seems very odd since the command

  python -c "from distutils.sysconfig import get_python_lib; print
get_python_lib(1)"

returns "/usr/lib64/python2.4/site-packages" which clearly has a leading
"/".  I'm not sure what's going on so any help or suggestions are 
appreciated.  And I'll try it again with a "fedora-6-i386-core" buildroot.

Hm, I have no idea. The package contains the guideline-required

%{!?python_sitearch: %define python_sitearch %(%{__python} -c "from
distutils.sysconfig import get_python_lib; print get_python_lib(1)")}

line. This ensures that python_sitearch gets set to something, but your error
message looks like it wasn't set at all, not even an invalid value.

Perhaps try first w/o using mock?

OK, I've now received the exact same error in mock with a "fedora-6-i386-core"
and a "fedora-6-x86_64-core" build root.  And, building outside mock gives 
exactly the same error, too.

I agree that you have set exactly the same (byte-for-byte!) python_sitearch 
macro as is suggested in the guidelines.  I can only conclude that the build 
process is either somehow either ignoring the macro or is un-defining it.
Neither of which makes sense to me.

Can someone (anyone?) please suggest a way to debug this problem?  Its 
frustrating since the VTK builds are a little time-consuming.

%bcond_* are known to cause odd issues in some setups, reverting them to plain
old %{?_with(out)_foo:bar} might be worth a try.

Hi Ville, thank you for the warning about %bcond_* but the problem seems to 
lie elsewhere.  I've tried the following builds:

  mock w/ fedora-6-i386-core
  mock w/ fedora-6-x86_64-core on two separate machines
  without mock on two different FC6 x86_64 system
  no-mock and removed all the %bcond_* bits from the spec file

and all fail with the exact same indication that %{python_sitearch} is 
undefined when reaching the %files sections.

And, just to be sure, I downloaded and built:

  http://people.atrpms.net/~athimm/fedorasubmit/vtk/vtk-5.0.2-13.at.src.rpm

from comment #20 and it builds and works nicely for me.  So something 
changed between the 5.0.2-13 and the 5.0.3-17 SRPMs that is causing 
this problem.

Unfortunately, I'm out of free time for the weekend and will have to 
continue this review at a later date.  Can someone please report whether
this build works for them (particularly in mock!) and/or suggest what is
causing this annoyance?  All help is appreciated!  :-)

(In reply to comment #43)
> Hi Axel, I'm working through a review right now and I've run into a problem
> where it fails to build in mock for a "fedora-6-x86_64-core" buildroot.  The
> reported error is:
>   RPM build errors:
>     File must begin with "/": %{python_sitearch}/vtk
> which seems very odd since the command

(In reply to comment #45)
> OK, I've now received the exact same error in mock with a "fedora-6-i386-core"
> and a "fedora-6-x86_64-core" build root.  

Removing all macros which includes white space like:
-----------------------------------------
%if %{with java}
%if %{with qt4} ........
-----------------------------------------
seems okay.

Please replace the %define in the definition of python_sitearch with a %global.

I spent quite a few hours to debug this and it looks like a bug in rpm, see bug
#237448.

The reason why I was not seeing it it that python_sitearch was already defined
for me. The consequence will be to get the FPC to change conditionalized
%defines to %globals until rpm is fixed and until all supported releases contain
this fix (so it may be that it will have to wait until F9+).

Hello
A quick search for ftgl show this review report...

Might be interrested in https://bugzilla.redhat.com/240090

Hi Axel, may sincere apologies for the delays.  I've been using your VTK 
package for a few weeks and it appears to function nicely for me.  Here
is a quick review:

GOOD:
+ source matches upstream -- sha1sum:
   0a574f481c65a3d188c48dfc4e284aa8f70bad84  vtk-5.0.2.tar.gz.1
   0a574f481c65a3d188c48dfc4e284aa8f70bad84  vtk-5.0.2.tar.gz
+ spec is correctly named, legible, and appears to meet the guidelines
+ license OK and correctly included
+ package builds in mock on FC6-x86_64 using the one small change:

   %{!?python_sitearch: %global python_sitearch ...

+ proper use of ldconfig
+ dir ownership looks OK
+ proper use of %clean in spec and at start of %install
+ proper use of -devel, -examples, etc.
+ no *.la

NEEDSWORK:
+ Please put the /usr/include/* files in /usr/include/vtk/* or a 
  similar location since some have rather generic names (e.g.,
  "internal") and there are a large number of them
+ rpmlint reports a number of warnings/errors and they are attached:
  + it would be nice to remove a lot of the unnecessary 
    executable permissions
  + the "vtk hardcoded-library-path in %{_prefix}/lib/*" looks 
    fine to me -- its just brain-dead parsing from rpmlint

Please consider the /usr/include/vtk subdir and I'll approve it.

Created attachment 155546 [details]
rpmlint output

rpmlint output

Thanks, would you like to also review bug #199406? It's rather trivial (just the
data files to vtk).

Here's what the specfile changes look like, the build will follow shortly:

--- vtk.spec.org        2007-04-16 19:48:30.000000000 +0200
+++ vtk.spec    2007-05-28 18:01:54.000000000 +0200
@@ -4,3 +4,3 @@
 
-%{!?python_sitearch: %define python_sitearch %(%{__python} -c "from
distutils.sysconfig import get_python_lib; print get_python_lib(1)")}
+%{!?python_sitearch:%global python_sitearch %(%{__python} -c "from
distutils.sysconfig import get_python_lib; print get_python_lib(1)")}
 
@@ -9,3 +9,3 @@
 Version: 5.0.3
-Release: 17%{?dist}
+Release: 18%{?dist}
 License: BSD-like
@@ -108,2 +108,5 @@
 
+# Remove executable bits from sources
+find . -name \*.c -or -name \*.cxx -or -name \*.h | xargs chmod -x
+
 # Save an unbuilt copy of the Example's sources for %doc
@@ -133,3 +136,3 @@
  -DVTK_INSTALL_BIN_DIR:PATH=%{_bindir} \
- -DVTK_INSTALL_INCLUDE_DIR:PATH=%{_includedir} \
+ -DVTK_INSTALL_INCLUDE_DIR:PATH=%{_includedir}/vtk \
  -DVTK_INSTALL_LIB_DIR:PATH=%{_libdir} \
@@ -316,3 +319,3 @@
 %{_libdir}/vtk-5.0/doxygen
-%{_includedir}/*
+%{_includedir}/vtk
 %{_libdir}/*.so
@@ -363,2 +366,6 @@
 %changelog
+* Mon May 28 2007 Axel Thimm <Axel.Thimm> - 5.0.3-18
+- Move headers to %%{_includedir}/vtk.
+- Remove executable bit from sources.
+
 * Mon Apr 16 2007 Axel Thimm <Axel.Thimm> - 5.0.3-17

OK, I don't see any blockers remaining so it's:  APPROVED.

Thanks, Ed!

New Package CVS Request
=======================
Package Name: vtk
Short Description: A high level 3D visualization library
Owners: Axel.Thimm
Branches: F-7 FC-6 FC-5

cvs done.

Hi Axel, is there any chance you could push builds for this on FC-6 
and/or F-7?  I don't mean to hassle you, I'm just curious... thanks!

Actually I'm pushing builds starting a month and they fail in the Java parts. I
can't reproduce it locally though.

http://buildsys.fedoraproject.org/logs/fedora-6-extras/33921-vtk-5.0.3-18.fc6/ppc/build.log

Similar for F7. I tend to turn off the java builds to get something in and then
care about the java errors. What do you say?

Oh, I wasn't aware of the Java build problems!  That's annoying.

VTK is very useful and, in my experience over the past few months, your 
package works quite nicely for C++, python, and tcl development.  I fully 
support your idea of initially disabling Java.  It can always be added as 
an update once the build problems get sorted out.

And I'm wondering -- are there any Java coders/packagers following this 
bug who might like to look into the above build problems?

Axel -

  Are you sure the %bcond_with/without macros work in the Fedora build system? 
Don't seem to do anything for me in mock.

Yes, the bcond macros could at most deactivate the Java builds, but the error is
due to the Java build being attempted. You can see from the build logs that the
bcond macros didn't mess up anything.

Could you reproduce the failure of the Java builds?

I could reproduce the java failure if I explicitly set with_java.  I fixed it by
removing %{?_smp_flags} from the make command.  Looks like a parallel build
dependency issue - probably trying to compile the wrappers before they are all
generated.

Also, why not build with OSMesa?  I believe it is only FC-5 that didn't have
OSMesa support.  Compiles fine for me on devel.

Next, you might consider providing a -mpi version like is done with paraview. 
Probably not a big win though since I think most mpi users will compile their own.

(In reply to comment #62)
> I could reproduce the java failure if I explicitly set with_java.

Yes, that's the default if you don't pass anything on the command line.

> I fixed it by removing %{?_smp_flags} from the make command.  Looks like a
> parallel build dependency issue

Argh, I hate %{?_smp_flags}. I only added it because I was hoping to get the
review happen due to comment #5. I should ask the FPC to forbid smp_flags.

Anyway thanks for spotting this!

> Also, why not build with OSMesa?

But, it's already built with it. The default it to build with java and OSMesa.

> Next, you might consider providing a -mpi version

Yes, but let's first make the first entry :)

(In reply to comment #63)
> (In reply to comment #62)
> > Also, why not build with OSMesa?
> 
> But, it's already built with it. The default it to build with java and OSMesa.
> 

Hmm, guess I really just don't understand %bcond.

> > Next, you might consider providing a -mpi version
> 
> Yes, but let's first make the first entry :)

Yeah, yeah... :-)

BTW - I usually do "make VERBOSE=1" with my cmake packages.

Hi Axel, is there anything I can do to help here?

I'd really like to see VTK in Fedora since I use it and have been 
recommending it to others.  I've done a number of vtk builds with
your spec-file (or slightly modified versions of it) on various 
FC-6 and F-7 systems (i386 and x86_64) and it works nicely.  When 
I remove the %bcond, java, and %{?_smp_flags} bits it also works 
nicely in mock.

Would you be willing to considfer me as a co-maintainer?  I use VTK 
(the C++ and python APIs, anyway), so I have an interest and am 
willing to test it, etc.

I Have a problem trying to compile SALOME wich needs vtk
----
checking for vtk python module... ./configure: line 2419: 24091 Segmentation
fault      $PYTHON -c "import vtk" >&/dev/null
no
----
rpm -q vtk (from Axel's rpms repository)
vtk-5.0.3-18.fc6

importing the module with ipython work fine but the segmentation fault appears
on only exit...

vtk-5.0.3-18.2.fc7 has been pushed to the Fedora 7 testing repository.  If problems still persist, please make note of it in this bug report.

Problems in building octaviz too.
There are some relative paths in /usr/lib/vtk-5.0/vtkCommonKit.cmake that
shouldn't be there.
There are already patches to solve the problem at http://tinyurl.com/3auz54.

Claudio, could you please open a separate bug for this (comment #68), and use

http://bugs.debian.org/cgi-bin/bugreport.cgi/99-port-to-vtk5.patch?bug=385519;msg=5;att=1

in the interim? Thanks!

vtk-5.0.3-18.2.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.

Package Change Request
======================
Package Name: vtk
New Branches: el6
Owners: jgu
InitialCC:

Git done (by process-git-requests).