-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
[VTK 8.2.0/HDF5 1.10.5] Upgrade VTK and HDF5 #5574
Conversation
fixes #5462 |
Thanks for taking this on! Here are the results I currently see in the CI system. We are currently seeing flakeyness in our CI system with all ports that depend on qt5-base due to the way it caches its results, so if you are not able to reproduce the osg-qt failures locally they can probably be ignored for now. Let me know if you would like me to attach any of the failure logs.
|
Current problem: Get VTK to link ogg (or Ogg? maybe a name conflict?) Ok got it to work but had to rename a OGG to Ogg in Thirdparty/ogg/CMakeLists.txt. Wondering what the correct libraryname for ogg is. |
@Rastaban: I need to know where the regressions are after the last commit and what happend. I succesfully build x64-windows but times up for today. |
ok i'll give up with trying to test pcl with vtk 8.2.0 on linux. So on x64-windows PCL builds fine. On linux vtk 8.2 builds fine. Currently testing x64-windows-static but I don't expect any surprises. So if there are still regression please let me know and attach the logs. |
I spoke too soon but the last commit fixed the x64-windows-static build. On my machine it build successfully but failed to copy to installed/x64-windows-static because I ran out of disk space. |
Sorry for the delay, here is what I see now. As mentioned earlier, any failure that has a dependancy on qt5-base can be ignored for now because of #5566. Let me know if you need any of the failure logs.
|
Error:
Can somebody report upstream that they need to use #include <H5Cpp.h> everywhere to correctly include this header? Maybe: @miurahr? |
@Rastaban: I cannot reproduce the vtk regression in x64-windows-static
|
@Neumann-A, check the latest results and attach the failure logs:
|
@PhoebeHui Seems to be a ninja issue... |
In the latest test run the only regressions are qhull on arm64-windows and x86-windows with the same |
@Rastaban: Hmm tried it at home for x86-windows and it worked the first time. Removed it and updated Visual Studio 2017 to newest version -> LNK 1201. Seems to be a regression in the build tools not correctly releasing the lock on the file or something similar. |
and to top it off.... if I readd |
This commit maybe by @gillins |
Is the HDF5 C++ library still being built? FYI might be worth upgrading to |
@Neumann-A you may be right about the toolset regression. Looking through the internal MSVC bug database there have been a few issues related to LNK1201 that have been fixed recently. |
Other than the qhull failure on x64-windows-static everything else passes the tests. Are there any other outstanding issue that need to be resolved before this is merged? |
see discussion in microsoft#5574
@Rastaban: should i clean up the commit history or do you want to merge it the way it is? There are probably other portfiles which can remove extra code changing *targets.cmake due to commit 0b5a576 but that could be done in another PR. The problem was that the release version of the targets file was missing the debug information due to the fact that cmake was simply not finding the debug versions due to the missing path. (folly is such an example I at least know of.) |
ports/qhull/portfile.cmake
Outdated
@@ -7,7 +7,20 @@ vcpkg_from_github( | |||
SHA512 8f5177ea45f82fa28f13e95105497e7e29086d7301e1cb8d3860fff09ebf8d0f01cfcb0f044c422f0ac0ba94b845bba223232e5eeb613bf671f65a569b8766d0 | |||
HEAD_REF master | |||
) | |||
|
|||
if(${VCPKG_TARGET_TRIPLET} STREQUAL "x64-windows-static") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be ${TARGET_TRIPLET}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in
1feb0dc
I was going to do a Squash and Merge, but if you are willing to cleanup the commit history then I can just merge it when you are done. |
extra code comments. Maybe the test should actually try to link targets from the package instead of just testing find_package
Also now builds dynamic and static libs in dynamic build due to the targets exported by hdf5. (Revert to default hdf5 build behavior)
HDF5_USE_STATIC_LIBRARIES must be set in static builds so that find_package(HDF5) finds the static hdf5 libraries
Ran out of time to test the build but I think it is a good start.