Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recipe for VTK #10776

Open
wants to merge 132 commits into
base: master
Choose a base branch
from
Open

Recipe for VTK #10776

wants to merge 132 commits into from

Conversation

paulharris
Copy link
Contributor

Specify library name and version: vtk/9.1.0

I'm putting this up for comments and further discussion.
It is getting there, but still needs some work.
Please help.


  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the conan-center hook activated.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@paulharris
Copy link
Contributor Author

Please check and respond here, and/or, slack me to discuss,
I need some help to make this battle station fully operational.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@paulharris
Copy link
Contributor Author

Note: I keep a list of known TODOs at the top of the conanfile.py

@conan-center-bot

This comment has been minimized.

@paulharris
Copy link
Contributor Author

I have hook warnings/errors related to extra .cmake files left from the cmake build+install.
I would appreciate some advice on what I should be doing about that.
eg it wasn't happy it found ./lib/cmake/vtk/vtk-config.cmake
which is weird because I thought that was the file I was generating?
I'm not understanding something here.,,

@conan-center-bot

This comment has been minimized.

@paulharris
Copy link
Contributor Author

I still have some things to do, would like to autogenerate the component list and dependencies etc.

@conan-center-bot

This comment has been minimized.

@paulharris
Copy link
Contributor Author

paulharris commented May 14, 2022

Seems to fail due to old CMake 3.18 on the CI.

VTK has patches to support older CMake up to 3.22,
I was hoping to avoid needing them ...
I suppose I can make cmake 3.23 a build requirement...

@SSE4
Copy link
Contributor

SSE4 commented May 14, 2022

Seems to fail due to old CMake 3.18 on the CI.

VTK has patches to support older CMake up to 3.22, I was hoping to avoid needing them ... I suppose I can make cmake 3.23 a build requirement...

that's not a problem - you always may add more recent CMake to build_requirements

recipes/vtk/all/conanfile.py Outdated Show resolved Hide resolved
recipes/vtk/all/conanfile.py Outdated Show resolved Hide resolved
recipes/vtk/all/conanfile.py Outdated Show resolved Hide resolved
@conan-center-bot

This comment has been minimized.

@paulharris
Copy link
Contributor Author

I am now thinking that I should use Kitware/VTK's bundled third party dependencies, even if the same (or more recent) version is available on conan as a package.

VTK has a lot of cmake magic to bring in their bundled dependencies, and is careful they do not clash with the same library if brought in independently.

Some of their dependencies are heavily patched, but it is not immediately obvious which ones they are, nor can I tell if a newer version of the same library might cause a problem with a VTK release.

So instead, just go with VTK's defaults and use their bundled externals.
ie VTK_USE_EXTERNAL = True and don't set a lot of requirements.

Thoughts?

@conan-center-bot

This comment has been minimized.

Still some things to TODO.
The recipe folder is still big, will slim down when I drop 9.2.6,
but for now it is useful to have more than one version to test the
recipe.
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@paulharris
Copy link
Contributor Author

I'm pretty happy with this recipe now, looking for feedback.
There are some patches that we use on the recipe, that we are pushing upstream.

@conan-center-bot

This comment has been minimized.

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ❌

Failure in build 4 (50d0a8bade38aec43e9b36bcd3f7907214a252ba):

  • vtk/9.3.0:
    Error running command conan export recipes/vtk/all/conanfile.py vtk/9.3.0@:
    WARN: *** Conan 1 is legacy and on a deprecation path ***
    WARN: *** Please upgrade to Conan 2 ***
    ERROR: Error loading conanfile at '/home/conan/workspace/prod-v1_cci_PR-10776/recipes/vtk/all/conanfile.py': : Error in init() method, line 317
    	self._override_options_values(extra_defaults)
    while calling '_override_options_values', line 195
    	self.options.update(overridden, new_values)
    	TypeError: update expected at most 1 arguments, got 2
    

Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability.


def _override_options_values(self, new_values):
overridden = { opt : ["DEFAULT", "YES", "NO", "WANT", "DONT_WANT"] for opt in new_values }
self.options.update(overridden, new_values)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only takes one argument that is a dict

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you mean self.options.update(),
the signature is conans/model/options.py:345 :
def update(self, options=None, options_values=None):

Copy link
Contributor

github-actions bot commented Jul 1, 2024

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet