New module system preview

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

New module system preview

Ben Boeckel
Hi all,

I've gotten the new module system to a point that I'd like to open up
testing to other developers. It's not 100% complete (see the known
issues listing below), but it's pretty close. I'd highly recommend using
a new build tree for building the branch.

Notes for WIP things on the branch:

  - There's a hard-coded list of "don't build this" in CMakeLists.txt
    currently just for my convenience. Feel free to edit the
    `REJECT_MODULES` argument list as needed for your machine(s).

Overview of changes:

  - Instead of `module.cmake`, there are `vtk.module` and `vtk.kit`
    files. These are basically CMake argument lists, but no variable
    expansion is allowed. If there are optional dependencies, they must
    be private dependencies. Optional public dependencies indicate that
    a new module should be made instead.

  - Modules may now be "rejected" and never built. This turns off
    *dependent* modules. If a module is required and also has a rejected
    module as a dependency, an error occurs.

  - Minimum CMake is still 3.3, but kit support requires CMake 3.12+ for
    object library features.

  - There are no more `vtk_mpi_link` or `vtk_opengl_link` "magic"
    functions. Instead, just depend on the `VTK::mpi` and `VTK::opengl`
    modules.

  - Third party modules now declare whether they support external copies
    or not (i.e., no more useless `VTK_USE_SYSTEM_kwsys` variable). See
    the `ThirdParty` directory for various examples. Other third parties
    only support external copies (e.g., `VTK::mpi` and `VTK::opengl`).

  - Imported targets are now preferred for `find_package` callers. Third
    party packages warn about this usage, but others do not. There is
    also the `vtk_module_find_package` call which also adds required
    `find_package` calls to the `vtk-config.cmake` file for external
    consumers (only necessary when using imported targets).

  - Module names now look like `VTK::CommonCore`. This is a target name
    which can be used anywhere, within or outside of the VTK source
    tree. Non-module projects now do:

```cmake
# Find VTK.
find_package(VTK REQUIRED COMPONENTS CommonCore)
# Make two programs.
add_executable(myexe1 myexe1.c)
add_executable(myexe2 myexe2.c)
# Add include directories, link line, compile definitions, etc.
# necessary to use VTK::CommonCore.
target_link_libraries(myexe1 PRIVATE VTK::CommonCore)
target_link_libraries(myexe2 PRIVATE VTK::CommonCore)
# Add VTK_AUTOINIT defines to the targets.
vtk_module_autoinit(TARGETS myexe1 myexe2 MODULES VTK::CommonCore)
```

  - To add requirements to a module, the module system offers some
    functions in lieu of CMake's `target_*`. This is in order to
    properly support kits (the `CommonCore` target is just an
    `INTERFACE` library in this case and `PRIVATE` link libraries need
    copied onto the containing kit). The functions:
    * `vtk_module_compile_options`
    * `vtk_module_definitions`
    * `vtk_module_include`
    * `vtk_module_link`
    * `vtk_module_link_options` (depends on `target_link_options` in
      CMake 3.13+)

It is currently based on `master` as of a week ago, but I'll probably
rebase again this week.

Please file issues found with the branch on my fork of VTK:

    https://gitlab.kitware.com/ben.boeckel/vtk/issues

to make things easier to track. Since Gitlab doesn't support MRs between
sibling forks (yet), please attach patches to issues if you have a fix.

Known issues:

  - Running generated Java wrappers needs some attention. Missing
    symbols (from Java code?) happen when running the tests.
  - Windows support (there are some `:` characters left in filenames
    yet).

Unimplemented:

  - Remote modules (conceptually the same as before, but there are no
    remote modules that have been ported right now)
  - `OPTIONAL_PYTHON_LINK` magic keyword. This can now just be supported
    in the `VTK::Python` module, but has not been implemented.

Untested:

  - Multi-config generators (e.g., Xcode and Visual Studio).
  - Mobile device support.

Thanks,

--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: New module system preview

Ben Boeckel
On Mon, Oct 29, 2018 at 12:07:20 -0400, Ben Boeckel wrote:
> I've gotten the new module system to a point that I'd like to open up
> testing to other developers. It's not 100% complete (see the known
> issues listing below), but it's pretty close. I'd highly recommend using
> a new build tree for building the branch.

It occurs to me that I forgot to add a link to the actual code:

    https://gitlab.kitware.com/ben.boeckel/vtk/tree/new-cmake-module

--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: New module system preview

Bill Lorensen
Will the old module system still work?

On Mon, Oct 29, 2018, 9:43 AM Ben Boeckel <[hidden email] wrote:
On Mon, Oct 29, 2018 at 12:07:20 -0400, Ben Boeckel wrote:
> I've gotten the new module system to a point that I'd like to open up
> testing to other developers. It's not 100% complete (see the known
> issues listing below), but it's pretty close. I'd highly recommend using
> a new build tree for building the branch.

It occurs to me that I forgot to add a link to the actual code:

    https://gitlab.kitware.com/ben.boeckel/vtk/tree/new-cmake-module

--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: New module system preview

Elvis Stansvik
In reply to this post by Ben Boeckel
Fantastic work Ben. One question below.

Den mån 29 okt. 2018 17:07Ben Boeckel <[hidden email]> skrev:
Hi all,

I've gotten the new module system to a point that I'd like to open up
testing to other developers. It's not 100% complete (see the known
issues listing below), but it's pretty close. I'd highly recommend using
a new build tree for building the branch.

Notes for WIP things on the branch:

  - There's a hard-coded list of "don't build this" in CMakeLists.txt
    currently just for my convenience. Feel free to edit the
    `REJECT_MODULES` argument list as needed for your machine(s).

Overview of changes:

  - Instead of `module.cmake`, there are `vtk.module` and `vtk.kit`
    files. These are basically CMake argument lists, but no variable
    expansion is allowed. If there are optional dependencies, they must
    be private dependencies. Optional public dependencies indicate that
    a new module should be made instead.

  - Modules may now be "rejected" and never built. This turns off
    *dependent* modules. If a module is required and also has a rejected
    module as a dependency, an error occurs.

  - Minimum CMake is still 3.3, but kit support requires CMake 3.12+ for
    object library features.

  - There are no more `vtk_mpi_link` or `vtk_opengl_link` "magic"
    functions. Instead, just depend on the `VTK::mpi` and `VTK::opengl`
    modules.

  - Third party modules now declare whether they support external copies
    or not (i.e., no more useless `VTK_USE_SYSTEM_kwsys` variable). See
    the `ThirdParty` directory for various examples. Other third parties
    only support external copies (e.g., `VTK::mpi` and `VTK::opengl`).

  - Imported targets are now preferred for `find_package` callers. Third
    party packages warn about this usage, but others do not. There is
    also the `vtk_module_find_package` call which also adds required
    `find_package` calls to the `vtk-config.cmake` file for external
    consumers (only necessary when using imported targets).

  - Module names now look like `VTK::CommonCore`. This is a target name
    which can be used anywhere, within or outside of the VTK source
    tree. Non-module projects now do:

```cmake
# Find VTK.
find_package(VTK REQUIRED COMPONENTS CommonCore)
# Make two programs.
add_executable(myexe1 myexe1.c)
add_executable(myexe2 myexe2.c)
# Add include directories, link line, compile definitions, etc.
# necessary to use VTK::CommonCore.
target_link_libraries(myexe1 PRIVATE VTK::CommonCore)
target_link_libraries(myexe2 PRIVATE VTK::CommonCore)
# Add VTK_AUTOINIT defines to the targets.
vtk_module_autoinit(TARGETS myexe1 myexe2 MODULES VTK::CommonCore)

Can this not be brought in automatically via the target's interface definitions?

With the current setup, I'm getting them via ${VTK_DEFINITIONS} I think.

Would be nice to avoid the repeated target name.

Elvis

```

  - To add requirements to a module, the module system offers some
    functions in lieu of CMake's `target_*`. This is in order to
    properly support kits (the `CommonCore` target is just an
    `INTERFACE` library in this case and `PRIVATE` link libraries need
    copied onto the containing kit). The functions:
    * `vtk_module_compile_options`
    * `vtk_module_definitions`
    * `vtk_module_include`
    * `vtk_module_link`
    * `vtk_module_link_options` (depends on `target_link_options` in
      CMake 3.13+)

It is currently based on `master` as of a week ago, but I'll probably
rebase again this week.

Please file issues found with the branch on my fork of VTK:

    https://gitlab.kitware.com/ben.boeckel/vtk/issues

to make things easier to track. Since Gitlab doesn't support MRs between
sibling forks (yet), please attach patches to issues if you have a fix.

Known issues:

  - Running generated Java wrappers needs some attention. Missing
    symbols (from Java code?) happen when running the tests.
  - Windows support (there are some `:` characters left in filenames
    yet).

Unimplemented:

  - Remote modules (conceptually the same as before, but there are no
    remote modules that have been ported right now)
  - `OPTIONAL_PYTHON_LINK` magic keyword. This can now just be supported
    in the `VTK::Python` module, but has not been implemented.

Untested:

  - Multi-config generators (e.g., Xcode and Visual Studio).
  - Mobile device support.

Thanks,

--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: New module system preview

Ben Boeckel
On Mon, Oct 29, 2018 at 18:13:43 +0100, Elvis Stansvik wrote:
> > # Add VTK_AUTOINIT defines to the targets.
> > vtk_module_autoinit(TARGETS myexe1 myexe2 MODULES VTK::CommonCore)
> >
>
> Can this not be brought in automatically via the target's interface
> definitions?

No, because the set of AUTOINIT defines depends on the modules you link.
Linking `VTK::ChemistryOpenGL2` and `VTK::ParallelChemistry` gives a
different AUTOINIT define than just linking `VTK::ChemistryOpenGL2`.

> With the current setup, I'm getting them via ${VTK_DEFINITIONS} I think.

This is true. Currently, it is for all modules that VTK is told to find.

> Would be nice to avoid the repeated target name.

I think instead of `MODULES`, it might be possible to instead rummage
around in the link properties of the passed targets. However, looking at
the documentation, this is not necessarily trivial to parse:

    https://cmake.org/cmake/help/latest/prop_tgt/LINK_LIBRARIES.html

I'll add a TODO item for it, but I imagine that it will not work for all
possible ways to use `target_link_libraries`.

--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: New module system preview

Ben Boeckel
In reply to this post by Bill Lorensen
On Mon, Oct 29, 2018 at 10:01:34 -0700, Bill Lorensen wrote:
> Will the old module system still work?

No, it is completely removed on the branch. This is mainly because the
old module system doesn't support imported targets at all (e.g., it
assumes that non-module dependencies are filesystem paths).

--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: New module system preview

David Thompson-2
In reply to this post by Ben Boeckel
Hi Ben,

I'm really looking forward to imported targets!

>  - Instead of `module.cmake`, there are `vtk.module` and `vtk.kit`
>    files. These are basically CMake argument lists, but no variable
>    expansion is allowed. If there are optional dependencies, they must
>    be private dependencies. Optional public dependencies indicate that
>    a new module should be made instead.

Is there a reason these things need to be separate files (vtk.module/vtk.kit) at all?

        Thanks,
        David
_______________________________________________
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: New module system preview

Bill Lorensen
In reply to this post by Ben Boeckel
Will I be able to support both old and new?

On Mon, Oct 29, 2018, 10:34 AM Ben Boeckel <[hidden email] wrote:
On Mon, Oct 29, 2018 at 10:01:34 -0700, Bill Lorensen wrote:
> Will the old module system still work?

No, it is completely removed on the branch. This is mainly because the
old module system doesn't support imported targets at all (e.g., it
assumes that non-module dependencies are filesystem paths).

--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: New module system preview

Ben Boeckel
In reply to this post by David Thompson-2
On Mon, Oct 29, 2018 at 13:57:02 -0400, David Thompson wrote:

> I'm really looking forward to imported targets!
>
> >  - Instead of `module.cmake`, there are `vtk.module` and `vtk.kit`
> >    files. These are basically CMake argument lists, but no variable
> >    expansion is allowed. If there are optional dependencies, they must
> >    be private dependencies. Optional public dependencies indicate that
> >    a new module should be made instead.
>
> Is there a reason these things need to be separate files
> (vtk.module/vtk.kit) at all?

The `vtk.kit` declares a kit. Membership into a kit is via `vtk.module`.
Example `vtk.kit`:

```
NAME
  VTK::Common
LIBRARY_NAME
  vtkCommon
DESCRIPTION
  Core utilities for VTK.
```

and `vtk.module`:

```
NAME
  VTK::CommonCore
LIBRARY_NAME
  vtkCommonCore
DESCRIPTION
  The base VTK library.
KIT
  VTK::Common
GROUPS
  StandAlone
DEPENDS
  VTK::kwiml
  VTK::vtksys
PRIVATE_DEPENDS
  VTK::utf8
TEST_DEPENDS
  VTK::CommonSystem
  VTK::CommonTransforms
  VTK::TestingCore
  VTK::vtksys
```

--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: New module system preview

Ben Boeckel
In reply to this post by Bill Lorensen
On Mon, Oct 29, 2018 at 10:57:10 -0700, Bill Lorensen wrote:
> Will I be able to support both old and new?

As a consumer of modules is much easier than as a module. The variable
names which control things are completely different (now "namespaced"
via naming conventions). `find_package(VTK)` is similar as long as
`VTK_LIBRARIES` was used and components are specified, but the old
module's CMake API is gone.

Old "is X enabled" detection (though this shouldn't be necessary in
general[1]):

```cmake
if (Module_X)
```

New:

```cmake
if (TARGET X)
```

But, the module names are also different now in the new VTK as well, so
that makes consuming harder (though predictable).

Building a library used to be:

```cmake
vtk_module_library(X ${srcs})
```

and now has a much richer API:

```cmake
vtk_module_add_module(X
  SOURCES ${srcs}
  HEADERS ${headers}
  PRIVATE_HEADERS ${private_headers})
```

though there is also `CLASSES` which does the de-facto standard
`.cxx`/`.h` pairing for `SOURCES` and `HEADERS` automatically. In
addition to the ineffectiveness of `target_*` functions on module names
due to the kits implementation, making a module which supports both APIs
is not going to be trivial.

--Ben

[1]Optional dependencies now pass `-DVTK_MODULE_ENABLE_X=` as 1 or 0 if
available where `X` is a "safe" name for the module (basically `::`
replaced by `_`). This is to promote `#if` rather than `#ifdef` usage
for optional dependency detection in C++ code which triggers errors if
present in public headers (though in consuming targets) and also
triggers warnings if the optional dependency is removed or made
non-optional (because the define is no longer present).
_______________________________________________
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: New module system preview

David Thompson-2
In reply to this post by Ben Boeckel
>>> - Instead of `module.cmake`, there are `vtk.module` and `vtk.kit`
>>>   files. These are basically CMake argument lists, but no variable
>>>   expansion is allowed. If there are optional dependencies, they must
>>>   be private dependencies. Optional public dependencies indicate that
>>>   a new module should be made instead.
>>
>> Is there a reason these things need to be separate files
>> (vtk.module/vtk.kit) at all?
>
> The `vtk.kit` declares a kit. Membership into a kit is via `vtk.module`.
> Example `vtk.kit`:

Yes, but why can't these declarations for modules and kits be inside CMakeLists.txt as function calls instead of separate files?

        David
_______________________________________________
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: New module system preview

Marcus D. Hanwell-2
On Mon, Oct 29, 2018 at 2:23 PM David Thompson <[hidden email]> wrote:
>>> - Instead of `module.cmake`, there are `vtk.module` and `vtk.kit`
>>>   files. These are basically CMake argument lists, but no variable
>>>   expansion is allowed. If there are optional dependencies, they must
>>>   be private dependencies. Optional public dependencies indicate that
>>>   a new module should be made instead.
>>
>> Is there a reason these things need to be separate files
>> (vtk.module/vtk.kit) at all?
>
> The `vtk.kit` declares a kit. Membership into a kit is via `vtk.module`.
> Example `vtk.kit`:

Yes, but why can't these declarations for modules and kits be inside CMakeLists.txt as function calls instead of separate files?

Guessing, but likely because we still use a two-pass approach - scan module/kit files, build up dep list, then actually add the things that are enabled in the second pass. 

_______________________________________________
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: New module system preview

Bill Lorensen
In reply to this post by Ben Boeckel
I will need the VTKExamples to support both api's. Also a few remote modules.

On Mon, Oct 29, 2018, 11:18 AM Ben Boeckel <[hidden email] wrote:
On Mon, Oct 29, 2018 at 10:57:10 -0700, Bill Lorensen wrote:
> Will I be able to support both old and new?

As a consumer of modules is much easier than as a module. The variable
names which control things are completely different (now "namespaced"
via naming conventions). `find_package(VTK)` is similar as long as
`VTK_LIBRARIES` was used and components are specified, but the old
module's CMake API is gone.

Old "is X enabled" detection (though this shouldn't be necessary in
general[1]):

```cmake
if (Module_X)
```

New:

```cmake
if (TARGET X)
```

But, the module names are also different now in the new VTK as well, so
that makes consuming harder (though predictable).

Building a library used to be:

```cmake
vtk_module_library(X ${srcs})
```

and now has a much richer API:

```cmake
vtk_module_add_module(X
  SOURCES ${srcs}
  HEADERS ${headers}
  PRIVATE_HEADERS ${private_headers})
```

though there is also `CLASSES` which does the de-facto standard
`.cxx`/`.h` pairing for `SOURCES` and `HEADERS` automatically. In
addition to the ineffectiveness of `target_*` functions on module names
due to the kits implementation, making a module which supports both APIs
is not going to be trivial.

--Ben

[1]Optional dependencies now pass `-DVTK_MODULE_ENABLE_X=` as 1 or 0 if
available where `X` is a "safe" name for the module (basically `::`
replaced by `_`). This is to promote `#if` rather than `#ifdef` usage
for optional dependency detection in C++ code which triggers errors if
present in public headers (though in consuming targets) and also
triggers warnings if the optional dependency is removed or made
non-optional (because the define is no longer present).

_______________________________________________
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: New module system preview

Ben Boeckel
In reply to this post by David Thompson-2
On Mon, Oct 29, 2018 at 14:23:27 -0400, David Thompson wrote:

> >>> - Instead of `module.cmake`, there are `vtk.module` and `vtk.kit`
> >>>   files. These are basically CMake argument lists, but no variable
> >>>   expansion is allowed. If there are optional dependencies, they must
> >>>   be private dependencies. Optional public dependencies indicate that
> >>>   a new module should be made instead.
> >>
> >> Is there a reason these things need to be separate files
> >> (vtk.module/vtk.kit) at all?
> >
> > The `vtk.kit` declares a kit. Membership into a kit is via `vtk.module`.
> > Example `vtk.kit`:
>
> Yes, but why can't these declarations for modules and kits be inside
> CMakeLists.txt as function calls instead of separate files?

There's still a two-pass build pipeline. First is to get the declaration
of the modules and build up the dependency graph. There is then a build
step which builds modules in dependency order. Unlike the old module
system, these steps are now distinct:

```cmake
vtk_module_find_modules(vtk_module_files ${vtk_source_directories})
vtk_module_find_kits(vtk_kit_files ${vtk_source_directories})
vtk_module_scan(
  MODULE_FILES        ${vtk_module_files}
  KIT_FILES           ${vtk_kit_files}
  REQUEST_MODULES     ${vtk_requested_modules}
  REJECT_MODULES      ${vtk_rejected_modules}
  PROVIDES_MODULES    vtk_modules
  PROVIDES_KITS       vtk_kits
  REQUIRES_MODULES    vtk_required_modules
  UNRECOGNIZED_MODULES vtk_unrecognized_modules
  WANT_BY_DEFAULT     ON
  ENABLE_TESTS        DEFAULT)

# Check to see if any modules were mentioned that we don't know how to
# build.
if (vtk_required_modules OR vtk_unrecognized_modules)
  message(FATAL_ERROR
    "The following modules were requested or required, but not found: "
    "${vtk_required_modules};${vtk_unrecognized_modules}.")
endif ()

vtk_module_build(
  MODULES             ${vtk_modules}
  KITS                ${vtk_kits}
  BUILD_WITH_KITS     ${VTK_ENABLE_KITS}
  INSTALL_EXPORT      VTK
  HEADERS_DESTINATION "include/vtk-${VTK_MAJOR_VERSION}.${VTK_MINOR_VERSION}"
  CMAKE_DESTINATION   "lib/cmake/vtk-${VTK_MAJOR_VERSION}.${VTK_MINOR_VERSION}"
  LIBRARY_NAME_SUFFIX "${VTK_MAJOR_VERSION}.${VTK_MINOR_VERSION}"
  VERSION             "${VTK_VERSION}"
  SOVERSION           "1"
  TEST_DATA_TARGET    VTKData
  TEST_INPUT_DATA_DIRECTORY   "${CMAKE_CURRENT_SOURCE_DIR}/Testing"
  TEST_OUTPUT_DATA_DIRECTORY  "${CMAKE_CURRENT_BINARY_DIR}/ExternalData/Testing")
```

The `scan` step processes dependencies found in the specified module and
kit files. This is also where we find out what modules we're actually
going to build. Once we have the list, we can then build them (assuming
that all mentioned modules actually exist). The reason for the two-phase
is to support embedding module-using projects (e.g., VTK in ParaView).

ParaView works something like this:

```cmake
vtk_module_find_modules(pv_module_files ${pv_source_directories})
vtk_module_find_kits(pv_kit_files ${pv_source_directories})
vtk_module_scan(
  MODULE_FILES        ${pv_module_files}
  KIT_FILES           ${pv_kit_files}
  REQUEST_MODULES     ${pv_requested_modules}
  REJECT_MODULES      ${pv_rejected_modules}
  PROVIDES_MODULES    pv_modules
  PROVIDES_KITS       pv_kits
  REQUIRES_MODULES    pv_required_modules
  UNRECOGNIZED_MODULES pv_unrecognized_modules
  ENABLE_TESTS        DEFAULT)

# Now build VTK modules required by the enabled ParaView modules.
# Alternatively, using an external VTK is now just crafting the
# appropriate `find_package(VTK)` component list.
vtk_module_find_modules(vtk_module_files ${vtk_source_directories})
vtk_module_find_kits(vtk_kit_files ${vtk_source_directories})
vtk_module_scan(
  MODULE_FILES        ${vtk_module_files}
  KIT_FILES           ${vtk_kit_files}
  REQUEST_MODULES     ${pv_required_modules}
                      ${pv_unrecognized_modules}
  PROVIDES_MODULES    vtk_modules
  PROVIDES_KITS       vtk_kits
  REQUIRES_MODULES    vtk_required_modules
  UNRECOGNIZED_MODULES vtk_unrecognized_modules
  WANT_BY_DEFAULT     OFF
  HIDE_MODULES_FROM_CACHE ON
  ENABLE_TESTS        DEFAULT)

# Check to see if any modules were mentioned that we don't know how to
# build.
if (vtk_required_modules OR vtk_unrecognized_modules)
  message(FATAL_ERROR
    "The following modules were requested or required, but not found: "
    "${vtk_required_modules};${vtk_unrecognized_modules}.")
endif ()

# Build VTK modules.
vtk_module_build(
  MODULES             ${vtk_modules}
  KITS                ${vtk_kits}
  BUILD_WITH_KITS     ${PARAVIEW_ENABLE_KITS}
  INSTALL_EXPORT      VTK
  HEADERS_DESTINATION "include/paraview-${PARAVIEW_MAJOR_VERSION}.${PARAVIEW_MINOR_VERSION}"
  CMAKE_DESTINATION   "lib/cmake/paraview-${PARAVIEW_MAJOR_VERSION}.${PARAVIEW_MINOR_VERSION}"
  LIBRARY_NAME_SUFFIX "-pv${PARAVIEW_MAJOR_VERSION}.${PARAVIEW_MINOR_VERSION}"
  VERSION             "${PARAVIEW_VERSION}"
  SOVERSION           "1"
  TEST_DATA_TARGET    ParaViewData
  TEST_INPUT_DATA_DIRECTORY   "${CMAKE_CURRENT_SOURCE_DIR}/VTK/Testing"
  TEST_OUTPUT_DATA_DIRECTORY  "${CMAKE_CURRENT_BINARY_DIR}/ExternalData/VTK/Testing")

# Build ParaView modules.
vtk_module_build(
  MODULES             ${pv_modules}
  KITS                ${pv_kits}
  BUILD_WITH_KITS     ${PARAVIEW_ENABLE_KITS}
  INSTALL_EXPORT      ParaView
  HEADERS_DESTINATION "include/paraview-${PARAVIEW_MAJOR_VERSION}.${PARAVIEW_MINOR_VERSION}"
  CMAKE_DESTINATION   "lib/cmake/paraview-${PARAVIEW_MAJOR_VERSION}.${PARAVIEW_MINOR_VERSION}"
  LIBRARY_NAME_SUFFIX "-pv${PARAVIEW_MAJOR_VERSION}.${PARAVIEW_MINOR_VERSION}"
  VERSION             "${PARAVIEW_VERSION}"
  SOVERSION           "1"
  TEST_DATA_TARGET    ParaViewData
  TEST_INPUT_DATA_DIRECTORY   "${CMAKE_CURRENT_SOURCE_DIR}/Testing"
  TEST_OUTPUT_DATA_DIRECTORY  "${CMAKE_CURRENT_BINARY_DIR}/ExternalData/Testing")
```

The no-extra-files approach is what the superbuild has. All code in
`CMakeLists.txt` now needs to handle being called twice which breaks
things like `cmake_dependent_option` and also means that
`configure_file` should be guarded by the appropriate phase. However,
the superbuild has a *much* simpler API and is smaller to boot.

--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: New module system preview

Ben Boeckel
In reply to this post by Bill Lorensen
On Mon, Oct 29, 2018 at 11:32:41 -0700, Bill Lorensen wrote:
> I will need the VTKExamples to support both api's. Also a few remote
> modules.

VTKExamples doesn't look too hard. Looking briefly at the remote
modules, they'd likely not be too hard to support both either since
they're relatively simple.

--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: New module system preview

David Thompson-2
In reply to this post by Ben Boeckel
>>>>> - Instead of `module.cmake`, there are `vtk.module` and `vtk.kit`
>>>>>  files. These are basically CMake argument lists, but no variable
>>>>>  expansion is allowed. If there are optional dependencies, they must
>>>>>  be private dependencies. Optional public dependencies indicate that
>>>>>  a new module should be made instead.
>>>>
>>>> Is there a reason these things need to be separate files
>>>> (vtk.module/vtk.kit) at all?
>>>
>>> The `vtk.kit` declares a kit. Membership into a kit is via `vtk.module`.
>>> Example `vtk.kit`:
>>
>> Yes, but why can't these declarations for modules and kits be inside
>> CMakeLists.txt as function calls instead of separate files?
>
> There's still a two-pass build pipeline. First is to get the declaration
> of the modules and build up the dependency graph. There is then a build
> step which builds modules in dependency order.

Even if it is two passes, why can't it be done from CMakeLists.txt? The second pass just adds library dependencies, header search paths, other target properties, and install rules, correct? Can't that be done any time after the libraries are declared?

It just feels like VTK and ParaView should be "best practices" examples. Your branch gets things a *lot* closer, but some twiddly details (the special .module/.kit files, no use of add_subdirectory, etc) get in the way of someone familiar with CMake but not these projects.

        David
_______________________________________________
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: New module system preview

Ben Boeckel
On Mon, Oct 29, 2018 at 15:45:36 -0400, David Thompson wrote:
> Even if it is two passes, why can't it be done from CMakeLists.txt?
> The second pass just adds library dependencies, header search paths,
> other target properties, and install rules, correct? Can't that be
> done any time after the libraries are declared?

First, the concept would require `target_link_libraries` support for
out-of-directory targets. That can't be done without 3.13.0 (still in
its rc cycle). It also means that we wouldn't be able to say "no" to
building some modules without somehow "undo"ing an `add_library` call.

Second, it's an ease-of-use problem. Say I don't have MPI on my machine.
I can't `add_subdirectory` *any* directory which requires `VTK::mpi`
because that module can't exist on my machine. How is the module system
supposed to know what directories can and cannot be included? If it is
`CMakeLists.txt` via `add_subdirectory` or `include`, the structure
would end up like:

```cmake
if (vtk_module_phase STREQUAL "scan")
  vtk_module_declare(<vtk.module args>)
elseif (vtk_module_phase STREQUAL "build")
  vtk_module_add_module(...)
endif ()
```

Third, part of the point of `vtk.module` is so that there *is no logic*
allowed. Making it part of `CMakeLists.txt` makes it much harder to
enforce this. Though this does make me think that we could stuff the
`vtk.module` contents in a comment header of the associated
`CMakeLists.txt`. That might be worth looking into. I'll add a TODO item
for it.

Towards the third point, it was a mistake to allow
`${VTK_RENDERING_BACKEND}` in the old module system's `module.cmake`
files. VTK should have supported building OpenGL1 *and* OpenGL2 backends
at the same time with executables using the object factory mechanism to
decide which one gets used. It also meant that some modules had classes
popping in and out of existence depending on other flags (e.g.,
`vtkIOMovie` contains `vtkOggTheoraWriter` depending on whether or not
`vtkoggtheora` is available…not something easy to have Just Work via
`find_package(VTK)` failing (the new module system now has a
`VTK::IOOggTheora` module for this class).

> It just feels like VTK and ParaView should be "best practices"
> examples. Your branch gets things a *lot* closer, but some twiddly
> details (the special .module/.kit files, no use of add_subdirectory,
> etc) get in the way of someone familiar with CMake but not these
> projects.

Unfortunately, things like kits, optional component building, complex
dependency graphs, etc. are not "common" and doing them via "best
practices" applicable to CMake projects in general isn't easy.

IMO, "best practices" would tell me that VTK should be many repositories
(or, many top-level builds at least) and each build has *no options* to
turn on or off bits of the build (modulo platform support and
deprecation). If one wants MPI support, go build the MPI subdirectory
and you get VTK MPI support.

Granted, there might be a top-level "enable these sections of code" flag
which runs all the subprojects like a superbuild, but I don't think the
downsides of the superbuild (i.e., ExternalProject) are ignorable for
projects with a developer and user community the size of VTK as-is.

--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: New module system preview

Elvis Stansvik
Den mån 29 okt. 2018 kl 21:42 skrev Ben Boeckel <[hidden email]>:

>
> On Mon, Oct 29, 2018 at 15:45:36 -0400, David Thompson wrote:
> > Even if it is two passes, why can't it be done from CMakeLists.txt?
> > The second pass just adds library dependencies, header search paths,
> > other target properties, and install rules, correct? Can't that be
> > done any time after the libraries are declared?
>
> First, the concept would require `target_link_libraries` support for
> out-of-directory targets. That can't be done without 3.13.0 (still in
> its rc cycle). It also means that we wouldn't be able to say "no" to
> building some modules without somehow "undo"ing an `add_library` call.
>
> Second, it's an ease-of-use problem. Say I don't have MPI on my machine.
> I can't `add_subdirectory` *any* directory which requires `VTK::mpi`
> because that module can't exist on my machine. How is the module system
> supposed to know what directories can and cannot be included? If it is
> `CMakeLists.txt` via `add_subdirectory` or `include`, the structure
> would end up like:
>
> ```cmake
> if (vtk_module_phase STREQUAL "scan")
>   vtk_module_declare(<vtk.module args>)
> elseif (vtk_module_phase STREQUAL "build")
>   vtk_module_add_module(...)
> endif ()
> ```
>
> Third, part of the point of `vtk.module` is so that there *is no logic*
> allowed. Making it part of `CMakeLists.txt` makes it much harder to
> enforce this. Though this does make me think that we could stuff the
> `vtk.module` contents in a comment header of the associated
> `CMakeLists.txt`. That might be worth looking into. I'll add a TODO item
> for it.

Let me just jump in and say that while it sounds neat to embed it
comments, it might make it slightly harder to make an updated version
of Utilities/Maintenance/WhatModulesVTK.py script (which I love!).

I kind of like the VTK system with these separate files. Even if it's
a bit unorthodox, they're very concise descriptions of the modules. I
like how I can do find . -name module.cmake to find all modules, or
cat Some/Dir/module.cmake to get info about a module. It's very
inspectable using standard tools.

It's just my 2 cents though, so take it or leave it :)

Elvis

>
> Towards the third point, it was a mistake to allow
> `${VTK_RENDERING_BACKEND}` in the old module system's `module.cmake`
> files. VTK should have supported building OpenGL1 *and* OpenGL2 backends
> at the same time with executables using the object factory mechanism to
> decide which one gets used. It also meant that some modules had classes
> popping in and out of existence depending on other flags (e.g.,
> `vtkIOMovie` contains `vtkOggTheoraWriter` depending on whether or not
> `vtkoggtheora` is available…not something easy to have Just Work via
> `find_package(VTK)` failing (the new module system now has a
> `VTK::IOOggTheora` module for this class).
>
> > It just feels like VTK and ParaView should be "best practices"
> > examples. Your branch gets things a *lot* closer, but some twiddly
> > details (the special .module/.kit files, no use of add_subdirectory,
> > etc) get in the way of someone familiar with CMake but not these
> > projects.
>
> Unfortunately, things like kits, optional component building, complex
> dependency graphs, etc. are not "common" and doing them via "best
> practices" applicable to CMake projects in general isn't easy.
>
> IMO, "best practices" would tell me that VTK should be many repositories
> (or, many top-level builds at least) and each build has *no options* to
> turn on or off bits of the build (modulo platform support and
> deprecation). If one wants MPI support, go build the MPI subdirectory
> and you get VTK MPI support.
>
> Granted, there might be a top-level "enable these sections of code" flag
> which runs all the subprojects like a superbuild, but I don't think the
> downsides of the superbuild (i.e., ExternalProject) are ignorable for
> projects with a developer and user community the size of VTK as-is.
>
> --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: New module system preview

Elvis Stansvik
In reply to this post by Ben Boeckel
Den mån 29 okt. 2018 kl 18:32 skrev Ben Boeckel <[hidden email]>:

>
> On Mon, Oct 29, 2018 at 18:13:43 +0100, Elvis Stansvik wrote:
> > > # Add VTK_AUTOINIT defines to the targets.
> > > vtk_module_autoinit(TARGETS myexe1 myexe2 MODULES VTK::CommonCore)
> > >
> >
> > Can this not be brought in automatically via the target's interface
> > definitions?
>
> No, because the set of AUTOINIT defines depends on the modules you link.
> Linking `VTK::ChemistryOpenGL2` and `VTK::ParallelChemistry` gives a
> different AUTOINIT define than just linking `VTK::ChemistryOpenGL2`.

Aha, thanks for clarifying. I never looked closely at how these things
work. It just worked :)

Elvis

>
> > With the current setup, I'm getting them via ${VTK_DEFINITIONS} I think.
>
> This is true. Currently, it is for all modules that VTK is told to find.
>
> > Would be nice to avoid the repeated target name.
>
> I think instead of `MODULES`, it might be possible to instead rummage
> around in the link properties of the passed targets. However, looking at
> the documentation, this is not necessarily trivial to parse:
>
>     https://cmake.org/cmake/help/latest/prop_tgt/LINK_LIBRARIES.html
>
> I'll add a TODO item for it, but I imagine that it will not work for all
> possible ways to use `target_link_libraries`.
>
> --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: New module system preview

Ben Boeckel
In reply to this post by Elvis Stansvik
On Mon, Oct 29, 2018 at 22:31:38 +0100, Elvis Stansvik wrote:
> Let me just jump in and say that while it sounds neat to embed it
> comments, it might make it slightly harder to make an updated version
> of Utilities/Maintenance/WhatModulesVTK.py script (which I love!).

Thanks for the reminder that this file needs updated.

CMake supports long-form comments now, so I'd think it'd be in blocks
like this:

```cmake
#[==[vtk.kit
...
#]==]
#[==[vtk.module
...
#]==]
```

with a limit of one `vtk.module` per file. Easy to slice out with a
simple `sed` call. Not so easy in CMake, but also not ridiculous.

This is similar to the way the new module system is documented:

```cmake
#[==[.md
...
#]==]
```

which will make it possible to extract out these blocks and feed them to
Doxygen and get the module system API docs on the web with the rest of
VTK.

There's also `#[==[.md INTERNAL` for internal function documentation as
well.

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