vtk-8.2.0-rc2 problem building wheels

classic Classic list List threaded Threaded
39 messages Options
12
Reply | Threaded
Open this post in threaded view
|

vtk-8.2.0-rc2 problem building wheels

Prabhu Ramachandran-3

Hi all,

Here is hopefully the last of the issues I am running into, I just tried to create wheels for 8.2.0-rc2 and have the following problem. 

First for some background.  Some of the VTK Python libraries seem to link to the PYTHON_LIBRARY, for the wheels, we do not do this.  JC pointed this out to me https://www.python.org/dev/peps/pep-0513/#libpythonx-y-so-1

Basically, if we build wheels linking to the libpython* the wheel may not work on Ubuntu/Debian which would be sad.

In the past what we did to build the wheels is provide a dummy VTKPythonPackage/scripts/internal/libpython-not-needed-symbols-exported-by-interpreter and passing that.  This somehow worked by tricking CMake into thinking there was a library specified but I am not sure how the linker did not complain.  Anyway, when I try to build the wheels now on MacOS I get this error:

Linking CXX shared library...ib/libvtkPythonInterpreter-8.2.1.dylib
FAILED: lib/libvtkPythonInterpreter-8.2.1.dylib
: && /Library/Developer/CommandLineTools/usr/bin/c++ -O3 -DNDEBUG -arch x86_64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk -mmacosx-version-min=10.9 -dynamiclib -Wl,-headerpad_max_install_names -undefined dynamic_lookup  -compatibility_version 1.0.0 -current_version 1.0.0 -o lib/libvtkPythonInterpreter-8.2.1.dylib -install_name @rpath/libvtkPythonInterpreter-8.2.1.dylib Utilities/PythonInterpreter/CMakeFiles/vtkPythonInterpreter.dir/vtkPythonInterpreter.cxx.o Utilities/PythonInterpreter/CMakeFiles/vtkPythonInterpreter.dir/vtkPythonInteractiveInterpreter.cxx.o  -Wl,-rpath,VTKPythonPackage/VTK-osx_3.7/lib VTKPythonPackage/scripts/internal/libpython-not-needed-symbols-exported-by-interpreter lib/libvtksys-8.2.1.dylib lib/libvtkCommon-8.2.1.dylib && :
ld: file too small (length=0) file 'VTKPythonPackage/scripts/internal/libpython-not-needed-symbols-exported-by-interpreter' for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)


Right now, I am able to build wheels only by leaving the PYTHON_LIBRARY entry blank.  This means that the libvtkPythonInterpreter-8.2.1.dylib does indeed link to the libpythonx.y.dylib. 

However, I do notice that the other libvtk*Python*8.2.1.dylib do not link to libpython.  So libvtkPythonInterpreter is the only one linking to libpython. So I think maybe if a user never uses the libvtkPythonInterpreter or if that is never explicitly imported or linked to in any Python code, we may be fine with these wheels.  Also I see that none of the Python extension modules, i.e. all the vtk*Python.so do not link to libpython or to libvtkPythonInterpreter.

Should I just ignore the libvtkPythonInterpreter and leave the PYTHON_LIBRARY field blank?

I would appreciate your thoughts on this matter. Thanks!

cheers,

Prabhu


_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: vtk-8.2.0-rc2 problem building wheels

Sreekanth Arikatla

On Tue, Nov 27, 2018 at 3:01 PM Prabhu Ramachandran <[hidden email]> wrote:

Hi all,

Here is hopefully the last of the issues I am running into, I just tried to create wheels for 8.2.0-rc2 and have the following problem. 

First for some background.  Some of the VTK Python libraries seem to link to the PYTHON_LIBRARY, for the wheels, we do not do this.  JC pointed this out to me https://www.python.org/dev/peps/pep-0513/#libpythonx-y-so-1

Basically, if we build wheels linking to the libpython* the wheel may not work on Ubuntu/Debian which would be sad.

In the past what we did to build the wheels is provide a dummy VTKPythonPackage/scripts/internal/libpython-not-needed-symbols-exported-by-interpreter and passing that.  This somehow worked by tricking CMake into thinking there was a library specified but I am not sure how the linker did not complain.  Anyway, when I try to build the wheels now on MacOS I get this error:

Linking CXX shared library...ib/libvtkPythonInterpreter-8.2.1.dylib
FAILED: lib/libvtkPythonInterpreter-8.2.1.dylib
: && /Library/Developer/CommandLineTools/usr/bin/c++ -O3 -DNDEBUG -arch x86_64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk -mmacosx-version-min=10.9 -dynamiclib -Wl,-headerpad_max_install_names -undefined dynamic_lookup  -compatibility_version 1.0.0 -current_version 1.0.0 -o lib/libvtkPythonInterpreter-8.2.1.dylib -install_name @rpath/libvtkPythonInterpreter-8.2.1.dylib Utilities/PythonInterpreter/CMakeFiles/vtkPythonInterpreter.dir/vtkPythonInterpreter.cxx.o Utilities/PythonInterpreter/CMakeFiles/vtkPythonInterpreter.dir/vtkPythonInteractiveInterpreter.cxx.o  -Wl,-rpath,VTKPythonPackage/VTK-osx_3.7/lib VTKPythonPackage/scripts/internal/libpython-not-needed-symbols-exported-by-interpreter lib/libvtksys-8.2.1.dylib lib/libvtkCommon-8.2.1.dylib && :
ld: file too small (length=0) file 'VTKPythonPackage/scripts/internal/libpython-not-needed-symbols-exported-by-interpreter' for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)


Right now, I am able to build wheels only by leaving the PYTHON_LIBRARY entry blank.  This means that the libvtkPythonInterpreter-8.2.1.dylib does indeed link to the libpythonx.y.dylib. 

However, I do notice that the other libvtk*Python*8.2.1.dylib do not link to libpython.  So libvtkPythonInterpreter is the only one linking to libpython. So I think maybe if a user never uses the libvtkPythonInterpreter or if that is never explicitly imported or linked to in any Python code, we may be fine with these wheels.  Also I see that none of the Python extension modules, i.e. all the vtk*Python.so do not link to libpython or to libvtkPythonInterpreter.

Should I just ignore the libvtkPythonInterpreter and leave the PYTHON_LIBRARY field blank?

I would appreciate your thoughts on this matter. Thanks!

cheers,

Prabhu

_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers



--
Sreekanth Arikatla, Ph.D.,
Senior R&D Engineer,
Kitware, Inc., Carrboro, NC.


_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: vtk-8.2.0-rc2 problem building wheels

Sreekanth Arikatla
Please ignore the previous email. I sent it by mistake.

On Tue, Nov 27, 2018 at 3:59 PM Sreekanth Arikatla <[hidden email]> wrote:

On Tue, Nov 27, 2018 at 3:01 PM Prabhu Ramachandran <[hidden email]> wrote:

Hi all,

Here is hopefully the last of the issues I am running into, I just tried to create wheels for 8.2.0-rc2 and have the following problem. 

First for some background.  Some of the VTK Python libraries seem to link to the PYTHON_LIBRARY, for the wheels, we do not do this.  JC pointed this out to me https://www.python.org/dev/peps/pep-0513/#libpythonx-y-so-1

Basically, if we build wheels linking to the libpython* the wheel may not work on Ubuntu/Debian which would be sad.

In the past what we did to build the wheels is provide a dummy VTKPythonPackage/scripts/internal/libpython-not-needed-symbols-exported-by-interpreter and passing that.  This somehow worked by tricking CMake into thinking there was a library specified but I am not sure how the linker did not complain.  Anyway, when I try to build the wheels now on MacOS I get this error:

Linking CXX shared library...ib/libvtkPythonInterpreter-8.2.1.dylib
FAILED: lib/libvtkPythonInterpreter-8.2.1.dylib
: && /Library/Developer/CommandLineTools/usr/bin/c++ -O3 -DNDEBUG -arch x86_64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk -mmacosx-version-min=10.9 -dynamiclib -Wl,-headerpad_max_install_names -undefined dynamic_lookup  -compatibility_version 1.0.0 -current_version 1.0.0 -o lib/libvtkPythonInterpreter-8.2.1.dylib -install_name @rpath/libvtkPythonInterpreter-8.2.1.dylib Utilities/PythonInterpreter/CMakeFiles/vtkPythonInterpreter.dir/vtkPythonInterpreter.cxx.o Utilities/PythonInterpreter/CMakeFiles/vtkPythonInterpreter.dir/vtkPythonInteractiveInterpreter.cxx.o  -Wl,-rpath,VTKPythonPackage/VTK-osx_3.7/lib VTKPythonPackage/scripts/internal/libpython-not-needed-symbols-exported-by-interpreter lib/libvtksys-8.2.1.dylib lib/libvtkCommon-8.2.1.dylib && :
ld: file too small (length=0) file 'VTKPythonPackage/scripts/internal/libpython-not-needed-symbols-exported-by-interpreter' for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)


Right now, I am able to build wheels only by leaving the PYTHON_LIBRARY entry blank.  This means that the libvtkPythonInterpreter-8.2.1.dylib does indeed link to the libpythonx.y.dylib. 

However, I do notice that the other libvtk*Python*8.2.1.dylib do not link to libpython.  So libvtkPythonInterpreter is the only one linking to libpython. So I think maybe if a user never uses the libvtkPythonInterpreter or if that is never explicitly imported or linked to in any Python code, we may be fine with these wheels.  Also I see that none of the Python extension modules, i.e. all the vtk*Python.so do not link to libpython or to libvtkPythonInterpreter.

Should I just ignore the libvtkPythonInterpreter and leave the PYTHON_LIBRARY field blank?

I would appreciate your thoughts on this matter. Thanks!

cheers,

Prabhu

_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers



--
Sreekanth Arikatla, Ph.D.,
Senior R&D Engineer,
Kitware, Inc., Carrboro, NC.



--
Sreekanth Arikatla, Ph.D.,
Senior R&D Engineer,
Kitware, Inc., Carrboro, NC.


_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: vtk-8.2.0-rc2 problem building wheels

Jean-Christophe Fillion-Robin-2
In reply to this post by Prabhu Ramachandran-3
Hi Prabhu,

> In the past what we did to build the wheels is provide a dummy VTKPythonPackage/scripts/internal/libpython-not-needed-symbols-exported-by-interpreter and passing that.  This somehow worked by tricking CMake into thinking there was a library specified but I am not sure how the linker did not complain.

This was done to ensure no library was strongly linking against python library. And if it did, the error message would be more explicit. It was working because the library was always weakly linked in older version of VTK.


> Should I just ignore the libvtkPythonInterpreter and leave the PYTHON_LIBRARY field blank?

It looks like introducing an option to conditionally build vtkPythonInterpreter would address the issue here.

@Ben: What do you think ?

Jc







On Tue, Nov 27, 2018 at 3:01 PM Prabhu Ramachandran <[hidden email]> wrote:

Hi all,

Here is hopefully the last of the issues I am running into, I just tried to create wheels for 8.2.0-rc2 and have the following problem. 

First for some background.  Some of the VTK Python libraries seem to link to the PYTHON_LIBRARY, for the wheels, we do not do this.  JC pointed this out to me https://www.python.org/dev/peps/pep-0513/#libpythonx-y-so-1

Basically, if we build wheels linking to the libpython* the wheel may not work on Ubuntu/Debian which would be sad.

In the past what we did to build the wheels is provide a dummy VTKPythonPackage/scripts/internal/libpython-not-needed-symbols-exported-by-interpreter and passing that.  This somehow worked by tricking CMake into thinking there was a library specified but I am not sure how the linker did not complain.  Anyway, when I try to build the wheels now on MacOS I get this error:

Linking CXX shared library...ib/libvtkPythonInterpreter-8.2.1.dylib
FAILED: lib/libvtkPythonInterpreter-8.2.1.dylib
: && /Library/Developer/CommandLineTools/usr/bin/c++ -O3 -DNDEBUG -arch x86_64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk -mmacosx-version-min=10.9 -dynamiclib -Wl,-headerpad_max_install_names -undefined dynamic_lookup  -compatibility_version 1.0.0 -current_version 1.0.0 -o lib/libvtkPythonInterpreter-8.2.1.dylib -install_name @rpath/libvtkPythonInterpreter-8.2.1.dylib Utilities/PythonInterpreter/CMakeFiles/vtkPythonInterpreter.dir/vtkPythonInterpreter.cxx.o Utilities/PythonInterpreter/CMakeFiles/vtkPythonInterpreter.dir/vtkPythonInteractiveInterpreter.cxx.o  -Wl,-rpath,VTKPythonPackage/VTK-osx_3.7/lib VTKPythonPackage/scripts/internal/libpython-not-needed-symbols-exported-by-interpreter lib/libvtksys-8.2.1.dylib lib/libvtkCommon-8.2.1.dylib && :
ld: file too small (length=0) file 'VTKPythonPackage/scripts/internal/libpython-not-needed-symbols-exported-by-interpreter' for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)


Right now, I am able to build wheels only by leaving the PYTHON_LIBRARY entry blank.  This means that the libvtkPythonInterpreter-8.2.1.dylib does indeed link to the libpythonx.y.dylib. 

However, I do notice that the other libvtk*Python*8.2.1.dylib do not link to libpython.  So libvtkPythonInterpreter is the only one linking to libpython. So I think maybe if a user never uses the libvtkPythonInterpreter or if that is never explicitly imported or linked to in any Python code, we may be fine with these wheels.  Also I see that none of the Python extension modules, i.e. all the vtk*Python.so do not link to libpython or to libvtkPythonInterpreter.

Should I just ignore the libvtkPythonInterpreter and leave the PYTHON_LIBRARY field blank?

I would appreciate your thoughts on this matter. Thanks!

cheers,

Prabhu


_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: vtk-8.2.0-rc2 problem building wheels

Sebastien Jourdain via vtk-developers
On Tue, Nov 27, 2018 at 19:02:14 -0500, Jean-Christophe Fillion-Robin wrote:
> > Should I just ignore the libvtkPythonInterpreter and leave the
> > PYTHON_LIBRARY field blank?

It being blank will cause `find_library` to try and find it itself. I
think the error is that now an empty linker script is not valid, but I'm
not sure. Is it possible to use an older compiler/linker to make the
wheels?

> It looks like introducing an option to conditionally build
> vtkPythonInterpreter would address the issue here.
>
> @Ben: What do you think ?

How would this work? If it is disabled, `vtkWrappingPythonCore` (a
dependency of all wrapped Python modules) and `vtkRenderingMatplotlib`
can't be built.

For the record, the new module system's way of doing this is here:

    https://gitlab.kitware.com/ben.boeckel/vtk/blob/new-cmake-module/Utilities/Python/CMakeLists.txt#L118

Basically, the `VTK::Python` module contains the library for
executables, but is just the magic flags to ignore the missing symbols
when linking libraries. So, it should "just work", but it does require
3.13+ for `target_link_options` (without it, direct linking is used). It
can't really be backported because the old module system is quite
allergic to CMake's target stuff (since it was designed basically as the
same thing, but predate's CMake's logic).

--Ben
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: vtk-8.2.0-rc2 problem building wheels

Prabhu Ramachandran-3
On 11/28/18 11:00 AM, Ben Boeckel wrote:
On Tue, Nov 27, 2018 at 19:02:14 -0500, Jean-Christophe Fillion-Robin wrote:
Should I just ignore the libvtkPythonInterpreter and leave the
PYTHON_LIBRARY field blank?
It being blank will cause `find_library` to try and find it itself. I
think the error is that now an empty linker script is not valid, but I'm
not sure. Is it possible to use an older compiler/linker to make the
wheels?

Unfortunately, that is a pain, so no, I don't think it would be convenient or meaningful to expect an older compiler/linker.


      
It looks like introducing an option to conditionally build
vtkPythonInterpreter would address the issue here.

@Ben: What do you think ?
How would this work? If it is disabled, `vtkWrappingPythonCore` (a
dependency of all wrapped Python modules) and `vtkRenderingMatplotlib`
can't be built.

Does vtkWrappingPythonCore depend on vtkPythonInterpreter?  vtkWrappingPythonCore does not link to libpython or to vtkPythonInterpreter.  The wheels by default do not seem to enable vtkRenderingMatplotlib although I am not sure if it should be added by default in the future, in which case this is a more serious issue.

For the record, the new module system's way of doing this is here:

    https://gitlab.kitware.com/ben.boeckel/vtk/blob/new-cmake-module/Utilities/Python/CMakeLists.txt#L118

Basically, the `VTK::Python` module contains the library for
executables, but is just the magic flags to ignore the missing symbols
when linking libraries. So, it should "just work", but it does require
3.13+ for `target_link_options` (without it, direct linking is used). It
can't really be backported because the old module system is quite
allergic to CMake's target stuff (since it was designed basically as the
same thing, but predate's CMake's logic).

I think requiring a newer CMake is fairly reasonable if that will solve the problem unless I am missing something obvious here (which is very likely).

cheers,

Prabhu



--Ben



_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: vtk-8.2.0-rc2 problem building wheels

Sebastien Jourdain via vtk-developers
On Wed, Nov 28, 2018 at 15:06:43 -0500, Prabhu Ramachandran wrote:
> Does vtkWrappingPythonCore depend on vtkPythonInterpreter? 
> vtkWrappingPythonCore does not link to libpython or to vtkPythonInterpreter. 

My bad, it's a COMPILE_DEPENDS. Though why that is isn't obvious.
Removing that dependency and then disabling the module should work.

> The wheels by default do not seem to enable vtkRenderingMatplotlib although I am
> not sure if it should be added by default in the future, in which case this is a
> more serious issue.

IMO, distribution builds should try to enable *everything* so there
isn't a question of "what do I have today?" when doing `import vtk`.
Keeping it disabled for 8.2 sounds fine, but 9.x will make it easier to
not have the linking problem.

--Ben
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: vtk-8.2.0-rc2 problem building wheels

Sebastien Jourdain via vtk-developers
I think the dependency comes in for the sake of VTK's python tests, as those call vtkpython.
Prabu can you try this and see if you get a pip worthy build out of it?
Note you must set these two configure flags to get it to compile.
BUILD_TESTING:BOOL=OFF
VTK_ENABLE_VTKPYTHON:BOOL=OFF
Longer term we should do a clean break by trying to move the bits of VTKPYTHON that remain in Wrapping/PythonCore to Utilities/PythonInterpretter and making sure the python tests agree.

Regarding matplotlib, for the moment just leave it off in the wheels. Ben's point of wrapping everything and the corrolary that VTK's modules should be atomic (options don't change the library internals) is an important goal that we won't fix in 8.2. We made strides in VTK 6.0 and will get much closer in 9.0, hopefully packagers and all consumers of VTK will have a better time of it after 9.

David E DeMarle
Kitware, Inc.
Principal Engineer
21 Corporate Drive
Clifton Park, NY 12065-8662
Phone: 518-881-4909



On Wed, Nov 28, 2018 at 3:44 PM Ben Boeckel via vtk-developers <[hidden email]> wrote:
On Wed, Nov 28, 2018 at 15:06:43 -0500, Prabhu Ramachandran wrote:
> Does vtkWrappingPythonCore depend on vtkPythonInterpreter? 
> vtkWrappingPythonCore does not link to libpython or to vtkPythonInterpreter. 

My bad, it's a COMPILE_DEPENDS. Though why that is isn't obvious.
Removing that dependency and then disabling the module should work.

> The wheels by default do not seem to enable vtkRenderingMatplotlib although I am
> not sure if it should be added by default in the future, in which case this is a
> more serious issue.

IMO, distribution builds should try to enable *everything* so there
isn't a question of "what do I have today?" when doing `import vtk`.
Keeping it disabled for 8.2 sounds fine, but 9.x will make it easier to
not have the linking problem.

--Ben
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers


_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: vtk-8.2.0-rc2 problem building wheels

Sebastien Jourdain via vtk-developers
[hidden email] [hidden email] FYI the python path improvments that you all did appear to have unintended consequences on wheel building for VTK.

David E DeMarle
Kitware, Inc.
Principal Engineer
21 Corporate Drive
Clifton Park, NY 12065-8662
Phone: 518-881-4909


On Thu, Dec 6, 2018 at 11:48 AM David E DeMarle <[hidden email]> wrote:
I think the dependency comes in for the sake of VTK's python tests, as those call vtkpython.
Prabu can you try this and see if you get a pip worthy build out of it?
Note you must set these two configure flags to get it to compile.
BUILD_TESTING:BOOL=OFF
VTK_ENABLE_VTKPYTHON:BOOL=OFF
Longer term we should do a clean break by trying to move the bits of VTKPYTHON that remain in Wrapping/PythonCore to Utilities/PythonInterpretter and making sure the python tests agree.

Regarding matplotlib, for the moment just leave it off in the wheels. Ben's point of wrapping everything and the corrolary that VTK's modules should be atomic (options don't change the library internals) is an important goal that we won't fix in 8.2. We made strides in VTK 6.0 and will get much closer in 9.0, hopefully packagers and all consumers of VTK will have a better time of it after 9.

David E DeMarle
Kitware, Inc.
Principal Engineer
21 Corporate Drive
Clifton Park, NY 12065-8662
Phone: 518-881-4909



On Wed, Nov 28, 2018 at 3:44 PM Ben Boeckel via vtk-developers <[hidden email]> wrote:
On Wed, Nov 28, 2018 at 15:06:43 -0500, Prabhu Ramachandran wrote:
> Does vtkWrappingPythonCore depend on vtkPythonInterpreter? 
> vtkWrappingPythonCore does not link to libpython or to vtkPythonInterpreter. 

My bad, it's a COMPILE_DEPENDS. Though why that is isn't obvious.
Removing that dependency and then disabling the module should work.

> The wheels by default do not seem to enable vtkRenderingMatplotlib although I am
> not sure if it should be added by default in the future, in which case this is a
> more serious issue.

IMO, distribution builds should try to enable *everything* so there
isn't a question of "what do I have today?" when doing `import vtk`.
Keeping it disabled for 8.2 sounds fine, but 9.x will make it easier to
not have the linking problem.

--Ben
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers


_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: vtk-8.2.0-rc2 problem building wheels

Sebastien Jourdain via vtk-developers
In reply to this post by Sebastien Jourdain via vtk-developers
> VTK_ENABLE_VTKPYTHON:BOOL=OFF

As a side note, would it make sense to renamed this option to VTK_BUILD_VTKPYTHON_EXECUTABLE (or VTK_ENABLE_VTKPYTHON_EXECUTABLE)

> distribution builds should try to enable *everything*


In that context, I don't think it makes sense to build VTK own python interpreter. Ideally, it should be possible to run VTK test with or without it.

I also envision we could have a "vtk-openvr" python module but it wouldn't be available by default when installing vtk wheel. To support this level of "modularity" , we would continue building part of VTK independently (for example, we can already do this with the VTKOpenVR module)





On Thu, Dec 6, 2018 at 11:48 AM David E DeMarle <[hidden email]> wrote:
I think the dependency comes in for the sake of VTK's python tests, as those call vtkpython.
Prabu can you try this and see if you get a pip worthy build out of it?
Note you must set these two configure flags to get it to compile.
BUILD_TESTING:BOOL=OFF
VTK_ENABLE_VTKPYTHON:BOOL=OFF
Longer term we should do a clean break by trying to move the bits of VTKPYTHON that remain in Wrapping/PythonCore to Utilities/PythonInterpretter and making sure the python tests agree.

Regarding matplotlib, for the moment just leave it off in the wheels. Ben's point of wrapping everything and the corrolary that VTK's modules should be atomic (options don't change the library internals) is an important goal that we won't fix in 8.2. We made strides in VTK 6.0 and will get much closer in 9.0, hopefully packagers and all consumers of VTK will have a better time of it after 9.

David E DeMarle
Kitware, Inc.
Principal Engineer
21 Corporate Drive
Clifton Park, NY 12065-8662
Phone: 518-881-4909



On Wed, Nov 28, 2018 at 3:44 PM Ben Boeckel via vtk-developers <[hidden email]> wrote:
On Wed, Nov 28, 2018 at 15:06:43 -0500, Prabhu Ramachandran wrote:
> Does vtkWrappingPythonCore depend on vtkPythonInterpreter? 
> vtkWrappingPythonCore does not link to libpython or to vtkPythonInterpreter. 

My bad, it's a COMPILE_DEPENDS. Though why that is isn't obvious.
Removing that dependency and then disabling the module should work.

> The wheels by default do not seem to enable vtkRenderingMatplotlib although I am
> not sure if it should be added by default in the future, in which case this is a
> more serious issue.

IMO, distribution builds should try to enable *everything* so there
isn't a question of "what do I have today?" when doing `import vtk`.
Keeping it disabled for 8.2 sounds fine, but 9.x will make it easier to
not have the linking problem.

--Ben
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers


_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: vtk-8.2.0-rc2 problem building wheels

Sebastien Jourdain via vtk-developers
On Fri, Dec 07, 2018 at 17:00:20 -0500, Jean-Christophe Fillion-Robin wrote:
> > distribution builds should try to enable *everything*
>
> In that context, I don't think it makes sense to build VTK own python
> interpreter. Ideally, it should be possible to run VTK test with or without
> it.

Yes, it is very odd that we've always built our own interpreter. IMO,
it'd be better to just set PYTHONPATH on the test suite and use a
standard Python executable.

> I also envision we could have a "vtk-openvr" python module but it wouldn't
> be available by default when installing vtk wheel. To support this level of
> "modularity" , we would continue building part of VTK independently (for
> example, we can already do this with the VTKOpenVR module)

The new module system doesn't support this at the moment (i.e., the
`find_package(VTK)` code has been ripped out of the OpenVR module). The
problem with this is that if a module can be built inside and outside of
VTK, dependent projects cannot know whether to do `find_package(VTK
COMPONENTS RenderingOpenVR)` or `find_package(VTKOpenVR)`.

--Ben
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: vtk-8.2.0-rc2 problem building wheels

Sebastien Jourdain via vtk-developers
> it'd be better to just set PYTHONPATH on the test suite and use a standard Python executable.

+1


> The new module system doesn't support this at the moment [...] The problem with this is that if a module can be built inside and outside of VTK

I see. That shouldn't be a problem, we can always do the following:

if(NOT VTK_SOURCE_DIR)
  // find method 1
else()
  // find method 2
endif()



1. Looking at the "new-cmake-module" branch, are the "vtkLocal" and "vtkMy" examples up-to-date ?


3. To test the updated build system with Slicer, would it be possible to rebase the branch "new-cmake-module" ?

Thanks
Jc





On Mon, Dec 10, 2018 at 10:07 AM Ben Boeckel <[hidden email]> wrote:
On Fri, Dec 07, 2018 at 17:00:20 -0500, Jean-Christophe Fillion-Robin wrote:
> > distribution builds should try to enable *everything*
>
> In that context, I don't think it makes sense to build VTK own python
> interpreter. Ideally, it should be possible to run VTK test with or without
> it.

Yes, it is very odd that we've always built our own interpreter. IMO,
it'd be better to just set PYTHONPATH on the test suite and use a
standard Python executable.

> I also envision we could have a "vtk-openvr" python module but it wouldn't
> be available by default when installing vtk wheel. To support this level of
> "modularity" , we would continue building part of VTK independently (for
> example, we can already do this with the VTKOpenVR module)

The new module system doesn't support this at the moment (i.e., the
`find_package(VTK)` code has been ripped out of the OpenVR module). The
problem with this is that if a module can be built inside and outside of
VTK, dependent projects cannot know whether to do `find_package(VTK
COMPONENTS RenderingOpenVR)` or `find_package(VTKOpenVR)`.

--Ben

_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: vtk-8.2.0-rc2 problem building wheels

Sebastien Jourdain via vtk-developers
On Tue, Dec 11, 2018 at 18:08:12 -0500, Jean-Christophe Fillion-Robin wrote:
> > The new module system doesn't support this at the moment [...] The
> > problem with this is that if a module can be built inside and outside of VTK
>
> I see. That shouldn't be a problem, we can always do the following:

You trimmed the important part of that. It isn't impossible. But the
better question is "should we?".

> if(NOT VTK_SOURCE_DIR)
>   // find method 1
> else()
>   // find method 2
> endif()

The thing is that the module is incompatible from a consumer point of
view in the two builds. One makes `VTK::RenderingOpenVR` and is provided
via `find_package(VTK COMPONENTS RenderingOpenVR)`. The other…isn't.
IMO, if the module can't be built as part of VTK often enough to be
expected in a VTK build (e.g., pip or a Linux package), then it should
just be in a separate repository.

Also, the amount of code in that first one is not trivial:

    find_package(VTK COMPONENTS CommonCore IOImage ...)
    vtk_module_scan(...)
    vtk_module_build(...)
    vtk_module_wrap_python(...)

and that still isn't going to work because it will need to detect when
the module system is including the CMakeLists.txt file (or it just needs
to be a separate directory completely). Would this outside-VTK build
expect to put its Python module under `vtkmodules` still? Either way, I
really don't want modules doing this kind of stuff. Either it's part of
VTK or it isn't.

> 1. Looking at the "new-cmake-module" branch, are the "vtkLocal" and "vtkMy"
> examples up-to-date ?

I haven't looked at them yet. Other examples have been updated though.

> https://gitlab.kitware.com/ben.boeckel/vtk/blob/new-cmake-module/Examples/Build/vtkLocal/CMakeLists.txt
> https://gitlab.kitware.com/ben.boeckel/vtk/blob/new-cmake-module/Examples/Build/vtkMy/CMakeLists.txt
>
> 2. Are the functions vtk_module_add_module, vtk_module_link, ... designed
> to be used outside of VTK build tree ?

Yes. VTK has no special inside-variable control over the code.
Everything is passed in via parameters to the functions, so VTK's
install directory variables are just parameters to this function. I'd
like to keep it that way.

> 3. To test the updated build system with Slicer, would it be possible to
> rebase the branch "new-cmake-module" ?

I've just rebased it today actually (for a ParaView rebase). Does Slicer
make VTK modules itself or just consume them?

--Ben
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: vtk-8.2.0-rc2 problem building wheels

Sebastien Jourdain via vtk-developers

>> module can be built inside and outside of VTK
> But the better question is "should we?".

Generally speaking yes, supporting the composition of CMake projects by both (1) "aggregation" (add_subdirectory)  and (2) "finding dependency" is important.

This applies to first class modules (e.g VTK module, ITK module, ... ) and also third-party project (e.g zlib, hdf5, ...)

Also, by having no differences between the build system of "module or project" that can be either be aggregated or built as dependency, it is easy to maintain and communicate a set of best practices, streamline use of projects in various scenarios without having to maintain forks.



> The thing is that the module is incompatible from a consumer point of view in the two builds. One makes `VTK::RenderingOpenVR` and is provided via `find_package(VTK COMPONENTS RenderingOpenVR)`.

I think this is something that we probably need to address in CMake. In the mean time, we could have a "vtk_ns" and vtk_ns_colon variables and reference target with "${vtk_ns_colon}RenderingOpenVR". Also wondering if introducing some new genex could also help.


> Would this outside-VTK build expect to put its Python module under `vtkmodules` still?

Good question. When building wheels, the answer is yes. That way, the content of all "VTK" wheels would end up in the same folder "vtkmodules" and shared library are in the same directory. Installing/uninstalling wheels also work well because pip has a manifest and only removes the files it needs to.




>> Are the functions vtk_module_add_module, vtk_module_link, ... designed to be used outside of VTK build tree ?

> Yes. VTK has no special inside-variable control over the code. Everything is passed in via parameters to the functions

This is great. This means that these functions could be used inside and outside VTK.

> I've just rebased it today actually

Great.


> Does Slicer make VTK modules itself or just consume them?

Almost both.

Consume them: yes

For the "make VTK modules", there are few cases:
- reuse of VTK wrapping infrastructure through Slicer/CMake/vtkMacroKitPythonWrap.cmake
- we have a lot of "pure" VTK library that currently use their own build system. In the future, it would be nice to standardize on reusing VTK build system.
- building of VTK/Rendering/OpenVR into a Slicer extension.







On Tue, Dec 11, 2018 at 6:55 PM Ben Boeckel <[hidden email]> wrote:
On Tue, Dec 11, 2018 at 18:08:12 -0500, Jean-Christophe Fillion-Robin wrote:
> > The new module system doesn't support this at the moment [...] The
> > problem with this is that if a module can be built inside and outside of VTK
>
> I see. That shouldn't be a problem, we can always do the following:

You trimmed the important part of that. It isn't impossible. But the
better question is "should we?".

> if(NOT VTK_SOURCE_DIR)
>   // find method 1
> else()
>   // find method 2
> endif()

The thing is that the module is incompatible from a consumer point of
view in the two builds. One makes `VTK::RenderingOpenVR` and is provided
via `find_package(VTK COMPONENTS RenderingOpenVR)`. The other…isn't.
IMO, if the module can't be built as part of VTK often enough to be
expected in a VTK build (e.g., pip or a Linux package), then it should
just be in a separate repository.

Also, the amount of code in that first one is not trivial:

    find_package(VTK COMPONENTS CommonCore IOImage ...)
    vtk_module_scan(...)
    vtk_module_build(...)
    vtk_module_wrap_python(...)

and that still isn't going to work because it will need to detect when
the module system is including the CMakeLists.txt file (or it just needs
to be a separate directory completely). Would this outside-VTK build
expect to put its Python module under `vtkmodules` still? Either way, I
really don't want modules doing this kind of stuff. Either it's part of
VTK or it isn't.

> 1. Looking at the "new-cmake-module" branch, are the "vtkLocal" and "vtkMy"
> examples up-to-date ?

I haven't looked at them yet. Other examples have been updated though.

> https://gitlab.kitware.com/ben.boeckel/vtk/blob/new-cmake-module/Examples/Build/vtkLocal/CMakeLists.txt
> https://gitlab.kitware.com/ben.boeckel/vtk/blob/new-cmake-module/Examples/Build/vtkMy/CMakeLists.txt
>
> 2. Are the functions vtk_module_add_module, vtk_module_link, ... designed
> to be used outside of VTK build tree ?

Yes. VTK has no special inside-variable control over the code.
Everything is passed in via parameters to the functions, so VTK's
install directory variables are just parameters to this function. I'd
like to keep it that way.

> 3. To test the updated build system with Slicer, would it be possible to
> rebase the branch "new-cmake-module" ?

I've just rebased it today actually (for a ParaView rebase). Does Slicer
make VTK modules itself or just consume them?

--Ben

_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: vtk-8.2.0-rc2 problem building wheels

Sebastien Jourdain via vtk-developers
On Wed, Dec 12, 2018 at 14:35:37 -0500, Jean-Christophe Fillion-Robin wrote:
> >> module can be built inside and outside of VTK
> > But the better question is "should we?".
>
> Generally speaking yes, supporting the composition of CMake projects by
> both (1) "aggregation" (add_subdirectory)  and (2) "finding dependency" is
> important.

I'm not seeing a rationale for this though. "Because it's important"
seems awfully tautological to me. VTK and other projects we maintain are
the only ones I'm aware of which support this kind of stuff. I don't see
it in the wider FOSS ecosystem. A similar situation happens nginx adding
modules to build by extracting into a certain subdirectory, but then
again, that's the *only* supported way of building nginx modules.

> This applies to first class modules (e.g VTK module, ITK module, ... ) and
> also third-party project (e.g zlib, hdf5, ...)

Third party projects are different. It's either given an already built
copy or it builds its own. It is done, basically, because Windows. If it
weren't for that, I'd be really partial to just say "use apt or brew".
Even there, the third party projects that get built as part of VTK are
pretty set-in-stone. I've removed all user-facing variables from them.
For example, if one wants an MPI-aware HDF5, it must be provided
externally; VTK's hdf5 is never MPI-aware. Similarly, VTK's png has been
stripped of its vectorized code (due to it simplifying the build by
getting rid of lots of try_compile checks), and other projects only
build what VTK needs from it.

> Also, by having no differences between the build system of "module or
> project" that can be either be aggregated or built as dependency, it is
> easy to maintain and communicate a set of best practices, streamline use of
> projects in various scenarios without having to maintain forks.

There's always going to be a difference. I'd rather just document how to
use VTK via `find_package(VTK)`.

> > The thing is that the module is incompatible from a consumer point of
> view in the two builds. One makes `VTK::RenderingOpenVR` and is provided
> via `find_package(VTK COMPONENTS RenderingOpenVR)`.
>
> I think this is something that we probably need to address in CMake. In the
> mean time, we could have a "vtk_ns" and vtk_ns_colon variables and
> reference target with "${vtk_ns_colon}RenderingOpenVR". Also wondering if
> introducing some new genex could also help.

This sounds super hacky to me. The module system certainly isn't going
to support it; it's complex enough as it is to deal with namespaces.
This is the best I can see right now:

    if (VTK_HAS_OPENVR) # variable provided by the user
      find_package(VTK REQUIRED COMPONENTS RenderingOpenVR)
      set(openvr_target VTK::RenderingOpenVR)
    else ()
      find_package(VTKRenderingOpenVR REQUIRED)
      # It really shouldn't provide VTK::RenderingOpenVR.
      set(openvr_target VTKRenderingOpenVR)
    endif ()

> > Would this outside-VTK build expect to put its Python module under
> `vtkmodules` still?
>
> Good question. When building wheels, the answer is yes. That way, the
> content of all "VTK" wheels would end up in the same folder "vtkmodules"
> and shared library are in the same directory. Installing/uninstalling
> wheels also work well because pip has a manifest and only removes the files
> it needs to.

The problem is that this breaks `vtkmodules.all.vtkOpenVROverlay`
because `vtkmodules.all` only includes modules built by VTK itself. It
means that we really shouldn't provide it at all if users have to know
the module name anyways.

> > Does Slicer make VTK modules itself or just consume them?
>
> Almost both.
>
> Consume them: yes
>
> For the "make VTK modules", there are few cases:
> - reuse of VTK wrapping infrastructure through
> Slicer/CMake/vtkMacroKitPythonWrap.cmake
> <https://github.com/Slicer/Slicer/blob/master/CMake/vtkMacroKitPythonWrap.cmake>

The new wrapping code assumes VTK modules (i.e., built via
`vtk_module_scan` and `vtk_module_build`). If the relevant properties
aren't set on the target to be wrapped, they'll not do the right things.

> - we have a lot of "pure" VTK library that currently use their own build
> system. In the future, it would be nice to standardize on reusing VTK build
> system.

That should be doable.

> - building of VTK/Rendering/OpenVR into a Slicer extension.

Why is it not just used as a dependency like this?

    find_package(VTK COMPONENTS RenderingOpenVR)
    if (VTK_RenderingOpenVR_FOUND)
      add_subdirectory(openvr_ext)
    endif ()

--Ben
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: vtk-8.2.0-rc2 problem building wheels

Sebastien Jourdain via vtk-developers
I am going to just respond to the comments about 'aggregation ' and 'finding'

On Wed, Dec 12, 2018 at 4:15 PM Ben Boeckel via vtk-developers
<[hidden email]> wrote:

>
> On Wed, Dec 12, 2018 at 14:35:37 -0500, Jean-Christophe Fillion-Robin wrote:
> > >> module can be built inside and outside of VTK
> > > But the better question is "should we?".
> >
> > Generally speaking yes, supporting the composition of CMake projects by
> > both (1) "aggregation" (add_subdirectory)  and (2) "finding dependency" is
> > important.
>
> I'm not seeing a rationale for this though. "Because it's important"
> seems awfully tautological to me. VTK and other projects we maintain are
> the only ones I'm aware of which support this kind of stuff. I don't see
> it in the wider FOSS ecosystem. A similar situation happens nginx adding
> modules to build by extracting into a certain subdirectory, but then
> again, that's the *only* supported way of building nginx modules.
>

This is common in the wild ( see catch2 ) to such a degree that CMake
now has FetchContent which explicitly supports the workflow of using
find_package and than falling back to add_subdirectory. I expect that
this workflow will increase as FetchContent becomes more wildly
available.




> > This applies to first class modules (e.g VTK module, ITK module, ... ) and
> > also third-party project (e.g zlib, hdf5, ...)
>
> Third party projects are different. It's either given an already built
> copy or it builds its own. It is done, basically, because Windows. If it
> weren't for that, I'd be really partial to just say "use apt or brew".
> Even there, the third party projects that get built as part of VTK are
> pretty set-in-stone. I've removed all user-facing variables from them.
> For example, if one wants an MPI-aware HDF5, it must be provided
> externally; VTK's hdf5 is never MPI-aware. Similarly, VTK's png has been
> stripped of its vectorized code (due to it simplifying the build by
> getting rid of lots of try_compile checks), and other projects only
> build what VTK needs from it.
>
> > Also, by having no differences between the build system of "module or
> > project" that can be either be aggregated or built as dependency, it is
> > easy to maintain and communicate a set of best practices, streamline use of
> > projects in various scenarios without having to maintain forks.
>
> There's always going to be a difference. I'd rather just document how to
> use VTK via `find_package(VTK)`.
>
> > > The thing is that the module is incompatible from a consumer point of
> > view in the two builds. One makes `VTK::RenderingOpenVR` and is provided
> > via `find_package(VTK COMPONENTS RenderingOpenVR)`.
> >
> > I think this is something that we probably need to address in CMake. In the
> > mean time, we could have a "vtk_ns" and vtk_ns_colon variables and
> > reference target with "${vtk_ns_colon}RenderingOpenVR". Also wondering if
> > introducing some new genex could also help.
>
> This sounds super hacky to me. The module system certainly isn't going
> to support it; it's complex enough as it is to deal with namespaces.
> This is the best I can see right now:
>
>     if (VTK_HAS_OPENVR) # variable provided by the user
>       find_package(VTK REQUIRED COMPONENTS RenderingOpenVR)
>       set(openvr_target VTK::RenderingOpenVR)
>     else ()
>       find_package(VTKRenderingOpenVR REQUIRED)
>       # It really shouldn't provide VTK::RenderingOpenVR.
>       set(openvr_target VTKRenderingOpenVR)
>     endif ()
>
> > > Would this outside-VTK build expect to put its Python module under
> > `vtkmodules` still?
> >
> > Good question. When building wheels, the answer is yes. That way, the
> > content of all "VTK" wheels would end up in the same folder "vtkmodules"
> > and shared library are in the same directory. Installing/uninstalling
> > wheels also work well because pip has a manifest and only removes the files
> > it needs to.
>
> The problem is that this breaks `vtkmodules.all.vtkOpenVROverlay`
> because `vtkmodules.all` only includes modules built by VTK itself. It
> means that we really shouldn't provide it at all if users have to know
> the module name anyways.
>
> > > Does Slicer make VTK modules itself or just consume them?
> >
> > Almost both.
> >
> > Consume them: yes
> >
> > For the "make VTK modules", there are few cases:
> > - reuse of VTK wrapping infrastructure through
> > Slicer/CMake/vtkMacroKitPythonWrap.cmake
> > <https://github.com/Slicer/Slicer/blob/master/CMake/vtkMacroKitPythonWrap.cmake>
>
> The new wrapping code assumes VTK modules (i.e., built via
> `vtk_module_scan` and `vtk_module_build`). If the relevant properties
> aren't set on the target to be wrapped, they'll not do the right things.
>
> > - we have a lot of "pure" VTK library that currently use their own build
> > system. In the future, it would be nice to standardize on reusing VTK build
> > system.
>
> That should be doable.
>
> > - building of VTK/Rendering/OpenVR into a Slicer extension.
>
> Why is it not just used as a dependency like this?
>
>     find_package(VTK COMPONENTS RenderingOpenVR)
>     if (VTK_RenderingOpenVR_FOUND)
>       add_subdirectory(openvr_ext)
>     endif ()
>
> --Ben
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
>
> Search the list archives at: http://markmail.org/search/?q=vtk-developers
>
> Follow this link to subscribe/unsubscribe:
> https://public.kitware.com/mailman/listinfo/vtk-developers
>
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: vtk-8.2.0-rc2 problem building wheels

Sebastien Jourdain via vtk-developers
On Wed, Dec 12, 2018 at 16:34:30 -0500, Robert Maynard wrote:
> This is common in the wild ( see catch2 ) to such a degree that CMake
> now has FetchContent which explicitly supports the workflow of using
> find_package and than falling back to add_subdirectory. I expect that
> this workflow will increase as FetchContent becomes more wildly
> available.

OK, so I do remember googletest also doing stuff like this. But these
are test dependencies. Are there any that you can think of that do this
for things like vtkzlib? Though there I suspect that no one does the
proper mangling that needs to be done…

Also, that's more like remote modules than what is being requested here.
If that's the case, maybe RenderingOpenVR should just become a remote
module instead?

--Ben
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: vtk-8.2.0-rc2 problem building wheels

Sebastien Jourdain via vtk-developers

> I'm not seeing a rationale

By ensuring the build system of a VTK/ITK/Slicer... modules is the same if it is built inside or outside the "main" application/libraries allows the following:

- avoid special case, tribal knowledge, ... : a module layout is consistent

- support for packaging independently some part


@Matt: Could you share your experience within the ITK community ?



> > > Would this outside-VTK build expect to put its Python module under `vtkmodules` still?
> > Good question. When building wheels, the answer is yes. [...]
> The problem is that this breaks `vtkmodules.all.vtkOpenVROverlay` because `vtkmodules.all` only includes modules built by VTK itself.
> It means that we really shouldn't provide it at all if users have to know the module name anyways.

In a nutshell, python support the concept of "entry_points". Each time a vtk wheel associated with a module is installed, the namespace "vtk.modules" could automatically be updated with a new entry (e.g "vtk.modules.vtkopenvr"). Then, in the "all.py" we would make use of "pkg_resources.iter_entry_points" to import the associated classes.

As similar approach was adopted by Girder, see here



> The new wrapping code assumes VTK modules (i.e., built via `vtk_module_scan` and `vtk_module_build`). If the relevant properties aren't set on the target to be wrapped, they'll not do the right things.

I see. By copying the relevant CMake functions from VTK 8.2, we should be able to keep things working.



> maybe RenderingOpenVR should just become a remote module instead?

Since adding a remote module simply means download the source, then call "add_subdirectory()" with two parameters. Unless a VTK module can be implemented so that it can be built inside and outside of the source tree, I don't think it will be supported.




> > - building of VTK/Rendering/OpenVR into a Slicer extension.

> Why is it not just used as a dependency like this?
>    find_package(VTK COMPONENTS RenderingOpenVR)
>    if (VTK_RenderingOpenVR_FOUND)
>      add_subdirectory(openvr_ext)
>    endif ()

Because:
* In order to reuse the install rules of the module and package it, we need the associated build tree.
* The build of VTK in Slicer proper has no dependency on openvr external project, this is lever to the SlicerOpenVR extension. See "SuperBuild" folder in https://github.com/KitwareMedical/SlicerVirtualReality






On Wed, Dec 12, 2018 at 5:17 PM Ben Boeckel <[hidden email]> wrote:
On Wed, Dec 12, 2018 at 16:34:30 -0500, Robert Maynard wrote:
> This is common in the wild ( see catch2 ) to such a degree that CMake
> now has FetchContent which explicitly supports the workflow of using
> find_package and than falling back to add_subdirectory. I expect that
> this workflow will increase as FetchContent becomes more wildly
> available.

OK, so I do remember googletest also doing stuff like this. But these
are test dependencies. Are there any that you can think of that do this
for things like vtkzlib? Though there I suspect that no one does the
proper mangling that needs to be done…

Also, that's more like remote modules than what is being requested here.
If that's the case, maybe RenderingOpenVR should just become a remote
module instead?

--Ben

_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: vtk-8.2.0-rc2 problem building wheels

Sebastien Jourdain via vtk-developers


On Wed, Dec 12, 2018 at 7:08 PM Jean-Christophe Fillion-Robin <[hidden email]> wrote:

> I'm not seeing a rationale

By ensuring the build system of a VTK/ITK/Slicer... modules is the same if it is built inside or outside the "main" application/libraries allows the following:

- avoid special case, tribal knowledge, ... : a module layout is consistent

- support for packaging independently some part


@Matt: Could you share your experience within the ITK community ?


We have seen success in the ITK community by maintaining a robust set of common functionality in the core repository and enabling extension modules in other repositories. This provides:

- Flexibility to mature a module in an incremental process.
- More contrained core repository build times and distributed builds of other modules.
- Enhanced backwards compatibility for the core and rapid changes in the peripherary.
- Enhanced participation from the huge talent pool in the broader FOSS ecosystem.
- Control and a sense of ownership on individual module repositories.

If the new module system for VTK improves the extensitibility potential of modules, it will be a big enhancement.

Ideally, we would have C++XX standard modules and a way to bulid them in place along with One Package Manager To Rule Them All. Since these are currently not available, the ability to share and extend VTK code is a good step. A bonus step forward would allow dependencies between ITK and VTK modules. 


These are all good discussion topics, but to come back to the thread topic, which is important to massively improve VTK's value by enabling integration with other Python packages: thanks to Dave for starting a patch. I wonder we can set a warning / error that BUILD_TESTING has to be disabled when VTK_ENABLE_VTKPYTHON is disabled, change the patch from a WIP, and use VTK_ENABLE_VTKPYTHON=OFF in the VTK Python wheel build. In the future, it would be helpful to have VTK_ENABLE_VTKPYTHON OFF by default and use a provided Python.

2 cents,
Matt

_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: vtk-8.2.0-rc2 problem building wheels

Sebastien Jourdain via vtk-developers
In reply to this post by Sebastien Jourdain via vtk-developers
On Wed, Dec 12, 2018 at 19:08:30 -0500, Jean-Christophe Fillion-Robin wrote:
> > I'm not seeing a rationale
>
> By ensuring the build system of a VTK/ITK/Slicer... modules is the same if
> it is built inside or outside the "main" application/libraries allows the
> following:

It *cannot* be the same. `find_package(VTK)` only contains the
information VTK had at the time of *its* build. Modules built outside of
it won't show up. If a module is part of VTK, VTK does lots of heavy
lifting one-time-setup things. Install directories, minimum CMake
version, CMake policies, etc. Any standalone module will have to copy
that information. I suspect that's at least 100 lines of CMake code
(some of which can't be abstracted out).

> - avoid special case, tribal knowledge, ... : a module layout is consistent

This support itself is a special case itself AFAICS. Module layouts are
consistent right now. Modules which do this are going to be the
different ones.

> - support for packaging independently some part

That's just more code that modules will have to duplicate from VTK.

> @Matt: Could you share your experience within the ITK community ?

<Matt's reply>

This seems more like remote modules as a path for migrating *into* VTK.
Building OpenVR separately is like moving it *out* of VTK. *That* is
what I don't understand.

> > > > Would this outside-VTK build expect to put its Python module under
> `vtkmodules` still?
> > > Good question. When building wheels, the answer is yes. [...]
> > The problem is that this breaks `vtkmodules.all.vtkOpenVROverlay` because
> `vtkmodules.all` only includes modules built by VTK itself.
> > It means that we really shouldn't provide it at all if users have to know
> the module name anyways.
>
> In a nutshell, python support the concept of "entry_points". Each time a
> vtk wheel associated with a module is installed, the namespace
> "vtk.modules" could automatically be updated with a new entry (e.g
> "vtk.modules.vtkopenvr"). Then, in the "all.py" we would make use of
> "pkg_resources.iter_entry_points" to import the associated classes.
>
> As similar approach was adopted by Girder, see here
> <https://github.com/girder/girder/blob/cfb58ef245384fff7f90cf4cd6c8cab94510b9f6/plugins/homepage/setup.py#L47-L51>

OK, maybe you've solved Python. Now what of CMake (`find_package`) and
Java?

> > The new wrapping code assumes VTK modules (i.e., built via
> `vtk_module_scan` and `vtk_module_build`). If the relevant properties
> aren't set on the target to be wrapped, they'll not do the right things.
>
> I see. By copying the relevant CMake functions from VTK 8.2, we should be
> able to keep things working.

What? I'm not maintaining that. The information that is used by the old
wrapping macros doesn't exist anymore either; it's all stored on
properites of the targets in which case it's basically the new wrapping
functions anyways. In addition, since PythonD libraries are now gone,
modules must import dependent modules first. Here's vtkPVCommon.py from
ParaView:

    from __future__ import absolute_import
    from . import vtkClientServer
    from vtkmodules import vtkCommonCore
    from vtkmodules import vtkIOXMLParser
    from .vtkPVCommonPython import *

If a module depends on one not using the standard macros, where the
Python bindings for the module is stored needs to be accessible somehow.

> > maybe RenderingOpenVR should just become a remote module instead?
>
> Since adding a remote module simply means download the source, then call
> "add_subdirectory()" with two parameters. Unless a VTK module can be
> implemented so that it can be built inside and outside of the source tree,
> I don't think it will be supported.

Remote modules with the new system would not be `add_subdirectory`. The
downloaded source directory is added to the `vtk_modules_find_modules`
function. What doesn't work right now is that it assumes everything is
in the source tree.

> > > - building of VTK/Rendering/OpenVR into a Slicer extension.
>
> > Why is it not just used as a dependency like this?
> >    find_package(VTK COMPONENTS RenderingOpenVR)
> >    if (VTK_RenderingOpenVR_FOUND)
> >      add_subdirectory(openvr_ext)
> >    endif ()
>
> Because:
> * In order to reuse the install rules of the module and package it, we need
> the associated build tree.

Not in general, you don't. For example, the paraview-superbuild makes
packages from the install trees of all of its projects by doing the
proper dependency traversal of the libraries. The new CMake install
rules work should make it easy for Slicer to package it up from a
separate build/install.

> * The build of VTK in Slicer proper has no dependency on openvr external
> project, this is lever to the SlicerOpenVR extension. See "SuperBuild"
> folder in https://github.com/KitwareMedical/SlicerVirtualReality

Again, this, to me, just points to OpenVR maybe being a separate
repository completely and optionally a remote module. Personally, I'd
have the superbuild turn on the OpenVR dependency in VTK if the
extension is wanted. If it isn't there, the VTK can't be used to build
the SlicerOpenVR extension.

--Ben
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers

12