[vtkm|vtk|paraview] update ports #37119
Conversation
# Conflicts: # ports/blas/CONTROL # ports/lapack-reference/CONTROL # ports/lapack-reference/portfile.cmake # ports/lapack/CONTROL # versions/b-/blas.json # versions/baseline.json # versions/l-/lapack-reference.json # versions/l-/lapack.json
rename -impl ports to old name move -test ports into script/test_ports
# Conflicts: # versions/o-/openblas.json
# Conflicts: # ports/lapack-reference/vcpkg.json # versions/baseline.json # versions/l-/lapack-reference.json
| - std::string libraryPath = vtkGetLibraryPathForSymbol(vtkPVInitializePythonModules); | ||
| - vtkPythonInterpreter::SetUserPythonPath( | ||
| - libraryPath.c_str(), "paraview/__init__.py" /*landmark*/); | ||
| + // Ignore this for now. Requires a slightly newer VTK version than PV 5.12 uses. |
There was a problem hiding this comment.
Can you explain why it is acceptable to just comment stuff out like this?
There was a problem hiding this comment.
What happens here in C++ code is the equivalent of setting PYTHONPATH in the environment. Since the VTK version here does not have these functions yet I commented them out. It is basically a partial revert of the external VTK patch in iparaview. (The current version of paraview also does not have these setup functions.)
Basically: It is a nice to have for paraview to have these functions but they are not a requirement. Users can manually set them up (or screw them up).
| { | ||
| std::vector<std::pair<int, vtkmdiy::MemoryBuffer>> vec; | ||
| - vec.push_back(std::move(entry)); | ||
| + vec.emplace_back(std::move(entry)); |
There was a problem hiding this comment.
Can you explain what changing this to emplace_back fixes?
There was a problem hiding this comment.
vtkmdiy::MemoryBuffer is an only moveable type and this moved the compiler error to a different location.
| -DVTKm_ENABLE_DOCUMENTATION=OFF | ||
| -DVTKm_ENABLE_BENCHMARKS=OFF | ||
| -DVTKm_USE_DEFAULT_TYPES_FOR_VTK=ON | ||
| -DBUILD_TESTING=OFF |
There was a problem hiding this comment.
Even if it is, better be explicit about it since it might change in the future or be dependent on some other options. (The default is not always set to OFF. Some projects set it to ON by default)
| remove-prefix-changes.patch | ||
| 10945.diff |
There was a problem hiding this comment.
Can you explain where these numbered patches came from? I assumed they were PRs in VTK's repo but https://gitlab.kitware.com/vtk/vtk/-/merge_requests/10945/diffs is not this change.
There was a problem hiding this comment.
I assumed they were PRs in VTK's repo but https://gitlab.kitware.com/vtk/vtk/-/merge_requests/10945/diffs is not this change.
It is just an earlier change. The problem with the latest is that it does no longer apply to the version here so I could not update it. Since the old version worked fine within vcpkg I used it instead of the newer one which would require a backport.
It is very probably the version after the fix for the error I posted a comment about in that MR
There was a problem hiding this comment.
I also keep the numbers as a reminder for: This can be removed in the next update.
There was a problem hiding this comment.
I'm just trying to follow up / verify that this is landed or at least submitted upstream, since some of these are big patches we wouldn't normally make ourselves. (No changes are requested to the content of the PR)
|
Thanks for the updates! |
Co-authored-by: Billy O'Neal <bion@microsoft.com>
|
Ready for review again since rtabmap is baseline. |
# Conflicts: # ports/openmvs/portfile.cmake # versions/o-/openmvs.json
#38035 broke it |
🤦♂️ sorry about that. |
|
@JavierMatosD CI is green. Ready to merge and fixing the baseline issues. |
|
thanks a lot for your work, when do you think we can merge this one? I am quite depended on it ? |
closes #36801
closes #36253
closes #36044
includes #24327 to fix dlib linkage in the osx pipelines.Need to look at
#36044#36345merge after