[vtk] Add option ENABLE_HDF5_PARALLEL to fit hdf5 feature parallel by JackBoosY · Pull Request #24740 · microsoft/vcpkg

JackBoosY · 2022-05-17T01:55:21Z

After installing hdf5[parallel] first, installing vtk will report configure error:

-- VTK module debug building: VTK::hdf5 is being built
-- Checking for module 'mpi-c'
--   Package 'mpi-c', required by 'virtual:world', not found
-- Checking for module 'mpi-cxx'
--   Package 'mpi-cxx', required by 'virtual:world', not found
CMake Error at ThirdParty/hdf5/CMakeLists.txt:21 (message):
  An external MPI-aware HDF5 requires that VTK be built with MPI support as
  well.

This is caused by vtk calling FindHDF5.cmake and getting HDF5_IS_PARALLEL in it.
Since using hdf5[parallel] needs to enable mpi, and using mpi does not necessarily require hdf5[parallel], add an error message to remind the user. See more details in https://gitlab.kitware.com/vtk/vtk/-/issues/18552.
Add test port vcpkg-ci-vtk to avoid this error message from appearing in CI tests.

Fixes #22892 #24740.

dg0yt

IMO this PR is creating a configuration that upstream explicitly considers unsupported. And vcpkg is know to not run any tests, so it creates a completely untested configuration.

ports/vtk/fix-hdf5-parallel.patch

ports/vtk/portfile.cmake

JackBoosY · 2022-05-17T07:05:18Z

IMO this PR is creating a configuration that upstream explicitly considers unsupported.

Any link to the upstream?

JackBoosY · 2022-05-17T08:38:27Z

This issue also exists in highfive:

CMake Warning at CMake/HighFiveTargetDeps.cmake:23 (message):
  Parallel HDF5 requested but libhdf5 doesnt support it
Call Stack (most recent call first):
  CMakeLists.txt:71 (include)

dg0yt · 2022-05-17T16:44:24Z

IMO this PR is creating a configuration that upstream explicitly considers unsupported.

Any link to the upstream?

Isn't it enough that upstream made it a FATAL_ERROR?

dg0yt · 2022-05-17T16:46:02Z

This issue also exists in highfive:

CMake Warning at CMake/HighFiveTargetDeps.cmake:23 (message):
  Parallel HDF5 requested but libhdf5 doesnt support it

You don't tell us the trigger, but the wording indicates a different, trivial issue.

JackBoosY · 2022-05-30T06:28:47Z

From the upstream reply:
MPI must be turned on when using hdf5[parallel], and hdf5[parallel] does not need to be turned on when turning on mpi.
I think we have two solutions:

Force feature mpi to depend on hdf5[parallel], but this is obviously not what upstream expects.
Report an error when feature parallel is selected for hdf5 and mpi is not selected for vtk.

github-actions

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

error: checked-in files for vtk have changed but the version was not updated
version: 9.0.3-pv5.9.1#11
old SHA: 277a83c960d3e5cddc283f65f9045b3664d071b7
new SHA: a4b3a0e5be24cb0c2ac714595e5d47e47f4e0139
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

You have modified or added at least one vcpkg.json where you should check the license field.

Details

If you feel able to do so, please consider adding a "license" field to the following files:

scripts/test_ports/vcpkg-ci-vtk/vcpkg.json

Valid values for the license field can be found in the documentation

github-actions

You have modified or added at least one vcpkg.json where you should check the license field.

Details

If you feel able to do so, please consider adding a "license" field to the following files:

scripts/test_ports/vcpkg-ci-vtk/vcpkg.json

Valid values for the license field can be found in the documentation

github-actions

You have modified or added at least one vcpkg.json where you should check the license field.

Details

If you feel able to do so, please consider adding a "license" field to the following files:

scripts/test_ports/vcpkg-ci-vtk/vcpkg.json

Valid values for the license field can be found in the documentation

JackBoosY · 2022-06-01T08:26:59Z

hdf5[parallel]:x64-linux issue: https://gitlab.kitware.com/cmake/cmake/-/issues/23574

dg0yt · 2022-06-01T18:55:55Z

hdf5[parallel]:x64-linux issue: https://gitlab.kitware.com/cmake/cmake/-/issues/23574

I doubt it is a CMake bug. At least not the syntax issue you claim there. We use it everywhere here in vcpkg.

I can reproduce the x64-linux build error. This is from the end of CMakeErrror.log:

FAILED: cmTC_5afa2 
: && /usr/bin/cc -std=c99 -fPIC -Og -ftrapv -fno-common -fmessage-length=0  CMakeFiles/cmTC_5afa2.dir/test_mpi.c.o -o cmTC_5afa2   && :
CMakeFiles/cmTC_5afa2.dir/test_mpi.c.o: In function `main':
test_mpi.c:(.text+0x21): undefined reference to `MPI_Init'
test_mpi.c:(.text+0x26): undefined reference to `MPI_Finalize'
collect2: error: ld returned 1 exit status

AFAIU it lacks -lmpi, but FindMPI.cmake could know it from mpicc -showme:link.

dg0yt · 2022-06-01T20:14:39Z

With --trace-expand, I find this:

/home/kpa/dg0yt/vcpkg/vcpkg/buildtrees/hdf5/src/df5-1_12_1-859faf3d27.clean/config/cmake/ConfigureChecks.cmake(286):  set(0 1 CACHE INTERNAL Have C function maximum decimal precision for C )

Wow, it sets a variable named 0 to 1.
... and now if(NOT WRAPPER_RETURN EQUAL 0) means if(NOT WRAPPER_RETURN EQUAL "1").
(And yes, setting set(0 "0") on top of FindMPI.cmake "fixes" the build. But this can't be the final fix.)

dg0yt · 2022-06-01T20:27:20Z

HDFGroup/hdf5#833

Neumann-A · 2022-06-02T12:25:36Z

Opened https://gitlab.kitware.com/cmake/cmake/-/issues/23580 since changing the meaning of stuff used for numeric comparisons should be disallowed by the build system.

…jack/22892

github-actions

You have modified or added at least one vcpkg.json where you should check the license field.

Details

If you feel able to do so, please consider adding a "license" field to the following files:

scripts/test_ports/vcpkg-ci-vtk/vcpkg.json

Valid values for the license field can be found in the documentation

JackBoosY · 2022-06-07T02:49:07Z

vtk:x64-windows regression: related upstream issue: https://gitlab.kitware.com/vtk/vtk/-/issues/18567

JackBoosY · 2022-06-16T02:10:40Z

Only the upstream bug remain.

JackBoosY · 2022-06-29T02:37:32Z

Draft this PR until the upstream fix this.

JackBoosY · 2022-07-08T06:16:52Z

Depends on #25636

JackBoosY · 2022-07-08T06:37:23Z

I think the features of vtk should be distinguished by components, not by dependencies.

…jack/22892

github-actions

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

error: checked-in files for vtk have changed but the version was not updated
version: 9.0.3-pv5.9.1#11
old SHA: a4b3a0e5be24cb0c2ac714595e5d47e47f4e0139
new SHA: 1f61c9a6e3896d1b8eba9de5830176a091cebcf1
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

JackBoosY · 2022-07-08T15:39:40Z

ports/vtk/vcpkg.json

    "all": {
      "description": "Build all vtk modules",
      "dependencies": [
+        "boost-assign",


Requires by internal dependency xdmf3, which requires by component VTK::xdmf3, VTK::IOXdmf3 and VTK::IOParallelXdmf3.

LilyWangLL · 2022-09-19T06:29:05Z

Closing this PR since it seems that no progress is being made. Please ping me to reopen if work is still being done.

JackBoosY added 2 commits May 16, 2022 18:50

[vtk] Add option ENABLE_HDF5_PARALLEL to fit hdf5 feature parallel

d9628c0

version

d502c75

JackBoosY added category:port-bug The issue is with a library, which is something the port should already support info:internal labels May 17, 2022

JackBoosY assigned LilyWangLL May 17, 2022

github-actions bot approved these changes May 17, 2022

View reviewed changes

LilyWangLL previously approved these changes May 17, 2022

View reviewed changes

dg0yt reviewed May 17, 2022

View reviewed changes

ports/vtk/fix-hdf5-parallel.patch Outdated Show resolved Hide resolved

ports/vtk/portfile.cmake Outdated Show resolved Hide resolved

JackBoosY added 3 commits May 30, 2022 01:12

[vtk] add error message instead of patching the source

ba76ee4

Add test port vcpkg-ci-vtk

cb94286

version

29a0b6a

github-actions bot requested changes May 30, 2022

View reviewed changes

github-actions bot approved these changes May 30, 2022

View reviewed changes

fix json

3bea547

github-actions bot approved these changes May 30, 2022

View reviewed changes

JackBoosY mentioned this pull request Jun 2, 2022

[hdf5] Fix feature parallel on Linux #25036

Merged

Merge branch 'master' of https://github.com/microsoft/vcpkg into dev/…

7131c41

…jack/22892

github-actions bot approved these changes Jun 6, 2022

View reviewed changes

Add license

4a35b81

github-actions bot previously approved these changes Jun 6, 2022

View reviewed changes

JackBoosY mentioned this pull request Jun 6, 2022

[hdf5] Refix msmpi library path in pkgconfig #25089

Merged

JackBoosY added the depends:different-pr This PR or Issue depends on a PR which has been filed label Jun 6, 2022

JackBoosY added depends:upstream-changes Waiting on a change to the upstream project and removed depends:different-pr This PR or Issue depends on a PR which has been filed labels Jun 16, 2022

ras0219-msft dismissed LilyWangLL’s stale review via 4a35b81 June 23, 2022 19:58

JackBoosY marked this pull request as draft June 29, 2022 02:37

JackBoosY mentioned this pull request Jul 7, 2022

[<vtk>] build failure #25579

Closed

JackBoosY mentioned this pull request Jul 8, 2022

[libgeotiff] Fix usage #25636

Closed

JackBoosY added 2 commits July 8, 2022 08:24

Merge branch 'master' of https://github.com/microsoft/vcpkg into dev/…

72de147

…jack/22892

Fix mpi4py, pdal and hdf5, add features, disable more components

f4a6a95

JackBoosY dismissed github-actions[bot]’s stale review via f4a6a95 July 8, 2022 15:27

github-actions bot requested changes Jul 8, 2022

View reviewed changes

JackBoosY commented Jul 8, 2022

View reviewed changes

LilyWangLL closed this Sep 19, 2022

Conversation

JackBoosY commented May 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dg0yt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JackBoosY commented May 17, 2022

Uh oh!

JackBoosY commented May 17, 2022

Uh oh!

dg0yt commented May 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dg0yt commented May 17, 2022

Uh oh!

JackBoosY commented May 30, 2022

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

JackBoosY commented Jun 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dg0yt commented Jun 1, 2022

Uh oh!

dg0yt commented Jun 1, 2022

Uh oh!

dg0yt commented Jun 1, 2022

Uh oh!

Neumann-A commented Jun 2, 2022

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

JackBoosY commented Jun 7, 2022

Uh oh!

JackBoosY commented Jun 16, 2022

Uh oh!

JackBoosY commented Jun 29, 2022

Uh oh!

JackBoosY commented Jul 8, 2022

Uh oh!

JackBoosY commented Jul 8, 2022

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

JackBoosY Jul 8, 2022

Choose a reason for hiding this comment

Uh oh!

LilyWangLL commented Sep 19, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JackBoosY commented May 17, 2022 •

edited

Loading

dg0yt commented May 17, 2022 •

edited

Loading

JackBoosY commented Jun 1, 2022 •

edited

Loading