Skip to content

[vtk] update to 9.0.1#12149

Merged
ras0219-msft merged 4 commits intomicrosoft:masterfrom
Neumann-A:vtk_9_0_1
Jul 7, 2020
Merged

[vtk] update to 9.0.1#12149
ras0219-msft merged 4 commits intomicrosoft:masterfrom
Neumann-A:vtk_9_0_1

Conversation

@Neumann-A
Copy link
Copy Markdown
Contributor

(also solves the paraview issue)

@JackBoosY JackBoosY self-assigned this Jun 30, 2020
@JackBoosY JackBoosY added the category:port-update The issue is with a library, which is requesting update new revision label Jun 30, 2020
@NancyLi1013
Copy link
Copy Markdown
Contributor

The features for vtk failed on x64-windows:

CMake Error at CMake/vtkDownload.cmake:8 (message):
  Attempted to download Module_MomentInvariants when VTK_FORBID_DOWNLOADS is
  ON
Call Stack (most recent call first):
  CMake/vtkModuleRemote.cmake:142 (vtk_download_attempt_check)
  Remote/MomentInvariants.remote.cmake:5 (vtk_fetch_module)
  Remote/CMakeLists.txt:9 (include)


-- Found Git: C:/Program Files/Git/cmd/git.exe (found version "2.20.1.windows.1") 
CMake Error at CMake/vtkModuleRemote.cmake:12 (message):
  Failed to clone repository:
  'https://gitlab.kitware.com/vtk/MomentInvariants.git' Clone error is:
  'fatal: could not create work tree dir
  'F:/12149/vcpkg/buildtrees/vtk/src/ad8774e85e-85de89847a/Remote/MomentInvariants':
  File exists

  '
Call Stack (most recent call first):
  CMake/vtkModuleRemote.cmake:107 (_git_clone)
  CMake/vtkModuleRemote.cmake:157 (_fetch_with_git)
  Remote/MomentInvariants.remote.cmake:5 (vtk_fetch_module)
  Remote/CMakeLists.txt:9 (include)

@Neumann-A
Copy link
Copy Markdown
Contributor Author

@NancyLi1013: The feature all is also broke in current master #9960 (comment). Due to that i added the suggestion here: #9960 (comment) and it should work now but I don't have time to test it.

@NancyLi1013
Copy link
Copy Markdown
Contributor

@Neumann-A Thanks for your update.
I will test these features again.

@NancyLi1013
Copy link
Copy Markdown
Contributor

NancyLi1013 commented Jul 6, 2020

The feature all is also broken due to the same reason as above.

cuda seems to be broken like this:

-- The CUDA compiler identification is unknown
CMake Error at CMakeLists.txt:126 (enable_language):
  No CMAKE_CUDA_COMPILER could be found.

  Tell CMake where to find the compiler by setting either the environment
  variable "CUDACXX" or the CMake cache entry CMAKE_CUDA_COMPILER to the full
  path to the compiler, or to the compiler name if it is in the PATH.

Other features failed due to this:

CMake Error at CMake/vtkModule.cmake:959 (message):
  The VTK::ParallelMPI4Py module requires the disabled module VTK::mpi.
Call Stack (most recent call first):
  CMakeLists.txt:270 (vtk_module_scan)
CMake Error at ThirdParty/hdf5/CMakeLists.txt:26 (message):
  An external MPI-aware HDF5 requires that VTK be built with MPI support as
  well.

@Neumann-A
Could you please help make a simple description about the relationships between these features?
I met the same problems on another PR, see comment #11399 (comment).
and as @larshg said here #11399 (comment).

Only on a clean environment can these features pass every time?

I test these features on the repo that I cloned last week and used git pull to update to the latest changes. Then I continued testing.

-DVTK_INSTALL_PACKAGE_DIR=share/vtk
-DVTK_INSTALL_RUNTIME_DIR=bin
-DVTK_FORBID_DOWNLOADS=ON
-DVTK_ENABLE_REMOTE_MODULES=OFF
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks like it is reducing the vtk featureset -- should this be turned into a feature?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You were the one adding:
-DVTK_FORBID_DOWNLOADS=ON
so
-DVTK_ENABLE_REMOTE_MODULES=OFF
is already a requirement to even have a chance of making the all feature work.
Either we allow downloads or we disallow downloads and remote modules.
Comment of one of the devs to vtk remote modules: #9960 (comment)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the link, this LGTM

@ras0219-msft
Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@JackBoosY JackBoosY requested a review from ras0219-msft July 7, 2020 01:55
@ras0219-msft ras0219-msft merged commit c6dafe1 into microsoft:master Jul 7, 2020
@ras0219-msft
Copy link
Copy Markdown
Contributor

Thanks for the great work yet again!

@Neumann-A Neumann-A deleted the vtk_9_0_1 branch July 17, 2020 19:24
@cenit
Copy link
Copy Markdown
Contributor

cenit commented Aug 6, 2020

@ras0219-msft @Neumann-A this PR broke opencv[vtk]

@Neumann-A
Copy link
Copy Markdown
Contributor Author

@cenit Logs?

@cenit
Copy link
Copy Markdown
Contributor

cenit commented Aug 6, 2020

VTK not found, that's all.
Vtk9 changed everything from the target point of view. It needs OpenCV 4.4 or a patch to use it correctly. It's a pity it went completely unnoticed, but again and again as happens so often for vcpkg, it breaks the experience for downstream users.

@Neumann-A
Copy link
Copy Markdown
Contributor Author

VTK changed their module system completely and made it finally sane. Before that VTK was just a buggy mess. If you want some features be explicitly tested in vcpkg's CI just add test port for it. See https://github.com/microsoft/vcpkg/tree/master/scripts/test_ports

I mean I also explicitly deactivated VTK in PCL due to upstream not making a lot of progress. At least OpenCV seem to be updateable to make it work. (Or people need to checkout the old VTK port and use a port-overlay.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:port-update The issue is with a library, which is requesting update new revision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants