[VTK] Fix build error when IOCCT is selected as feature - #53476
[VTK] Fix build error when IOCCT is selected as feature#53476Lars Glud (larshg) wants to merge 9 commits into
Conversation
…res is selected. Fix compiler warnings about types and collection. Rename opencascade patch filename, since it covers multiple versions.
Billy O'Neal (BillyONeal)
left a comment
Billy O'Neal (BillyONeal)
left a comment
There was a problem hiding this comment.
larshg#1 will fix version database
Renamed opencascaded patch file as it patches for multiple versions - should I remove old fixes for pre 8.0.1?
I don't see any reason to do that, at least until they no longer apply because upstream has effectively set their target version later than that.
Should I change:
vtk[ioocct]=skipto
vtk[ioocct]=cascade
in ci.feature.baseline.txt?
Looks like this was done back in #47210 (comment) . It is likely you will need to do further investigation in order to remove the skip but I think it would be welcome.
| endif() | ||
|
|
||
| if("ioocct" IN_LIST FEATURES) | ||
| # OCCT 8.0's headers require C++17. VTK's own CMake forces CMAKE_CXX_STANDARD to 11 unless told |
There was a problem hiding this comment.
Maybe we should be updating VTK instead?
There was a problem hiding this comment.
Yes, that could be done, but its a large effort with a lot of dependencies and currently out of my timeframe atm.
There was a problem hiding this comment.
The VTK et al. update is pending in #52532, only stopped by viskores (aka vtk-m) CUDA DLL woes.
There was a problem hiding this comment.
(The viskores troubles is split out to #53088.)
| - Standard_Integer shift = 0; | ||
| + int shift = 0; |
There was a problem hiding this comment.
It is not clear to me that these types of changes are safe; has this been submitted upstream?
There was a problem hiding this comment.
I'll revert these ones - they were only deprecation warnings that it should be changed to built in types. Not per se required for compilation.
| # otherwise, which combined with Eigen3's cxx_std_14 requirement causes some targets to compile | ||
| # as c++14 instead of 17, so both need to be overridden together. | ||
| list(APPEND ADDITIONAL_OPTIONS | ||
| -DVTK_IGNORE_CMAKE_CXX11_CHECKS=ON |
There was a problem hiding this comment.
This seems like it might be a dangerous option to turn on without justification here?
There was a problem hiding this comment.
If this is not on, it defaults to c++11, but eigens requirements raises that to c++14 and somehow enforces this on some targets, but having this all of them becomes c++17 targets.
There was a problem hiding this comment.
Can you add a permalink comment which helps describe that it is safe to do?
3 participants
Fixes #53475
./vcpkg x-add-version --alland committing the result.Renamed opencascaded patch file as it patches for multiple versions